Add new default method to IClientCertificate to handle SHA256

Author: Avery-Dunn

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java

@@ -46,14 +46,15 @@ final class ClientCertificate implements IClientCertificate {
         this.publicKeyCertificateChain = publicKeyCertificateChain;
     }
 
-    public String publicCertificateHash()
+    @Override
+    public String publicCertificateHash256()
             throws CertificateEncodingException, NoSuchAlgorithmException {
 
         return Base64.getEncoder().encodeToString(ClientCertificate
                 .getHashSha256(publicKeyCertificateChain.get(0).getEncoded()));
     }
 
-    public String publicCertificateHashSha1()
+    public String publicCertificateHash()
             throws CertificateEncodingException, NoSuchAlgorithmException {
 
         return Base64.getEncoder().encodeToString(ClientCertificate

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java

@@ -102,9 +102,10 @@ private void initClientAuthentication(IClientCredential clientCredential) {
             this.clientCertAuthentication = true;
             this.clientCertificate = (ClientCertificate) clientCredential;
             if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
-                clientAuthentication = buildValidClientCertificateAuthoritySha1();
+                //When this was added, ADFS did not support SHA256 hashes for client certificates
+                clientAuthentication = buildValidClientCertificateAuthority(true);
             } else  {
-                clientAuthentication = buildValidClientCertificateAuthority();
+                clientAuthentication = buildValidClientCertificateAuthority(false);
             }
         } else if (clientCredential instanceof ClientAssertion) {
             clientAuthentication = createClientAuthFromClientAssertion((ClientAssertion) clientCredential);
@@ -119,32 +120,25 @@ protected ClientAuthentication clientAuthentication() {
             final Date currentDateTime = new Date(System.currentTimeMillis());
             final Date expirationTime = ((PrivateKeyJWT) clientAuthentication).getJWTAuthenticationClaimsSet().getExpirationTime();
             if (expirationTime.before(currentDateTime)) {
-                //The asserted private jwt with the client certificate can expire so rebuild it when the
-                clientAuthentication = buildValidClientCertificateAuthority();
+                if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
+                    clientAuthentication = buildValidClientCertificateAuthority(true);
+                } else  {
+                    clientAuthentication = buildValidClientCertificateAuthority(false);
+                }
             }
         }
         return clientAuthentication;
     }
 
