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
--- a/jdk/src/share/classes/sun/security/provider/certpath/CrlRevocationChecker.java Tue Apr 12 13:14:05 2011 +0200
+++ b/jdk/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/jdk/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java Tue Apr 12 13:14:05 2011 +0200
+++ b/jdk/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;
}
}