changeset 3998:6e306c3aa17b

6882437: CertPath/X509CertPathDiscovery/Test fails on jdk7/pit/b62 Summary: Pass trust anchors to CRL certification path building, support CRLs without AKID extension. Reviewed-by: mullan
author xuelei
date Tue, 12 Apr 2011 08:27:00 -0700
parents 9128eace50f5
children 1bb95f6ac753
files src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java
diffstat 2 files changed, 70 insertions(+), 60 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java	Tue Apr 12 13:14:05 2011 +0200
+++ b/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java	Tue Apr 12 08:27:00 2011 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -249,7 +249,7 @@
         throws CertPathValidatorException
     {
         verifyRevocationStatus(currCert, prevKey, signFlag,
-                               allowSeparateKey, null);
+                   allowSeparateKey, null, mParams.getTrustAnchors());
     }
 
     /**
@@ -260,11 +260,12 @@
      *                     circular dependencies, we assume they're
      *                     revoked while checking the revocation
      *                     status of this cert.
+     * @param trustAnchors a <code>Set</code> of <code>TrustAnchor</code>s
      */
     private void verifyRevocationStatus(X509Certificate currCert,
         PublicKey prevKey, boolean signFlag, boolean allowSeparateKey,
-        Set<X509Certificate> stackedCerts) throws CertPathValidatorException
-    {
+        Set<X509Certificate> stackedCerts,
+        Set<TrustAnchor> trustAnchors) throws CertPathValidatorException {
 
         String msg = "revocation status";
         if (debug != null) {
@@ -311,7 +312,7 @@
                 DistributionPointFetcher.getInstance();
             // all CRLs returned by the DP Fetcher have also been verified
             mApprovedCRLs.addAll(store.getCRLs(sel, signFlag, prevKey,
-                mSigProvider, mStores, reasonsMask, mAnchor));
+                mSigProvider, mStores, reasonsMask, trustAnchors));
         } catch (Exception e) {
             if (debug != null) {
                 debug.println("CrlRevocationChecker.verifyRevocationStatus() "
@@ -328,7 +329,7 @@
             // Now that we have a list of possible CRLs, see which ones can
             // be approved
             mApprovedCRLs.addAll(verifyPossibleCRLs(mPossibleCRLs, currCert,
-                signFlag, prevKey, reasonsMask));
+                signFlag, prevKey, reasonsMask, trustAnchors));
         }
         if (debug != null) {
             debug.println("CrlRevocationChecker.verifyRevocationStatus() " +
@@ -353,9 +354,10 @@
         // See if the cert is in the set of approved crls.
         if (debug != null) {
             BigInteger sn = currCert.getSerialNumber();
-            debug.println("starting the final sweep...");
+            debug.println("CrlRevocationChecker.verifyRevocationStatus() " +
+                            "starting the final sweep...");
             debug.println("CrlRevocationChecker.verifyRevocationStatus" +
-                          " cert SN: " + sn.toString());
+                            " cert SN: " + sn.toString());
         }
 
         CRLReason reasonCode = CRLReason.UNSPECIFIED;
@@ -497,9 +499,9 @@
         certSel.setSubject(currCert.getIssuerX500Principal());
         certSel.setKeyUsage(mCrlSignUsage);
 
-        Set<TrustAnchor> newAnchors = mAnchor == null
-            ? mParams.getTrustAnchors()
-            : Collections.singleton(mAnchor);
+        Set<TrustAnchor> newAnchors =
+            (mAnchor == null ? mParams.getTrustAnchors() :
+                                Collections.singleton(mAnchor));
 
         PKIXBuilderParameters builderParams;
         if (mParams instanceof PKIXBuilderParameters) {
@@ -617,8 +619,8 @@
                             debug.println("CrlRevocationChecker.buildToNewKey()"
                                 + " index " + i + " checking " + cert);
                         }
-                        verifyRevocationStatus(cert, prevKey2, signFlag,
-                                               true, stackedCerts);
+                        verifyRevocationStatus(cert, prevKey2, signFlag, true,
+                                stackedCerts, newAnchors);
                         signFlag = certCanSignCrl(cert);
                         prevKey2 = cert.getPublicKey();
                     }
