From 9e15868483507b32dc686ef2647e6f93e44415a9 Mon Sep 17 00:00:00 2001 From: xuelei Date: Tue, 12 Apr 2011 08:27:00 -0700 Subject: [PATCH] 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 --- .../certpath/CrlRevocationChecker.java | 37 ++++---- .../certpath/DistributionPointFetcher.java | 93 ++++++++++--------- 2 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java b/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java index 6738c3926..1f692f235 100644 --- a/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java +++ b/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java @@ -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 @@ class CrlRevocationChecker extends PKIXCertPathChecker { throws CertPathValidatorException { verifyRevocationStatus(currCert, prevKey, signFlag, - allowSeparateKey, null); + allowSeparateKey, null, mParams.getTrustAnchors()); } /** @@ -260,11 +260,12 @@ class CrlRevocationChecker extends PKIXCertPathChecker { * circular dependencies, we assume they're * revoked while checking the revocation * status of this cert. + * @param trustAnchors a Set of TrustAnchors */ private void verifyRevocationStatus(X509Certificate currCert, PublicKey prevKey, boolean signFlag, boolean allowSeparateKey, - Set stackedCerts) throws CertPathValidatorException - { + Set stackedCerts, + Set trustAnchors) throws CertPathValidatorException { String msg = "revocation status"; if (debug != null) { @@ -311,7 +312,7 @@ class CrlRevocationChecker extends PKIXCertPathChecker { 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 @@ class CrlRevocationChecker extends PKIXCertPathChecker { // 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 @@ class CrlRevocationChecker extends PKIXCertPathChecker { // 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 @@ class CrlRevocationChecker extends PKIXCertPathChecker { certSel.setSubject(currCert.getIssuerX500Principal()); certSel.setKeyUsage(mCrlSignUsage); - Set newAnchors = mAnchor == null - ? mParams.getTrustAnchors() - : Collections.singleton(mAnchor); + Set newAnchors = + (mAnchor == null ? mParams.getTrustAnchors() : + Collections.singleton(mAnchor)); PKIXBuilderParameters builderParams; if (mParams instanceof PKIXBuilderParameters) { @@ -617,8 +619,8 @@ class CrlRevocationChecker extends PKIXCertPathChecker { 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 @@ class CrlRevocationChecker extends PKIXCertPathChecker { * @param signFlag true 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 Set of TrustAnchors> * @return a collection of approved crls (or an empty collection) */ private Collection verifyPossibleCRLs(Set crls, X509Certificate cert, boolean signFlag, PublicKey prevKey, - boolean[] reasonsMask) throws CertPathValidatorException - { + boolean[] reasonsMask, + Set trustAnchors) throws CertPathValidatorException { + try { X509CertImpl certImpl = X509CertImpl.toImpl(cert); if (debug != null) { @@ -764,7 +768,8 @@ class CrlRevocationChecker extends PKIXCertPathChecker { 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); } } diff --git a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java index e367a1fae..56c5ab91c 100644 --- a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java +++ b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java @@ -90,8 +90,9 @@ class DistributionPointFetcher { */ Collection getCRLs(X509CRLSelector selector, boolean signFlag, PublicKey prevKey, String provider, List certStores, - boolean[] reasonsMask, TrustAnchor anchor) throws CertStoreException - { + boolean[] reasonsMask, + Set trustAnchors) throws CertStoreException { + if (USE_CRLDP == false) { return Collections.emptySet(); } @@ -121,7 +122,7 @@ class DistributionPointFetcher { DistributionPoint point = t.next(); Collection crls = getCRLs(selector, certImpl, point, reasonsMask, signFlag, prevKey, provider, - certStores, anchor); + certStores, trustAnchors); results.addAll(crls); } if (debug != null) { @@ -142,8 +143,8 @@ class DistributionPointFetcher { private Collection getCRLs(X509CRLSelector selector, X509CertImpl certImpl, DistributionPoint point, boolean[] reasonsMask, boolean signFlag, PublicKey prevKey, String provider, - List certStores, TrustAnchor anchor) - { + List certStores, Set trustAnchors) { + // check for full name GeneralNames fullName = point.getFullName(); if (fullName == null) { @@ -194,7 +195,7 @@ class DistributionPointFetcher { // 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 @@ class DistributionPointFetcher { * @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 trustAnchors, List certStores) throws CRLException, IOException { + boolean indirectCRL = false; X509CRLImpl crlImpl = X509CRLImpl.toImpl(crl); IssuingDistributionPointExtension idpExt = @@ -335,7 +341,16 @@ class DistributionPointFetcher { 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 @@ class DistributionPointFetcher { // 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 trustAnchors = new HashSet(); - if (anchor != null) { - trustAnchors.add(anchor); - } + Set 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 @@ class DistributionPointFetcher { private static boolean issues(X509CertImpl cert, X509CRLImpl crl, String provider) throws IOException { + boolean matched = false; + AdaptableX509CertSelector issuerSelector = new AdaptableX509CertSelector(); @@ -719,9 +709,24 @@ class DistributionPointFetcher { * 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); + + // 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 issuerSelector.match(cert); + return matched; } } -- GitLab