Skip to content

Bind revocation responses to the issuer before trusting them #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ThisaraWeerakoon wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Bind revocation responses to the issuer before trusting them #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions ...rts/core/nhttp/src/main/java/org/apache/synapse/transport/utils/sslcert/crl/CRLCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ public class CRLCache implements ManageableCache {
private final Map<String, CRLCacheValue> hashMap;
private Iterator<Map.Entry<String, CRLCacheValue>> iterator;
private volatile CacheManager cacheManager;
private final CRLVerifier crlVerifier;

private CRLCache() {
hashMap = new ConcurrentHashMap<String, CRLCacheValue>();
iterator = hashMap.entrySet().iterator();
crlVerifier = new CRLVerifier(null);
}

public static CRLCache getCache() {
Expand Down Expand Up @@ -116,15 +114,7 @@ private synchronized void replaceNewCacheValue(CRLCacheValue cacheValue) {
return;
}

try {
String crlUrl = cacheValue.crlUrl;
X509CRL x509CRL = crlVerifier.downloadCRLFromWeb(crlUrl);
this.setCacheValue(crlUrl, x509CRL);
} catch (Exception e) {
log.debug("Cant replace old CacheValue with new CacheValue. So remove", e);
//If cant be replaced remove.
cacheValue.removeThisCacheValue();
}
cacheValue.removeThisCacheValue();
}