-    private ClientAuthentication buildValidClientCertificateAuthority() {
-        ClientAssertion clientAssertion = JwtHelper.buildJwt(
-                clientId(),
-                clientCertificate,
-                this.authenticationAuthority.selfSignedJwtAudience(),
-                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() {
+    private ClientAuthentication buildValidClientCertificateAuthority(boolean useSha1) {
         ClientAssertion clientAssertion = JwtHelper.buildJwt(
                 clientId(),
                 clientCertificate,
                 this.authenticationAuthority.selfSignedJwtAudience(),
                 sendX5c,
-                true);
+                useSha1);
         return createClientAuthFromClientAssertion(clientAssertion);
     }
 

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IClientCertificate.java

@@ -23,7 +23,20 @@ public interface IClientCertificate extends IClientCredential {
     PrivateKey privateKey();
 
     /**
-     * Base64 encoded hash of the the public certificate.
+     * Base64 encoded SHA-256 hash of the public certificate.
+     *
+     * @return base64 encoded string
+     * @throws CertificateEncodingException if an encoding error occurs
+     * @throws NoSuchAlgorithmException     if requested algorithm is not available in the environment
+     */
+    default String publicCertificateHash256() throws CertificateEncodingException, NoSuchAlgorithmException {
+        //Default implementation that returns null, to add backwards compatibility for those implementing this public interface.
+        //If left as null, the library will default to the older publicCertificateHash() method and SHA-1 hashing.
+        return null;
+    }
+
+    /**
+     * Base64 encoded SHA-1 hash of the public certificate.
      *
      * @return base64 encoded string
      * @throws CertificateEncodingException if an encoding error occurs

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java

@@ -56,11 +56,12 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent
                 builder.x509CertChain(certs);
             }
 
-            //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()));
+            //SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side. If SHA-256
+            // is not supported or the IClientCredential.publicCertificateHash256() method is not implemented, the library will default to SHA-1.
+            if (useSha1 || credential.publicCertificateHash256() == null) {
+                builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHash()));
             } else {
-                builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash()));
+                builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash256()));
             }
 
             jwt = new SignedJWT(builder.build(), claimsSet);

msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/ClientCertificateTest.java

@@ -8,13 +8,18 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
 import java.math.BigInteger;
+import java.security.NoSuchAlgorithmException;
 import java.security.PrivateKey;
+import java.security.cert.CertificateException;
 import java.security.interfaces.RSAPrivateKey;
+import java.util.Collections;
+import java.util.List;
 
 @TestInstance(TestInstance.Lifecycle.PER_CLASS)
 class ClientCertificateTest {
@@ -36,4 +41,35 @@ void testGetClient() {
         final ClientCertificate kc = ClientCertificate.create(key, null);
         assertNotNull(kc);
     }
+
+    @Test
+    void testIClientCertificateInterface_Sha256AndSha1() throws NoSuchAlgorithmException, CertificateException {
+        //See https://github.com/AzureAD/microsoft-authentication-library-for-java/issues/863 for context on this test.
+        //Essentially, it aims to test compatibility for customers that implemented IClientCertificate in older versions of the library.
+
+        //IClientCertificate.publicCertificateHash256() returns null by default if not implemented...
+        IClientCertificate certificate = new TestClientCredential();
+        assertNull(certificate.publicCertificateHash256());
+
+        //... but ClientCredentialFactory has an implemented version, so it should not be null.
+        certificate = ClientCredentialFactory.createFromCertificate(TestHelper.getPrivateKey(), TestHelper.getX509Cert());
+        assertNotNull(certificate.publicCertificateHash256());
+    }
+
+    class TestClientCredential implements IClientCertificate {
+        @Override
+        public PrivateKey privateKey() {
+            return null;
+        }
+
+        @Override
+        public String publicCertificateHash() {
+            return "";
+        }
+
+        @Override
+        public List<String> getEncodedPublicKeyCertificateChain() {
+            return Collections.emptyList();
+        }
+    }
 }

msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java

@@ -7,13 +7,18 @@
 import com.nimbusds.jose.crypto.RSASSASigner;
 import com.nimbusds.jose.jwk.RSAKey;
 import com.nimbusds.jose.jwk.gen.RSAKeyGenerator;
+import sun.security.tools.keytool.CertAndKeyGen;
+import sun.security.x509.X500Name;
 
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Paths;
+import java.security.*;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -29,6 +34,9 @@ class TestHelper {
             "\"expires_on\": %d ,\"expires_in\": %d," +
             "\"token_type\":\"Bearer\"}";
 
+    static X509Certificate x509Cert = getX509Cert();
+    static PrivateKey privateKey = getPrivateKey();
+
     static String readResource(Class<?> classInstance, String resource) {
         try {
             return new String(Files.readAllBytes(Paths.get(classInstance.getResource(resource).toURI())));
@@ -97,4 +105,34 @@ static HttpResponse expectedResponse(int statusCode, String response) {
 
         return httpResponse;
     }
+
+    static void setPrivateKeyAndCert() {
+        try {
+            CertAndKeyGen certGen = new CertAndKeyGen("RSA", "SHA256WithRSA", null);
+            certGen.generate(2048);
+            X509Certificate cert = certGen.getSelfCertificate(
+                    new X500Name(""), 3600);
+
+            x509Cert = cert;
+            privateKey = certGen.getPrivateKey();
+        } catch (NoSuchAlgorithmException | NoSuchProviderException | InvalidKeyException | IOException | CertificateException | SignatureException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    static X509Certificate getX509Cert() {
+        if (x509Cert == null) {
+            setPrivateKeyAndCert();
+        }
+
+        return x509Cert;
+    }
+
+    static PrivateKey getPrivateKey() {
+        if (privateKey == null) {
+            setPrivateKeyAndCert();
+        }
+
+        return privateKey;
+    }
 }