Skip to content

Commit

Permalink
Validate the shortest certificate chain
Browse files Browse the repository at this point in the history
Do not download remote certificate for issuer X if the received server
certificate (signed by X) can be validated using a locally available CA
certificate. According to our tests, a typical browser does not follow
'CA Issuers' references to download 'missing' certificates when the
browser can validate the origin server certificate (or its chain) using
a local CA certificate. Avoiding unnecessary validations and downloads
not only saves time, but can prevent validation failures as well!

This is Measurement Factory project
  • Loading branch information
chtsanti committed Dec 1, 2017
1 parent 6f518cf commit 9ef7d9d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 11 deletions.
6 changes: 4 additions & 2 deletions src/security/PeerConnector.cc
Expand Up @@ -656,8 +656,9 @@ Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus)
if (X509 *cert = d2i_X509(NULL, &raw, obj.length())) {
char buffer[1024];
debugs(81, 5, "Retrieved certificate: " << X509_NAME_oneline(X509_get_subject_name(cert), buffer, 1024));
ContextPointer ctx(getTlsContext());
const Security::CertList &certsList = srvBio->serverCertificatesIfAny();
if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert, certsList)) {
if (const char *issuerUri = Ssl::uriOfIssuerIfMissing(cert, certsList, ctx)) {
urlsOfMissingCerts.push(SBuf(issuerUri));
}
Ssl::SSL_add_untrusted_cert(session.get(), cert);
Expand Down Expand Up @@ -694,7 +695,8 @@ Security::PeerConnector::checkForMissingCertificates()

if (certs.size()) {
debugs(83, 5, "SSL server sent " << certs.size() << " certificates");
Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs);
ContextPointer ctx(getTlsContext());
Ssl::missingChainCertificatesUrls(urlsOfMissingCerts, certs, ctx);
if (urlsOfMissingCerts.size()) {
startCertDownloading(urlsOfMissingCerts.front());
urlsOfMissingCerts.pop();
Expand Down
45 changes: 38 additions & 7 deletions src/ssl/support.cc
Expand Up @@ -1046,18 +1046,49 @@ findCertIssuer(Security::CertList const &list, X509 *cert)
return false;
}

/// \return true if the cert issuer exist in the certificates stored in connContext
static bool
issuerExistInCaDb(X509 *cert, const Security::ContextPointer &connContext)
{
if (!connContext)
return false;

X509_STORE_CTX *storeCtx = X509_STORE_CTX_new();
if (!storeCtx) {
debugs(83, DBG_IMPORTANT, "Failed to allocate STORE_CTX object");
return false;
}

bool gotIssuer = false;
X509_STORE *store = SSL_CTX_get_cert_store(connContext.get());
if (X509_STORE_CTX_init(storeCtx, store, nullptr, nullptr)) {
X509 *issuer = nullptr;
gotIssuer = (X509_STORE_CTX_get1_issuer(&issuer, storeCtx, cert) > 0);
if (issuer)
X509_free(issuer);
} else {
const int ssl_error = ERR_get_error();
debugs(83, DBG_IMPORTANT, "Failed to initialize STORE_CTX object: " << Security::ErrorString(ssl_error));
}
X509_STORE_CTX_free(storeCtx);

return gotIssuer;
}

const char *
Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates)
Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context)
{
if (!cert || !serverCertificates.size())
return nullptr;

if (!findCertIssuer(serverCertificates, cert)) {
//if issuer is missing
if (!findCertIssuerFast(SquidUntrustedCerts, cert)) {
// and issuer not found in local untrusted certificates database
if (const char *issuerUri = hasAuthorityInfoAccessCaIssuers(cert)) {
// There is a URI where we can download a certificate.
if (const char *issuerUri = hasAuthorityInfoAccessCaIssuers(cert)) {
// There is a URI where we can download a certificate.
if (!findCertIssuerFast(SquidUntrustedCerts, cert) &&
!issuerExistInCaDb(cert, context)) {
// and issuer not found in local databases containing
// untrusted certificates and trusted CA certificates
return issuerUri;
}
}
Expand All @@ -1066,13 +1097,13 @@ Ssl::uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificat
}

void
Ssl::missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates)
Ssl::missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context)
{
if (!serverCertificates.size())
return;

for (const auto &i : serverCertificates) {
if (const char *issuerUri = uriOfIssuerIfMissing(i.get(), serverCertificates))
if (const char *issuerUri = uriOfIssuerIfMissing(i.get(), serverCertificates, context))
URIs.push(SBuf(issuerUri));
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/ssl/support.h
Expand Up @@ -173,13 +173,13 @@ void SSL_add_untrusted_cert(SSL *ssl, X509 *cert);
* Searches in serverCertificates list for the cert issuer and if not found
* and Authority Info Access of cert provides a URI return it.
*/
const char *uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates);
const char *uriOfIssuerIfMissing(X509 *cert, Security::CertList const &serverCertificates, const Security::ContextPointer &context);

/**
* Fill URIs queue with the uris of missing certificates from serverCertificate chain
* if this information provided by Authority Info Access.
*/
void missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates);
void missingChainCertificatesUrls(std::queue<SBuf> &URIs, Security::CertList const &serverCertificates, const Security::ContextPointer &context);

/**
\ingroup ServerProtocolSSLAPI
Expand Down

0 comments on commit 9ef7d9d

Please sign in to comment.