public synchronized X509CRL getCacheValue(String crlUrl) {
Expand Down
81 changes: 77 additions & 4 deletions .../core/nhttp/src/main/java/org/apache/synapse/transport/utils/sslcert/crl/CRLVerifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.security.GeneralSecurityException;
import java.security.cert.*;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;

/**
Expand All @@ -46,6 +48,18 @@ public class CRLVerifier implements RevocationVerifier {

private CRLCache cache;

/**
* Tolerance applied to both ends of a CRL's validity window, to allow for clock skew between
* this host and the CA that published it.
*/
private static final long CLOCK_SKEW_MILLIS = 5 * 60 * 1000L;

/**
* How old a CRL may be when it carries no nextUpdate, which RFC 5280 section 5.1.2.5
* leaves optional.
*/
private static final long MAX_CRL_AGE_MILLIS = 24 * 60 * 60 * 1000L;

public CRLVerifier(CRLCache cache) {
this.cache = cache;
}
Expand All @@ -54,7 +68,7 @@ public CRLVerifier(CRLCache cache) {
* Checks revocation status (Good, Revoked) of the peer certificate.
*
* @param peerCert peer certificate
* @param issuerCert issuer certificate of the peer. not used currently.
* @param issuerCert issuer of the peer certificate; every CRL used here is verified against it.
* @return revocation status of the peer certificate.
* @throws CertificateVerificationException
*
Expand All @@ -74,16 +88,26 @@ public RevocationStatus checkRevocationStatus(X509Certificate peerCert, X509Cert
X509CRL x509CRL = cache.getCacheValue(crlUrl);
if (x509CRL != null) {
//If cant be casted, we have used the wrong cache.
RevocationStatus status = getRevocationStatus(x509CRL, peerCert);
log.debug("CRL taken from cache....");
return status;
try {
verifyCRL(x509CRL, issuerCert);
RevocationStatus status = getRevocationStatus(x509CRL, peerCert);
log.debug("CRL taken from cache....");
return status;
} catch (CertificateVerificationException e) {
if (log.isDebugEnabled()) {
log.debug("Discarding the cached CRL for " + crlUrl +
" and downloading it again.", e);
}
cache.removeCacheValue(crlUrl);
}
}
}

//todo: Do we need to check if URL has the same domain name as issuerCert?
try {
X509CRL x509CRL = downloadCRLFromWeb(crlUrl);
if (x509CRL != null) {
verifyCRL(x509CRL, issuerCert);
if (cache != null) {
cache.setCacheValue(crlUrl, x509CRL);
}
Expand All @@ -108,6 +132,55 @@ private RevocationStatus getRevocationStatus(X509CRL x509CRL, X509Certificate pe
}
}

/**
* Verifies a CRL was published by the issuer of the certificate being checked and is still
* current, rejecting forged or stale CRLs and indirect CRLs signed by a separate CRL issuer.
*
* @param x509CRL the CRL that was fetched or taken from the cache.
* @param issuerCert the issuer of the certificate being checked.
* @throws CertificateVerificationException if the CRL is not validly signed by that issuer or
* is no longer current.
*/
private void verifyCRL(X509CRL x509CRL, X509Certificate issuerCert)
throws CertificateVerificationException {

if (issuerCert == null) {
throw new CertificateVerificationException("Cannot verify a CRL without the issuer " +
"certificate");
}

if (!x509CRL.getIssuerX500Principal().equals(issuerCert.getSubjectX500Principal())) {
throw new CertificateVerificationException("CRL issuer " +
x509CRL.getIssuerX500Principal() + " is not the issuer of the certificate " +
"being checked");
}

try {
x509CRL.verify(issuerCert.getPublicKey());
} catch (GeneralSecurityException e) {
throw new CertificateVerificationException("CRL is not signed by the issuer of the " +
"certificate being checked", e);
}

long now = System.currentTimeMillis();
Date thisUpdate = x509CRL.getThisUpdate();
Date nextUpdate = x509CRL.getNextUpdate();

if (thisUpdate == null) {
throw new CertificateVerificationException("CRL does not say when it was issued");
}
if (thisUpdate.getTime() > now + CLOCK_SKEW_MILLIS) {
throw new CertificateVerificationException("CRL is not valid yet. It is dated " +
thisUpdate);
}
if (nextUpdate != null && nextUpdate.getTime() < now - CLOCK_SKEW_MILLIS) {
throw new CertificateVerificationException("CRL expired on " + nextUpdate);
} else if (nextUpdate == null && thisUpdate.getTime() < now - MAX_CRL_AGE_MILLIS) {
throw new CertificateVerificationException("CRL carries no nextUpdate and is older " +
"than the accepted maximum. It is dated " + thisUpdate);
}
}

/**
* Downloads CRL from the crlUrl. Does not support HTTPS
*/
Expand Down
28 changes: 1 addition & 27 deletions ...s/core/nhttp/src/main/java/org/apache/synapse/transport/utils/sslcert/ocsp/OCSPCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,11 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.synapse.commons.jmx.MBeanRegistrar;
import org.apache.synapse.transport.utils.sslcert.CertificateVerificationException;
import org.apache.synapse.transport.utils.sslcert.cache.CacheController;
import org.apache.synapse.transport.utils.sslcert.cache.CacheManager;
import org.apache.synapse.transport.utils.sslcert.cache.ManageableCache;
import org.apache.synapse.transport.utils.sslcert.cache.ManageableCacheValue;
import org.bouncycastle.cert.ocsp.BasicOCSPResp;
import org.bouncycastle.cert.ocsp.OCSPReq;
import org.bouncycastle.cert.ocsp.OCSPResp;
import org.bouncycastle.cert.ocsp.SingleResp;

import java.math.BigInteger;
Expand All @@ -52,12 +49,10 @@ public class OCSPCache implements ManageableCache {
private final Map<BigInteger,OCSPCacheValue> hashMap;
private Iterator<Map.Entry<BigInteger,OCSPCacheValue>> iterator;
private volatile CacheManager cacheManager;
private final OCSPVerifier ocspVerifier;

private OCSPCache() {
this.hashMap = new ConcurrentHashMap<BigInteger, OCSPCacheValue>();
this.iterator = hashMap.entrySet().iterator();
this.ocspVerifier = new OCSPVerifier(null);
}

public static OCSPCache getCache() {
Expand Down Expand Up @@ -120,28 +115,7 @@ private synchronized void replaceNewCacheValue(OCSPCacheValue cacheValue){
return;
}

try {
String serviceUrl = cacheValue.serviceUrl;
OCSPReq request = cacheValue.request;
OCSPResp response= ocspVerifier.getOCSPResponse(serviceUrl, request);

if (OCSPResp.SUCCESSFUL != response.getStatus())
throw new CertificateVerificationException("OCSP response status not SUCCESSFUL");

BasicOCSPResp basicResponse = (BasicOCSPResp) response.getResponseObject();
SingleResp[] responses = (basicResponse == null) ? null : basicResponse.getResponses();

if (responses == null)
throw new CertificateVerificationException("Cant get OCSP response");

SingleResp resp = responses[0];
this.setCacheValue(cacheValue.serialNumber, resp, request, serviceUrl);

} catch (Exception e){
log.debug("Cant replace old CacheValue with new CacheValue. So remove", e);
//If cant be replaced remove.
cacheValue.removeThisCacheValue();
}
cacheValue.removeThisCacheValue();
}

public synchronized SingleResp getCacheValue(BigInteger serialNumber) {
Expand Down
Loading
Loading