7176627: CertPath/jep124/PreferCRL_SoftFail test fails (Could not determine revocation status)

Reviewed-by: xuelei
This commit is contained in:
Sean Mullan 2012-09-14 10:13:04 -04:00
parent e0782017fe
commit 5e99983092
9 changed files with 189 additions and 63 deletions

View File

@ -35,6 +35,7 @@ import java.security.InvalidAlgorithmParameterException;
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 @@ public abstract class CertStoreHelper {
}
}
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 @@ public abstract class CertStoreHelper {
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);
}

View File

@ -116,12 +116,17 @@ class DistributionPointFetcher {
/**
* 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 @@ class DistributionPointFetcher {
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 @@ class DistributionPointFetcher {
/**
* 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 (Exception e) {
} catch (InvalidAlgorithmParameterException |
NoSuchAlgorithmException e) {
if (debug != null) {
debug.println("Exception getting CRL from CertStore: " + e);
e.printStackTrace();
debug.println("Can't create URICertStore: " + e.getMessage());
}
return null;
}
Collection<? extends CRL> crls = ucs.getCRLs(null);
if (crls.isEmpty()) {
return null;
} else {
return (X509CRL) crls.iterator().next();
}
return null;
}
/**
* 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 @@ class DistributionPointFetcher {
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;
}
}
/**

View File

@ -369,20 +369,21 @@ class ForwardBuilder extends Builder {
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;

View File

@ -335,8 +335,8 @@ public final class OCSP {
static class NetworkFailureException extends CertPathValidatorException {
private static final long serialVersionUID = 0l;
private NetworkFailureException(IOException ioe) {
super(ioe);
NetworkFailureException(Throwable t) {
super(t);
}
@Override

View File

@ -270,6 +270,24 @@ class PKIX {
}
}
/**
* 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.

View File

@ -50,7 +50,7 @@ import java.util.Set;
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 @@ class RevocationChecker extends PKIXRevocationChecker {
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 @@ class RevocationChecker extends PKIXRevocationChecker {
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 @@ class RevocationChecker extends PKIXRevocationChecker {
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);

View File

@ -340,7 +340,11 @@ class URICertStore extends CertStoreSpi {
// 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 @@ class URICertStore extends CertStoreSpi {
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();
}
/**

View File

@ -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 @@ public final class LDAPCertStoreHelper
{
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));
}
}

View File

@ -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 @@ public final class SSLServerCertStoreHelper extends CertStoreHelper {
{
throw new UnsupportedOperationException();
}
@Override
public boolean isCausedByNetworkIssue(CertStoreException e) {
Throwable t = e.getCause();
return (t != null && t instanceof IOException);
}
}