@@ -727,12 +729,14 @@
      * @param signFlag <code>true</code> if prevKey was trusted to sign CRLs
      * @param prevKey the public key of the issuer of cert
      * @param reasonsMask the reason code mask
+     * @param trustAnchors a <code>Set</code> of <code>TrustAnchor</code>s>
      * @return a collection of approved crls (or an empty collection)
      */
     private Collection<X509CRL> verifyPossibleCRLs(Set<X509CRL> crls,
         X509Certificate cert, boolean signFlag, PublicKey prevKey,
-        boolean[] reasonsMask) throws CertPathValidatorException
-    {
+        boolean[] reasonsMask,
+        Set<TrustAnchor> trustAnchors) throws CertPathValidatorException {
+
         try {
             X509CertImpl certImpl = X509CertImpl.toImpl(cert);
             if (debug != null) {
@@ -764,7 +768,8 @@
                 DistributionPoint point = t.next();
                 for (X509CRL crl : crls) {
                     if (dpf.verifyCRL(certImpl, point, crl, reasonsMask,
-                        signFlag, prevKey, mSigProvider, mAnchor, mStores)) {
+                            signFlag, prevKey, mSigProvider,
+                            trustAnchors, mStores)) {
                         results.add(crl);
                     }
                 }
--- a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java	Tue Apr 12 13:14:05 2011 +0200
+++ b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java	Tue Apr 12 08:27:00 2011 -0700
@@ -90,8 +90,9 @@
      */
     Collection<X509CRL> getCRLs(X509CRLSelector selector, boolean signFlag,
         PublicKey prevKey, String provider, List<CertStore> certStores,
-        boolean[] reasonsMask, TrustAnchor anchor) throws CertStoreException
-    {
+        boolean[] reasonsMask,
+        Set<TrustAnchor> trustAnchors) throws CertStoreException {
+
         if (USE_CRLDP == false) {
             return Collections.emptySet();
         }
@@ -121,7 +122,7 @@
                 DistributionPoint point = t.next();
                 Collection<X509CRL> crls = getCRLs(selector, certImpl,
                     point, reasonsMask, signFlag, prevKey, provider,
-                    certStores, anchor);
+                    certStores, trustAnchors);
                 results.addAll(crls);
             }
             if (debug != null) {
@@ -142,8 +143,8 @@
     private Collection<X509CRL> getCRLs(X509CRLSelector selector,
         X509CertImpl certImpl, DistributionPoint point, boolean[] reasonsMask,
         boolean signFlag, PublicKey prevKey, String provider,
-        List<CertStore> certStores, TrustAnchor anchor)
-    {
+        List<CertStore> certStores, Set<TrustAnchor> trustAnchors) {
+
         // check for full name
         GeneralNames fullName = point.getFullName();
         if (fullName == null) {
@@ -194,7 +195,7 @@
                 // we check the issuer in verifyCRLs method
                 selector.setIssuerNames(null);
                 if (selector.match(crl) && verifyCRL(certImpl, point, crl,
-                        reasonsMask, signFlag, prevKey, provider, anchor,
+                        reasonsMask, signFlag, prevKey, provider, trustAnchors,
                         certStores)) {
                     crls.add(crl);
                 }
@@ -276,12 +277,17 @@
      * @param signFlag true if prevKey can be used to verify the CRL
      * @param prevKey the public key that verifies the certificate's signature
      * @param provider the Signature provider to use
+     * @param trustAnchors a {@code Set} of {@code TrustAnchor}s
+     * @param certStores a {@code List} of {@code CertStore}s to be used in
+     *        finding certificates and CRLs
      * @return true if ok, false if not
      */
     boolean verifyCRL(X509CertImpl certImpl, DistributionPoint point,
         X509CRL crl, boolean[] reasonsMask, boolean signFlag,
-        PublicKey prevKey, String provider, TrustAnchor anchor,
+        PublicKey prevKey, String provider,
+        Set<TrustAnchor> trustAnchors,
         List<CertStore> certStores) throws CRLException, IOException {
+
         boolean indirectCRL = false;
         X509CRLImpl crlImpl = X509CRLImpl.toImpl(crl);
         IssuingDistributionPointExtension idpExt =
@@ -335,7 +341,16 @@
             byte[] crlAKID = crlImpl.getExtensionValue(
                                 PKIXExtensions.AuthorityKey_Id.toString());
 
-            if (!Arrays.equals(certAKID, crlAKID)) {
+            if (certAKID == null || crlAKID == null) {
+                // cannot recognize indirect CRL without AKID
+
+                // we accept the case that a CRL issuer provide status
+                // information for itself.
+                if (issues(certImpl, crlImpl, provider)) {
+                    // reset the public key used to verify the CRL's signature
+                    prevKey = certImpl.getPublicKey();
+                }
+            } else if (!Arrays.equals(certAKID, crlAKID)) {
                 // we accept the case that a CRL issuer provide status
                 // information for itself.
                 if (issues(certImpl, crlImpl, provider)) {
@@ -572,46 +587,19 @@
             // Except the performance improvement, another benefit is to break
             // the dead loop while looking for the issuer back and forth
             // between the delegated self-issued certificate and its issuer.
-            Set<TrustAnchor> trustAnchors = new HashSet<TrustAnchor>();
-            if (anchor != null) {
-                trustAnchors.add(anchor);
-            }
+            Set<TrustAnchor> newTrustAnchors = new HashSet<>(trustAnchors);
 
             if (prevKey != null) {
-                // if the previous key is of the anchor, don't bother to
-                // duplicate the trust.
-                boolean duplicated = false;
-                PublicKey publicKey = prevKey;
+                // Add the previous certificate as a trust anchor.
                 X500Principal principal = certImpl.getIssuerX500Principal();
-
-                if (anchor != null) {
-                    X509Certificate trustedCert = anchor.getTrustedCert();
-                    X500Principal trustedPrincipal;
-                    PublicKey trustedPublicKey;
-                    if (trustedCert != null) {
-                        trustedPrincipal = trustedCert.getSubjectX500Principal();
-                        trustedPublicKey = trustedCert.getPublicKey();
-                    } else {
-                        trustedPrincipal = anchor.getCA();
-                        trustedPublicKey = anchor.getCAPublicKey();
-                    }
-
-                    if (principal.equals(trustedPrincipal) &&
-                        publicKey.equals(trustedPublicKey)) {
-                        duplicated = true;
-                    }
-                }
-
-                if (!duplicated) {
-                    TrustAnchor temporary =
-                        new TrustAnchor(principal, publicKey, null);
-                    trustAnchors.add(temporary);
-                }
+                TrustAnchor temporary =
+                        new TrustAnchor(principal, prevKey, null);
+                newTrustAnchors.add(temporary);
             }
 
             PKIXBuilderParameters params = null;
             try {
-                params = new PKIXBuilderParameters(trustAnchors, certSel);
+                params = new PKIXBuilderParameters(newTrustAnchors, certSel);
             } catch (InvalidAlgorithmParameterException iape) {
                 throw new CRLException(iape);
             }
@@ -697,6 +685,8 @@
     private static boolean issues(X509CertImpl cert, X509CRLImpl crl,
             String provider) throws IOException {
 
+        boolean matched = false;
+
         AdaptableX509CertSelector issuerSelector =
                                     new AdaptableX509CertSelector();
 
@@ -719,9 +709,24 @@
          * and MUST include authority key identifier extension in all CRLs
          * issued. [section 5.2.1, RFC 2459]
          */
-        issuerSelector.parseAuthorityKeyIdentifierExtension(
-                                        crl.getAuthKeyIdExtension());
+        AuthorityKeyIdentifierExtension crlAKID = crl.getAuthKeyIdExtension();
+        if (crlAKID != null) {
+            issuerSelector.parseAuthorityKeyIdentifierExtension(crlAKID);
+        }
+
+        matched = issuerSelector.match(cert);
 
-        return issuerSelector.match(cert);
+        // if AKID is unreliable, verify the CRL signature with the cert
+        if (matched && (crlAKID == null ||
+                cert.getAuthorityKeyIdentifierExtension() == null)) {
+            try {
+                crl.verify(cert.getPublicKey(), provider);
+                matched = true;
+            } catch (Exception e) {
+                matched = false;
+            }
+        }
+
+        return matched;
     }
 }