mirror of
https://github.com/openjdk/jdk.git
synced 2026-02-15 21:05:11 +00:00
8007967: Infinite loop can happen in sun.security.provider.certpath.SunCertPathBuilder.depthFirstSearchForward()
Reviewed-by: mullan
This commit is contained in:
Jason Uh
parent
2da6ed45d3
commit
d8cb69a469
@ -75,6 +75,25 @@ public class DistributionPointFetcher {
|
||||
Set<TrustAnchor> trustAnchors,
|
||||
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) {
|
||||
@ -101,7 +120,7 @@ public class DistributionPointFetcher {
|
||||
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 @@ public class DistributionPointFetcher {
|
||||
*/
|
||||
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 @@ public class DistributionPointFetcher {
|
||||
// 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 @@ public class DistributionPointFetcher {
|
||||
* @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 @@ public class DistributionPointFetcher {
|
||||
*/
|
||||
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 @@ public class DistributionPointFetcher {
|
||||
// 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);
|
||||
}
|
||||
|
||||
|
||||
@ -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.util.Debug;
|
||||
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 @@ class ForwardBuilder extends Builder {
|
||||
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");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -456,12 +456,13 @@ class RevocationChecker extends PKIXRevocationChecker {
|
||||
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 @@ class RevocationChecker extends PKIXRevocationChecker {
|
||||
try {
|
||||
if (crlDP) {
|
||||
approvedCRLs.addAll(DistributionPointFetcher.getCRLs(
|
||||
sel, signFlag, prevKey,
|
||||
sel, signFlag, prevKey, prevCert,
|
||||
params.sigProvider(), certStores,
|
||||
reasonsMask, anchors, null));
|
||||
}
|
||||
@ -825,7 +826,7 @@ class RevocationChecker extends PKIXRevocationChecker {
|
||||
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 @@ class RevocationChecker extends PKIXRevocationChecker {
|
||||
+ " 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 @@ class RevocationChecker extends PKIXRevocationChecker {
|
||||
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) {
|
||||
|
||||
@ -30,6 +30,7 @@ import java.security.GeneralSecurityException;
|
||||
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 @@ import sun.security.util.Debug;
|
||||
* 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 @@ public final class SunCertPathBuilder extends CertPathBuilderSpi {
|
||||
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;
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user