Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[draft] sni + mtls #4965

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ public AcquireTokenForClientParameterBuilder WithSendX5C(bool withSendX5C)
return this;
}

/// <summary>
/// Specifies that the certificate provided will be used for PoP tokens with MTLS (Mutual TLS) authentication.
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <returns>The current instance of <see cref="AcquireTokenForClientParameterBuilder"/> to enable method chaining.</returns>
public AcquireTokenForClientParameterBuilder WithMtlsPop()
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
{
Parameters.UseMtlsPop = true;
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
return this; // Return the builder to allow method chaining
}

/// <summary>
/// Please use WithAzureRegion on the ConfidentialClientApplicationBuilder object
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Identity.Client.ApiConfig.Parameters;
using Microsoft.Identity.Client.AuthScheme.PoP;
using Microsoft.Identity.Client.Instance.Discovery;
using Microsoft.Identity.Client.Internal;
using Microsoft.Identity.Client.Internal.Requests;
Expand Down Expand Up @@ -63,6 +64,16 @@ public async Task<AuthenticationResult> ExecuteAsync(

requestParams.SendX5C = clientParameters.SendX5C ?? false;

requestParams.UseMtlsPop = clientParameters.UseMtlsPop;

if (requestParams.UseMtlsPop)
{
commonParameters.MtlsCertificate = _confidentialClientApplication.Certificate;
commonParameters.AuthenticationOperation = new MtlsPopAuthenticationOperation(_confidentialClientApplication.Certificate);
ServiceBundle.Config.ClientCredential = null;
ServiceBundle.Config.UseMtlsPop = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid having this as the request level and at the app level.

}

var handler = new ClientCredentialRequest(
ServiceBundle,
requestParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,10 @@ internal abstract class AbstractAcquireTokenConfidentialClientParameters
/// This overrides application config settings.
/// </summary>
public bool? SendX5C { get; set; }

/// <summary>
/// Whether to use MTLS Proof of Possession (PoP)
/// </summary>
public bool UseMtlsPop { get; set; } = false;
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,9 @@ public string ClientVersion
public bool IsPublicClient => !IsConfidentialClient && !IsManagedIdentity;

public Func<AppTokenProviderParameters, Task<AppTokenProviderResult>> AppTokenProvider;
public bool UseMtlsPop { get; internal set; } = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not be needed.


#region ClientCredentials
#region ClientCredentials

public IClientCredential ClientCredential { get; internal set; }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using Microsoft.Identity.Client.AppConfig;
using Microsoft.Identity.Client.Cache.Items;
using Microsoft.Identity.Client.Internal;
using Microsoft.Identity.Client.OAuth2;
using Microsoft.Identity.Client.Utils;
#if SUPPORTS_SYSTEM_TEXT_JSON
using JObject = System.Text.Json.Nodes.JsonObject;
using JToken = System.Text.Json.Nodes.JsonNode;
#else
using Microsoft.Identity.Json;
using Microsoft.Identity.Json.Linq;
#endif

namespace Microsoft.Identity.Client.AuthScheme.PoP
{
internal class MtlsPopAuthenticationOperation : IAuthenticationOperation
{
private readonly X509Certificate2 _mtlsCert;

public MtlsPopAuthenticationOperation(X509Certificate2 mtlsCert)
{
_mtlsCert = mtlsCert;
KeyId = mtlsCert.Thumbprint;
}

public int TelemetryTokenType => (int)TokenType.Pop;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls use a different value here. This one is reserved fro SHR POP

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gladjohn @bgavrilMS is the mapping maintained in the enum?


public string AuthorizationHeaderPrefix => Constants.MtlsPoPAuthHeaderPrefix;

public string AccessTokenType => Constants.MtlsPoPTokenType;

/// <summary>
/// For PoP, we chose to use the base64(jwk_thumbprint)
/// </summary>
public string KeyId { get; }

public IReadOnlyDictionary<string, string> GetTokenRequestParams()
{
return CollectionHelpers.GetEmptyDictionary<string, string>();
}

public void FormatResult(AuthenticationResult authenticationResult)
{
var header = new JObject();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

header[JsonWebTokenConstants.KeyId] = KeyId;
header[JsonWebTokenConstants.Type] = Constants.MtlsPoPTokenType;

//authenticationResult.Certificate = _mtlsCert;
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
}

private JObject CreateBody(string accessToken)
{
return null;
}

/// <summary>
/// A key ID that uniquely describes a public / private key pair. While KeyID is not normally
/// strict, AAD support for PoP requires that we use the base64 encoded JWK thumbprint, as described by
/// https://tools.ietf.org/html/rfc7638
/// </summary>
private static byte[] ComputeThumbprint(string canonicalJwk)
gladjohn marked this conversation as resolved.
Show resolved Hide resolved
{
using (SHA256 hash = SHA256.Create())
{
return hash.ComputeHash(Encoding.UTF8.GetBytes(canonicalJwk));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal class RegionDiscoveryProvider : IRegionDiscoveryProvider
{
private readonly IRegionManager _regionManager;
public const string PublicEnvForRegional = "login.microsoft.com";
public const string PublicEnvForMtls = "mtlsauth.microsoft.com";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend you rename this to RegionAndMtlsProvider

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgavrilMS but the region is not included in the const?


public RegionDiscoveryProvider(IHttpManager httpManager, bool clearCache)
{
Expand Down Expand Up @@ -56,9 +57,14 @@ private static InstanceDiscoveryMetadataEntry CreateEntry(string originalEnv, st

private static string GetRegionalizedEnvironment(Uri authority, string region, RequestContext requestContext)
{

string host = authority.Host;

if (requestContext.ServiceBundle.Config.UseMtlsPop)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the case of non-regional + mtls POP config? Do we want to support this or just error out?

{
requestContext.Logger.Info(() => $"[Region discovery] Using MTLS public endpoint: {PublicEnvForMtls}");
return $"{region}.{PublicEnvForMtls}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong for non public cloud.

}

if (KnownMetadataProvider.IsPublicEnvironment(host))
{
requestContext.Logger.Info(() => $"[Region discovery] Regionalized Environment is : {region}.{PublicEnvForRegional}. ");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ internal static class Constants
public static readonly TimeSpan AccessTokenExpirationBuffer = TimeSpan.FromMinutes(5);
public const string EnableSpaAuthCode = "1";
public const string PoPTokenType = "pop";
public const string PoPAuthHeaderPrefix = "PoP";
public const string MtlsPoPTokenType = "mtls_pop";
public const string PoPAuthHeaderPrefix = "PoP";
public const string MtlsPoPAuthHeaderPrefix = "mtls_pop";
public const string RequestConfirmation = "req_cnf";
public const string BearerAuthHeaderPrefix = "Bearer";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public AuthenticationRequestParameters(

public AuthorityInfo AuthorityInfo => AuthorityManager.Authority.AuthorityInfo;

public AuthorityInfo AuthorityOverride => _commonParameters.AuthorityOverride;
public AuthorityInfo AuthorityOverride => _commonParameters.AuthorityOverride;

#endregion

Expand Down Expand Up @@ -163,6 +163,12 @@ public string LoginHint
public string LongRunningOboCacheKey { get; set; }

public KeyValuePair<string, string>? CcsRoutingHint { get; set; }

/// <summary>
/// Indicates if MTLS Proof of Possession should be used.
/// </summary>
public bool UseMtlsPop { get; set; } = false;

#endregion

public void LogParameters()
Expand All @@ -185,6 +191,7 @@ public void LogParameters()
builder.AppendLine("ApiId - " + ApiId);
builder.AppendLine("IsConfidentialClient - " + AppConfig.IsConfidentialClient);
builder.AppendLine("SendX5C - " + SendX5C);
builder.AppendLine("UseMtlsPop - " + UseMtlsPop);
builder.AppendLine("LoginHint - " + LoginHint);
builder.AppendLine("IsBrokerConfigured - " + AppConfig.IsBrokerEnabled);
builder.AppendLine("HomeAccountId - " + HomeAccountId);
Expand All @@ -205,6 +212,7 @@ public void LogParameters()
builder.AppendLine("ApiId - " + ApiId);
builder.AppendLine("IsConfidentialClient - " + AppConfig.IsConfidentialClient);
builder.AppendLine("SendX5C - " + SendX5C);
builder.AppendLine("UseMtlsPop - " + UseMtlsPop);
builder.AppendLine("LoginHint ? " + !string.IsNullOrEmpty(LoginHint));
builder.AppendLine("IsBrokerConfigured - " + AppConfig.IsBrokerEnabled);
builder.AppendLine("HomeAccountId - " + !string.IsNullOrEmpty(HomeAccountId));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private async Task<AuthenticationResult> GetAccessTokenAsync(
// Get a token from AAD
if (ServiceBundle.Config.AppTokenProvider == null)
{
MsalTokenResponse msalTokenResponse = await SendTokenRequestAsync(GetBodyParameters(), cancellationToken).ConfigureAwait(false);
MsalTokenResponse msalTokenResponse = await SendTokenRequestAsync(GetBodyParameters(_clientParameters.UseMtlsPop), cancellationToken).ConfigureAwait(false);
return await CacheTokenResponseAndCreateAuthenticationResultAsync(msalTokenResponse).ConfigureAwait(false);
}

Expand Down Expand Up @@ -224,13 +224,20 @@ protected override SortedSet<string> GetOverriddenScopes(ISet<string> inputScope
return new SortedSet<string>(inputScopes);
}

private Dictionary<string, string> GetBodyParameters()
private Dictionary<string, string> GetBodyParameters(bool useMtlsPop)
{
var dict = new Dictionary<string, string>
{
[OAuth2Parameter.GrantType] = OAuth2GrantType.ClientCredentials,
[OAuth2Parameter.Scope] = AuthenticationRequestParameters.Scope.AsSingleString()
};

// Only add TokenType if useMtlsPop is true
if (useMtlsPop)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to do this in MtlsPopAuthenticationOperation.cs

{
dict[OAuth2Parameter.TokenType] = RequestTokenType.MTLSPop;
}

return dict;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ internal static class OAuth2Error
public const string AuthorizationPending = "authorization_pending";
}

internal static class RequestTokenType
{
public const string Bearer = "bearer";
public const string MTLSPop = "mtls_pop";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls consolidate with SHR POP.

}

internal static class OAuth2Value
{
public const string CodeChallengeMethodValue = "S256";
Expand Down
Loading