-
Notifications
You must be signed in to change notification settings - Fork 161
Add SN/I certificate support over mTLS Proof-of-Possession (PoP) #1040
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
base: dev
Are you sure you want to change the base?
Changes from 10 commits
d8cac48
150a554
68d3715
66d59db
e6dee5f
288df02
6bf6024
f848b54
3ff929c
3ef7681
d576db6
683e54e
3858dc3
d22059a
48fd92d
29fc91b
1805895
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| { | ||
| "tool": "Credential Scanner", | ||
| "suppressions": [ | ||
| { | ||
| "file": "msal4j-sdk/src/test/resources/mtls_test_cert.p12", | ||
| "_justification": "Self-signed, test-only certificate (CN=msal4j-mtls-test) used by unit tests for mTLS Proof-of-Possession. Contains no production secret." | ||
| } | ||
| ] | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,205 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.microsoft.aad.msal4j; | ||
|
|
||
| import com.microsoft.aad.msal4j.labapi.KeyVaultSecretsProvider; | ||
| import org.junit.jupiter.api.Assumptions; | ||
| import org.junit.jupiter.api.BeforeAll; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.TestInstance; | ||
|
|
||
| import java.io.IOException; | ||
| import java.security.KeyStore; | ||
| import java.security.KeyStoreException; | ||
| import java.security.NoSuchAlgorithmException; | ||
| import java.security.NoSuchProviderException; | ||
| import java.security.PrivateKey; | ||
| import java.security.UnrecoverableKeyException; | ||
| import java.security.cert.CertificateException; | ||
| import java.security.cert.X509Certificate; | ||
| import java.util.Collections; | ||
| import java.util.concurrent.ExecutionException; | ||
|
|
||
| import static com.microsoft.aad.msal4j.TestConstants.KEYVAULT_DEFAULT_SCOPE; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertNotEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
|
|
||
| /** | ||
| * End-to-end integration tests for SN/I certificate over mTLS Proof-of-Possession (PoP). | ||
| * | ||
| * <p>These exercise the primary deliverable of this work: a confidential-client app configured with a | ||
| * Subject-Name/Issuer (SN/I) certificate obtains an <b>mTLS-bound PoP access token</b> from Entra ID | ||
| * (ESTS), where that same SNI cert is presented as the client TLS certificate in the mutual-TLS | ||
| * handshake to the token endpoint (no {@code private_key_jwt} / x5c client assertion on the direct | ||
| * path). | ||
| * | ||
| * <p>The primary scenario is covered: | ||
| * <ul> | ||
| * <li><b>Direct SNI cert → mTLS PoP</b> (client credentials), global and regional endpoints.</li> | ||
| * </ul> | ||
| * | ||
| * <p><b>Testability gate (SME note A):</b> ESTS gates mTLS PoP on the <i>final resource audience</i>, | ||
| * which must be an ESTS allow-listed resource (e.g. Azure Key Vault or MS Graph) — not the client app. | ||
| * Every test below therefore requests a token for an allow-listed resource. | ||
| * | ||
| * <p>The lab SN/I certificate is <b>non-CNG</b>, so these tests are E2E-runnable in CI/CD using the same | ||
| * certificate the pipelines already provision for {@code ClientCredentialsIT} and {@code AgenticIT} (the | ||
| * OS keystore alias {@link KeyVaultSecretsProvider#CERTIFICATE_ALIAS}). They require lab credentials and | ||
| * network access and only pass in CI (like the other {@code *IT} tests, they are not run by the unit-test | ||
| * surefire pass). | ||
| * | ||
| * <p><b>App/tenant requirement:</b> ESTS only issues mTLS PoP tokens for the SN/I-allow-listed app in the | ||
| * MSI team tenant. This mirrors MSAL .NET's {@code ClientCredentialsMtlsPopTests}: any other app (e.g. the | ||
| * general {@code LabVaultAppID}) is rejected with {@code AADSTS700025: Client is public...}. The lab cert | ||
| * ({@link KeyVaultSecretsProvider#CERTIFICATE_ALIAS}) is registered for SN/I on this app. | ||
| */ | ||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| class MtlsPopIT { | ||
|
|
||
| // SN/I-allow-listed app and MSI team tenant. mTLS PoP only works on this app/tenant pair; mirrors | ||
| // MSAL .NET's ClientCredentialsMtlsPopTests. App and tenant IDs are public identifiers, not secrets. | ||
| private static final String SNI_ALLOWLISTED_APP_ID = "163ffef9-a313-45b4-ab2f-c7e2f5e0e23e"; | ||
| private static final String SNI_ALLOWLISTED_AUTHORITY = | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have so much reliance on this MSI team's test tenant. Can we have our own SNI test setup in IDLABS1? |
||
| "https://login.microsoftonline.com/bea21ebe-8b64-4d06-9f6d-6a889b120a7c"; | ||
| private static final String TEST_SLICE_REGION = "westus3"; | ||
|
|
||
| private PrivateKey privateKey; | ||
| private X509Certificate publicCertificate; | ||
| private IClientCertificate certificate; | ||
|
|
||
| @BeforeAll | ||
| void init() throws KeyStoreException, NoSuchProviderException, IOException, | ||
| NoSuchAlgorithmException, CertificateException, UnrecoverableKeyException { | ||
| KeyStore keystore = CertificateHelper.createKeyStore(); | ||
| keystore.load(null, null); | ||
|
|
||
| privateKey = (PrivateKey) keystore.getKey(KeyVaultSecretsProvider.CERTIFICATE_ALIAS, null); | ||
| publicCertificate = (X509Certificate) keystore.getCertificate(KeyVaultSecretsProvider.CERTIFICATE_ALIAS); | ||
|
|
||
| assertNotNull(privateKey, "Lab private key not found. Ensure the lab cert is installed."); | ||
| assertNotNull(publicCertificate, "Lab certificate not found. Ensure the lab cert is installed."); | ||
|
|
||
| certificate = ClientCredentialFactory.createFromCertificate(privateKey, publicCertificate); | ||
| } | ||
|
|
||
| /** | ||
| * Direct SNI cert → mTLS PoP with <b>no region</b> (exercises the global | ||
| * {@code mtlsauth.microsoft.com} endpoint). The lab cert is presented as the client TLS certificate; | ||
| * the request carries {@code token_type=mtls_pop} and <b>no</b> client assertion. Requests an | ||
| * allow-listed resource (Key Vault) so ESTS issues the bound token. | ||
| */ | ||
| @Test | ||
| void acquireTokenClientCredentials_Certificate_MtlsPop() throws Exception { | ||
| ConfidentialClientApplication cca = ConfidentialClientApplication.builder(SNI_ALLOWLISTED_APP_ID, certificate) | ||
| .authority(SNI_ALLOWLISTED_AUTHORITY) // tenanted authority (required for mTLS PoP) | ||
| .build(); | ||
|
|
||
| IAuthenticationResult result = acquireMtlsPopOrSkipOnDowngrade(cca, ClientCredentialParameters | ||
| .builder(Collections.singleton(KEYVAULT_DEFAULT_SCOPE)) | ||
| .mtlsProofOfPossession() | ||
| .build()); | ||
|
|
||
| assertMtlsPopResult(result, expectedLabThumbprint()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Acceptance blocker] This resource call uses the original credential rather than the binding returned by MSAL, so it does not satisfy BIND-01/BIND-07. Return |
||
| } | ||
|
|
||
| /** | ||
| * Direct SNI cert → mTLS PoP with a region configured (exercises the regional | ||
| * {@code <region>.mtlsauth.microsoft.com} endpoint), and verifies the bound token is cached and | ||
| * retrieved on a second call. | ||
| */ | ||
| @Test | ||
| void acquireTokenClientCredentials_Certificate_MtlsPop_Regional() throws Exception { | ||
| ConfidentialClientApplication cca = ConfidentialClientApplication.builder(SNI_ALLOWLISTED_APP_ID, certificate) | ||
| .authority(SNI_ALLOWLISTED_AUTHORITY) | ||
| .azureRegion(TEST_SLICE_REGION) | ||
| .build(); | ||
|
|
||
| IAuthenticationResult result = acquireMtlsPopOrSkipOnDowngrade(cca, ClientCredentialParameters | ||
| .builder(Collections.singleton(KEYVAULT_DEFAULT_SCOPE)) | ||
| .mtlsProofOfPossession() | ||
| .build()); | ||
|
|
||
| assertMtlsPopResult(result, expectedLabThumbprint()); | ||
|
|
||
| // The mTLS-PoP token must be cached under {token_type + cert KeyId} and returned on lookup. | ||
| IAuthenticationResult cached = acquireMtlsPopOrSkipOnDowngrade(cca, ClientCredentialParameters | ||
| .builder(Collections.singleton(KEYVAULT_DEFAULT_SCOPE)) | ||
| .mtlsProofOfPossession() | ||
| .build()); | ||
|
|
||
| assertEquals(result.accessToken(), cached.accessToken(), | ||
| "Second mTLS-PoP request should return the cached bound token"); | ||
| } | ||
|
|
||
| /** | ||
| * Requesting a Bearer token and an mTLS-PoP token for the same scope on the same app must yield two | ||
| * distinct tokens (cache isolation on {token_type + cert KeyId}), confirming the PoP path never | ||
| * aliases the existing SNI+Bearer path. | ||
| */ | ||
| @Test | ||
| void acquireTokenClientCredentials_BearerAndMtlsPop_AreCacheIsolated() throws Exception { | ||
| ConfidentialClientApplication cca = ConfidentialClientApplication.builder(SNI_ALLOWLISTED_APP_ID, certificate) | ||
| .authority(SNI_ALLOWLISTED_AUTHORITY) | ||
| .build(); | ||
|
|
||
| // Existing SNI + Bearer path (unchanged). | ||
| IAuthenticationResult bearer = cca.acquireToken(ClientCredentialParameters | ||
| .builder(Collections.singleton(KEYVAULT_DEFAULT_SCOPE)) | ||
| .build()) | ||
| .get(); | ||
| assertEquals(TokenType.BEARER, bearer.metadata().tokenType()); | ||
|
|
||
| // New SNI + mTLS PoP path. | ||
| IAuthenticationResult pop = acquireMtlsPopOrSkipOnDowngrade(cca, ClientCredentialParameters | ||
| .builder(Collections.singleton(KEYVAULT_DEFAULT_SCOPE)) | ||
| .mtlsProofOfPossession() | ||
| .build()); | ||
| assertEquals(TokenType.MTLS_POP, pop.metadata().tokenType()); | ||
|
|
||
| assertNotEquals(bearer.accessToken(), pop.accessToken(), | ||
| "Bearer and mTLS-PoP tokens for the same scope must be distinct cache entries"); | ||
| assertEquals(2, cca.tokenCache.accessTokens.size(), | ||
| "Bearer and mTLS-PoP tokens must occupy separate cache entries"); | ||
| } | ||
|
|
||
| // ESTS's mTLS PoP test slice is a known intermittent token_type downgrader. When it returns a | ||
| // non-mtls_pop token the access token is not certificate-bound, and MSAL now fails closed with | ||
| // TOKEN_TYPE_MISMATCH. Treat that specific outcome as inconclusive (skip) rather than a hard failure, | ||
| // mirroring MSAL .NET's ExecuteOrInconclusiveOnTokenTypeMismatchAsync. | ||
| private static IAuthenticationResult acquireMtlsPopOrSkipOnDowngrade( | ||
| ConfidentialClientApplication cca, ClientCredentialParameters parameters) throws Exception { | ||
| try { | ||
| return cca.acquireToken(parameters).get(); | ||
| } catch (ExecutionException e) { | ||
| if (e.getCause() instanceof MsalClientException | ||
| && AuthenticationErrorCode.TOKEN_TYPE_MISMATCH.equals( | ||
| ((MsalClientException) e.getCause()).errorCode())) { | ||
| Assumptions.abort("ESTS returned a non-mtls_pop token_type (downgrade); treating as " | ||
| + "inconclusive: " + e.getCause().getMessage()); | ||
| } | ||
| throw e; | ||
| } | ||
| } | ||
|
|
||
| private void assertMtlsPopResult(IAuthenticationResult result, String expectedThumbprint) { | ||
| assertNotNull(result, "Auth result should not be null"); | ||
| assertNotNull(result.accessToken(), "Access token should not be null"); | ||
| assertFalse(result.accessToken().isEmpty(), "Access token should not be empty"); | ||
| assertEquals(TokenType.MTLS_POP, result.metadata().tokenType(), | ||
| "Result token type should be MTLS_POP"); | ||
|
|
||
| BindingCertificate binding = result.metadata().bindingCertificate(); | ||
| assertNotNull(binding, "mTLS-PoP result must expose a binding certificate"); | ||
| assertNotNull(binding.thumbprintSha256(), "Binding certificate must expose its SHA-256 thumbprint"); | ||
| assertFalse(binding.certificateChain().isEmpty(), "Binding certificate must expose its x5c chain"); | ||
| assertEquals(expectedThumbprint, binding.thumbprintSha256(), | ||
| "Binding certificate thumbprint must match the lab SNI cert (x5t#S256)"); | ||
| } | ||
|
|
||
| private String expectedLabThumbprint() { | ||
| return MtlsClientCertificateHelper.computeThumbprintSha256(publicCertificate); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,17 @@ class AcquireTokenByClientCredentialSupplier extends AuthenticationResultSupplie | |
|
|
||
| @Override | ||
| AuthenticationResult execute() throws Exception { | ||
| // For mTLS Proof-of-Possession, isolate the access token in the cache by the binding certificate's | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Blocker] |
||
| // KeyId (x5t#S256) in addition to the token_type dimension, so PoP tokens bound to different | ||
| // certificates never alias. Stamped before the cache lookup so reads and writes hash identically. | ||
| IClientCertificate resolvedBindingCertificate = null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Blocker] This writes application-specific certificate identity into a public reusable parameters object. Two CCAs using certificates A and B can concurrently use the same parameters instance, causing one request to compute, read or save using the other application’s partition. Please keep public parameters immutable and carry an acquisition-local binding snapshot/cache key through lookup, execution and cache save. |
||
| if (clientCredentialRequest.parameters.mtlsProofOfPossession()) { | ||
| resolvedBindingCertificate = MtlsClientCertificateHelper.resolveBindingCertificate( | ||
| (ConfidentialClientApplication) this.clientApplication, clientCredentialRequest.parameters); | ||
| clientCredentialRequest.parameters.bindingCertificateKeyId( | ||
| MtlsClientCertificateHelper.computeCertificateKeyId(resolvedBindingCertificate)); | ||
| } | ||
|
|
||
| if (clientCredentialRequest.parameters.skipCache() != null && | ||
| !clientCredentialRequest.parameters.skipCache()) { | ||
| LOG.debug("SkipCache set to false. Attempting cache lookup"); | ||
|
|
@@ -50,7 +61,9 @@ AuthenticationResult execute() throws Exception { | |
| this.clientApplication, | ||
| silentRequest); | ||
|
|
||
| return supplier.execute(); | ||
| AuthenticationResult cachedResult = supplier.execute(); | ||
| restoreMtlsProofOfPossessionMetadata(cachedResult, resolvedBindingCertificate); | ||
| return cachedResult; | ||
| } catch (MsalClientException ex) { | ||
| LOG.debug("Cache lookup failed: {}", ex.getMessage()); | ||
| return acquireTokenByClientCredential(); | ||
|
|
@@ -61,6 +74,29 @@ AuthenticationResult execute() throws Exception { | |
| return acquireTokenByClientCredential(); | ||
| } | ||
|
|
||
| /** | ||
| * Restores the mTLS Proof-of-Possession metadata (token type and binding certificate) on a result | ||
| * served from the cache. The network path stamps these in {@link TokenRequestExecutor}, but the | ||
| * silent/cache read path does not, so without this a cached PoP token would report {@code BEARER} | ||
| * and a null binding certificate. | ||
| * | ||
| * <p>The cache is isolated by {@code token_type} and the certificate KeyId (x5t#S256), so a cache | ||
| * hit for an mTLS PoP request is guaranteed to be an {@code mtls_pop} token bound to this exact | ||
| * certificate; re-stamping it makes a cached PoP token report the same {@code tokenType()} and | ||
| * {@code bindingCertificate()} as a freshly issued one. Mirrors MSAL.NET, which persists/restores | ||
| * the token type from the cache item and re-runs the mTLS operation (re-stamping the binding | ||
| * certificate) on cache hits. | ||
| */ | ||
| private void restoreMtlsProofOfPossessionMetadata(AuthenticationResult result, | ||
| IClientCertificate bindingCertificate) { | ||
| if (result == null || bindingCertificate == null) { | ||
| return; | ||
| } | ||
| result.metadata().tokenType(TokenType.MTLS_POP); | ||
| result.metadata().bindingCertificate( | ||
| MtlsClientCertificateHelper.buildBindingCertificate(bindingCertificate)); | ||
| } | ||
|
|
||
| private AuthenticationResult acquireTokenByClientCredential() throws Exception { | ||
|
|
||
| if (this.clientCredentialRequest.appTokenProvider != null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this PR definitely weren't released a month ago.