7176627: CertPath/jep124/PreferCRL_SoftFail test fails (Could not determine revocation status)
Reviewed-by: xuelei
--- a/jdk/src/share/classes/sun/security/provider/certpath/CertStoreHelper.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/CertStoreHelper.java Fri Sep 14 10:13:04 2012 -0400
@@ -35,6 +35,7 @@
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.security.cert.CertStore;
+import java.security.cert.CertStoreException;
import java.security.cert.X509CertSelector;
import java.security.cert.X509CRLSelector;
import javax.security.auth.x500.X500Principal;
@@ -96,6 +97,25 @@
}
}
+ static boolean isCausedByNetworkIssue(String type, CertStoreException cse) {
+ switch (type) {
+ case "LDAP":
+ case "SSLServer":
+ try {
+ CertStoreHelper csh = CertStoreHelper.getInstance(type);
+ return csh.isCausedByNetworkIssue(cse);
+ } catch (NoSuchAlgorithmException nsae) {
+ return false;
+ }
+ case "URI":
+ Throwable t = cse.getCause();
+ return (t != null && t instanceof IOException);
+ default:
+ // we don't know about any other remote CertStore types
+ return false;
+ }
+ }
+
/**
* Returns a CertStore using the given URI as parameters.
*/
@@ -119,4 +139,10 @@
Collection<X500Principal> certIssuers,
String dn)
throws IOException;
+
+ /**
+ * Returns true if the cause of the CertStoreException is a network
+ * related issue.
+ */
+ public abstract boolean isCausedByNetworkIssue(CertStoreException e);
}
--- a/jdk/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java Fri Sep 14 10:13:04 2012 -0400
@@ -116,12 +116,17 @@
/**
* Download CRLs from the given distribution point, verify and return them.
* See the top of the class for current limitations.
+ *
+ * @throws CertStoreException if there is an error retrieving the CRLs
+ * from one of the GeneralNames and no other CRLs are retrieved from
+ * the other GeneralNames. If more than one GeneralName throws an
+ * exception then the one from the last GeneralName is thrown.
*/
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) {
+ Date validity) throws CertStoreException {
// check for full name
GeneralNames fullName = point.getFullName();
@@ -149,24 +154,33 @@
return Collections.emptySet();
}
}
- Collection<X509CRL> possibleCRLs = new ArrayList<X509CRL>();
- Collection<X509CRL> crls = new ArrayList<X509CRL>(2);
+ Collection<X509CRL> possibleCRLs = new ArrayList<>();
+ CertStoreException savedCSE = null;
for (Iterator<GeneralName> t = fullName.iterator(); t.hasNext(); ) {
- GeneralName name = t.next();
- if (name.getType() == GeneralNameInterface.NAME_DIRECTORY) {
- X500Name x500Name = (X500Name) name.getName();
- possibleCRLs.addAll(
- getCRLs(x500Name, certImpl.getIssuerX500Principal(),
- certStores));
- } else if (name.getType() == GeneralNameInterface.NAME_URI) {
- URIName uriName = (URIName)name.getName();
- X509CRL crl = getCRL(uriName);
- if (crl != null) {
- possibleCRLs.add(crl);
+ try {
+ GeneralName name = t.next();
+ if (name.getType() == GeneralNameInterface.NAME_DIRECTORY) {
+ X500Name x500Name = (X500Name) name.getName();
+ possibleCRLs.addAll(
+ getCRLs(x500Name, certImpl.getIssuerX500Principal(),
+ certStores));
+ } else if (name.getType() == GeneralNameInterface.NAME_URI) {
+ URIName uriName = (URIName)name.getName();
+ X509CRL crl = getCRL(uriName);
+ if (crl != null) {
+ possibleCRLs.add(crl);
+ }
}
+ } catch (CertStoreException cse) {
+ savedCSE = cse;
}
}
+ // only throw CertStoreException if no CRLs are retrieved
+ if (possibleCRLs.isEmpty() && savedCSE != null) {
+ throw savedCSE;
+ }
+ Collection<X509CRL> crls = new ArrayList<>(2);
for (X509CRL crl : possibleCRLs) {
try {
// make sure issuer is not set
@@ -191,34 +205,43 @@
/**
* Download CRL from given URI.
*/
- private static X509CRL getCRL(URIName name) {
+ private static X509CRL getCRL(URIName name) throws CertStoreException {
URI uri = name.getURI();
if (debug != null) {
debug.println("Trying to fetch CRL from DP " + uri);
}
+ CertStore ucs = null;
try {
- CertStore ucs = URICertStore.getInstance
+ ucs = URICertStore.getInstance
(new URICertStore.URICertStoreParameters(uri));
- Collection<? extends CRL> crls = ucs.getCRLs(null);
- if (crls.isEmpty()) {
- return null;
- } else {
- return (X509CRL) crls.iterator().next();
+ } catch (InvalidAlgorithmParameterException |
+ NoSuchAlgorithmException e) {
+ if (debug != null) {
+ debug.println("Can't create URICertStore: " + e.getMessage());
}
- } catch (Exception e) {
- if (debug != null) {
- debug.println("Exception getting CRL from CertStore: " + e);
- e.printStackTrace();
- }
+ return null;
}
- return null;
+
+ Collection<? extends CRL> crls = ucs.getCRLs(null);
+ if (crls.isEmpty()) {
+ return null;
+ } else {
+ return (X509CRL) crls.iterator().next();
+ }
}
/**
* Fetch CRLs from certStores.
+ *
+ * @throws CertStoreException if there is an error retrieving the CRLs from
+ * one of the CertStores and no other CRLs are retrieved from
+ * the other CertStores. If more than one CertStore throws an
+ * exception then the one from the last CertStore is thrown.
*/
private static Collection<X509CRL> getCRLs(X500Name name,
- X500Principal certIssuer, List<CertStore> certStores)
+ X500Principal certIssuer,
+ List<CertStore> certStores)
+ throws CertStoreException
{
if (debug != null) {
debug.println("Trying to fetch CRL from DP " + name);
@@ -227,21 +250,27 @@
xcs.addIssuer(name.asX500Principal());
xcs.addIssuer(certIssuer);
Collection<X509CRL> crls = new ArrayList<>();
+ CertStoreException savedCSE = null;
for (CertStore store : certStores) {
try {
for (CRL crl : store.getCRLs(xcs)) {
crls.add((X509CRL)crl);
}
} catch (CertStoreException cse) {
- // don't add the CRL
if (debug != null) {
- debug.println("Non-fatal exception while retrieving " +
+ debug.println("Exception while retrieving " +
"CRLs: " + cse);
cse.printStackTrace();
}
+ savedCSE = new PKIX.CertStoreTypeException(store.getType(),cse);
}
}
- return crls;
+ // only throw CertStoreException if no CRLs are retrieved
+ if (crls.isEmpty() && savedCSE != null) {
+ throw savedCSE;
+ } else {
+ return crls;
+ }
}
/**
--- a/jdk/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java Fri Sep 14 10:13:04 2012 -0400
@@ -369,20 +369,21 @@
boolean add = false;
for (AccessDescription ad : adList) {
CertStore cs = URICertStore.getInstance(ad);
- try {
- if (certs.addAll((Collection<X509Certificate>)
- cs.getCertificates(caSelector))) {
- add = true;
- if (!searchAllCertStores) {
- return true;
+ if (cs != null) {
+ try {
+ if (certs.addAll((Collection<X509Certificate>)
+ cs.getCertificates(caSelector))) {
+ add = true;
+ if (!searchAllCertStores) {
+ return true;
+ }
+ }
+ } catch (CertStoreException cse) {
+ if (debug != null) {
+ debug.println("exception getting certs from CertStore:");
+ cse.printStackTrace();
}
}
- } catch (CertStoreException cse) {
- if (debug != null) {
- debug.println("exception getting certs from CertStore:");
- cse.printStackTrace();
- }
- continue;
}
}
return add;
--- a/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java Fri Sep 14 10:13:04 2012 -0400
@@ -335,8 +335,8 @@
static class NetworkFailureException extends CertPathValidatorException {
private static final long serialVersionUID = 0l;
- private NetworkFailureException(IOException ioe) {
- super(ioe);
+ NetworkFailureException(Throwable t) {
+ super(t);
}
@Override
--- a/jdk/src/share/classes/sun/security/provider/certpath/PKIX.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/PKIX.java Fri Sep 14 10:13:04 2012 -0400
@@ -271,6 +271,24 @@
}
/**
+ * A CertStoreException with additional information about the type of
+ * CertStore that generated the exception.
+ */
+ static class CertStoreTypeException extends CertStoreException {
+ private static final long serialVersionUID = 7463352639238322556L;
+
+ private final String type;
+
+ CertStoreTypeException(String type, CertStoreException cse) {
+ super(cse.getMessage(), cse.getCause());
+ this.type = type;
+ }
+ String getType() {
+ return type;
+ }
+ }
+
+ /**
* Comparator that orders CertStores so that local CertStores come before
* remote CertStores.
*/
--- a/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java Fri Sep 14 10:13:04 2012 -0400
@@ -50,7 +50,7 @@
import javax.security.auth.x500.X500Principal;
import static sun.security.provider.certpath.OCSP.*;
-import sun.security.provider.certpath.PKIX.ValidatorParams;
+import static sun.security.provider.certpath.PKIX.*;
import sun.security.action.GetPropertyAction;
import sun.security.x509.*;
import static sun.security.x509.PKIXExtensions.*;
@@ -458,14 +458,23 @@
sel.setCertificateChecking(cert);
CertPathHelper.setDateAndTime(sel, params.date(), MAX_CLOCK_SKEW);
- // First, check cached CRLs
+ // First, check user-specified CertStores
+ NetworkFailureException nfe = null;
for (CertStore store : certStores) {
try {
for (CRL crl : store.getCRLs(sel)) {
possibleCRLs.add((X509CRL)crl);
}
} catch (CertStoreException e) {
- // XXX ignore?
+ if (debug != null) {
+ debug.println("RevocationChecker.checkCRLs() " +
+ "CertStoreException: " + e.getMessage());
+ }
+ if (softFail && nfe == null &&
+ CertStoreHelper.isCausedByNetworkIssue(store.getType(),e)) {
+ // save this exception, we may need to throw it later
+ nfe = new NetworkFailureException(e);
+ }
}
}
@@ -504,9 +513,12 @@
reasonsMask, anchors, params.date()));
}
} catch (CertStoreException e) {
- if (debug != null) {
- debug.println("RevocationChecker.checkCRLs() " +
- "unexpected exception: " + e.getMessage());
+ if (softFail && e instanceof CertStoreTypeException) {
+ CertStoreTypeException cste = (CertStoreTypeException)e;
+ if (CertStoreHelper.isCausedByNetworkIssue(cste.getType(),
+ e)) {
+ throw new NetworkFailureException(e);
+ }
}
throw new CertPathValidatorException(e);
}
@@ -516,10 +528,28 @@
checkApprovedCRLs(cert, approvedCRLs);
} else {
if (allowSeparateKey) {
- verifyWithSeparateSigningKey(cert, prevKey, signFlag,
- stackedCerts);
- return;
+ try {
+ verifyWithSeparateSigningKey(cert, prevKey, signFlag,
+ stackedCerts);
+ return;
+ } catch (CertPathValidatorException cpve) {
+ if (nfe != null) {
+ // if a network issue previously prevented us from
+ // retrieving a CRL from one of the user-specified
+ // CertStores and SOFT_FAIL is enabled, throw it now
+ // so it can be handled appropriately
+ throw nfe;
+ }
+ throw cpve;
+ }
} else {
+ if (nfe != null) {
+ // if a network issue previously prevented us from
+ // retrieving a CRL from one of the user-specified
+ // CertStores and SOFT_FAIL is enabled, throw it now
+ // so it can be handled appropriately
+ throw nfe;
+ }
throw new CertPathValidatorException
("Could not determine revocation status", null, null, -1,
BasicReason.UNDETERMINED_REVOCATION_STATUS);
--- a/jdk/src/share/classes/sun/security/provider/certpath/URICertStore.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/URICertStore.java Fri Sep 14 10:13:04 2012 -0400
@@ -340,7 +340,11 @@
// Fetch the CRLs via LDAP. LDAPCertStore has its own
// caching mechanism, see the class description for more info.
// Safe cast since xsel is an X509 certificate selector.
- return (Collection<X509CRL>) ldapCertStore.getCRLs(xsel);
+ try {
+ return (Collection<X509CRL>) ldapCertStore.getCRLs(xsel);
+ } catch (CertStoreException cse) {
+ throw new PKIX.CertStoreTypeException("LDAP", cse);
+ }
}
// Return the CRLs for this entry. It returns the cached value
@@ -391,11 +395,12 @@
debug.println("Exception fetching CRL:");
e.printStackTrace();
}
+ // exception, forget previous values
+ lastModified = 0;
+ crl = null;
+ throw new PKIX.CertStoreTypeException("URI",
+ new CertStoreException(e));
}
- // exception, forget previous values
- lastModified = 0;
- crl = null;
- return Collections.emptyList();
}
/**
--- a/jdk/src/share/classes/sun/security/provider/certpath/ldap/LDAPCertStoreHelper.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/ldap/LDAPCertStoreHelper.java Fri Sep 14 10:13:04 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2012, 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
@@ -25,15 +25,18 @@
package sun.security.provider.certpath.ldap;
+import java.io.IOException;
import java.net.URI;
import java.util.Collection;
import java.security.NoSuchAlgorithmException;
import java.security.InvalidAlgorithmParameterException;
import java.security.cert.CertStore;
+import java.security.cert.CertStoreException;
import java.security.cert.X509CertSelector;
import java.security.cert.X509CRLSelector;
+import javax.naming.CommunicationException;
+import javax.naming.ServiceUnavailableException;
import javax.security.auth.x500.X500Principal;
-import java.io.IOException;
import sun.security.provider.certpath.CertStoreHelper;
@@ -68,4 +71,11 @@
{
return new LDAPCertStore.LDAPCRLSelector(selector, certIssuers, ldapDN);
}
+
+ @Override
+ public boolean isCausedByNetworkIssue(CertStoreException e) {
+ Throwable t = e.getCause();
+ return (t != null && (t instanceof ServiceUnavailableException ||
+ t instanceof CommunicationException));
+ }
}
--- a/jdk/src/share/classes/sun/security/provider/certpath/ssl/SSLServerCertStoreHelper.java Tue Sep 04 02:24:51 2012 -0700
+++ b/jdk/src/share/classes/sun/security/provider/certpath/ssl/SSLServerCertStoreHelper.java Fri Sep 14 10:13:04 2012 -0400
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2012, 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
@@ -25,15 +25,16 @@
package sun.security.provider.certpath.ssl;
+import java.io.IOException;
import java.net.URI;
-import java.util.Collection;
import java.security.NoSuchAlgorithmException;
import java.security.InvalidAlgorithmParameterException;
import java.security.cert.CertStore;
+import java.security.cert.CertStoreException;
import java.security.cert.X509CertSelector;
import java.security.cert.X509CRLSelector;
+import java.util.Collection;
import javax.security.auth.x500.X500Principal;
-import java.io.IOException;
import sun.security.provider.certpath.CertStoreHelper;
@@ -66,4 +67,10 @@
{
throw new UnsupportedOperationException();
}
+
+ @Override
+ public boolean isCausedByNetworkIssue(CertStoreException e) {
+ Throwable t = e.getCause();
+ return (t != null && t instanceof IOException);
+ }
}