8165274: SHA1 certpath constraint check fails with OCSP certificate

Reviewed-by: mullan, jnimeh

jdk/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java

@@ -185,11 +185,7 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
             AlgorithmConstraints constraints,
             Date pkixdate) {
 
-        if (anchor == null) {
+        if (anchor != null) {
-            throw new IllegalArgumentException(
-                    "The trust anchor cannot be null");
-        }
-
             if (anchor.getTrustedCert() != null) {
                 this.trustedPubKey = anchor.getTrustedCert().getPublicKey();
                 // Check for anchor certificate restrictions
@@ -200,6 +196,12 @@ public final class AlgorithmChecker extends PKIXCertPathChecker {
             } else {
                 this.trustedPubKey = anchor.getCAPublicKey();
             }
+        } else {
+            this.trustedPubKey = null;
+            if (debug != null) {
+                debug.println("TrustAnchor is null, trustedMatch is false.");
+            }
+        }
 
         this.prevPubKey = trustedPubKey;
         this.constraints = constraints;

jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSP.java

@@ -35,6 +35,7 @@ import java.security.cert.CertPathValidatorException;
 import java.security.cert.CertPathValidatorException.BasicReason;
 import java.security.cert.CRLReason;
 import java.security.cert.Extension;
+import java.security.cert.TrustAnchor;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.Collections;
@@ -163,6 +164,15 @@ public final class OCSP {
                                          X509Certificate responderCert,
                                          Date date, List<Extension> extensions)
         throws IOException, CertPathValidatorException
+    {
+        return check(cert, responderURI, null, issuerCert, responderCert, date, extensions);
+    }
+
+    public static RevocationStatus check(X509Certificate cert,
+            URI responderURI, TrustAnchor anchor, X509Certificate issuerCert,
+            X509Certificate responderCert, Date date,
+            List<Extension> extensions)
+            throws IOException, CertPathValidatorException
     {
         CertId certId = null;
         try {
@@ -173,7 +183,7 @@ public final class OCSP {
                 ("Exception while encoding OCSPRequest", e);
         }
         OCSPResponse ocspResponse = check(Collections.singletonList(certId),
-            responderURI, new OCSPResponse.IssuerInfo(issuerCert),
+                responderURI, new OCSPResponse.IssuerInfo(anchor, issuerCert),
                 responderCert, date, extensions);
         return (RevocationStatus) ocspResponse.getSingleResponse(certId);
     }

jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java

@@ -507,9 +507,8 @@ public final class OCSPResponse {
 
                 // Check algorithm constraints specified in security property
                 // "jdk.certpath.disabledAlgorithms".
-                AlgorithmChecker algChecker = new AlgorithmChecker(
+                AlgorithmChecker algChecker =
-                        new TrustAnchor(issuerInfo.getName(),
+                        new AlgorithmChecker(issuerInfo.getAnchor(), date);
-                                issuerInfo.getPublicKey(), null));
                 algChecker.init(false);
                 algChecker.check(signerCert, Collections.<String>emptySet());
 
@@ -982,36 +981,38 @@ public final class OCSPResponse {
     /**
      * Helper class that allows consumers to pass in issuer information.  This
      * will always consist of the issuer's name and public key, but may also
-     * contain a certificate if the originating data is in that form.
+     * contain a certificate if the originating data is in that form.  The
+     * trust anchor for the certificate chain will be included for certpath
+     * disabled algorithm checking.
      */
     static final class IssuerInfo {
+        private final TrustAnchor anchor;
         private final X509Certificate certificate;
         private final X500Principal name;
         private final PublicKey pubKey;
 
-        IssuerInfo(X509Certificate issuerCert) {
-            certificate = Objects.requireNonNull(issuerCert,
-                    "Constructor requires non-null certificate");
-            name = certificate.getSubjectX500Principal();
-            pubKey = certificate.getPublicKey();
-        }
-
-        IssuerInfo(X500Principal subjectName, PublicKey key) {
-            certificate = null;
-            name = Objects.requireNonNull(subjectName,
-                    "Constructor requires non-null subject");
-            pubKey = Objects.requireNonNull(key,
-                    "Constructor requires non-null public key");
-        }
-
         IssuerInfo(TrustAnchor anchor) {
-            certificate = anchor.getTrustedCert();
+            this(anchor, (anchor != null) ? anchor.getTrustedCert() : null);
-            if (certificate != null) {
+        }
-                name = certificate.getSubjectX500Principal();
+
-                pubKey = certificate.getPublicKey();
+        IssuerInfo(X509Certificate issuerCert) {
+            this(null, issuerCert);
+        }
+
+        IssuerInfo(TrustAnchor anchor, X509Certificate issuerCert) {
+            if (anchor == null && issuerCert == null) {
+                throw new NullPointerException("TrustAnchor and issuerCert " +
+                        "cannot be null");
+            }
+            this.anchor = anchor;
+            if (issuerCert != null) {
+                name = issuerCert.getSubjectX500Principal();
+                pubKey = issuerCert.getPublicKey();
+                certificate = issuerCert;
             } else {
                 name = anchor.getCA();
                 pubKey = anchor.getCAPublicKey();
+                certificate = anchor.getTrustedCert();
             }
         }
 
@@ -1046,6 +1047,15 @@ public final class OCSPResponse {
             return pubKey;
         }
 
+        /**
+         * Get the TrustAnchor for the certificate chain.
+         *
+         * @return a {@code TrustAnchor}.
+         */
+        TrustAnchor getAnchor() {
+            return anchor;
+        }
+
         /**
          * Create a string representation of this IssuerInfo.
          *

jdk/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java

@@ -437,7 +437,7 @@ class RevocationChecker extends PKIXRevocationChecker {
     private void updateState(X509Certificate cert)
         throws CertPathValidatorException
     {
-        issuerInfo = new OCSPResponse.IssuerInfo(cert);
+        issuerInfo = new OCSPResponse.IssuerInfo(anchor, cert);
 
         // Make new public key if parameters are missing
         PublicKey pubKey = cert.getPublicKey();
@@ -740,8 +740,8 @@ class RevocationChecker extends PKIXRevocationChecker {
                 }
 
                 response = OCSP.check(Collections.singletonList(certId),
-                                      responderURI, issuerInfo,
+                        responderURI, issuerInfo, responderCert, params.date(),
-                                      responderCert, null, ocspExtensions);
+                        ocspExtensions);
             }
         } catch (IOException e) {
             throw new CertPathValidatorException(

jdk/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java

@@ -530,7 +530,8 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                 }
                 throw new CertPathValidatorException(
                         "Algorithm constraints check failed on certificate " +
-                                "anchor limits",
+                                "anchor limits. " + algorithm + " used with " +
+                                cp.getCertificate().getSubjectX500Principal(),
                         null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
             }
         }
@@ -611,8 +612,8 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                      return;
                  }
                  throw new CertPathValidatorException(
-                         "denyAfter constraint check failed.  " +
+                         "denyAfter constraint check failed: " + algorithm +
-                                 "Constraint date: " +
+                                 " used with Constraint date: " +
                                  dateFormat.format(denyAfterDate) + "; "
                                  + errmsg + dateFormat.format(currentDate),
                          null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
@@ -644,6 +645,7 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
         private int minSize;            // the minimal available key size
         private int maxSize;            // the maximal available key size
         private int prohibitedSize = -1;    // unavailable key sizes
+        private int size;
 
         public KeySizeConstraint(String algo, Operator operator, int length) {
             algorithm = algo;
@@ -695,7 +697,9 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                     return;
                 }
                 throw new CertPathValidatorException(
-                        "Algorithm constraints check failed on keysize limits",
+                        "Algorithm constraints check failed on keysize limits. "
+                                + algorithm + " " + size + "bit key used with "
+                                + cp.getCertificate().getSubjectX500Principal(),
                         null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
             }
         }
@@ -722,7 +726,7 @@ public class DisabledAlgorithmConstraints extends AbstractAlgorithmConstraints {
                 return true;
             }
 
-            int size = KeyUtil.getKeySize(key);
+            size = KeyUtil.getKeySize(key);
             if (size == 0) {
                 return false;    // we don't allow any key of size 0.
             } else if (size > 0) {