OpenJDK / jdk8u / jdk8u / jdk
changeset 8972:d6c4ae56c079
8007967: Infinite loop can happen in sun.security.provider.certpath.SunCertPathBuilder.depthFirstSearchForward()
Reviewed-by: mullan
author | juh |
---|---|
date | Fri, 06 Dec 2013 11:36:04 -0800 |
parents | f8da1f34c65c |
children | 9e579a2329c0 |
files | src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java src/share/classes/sun/security/provider/certpath/ForwardBuilder.java src/share/classes/sun/security/provider/certpath/RevocationChecker.java src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java |
diffstat | 4 files changed, 68 insertions(+), 47 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java Fri Dec 06 11:28:24 2013 -0800 +++ b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java Fri Dec 06 11:36:04 2013 -0800 @@ -76,6 +76,25 @@ Date validity) throws CertStoreException { + return getCRLs(selector, signFlag, prevKey, null, provider, certStores, + reasonsMask, trustAnchors, validity); + } + + /** + * Return the X509CRLs matching this selector. The selector must be + * an X509CRLSelector with certificateChecking set. + */ + public static Collection<X509CRL> getCRLs(X509CRLSelector selector, + boolean signFlag, + PublicKey prevKey, + X509Certificate prevCert, + String provider, + List<CertStore> certStores, + boolean[] reasonsMask, + Set<TrustAnchor> trustAnchors, + Date validity) + throws CertStoreException + { X509Certificate cert = selector.getCertificateChecking(); if (cert == null) { return Collections.emptySet(); @@ -101,7 +120,7 @@ t.hasNext() && !Arrays.equals(reasonsMask, ALL_REASONS); ) { DistributionPoint point = t.next(); Collection<X509CRL> crls = getCRLs(selector, certImpl, - point, reasonsMask, signFlag, prevKey, provider, + point, reasonsMask, signFlag, prevKey, prevCert, provider, certStores, trustAnchors, validity); results.addAll(crls); } @@ -125,9 +144,10 @@ */ private static Collection<X509CRL> getCRLs(X509CRLSelector selector, X509CertImpl certImpl, DistributionPoint point, boolean[] reasonsMask, - boolean signFlag, PublicKey prevKey, String provider, - List<CertStore> certStores, Set<TrustAnchor> trustAnchors, - Date validity) throws CertStoreException { + boolean signFlag, PublicKey prevKey, X509Certificate prevCert, + String provider, List<CertStore> certStores, + Set<TrustAnchor> trustAnchors, Date validity) + throws CertStoreException { // check for full name GeneralNames fullName = point.getFullName(); @@ -188,8 +208,8 @@ // we check the issuer in verifyCRLs method selector.setIssuerNames(null); if (selector.match(crl) && verifyCRL(certImpl, point, crl, - reasonsMask, signFlag, prevKey, provider, trustAnchors, - certStores, validity)) { + reasonsMask, signFlag, prevKey, prevCert, provider, + trustAnchors, certStores, validity)) { crls.add(crl); } } catch (IOException | CRLException e) { @@ -284,6 +304,8 @@ * @param reasonsMask the interim reasons mask * @param signFlag true if prevKey can be used to verify the CRL * @param prevKey the public key that verifies the certificate's signature + * @param prevCert the certificate whose public key verifies + * {@code certImpl}'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 @@ -294,7 +316,7 @@ */ static boolean verifyCRL(X509CertImpl certImpl, DistributionPoint point, X509CRL crl, boolean[] reasonsMask, boolean signFlag, - PublicKey prevKey, String provider, + PublicKey prevKey, X509Certificate prevCert, String provider, Set<TrustAnchor> trustAnchors, List<CertStore> certStores, Date validity) throws CRLException, IOException { @@ -591,18 +613,26 @@ // the subject criterion will be set by builder automatically. } - // by far, we have validated the previous certificate, we can - // trust it during validating the CRL issuer. - // Except the performance improvement, another benefit is to break - // the dead loop while looking for the issuer back and forth + // By now, we have validated the previous certificate, so we can + // trust it during the validation of the CRL issuer. + // In addition to 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> newTrustAnchors = new HashSet<>(trustAnchors); if (prevKey != null) { // Add the previous certificate as a trust anchor. - X500Principal principal = certImpl.getIssuerX500Principal(); - TrustAnchor temporary = - new TrustAnchor(principal, prevKey, null); + // If prevCert is not null, we want to construct a TrustAnchor + // using the cert object because when the certpath for the CRL + // is built later, the CertSelector will make comparisons with + // the TrustAnchor's trustedCert member rather than its pubKey. + TrustAnchor temporary; + if (prevCert != null) { + temporary = new TrustAnchor(prevCert, null); + } else { + X500Principal principal = certImpl.getIssuerX500Principal(); + temporary = new TrustAnchor(principal, prevKey, null); + } newTrustAnchors.add(temporary); }
--- a/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java Fri Dec 06 11:28:24 2013 -0800 +++ b/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java Fri Dec 06 11:36:04 2013 -0800 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2013, 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 @@ -47,9 +47,7 @@ import sun.security.x509.AccessDescription; import sun.security.x509.AuthorityInfoAccessExtension; import static sun.security.x509.PKIXExtensions.*; -import sun.security.x509.PolicyMappingsExtension; import sun.security.x509.X500Name; -import sun.security.x509.X509CertImpl; import sun.security.x509.AuthorityKeyIdentifierExtension; /** @@ -672,32 +670,16 @@ currState.untrustedChecker.check(cert, Collections.<String>emptySet()); /* - * check for looping - abort a loop if - * ((we encounter the same certificate twice) AND - * ((policyMappingInhibited = true) OR (no policy mapping - * extensions can be found between the occurrences of the same - * certificate))) + * check for looping - abort a loop if we encounter the same + * certificate twice */ if (certPathList != null) { - boolean policyMappingFound = false; for (X509Certificate cpListCert : certPathList) { - X509CertImpl cpListCertImpl = X509CertImpl.toImpl(cpListCert); - PolicyMappingsExtension policyMappingsExt - = cpListCertImpl.getPolicyMappingsExtension(); - if (policyMappingsExt != null) { - policyMappingFound = true; - } - if (debug != null) { - debug.println("policyMappingFound = " + policyMappingFound); - } if (cert.equals(cpListCert)) { - if ((buildParams.policyMappingInhibited()) || - (!policyMappingFound)) { - if (debug != null) { - debug.println("loop detected!!"); - } - throw new CertPathValidatorException("loop detected"); + if (debug != null) { + debug.println("loop detected!!"); } + throw new CertPathValidatorException("loop detected"); } } }
--- a/src/share/classes/sun/security/provider/certpath/RevocationChecker.java Fri Dec 06 11:28:24 2013 -0800 +++ b/src/share/classes/sun/security/provider/certpath/RevocationChecker.java Fri Dec 06 11:36:04 2013 -0800 @@ -456,12 +456,13 @@ PublicKey pubKey, boolean signFlag) throws CertPathValidatorException { - checkCRLs(cert, pubKey, signFlag, true, + checkCRLs(cert, pubKey, null, signFlag, true, stackedCerts, params.trustAnchors()); } private void checkCRLs(X509Certificate cert, PublicKey prevKey, - boolean signFlag, boolean allowSeparateKey, + X509Certificate prevCert, boolean signFlag, + boolean allowSeparateKey, Set<X509Certificate> stackedCerts, Set<TrustAnchor> anchors) throws CertPathValidatorException @@ -543,7 +544,7 @@ try { if (crlDP) { approvedCRLs.addAll(DistributionPointFetcher.getCRLs( - sel, signFlag, prevKey, + sel, signFlag, prevKey, prevCert, params.sigProvider(), certStores, reasonsMask, anchors, null)); } @@ -825,7 +826,7 @@ for (X509CRL crl : crls) { if (DistributionPointFetcher.verifyCRL( certImpl, point, crl, reasonsMask, signFlag, - prevKey, params.sigProvider(), anchors, + prevKey, null, params.sigProvider(), anchors, certStores, params.date())) { results.add(crl); @@ -1043,7 +1044,7 @@ + " index " + i + " checking " + cert); } - checkCRLs(cert, prevKey2, signFlag, true, + checkCRLs(cert, prevKey2, null, signFlag, true, stackedCerts, newAnchors); signFlag = certCanSignCrl(cert); prevKey2 = cert.getPublicKey(); @@ -1058,13 +1059,14 @@ debug.println("RevocationChecker.buildToNewKey()" + " got key " + cpbr.getPublicKey()); } - // Now check revocation on the current cert using that key. + // Now check revocation on the current cert using that key and + // the corresponding certificate. // If it doesn't check out, try to find a different key. // And if we can't find a key, then return false. PublicKey newKey = cpbr.getPublicKey(); try { - checkCRLs(currCert, newKey, true, false, null, - params.trustAnchors()); + checkCRLs(currCert, newKey, (X509Certificate) cpList.get(0), + true, false, null, params.trustAnchors()); // If that passed, the cert is OK! return; } catch (CertPathValidatorException cpve) {
--- a/src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java Fri Dec 06 11:28:24 2013 -0800 +++ b/src/share/classes/sun/security/provider/certpath/SunCertPathBuilder.java Fri Dec 06 11:36:04 2013 -0800 @@ -30,6 +30,7 @@ import java.security.InvalidAlgorithmParameterException; import java.security.PublicKey; import java.security.cert.*; +import java.security.cert.CertPathValidatorException.BasicReason; import java.security.cert.PKIXReason; import java.util.ArrayList; import java.util.Collection; @@ -49,7 +50,7 @@ * This class is able to build certification paths in either the forward * or reverse directions. * - * <p> If successful, it returns a certification path which has succesfully + * <p> If successful, it returns a certification path which has successfully * satisfied all the constraints and requirements specified in the * PKIXBuilderParameters object and has been validated according to the PKIX * path validation algorithm defined in RFC 3280. @@ -510,6 +511,12 @@ debug.println ("SunCertPathBuilder.depthFirstSearchForward(): " + "final verification failed: " + cpve); + // If the target cert itself is revoked, we + // cannot trust it. We can bail out here. + if (buildParams.targetCertConstraints().match(currCert) + && cpve.getReason() == BasicReason.REVOKED) { + throw cpve; + } vertex.setThrowable(cpve); continue vertices; }