Merge pull request #840 from AzureAD/avdunn/update-sha-method

Use SHA256 thumbprints in non-ADFS cert flows
AzureAD · Jul 23, 2024 · c83185c · c83185c
2 parents d3faa28 + 08a5189
commit c83185c
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 10 deletions.
diff --git a/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java b/msal4j-sdk/src/integrationtest/java/com.microsoft.aad.msal4j/ClientCredentialsIT.java
@@ -160,7 +160,7 @@ private ClientAssertion getClientAssertion(String clientId) {
                 clientId,
                 (ClientCertificate) certificate,
                 "https://login.microsoftonline.com/common/oauth2/v2.0/token",
-                true);
+                true, false);
     }
 
     private void assertAcquireTokenCommon(String clientId, IClientCredential credential, String authority) throws Exception {

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java
@@ -8,8 +8,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.MessageDigest;
@@ -21,7 +19,6 @@
 import java.security.cert.CertificateEncodingException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
-import java.security.interfaces.RSAPrivateKey;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Base64;
@@ -53,7 +50,14 @@ public String publicCertificateHash()
             throws CertificateEncodingException, NoSuchAlgorithmException {
 
         return Base64.getEncoder().encodeToString(ClientCertificate
-                .getHash(publicKeyCertificateChain.get(0).getEncoded()));
+                .getHashSha256(publicKeyCertificateChain.get(0).getEncoded()));
+    }
+
+    public String publicCertificateHashSha1()
+            throws CertificateEncodingException, NoSuchAlgorithmException {
+
+        return Base64.getEncoder().encodeToString(ClientCertificate
+                .getHashSha1(publicKeyCertificateChain.get(0).getEncoded()));
     }
 
     public List<String> getEncodedPublicKeyCertificateChain() throws CertificateEncodingException {
@@ -119,9 +123,15 @@ static ClientCertificate create(final PrivateKey key, final X509Certificate publ
         return new ClientCertificate(key, Arrays.asList(publicKeyCertificate));
     }
 
-    private static byte[] getHash(final byte[] inputBytes) throws NoSuchAlgorithmException {
+    private static byte[] getHashSha1(final byte[] inputBytes) throws NoSuchAlgorithmException {
         final MessageDigest md = MessageDigest.getInstance("SHA-1");
         md.update(inputBytes);
         return md.digest();
     }
+
+    private static byte[] getHashSha256(final byte[] inputBytes) throws NoSuchAlgorithmException {
+        final MessageDigest md = MessageDigest.getInstance("SHA-256");
+        md.update(inputBytes);
+        return md.digest();
+    }
 }
diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java
@@ -101,7 +101,11 @@ private void initClientAuthentication(IClientCredential clientCredential) {
         } else if (clientCredential instanceof ClientCertificate) {
             this.clientCertAuthentication = true;
             this.clientCertificate = (ClientCertificate) clientCredential;
-            clientAuthentication = buildValidClientCertificateAuthority();
+            if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
+                clientAuthentication = buildValidClientCertificateAuthoritySha1();
+            } else  {
+                clientAuthentication = buildValidClientCertificateAuthority();
+            }
         } else if (clientCredential instanceof ClientAssertion) {
             clientAuthentication = createClientAuthFromClientAssertion((ClientAssertion) clientCredential);
         } else {
@@ -127,7 +131,20 @@ private ClientAuthentication buildValidClientCertificateAuthority() {
                 clientId(),
                 clientCertificate,
                 this.authenticationAuthority.selfSignedJwtAudience(),
-                sendX5c);
+                sendX5c,
+                false);
+        return createClientAuthFromClientAssertion(clientAssertion);
+    }
+
+    //The library originally used SHA-1 for thumbprints as other algorithms were not supported server-side,
+    //  and while support for SHA-256 has been added certain flows still only allow SHA-1
+    private ClientAuthentication buildValidClientCertificateAuthoritySha1() {
+        ClientAssertion clientAssertion = JwtHelper.buildJwt(
+                clientId(),
+                clientCertificate,
+                this.authenticationAuthority.selfSignedJwtAudience(),
+                sendX5c,
+                true);
         return createClientAuthFromClientAssertion(clientAssertion);
     }
 

diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java
@@ -21,7 +21,8 @@
 final class JwtHelper {
 
     static ClientAssertion buildJwt(String clientId, final ClientCertificate credential,
-                                    final String jwtAudience, boolean sendX5c) throws MsalClientException {
+                                    final String jwtAudience, boolean sendX5c,
+                                    boolean useSha1) throws MsalClientException {
         if (StringHelper.isBlank(clientId)) {
             throw new IllegalArgumentException("clientId is null or empty");
         }
@@ -55,7 +56,12 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent
                 builder.x509CertChain(certs);
             }
 
-            builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHash()));
+            //SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side
+            if (useSha1) {
+                builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHashSha1()));
+            } else {
+                builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash()));
+            }
 
             jwt = new SignedJWT(builder.build(), claimsSet);
             final RSASSASigner signer = new RSASSASigner(credential.privateKey());