Fixed reporting of validation errors for downloaded intermediate certs. (#72)

When Squid or its helper could not validate a downloaded intermediate
certificate (or the root certificate), Squid error page contained
'[Not available]' instead of the broken certificate details, and '-1'
instead of depth of the broken certificate in logs.

This is a Measurement Factory project.

diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc
index 6680053b528d88f7ae6af0e074540e0200d77c74..0ff77aac115bed01a386a81ca131219f8c46a823 100644 (file)
--- a/src/ssl/cert_validate_message.cc
+++ b/src/ssl/cert_validate_message.cc
@@ -16,12 +16,28 @@
 #include "ssl/support.h"
 #include "util.h"
 
+/// Retrieves the certificates chain used to verify the peer.
+/// This is the full chain built by OpenSSL while verifying the server
+/// certificate or, if this is not available, the chain sent by server.
+/// \return the certificates chain or nil
+static STACK_OF(X509) *
+PeerValidationCertificatesChain(const Security::SessionPointer &ssl)
+{
+    assert(ssl);
+    // The full chain built by openSSL while verifying the server cert,
+    // retrieved from verify callback:
+    if (const auto certs = static_cast<STACK_OF(X509) *>(SSL_get_ex_data(ssl.get(), ssl_ex_index_ssl_cert_chain)))
+        return certs;
+
+    /// Last resort: certificates chain sent by server
+    return SSL_get_peer_cert_chain(ssl.get()); // may be nil
+}
+
 void
 Ssl::CertValidationMsg::composeRequest(CertValidationRequest const &vcert)
 {
     body.clear();
     body += Ssl::CertValidationMsg::param_host + "=" + vcert.domainName;
-    STACK_OF(X509) *peerCerts = static_cast<STACK_OF(X509) *>(SSL_get_ex_data(vcert.ssl.get(), ssl_ex_index_ssl_cert_chain));
 
     if (const char *sslVersion = SSL_get_version(vcert.ssl.get()))
         body += "\n" +  Ssl::CertValidationMsg::param_proto_version + "=" + sslVersion;
@@ -29,9 +45,7 @@ Ssl::CertValidationMsg::composeRequest(CertValidationRequest const &vcert)
     if (const char *cipherName = SSL_CIPHER_get_name(SSL_get_current_cipher(vcert.ssl.get())))
         body += "\n" +  Ssl::CertValidationMsg::param_cipher + "=" + cipherName;
 
-    if (!peerCerts)
-        peerCerts = SSL_get_peer_cert_chain(vcert.ssl.get());
-
+    STACK_OF(X509) *peerCerts = PeerValidationCertificatesChain(vcert.ssl);
     if (peerCerts) {
         Ssl::BIO_Pointer bio(BIO_new(BIO_s_mem()));
         for (int i = 0; i < sk_X509_num(peerCerts); ++i) {
@@ -75,10 +89,12 @@ get_error_id(const char *label, size_t len)
 }
 
 bool
-Ssl::CertValidationMsg::parseResponse(CertValidationResponse &resp, STACK_OF(X509) *peerCerts, std::string &error)
+Ssl::CertValidationMsg::parseResponse(CertValidationResponse &resp, std::string &error)
 {
     std::vector<CertItem> certs;
 
+    const STACK_OF(X509) *peerCerts = PeerValidationCertificatesChain(resp.ssl);
+
     const char *param = body.c_str();
     while (*param) {
         while (xisspace(*param)) param++;

diff --git a/src/ssl/cert_validate_message.h b/src/ssl/cert_validate_message.h
index ad5b886ca741cbd5e754b7b6567fa0cff3a3f3b5..1995e25ff01e955565c121522ce2d85294fae852 100644 (file)
--- a/src/ssl/cert_validate_message.h
+++ b/src/ssl/cert_validate_message.h
@@ -59,12 +59,13 @@ public:
     };
 
     typedef std::vector<RecvdError> RecvdErrors;
-
+    explicit CertValidationResponse(const Security::SessionPointer &aSession) : ssl(aSession) {}
     /// Search in errors list for the error item with id=errorId.
     /// If none found a new RecvdError item added with the given id;
     RecvdError &getError(int errorId);
     RecvdErrors errors; ///< The list of parsed errors
     Helper::ResultCode resultCode; ///< The helper result code
+    Security::SessionPointer ssl;
 };
 
 /**
@@ -99,7 +100,7 @@ public:
     void composeRequest(CertValidationRequest const &vcert);
 
     /// Parse a response message and fill the resp object with parsed informations
-    bool parseResponse(CertValidationResponse &resp, STACK_OF(X509) *peerCerts, std::string &error);
+    bool parseResponse(CertValidationResponse &resp, std::string &error);
 
     /// Search a CertItems list for the certificate with ID "name"
     X509 *getCertByName(std::vector<CertItem> const &, std::string const & name);

diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc
index 9d9723b4ab18b874e703feb573495dfd049bad13..f625e4d2e9cf18ed12deeaac98dd36f18d524b6a 100644 (file)
--- a/src/ssl/helper.cc
+++ b/src/ssl/helper.cc
@@ -264,11 +264,11 @@ static void
 sslCrtvdHandleReplyWrapper(void *data, const ::Helper::Reply &reply)
 {
     Ssl::CertValidationMsg replyMsg(Ssl::CrtdMessage::REPLY);
-    Ssl::CertValidationResponse::Pointer validationResponse = new Ssl::CertValidationResponse;
     std::string error;
 
     submitData *crtdvdData = static_cast<submitData *>(data);
-    STACK_OF(X509) *peerCerts = SSL_get_peer_cert_chain(crtdvdData->ssl.get());
+    assert(crtdvdData->ssl.get());
+    Ssl::CertValidationResponse::Pointer validationResponse = new Ssl::CertValidationResponse(crtdvdData->ssl);
     if (reply.result == ::Helper::BrokenHelper) {
         debugs(83, DBG_IMPORTANT, "\"ssl_crtvd\" helper error response: " << reply.other().content());
         validationResponse->resultCode = ::Helper::BrokenHelper;
@@ -276,7 +276,7 @@ sslCrtvdHandleReplyWrapper(void *data, const ::Helper::Reply &reply)
         debugs(83, DBG_IMPORTANT, "\"ssl_crtvd\" helper returned NULL response");
         validationResponse->resultCode = ::Helper::BrokenHelper;
     } else if (replyMsg.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK ||
-               !replyMsg.parseResponse(*validationResponse, peerCerts, error) ) {
+               !replyMsg.parseResponse(*validationResponse, error) ) {
         debugs(83, DBG_IMPORTANT, "WARNING: Reply from ssl_crtvd for " << " is incorrect");
         debugs(83, DBG_IMPORTANT, "Certificate cannot be validated. ssl_crtvd response: " << replyMsg.getBody());
         validationResponse->resultCode = ::Helper::BrokenHelper;
@@ -326,7 +326,7 @@ void Ssl::CertValidationHelper::sslSubmit(Ssl::CertValidationRequest const &requ
     }
 
     if (!ssl_crt_validator->trySubmit(crtdvdData->query.c_str(), sslCrtvdHandleReplyWrapper, crtdvdData)) {
-        Ssl::CertValidationResponse::Pointer resp = new Ssl::CertValidationResponse;;
+        Ssl::CertValidationResponse::Pointer resp = new Ssl::CertValidationResponse(crtdvdData->ssl);
         resp->resultCode = ::Helper::BrokenHelper;
         Ssl::CertValidationHelper::CbDialer *dialer = dynamic_cast<Ssl::CertValidationHelper::CbDialer*>(callback->getDialer());
         Must(dialer);