8165274: SHA1 certpath constraint check fails with OCSP certificate
authorascarpino
Tue, 18 Oct 2016 15:13:11 -0700
changeset 41562 1e040ccac110
parent 41561 0c6942d13f2e
child 41563 af8478174e7b
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
jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSP.java
jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java
jdk/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java
jdk/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java
--- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java	Tue Oct 18 13:27:00 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java	Tue Oct 18 15:13:11 2016 -0700
@@ -185,20 +185,22 @@
             AlgorithmConstraints constraints,
             Date pkixdate) {
 
-        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
-            trustedMatch = checkFingerprint(anchor.getTrustedCert());
-            if (trustedMatch && debug != null) {
-                debug.println("trustedMatch = true");
+        if (anchor != null) {
+            if (anchor.getTrustedCert() != null) {
+                this.trustedPubKey = anchor.getTrustedCert().getPublicKey();
+                // Check for anchor certificate restrictions
+                trustedMatch = checkFingerprint(anchor.getTrustedCert());
+                if (trustedMatch && debug != null) {
+                    debug.println("trustedMatch = true");
+                }
+            } else {
+                this.trustedPubKey = anchor.getCAPublicKey();
             }
         } else {
-            this.trustedPubKey = anchor.getCAPublicKey();
+            this.trustedPubKey = null;
+            if (debug != null) {
+                debug.println("TrustAnchor is null, trustedMatch is false.");
+            }
         }
 
         this.prevPubKey = trustedPubKey;
--- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSP.java	Tue Oct 18 13:27:00 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSP.java	Tue Oct 18 15:13:11 2016 -0700
@@ -35,6 +35,7 @@
 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;
@@ -164,6 +165,15 @@
                                          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 {
             X509CertImpl certImpl = X509CertImpl.toImpl(cert);
@@ -173,8 +183,8 @@
                 ("Exception while encoding OCSPRequest", e);
         }
         OCSPResponse ocspResponse = check(Collections.singletonList(certId),
-            responderURI, new OCSPResponse.IssuerInfo(issuerCert),
-            responderCert, date, extensions);
+                responderURI, new OCSPResponse.IssuerInfo(anchor, issuerCert),
+                responderCert, date, extensions);
         return (RevocationStatus) ocspResponse.getSingleResponse(certId);
     }
 
--- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java	Tue Oct 18 13:27:00 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java	Tue Oct 18 15:13:11 2016 -0700
@@ -507,9 +507,8 @@
 
                 // Check algorithm constraints specified in security property
                 // "jdk.certpath.disabledAlgorithms".
-                AlgorithmChecker algChecker = new AlgorithmChecker(
-                        new TrustAnchor(issuerInfo.getName(),
-                                issuerInfo.getPublicKey(), null));
+                AlgorithmChecker algChecker =
+                        new AlgorithmChecker(issuerInfo.getAnchor(), date);
                 algChecker.init(false);
                 algChecker.check(signerCert, Collections.<String>emptySet());
 
@@ -982,36 +981,38 @@
     /**
      * 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(TrustAnchor anchor) {
+            this(anchor, (anchor != null) ? anchor.getTrustedCert() : null);
+        }
+
         IssuerInfo(X509Certificate issuerCert) {
-            certificate = Objects.requireNonNull(issuerCert,
-                    "Constructor requires non-null certificate");
-            name = certificate.getSubjectX500Principal();
-            pubKey = certificate.getPublicKey();
+            this(null, issuerCert);
         }
 
-        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();
-            if (certificate != null) {
-                name = certificate.getSubjectX500Principal();
-                pubKey = certificate.getPublicKey();
+        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();
             }
         }
 
@@ -1047,6 +1048,15 @@
         }
 
         /**
+         * Get the TrustAnchor for the certificate chain.
+         *
+         * @return a {@code TrustAnchor}.
+         */
+        TrustAnchor getAnchor() {
+            return anchor;
+        }
+
+        /**
          * Create a string representation of this IssuerInfo.
          *
          * @return a {@code String} form of this IssuerInfo object.
--- a/jdk/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java	Tue Oct 18 13:27:00 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java	Tue Oct 18 15:13:11 2016 -0700
@@ -437,7 +437,7 @@
     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 @@
                 }
 
                 response = OCSP.check(Collections.singletonList(certId),
-                                      responderURI, issuerInfo,
-                                      responderCert, null, ocspExtensions);
+                        responderURI, issuerInfo, responderCert, params.date(),
+                        ocspExtensions);
             }
         } catch (IOException e) {
             throw new CertPathValidatorException(
--- a/jdk/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java	Tue Oct 18 13:27:00 2016 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java	Tue Oct 18 15:13:11 2016 -0700
@@ -530,7 +530,8 @@
                 }
                 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 @@
                      return;
                  }
                  throw new CertPathValidatorException(
-                         "denyAfter constraint check failed.  " +
-                                 "Constraint date: " +
+                         "denyAfter constraint check failed: " + algorithm +
+                                 " used with Constraint date: " +
                                  dateFormat.format(denyAfterDate) + "; "
                                  + errmsg + dateFormat.format(currentDate),
                          null, null, -1, BasicReason.ALGORITHM_CONSTRAINED);
@@ -644,6 +645,7 @@
         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 @@
                     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 @@
                 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) {