- Remove the ValidateCertificateResponse::ErrorItem::certId member. The

  user should always link with a certificate, not with an index to certificate.
- The CertValidateMessage::parseResponse takes as argument the list of peer
  Certificates. It uses this list to fill the ValidateCertificateResponse
  object.
- Return ERR_GATEWAY_FAILURE/HTTP_INTERNAL_SERVER_ERROR error if:
   * Failed to compose the  Ssl::CertValidateMessage message to sent to
     cert validator
   * The response from cert validator is wrong
   * The cert validator returns an error.

diff --git a/src/forward.cc b/src/forward.cc
index 038bc27b7e3f2d53bf57944dbda57e13cbc949b4..ee48acaba23e1851200add204fad86e4427e3004 100644 (file)
--- a/src/forward.cc
+++ b/src/forward.cc
@@ -790,6 +790,14 @@ FwdState::negotiateSSL(int fd)
                    " certificate: " << e.what() << "; will now block to " <<
                    "validate that certificate.");
             // fall through to do blocking in-process generation.
+            ErrorState *anErr = new ErrorState(ERR_GATEWAY_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request);
+            fail(anErr);
+            if (serverConnection()->getPeer()) {
+                peerConnectFailed(serverConnection()->getPeer());
+            }
+            serverConn->close();
+            self = NULL;
+            return;
         }
     }
 #endif // USE_SSL_CERT_VALIDATOR
@@ -810,6 +818,7 @@ FwdState::sslCrtvdHandleReply(const char *reply)
 {
     Ssl::Errors *errs = NULL;
     Ssl::ErrorDetail *errDetails = NULL;
+    bool validatorFailed = false;
     if (!Comm::IsConnOpen(serverConnection())) {
         return;
     }
@@ -817,24 +826,27 @@ FwdState::sslCrtvdHandleReply(const char *reply)
 
     if (!reply) {
         debugs(83, 1, HERE << "\"ssl_crtd\" helper return <NULL> reply");
+        validatorFailed = true;
     } else {
         Ssl::CertValidateMessage reply_message;
         Ssl::ValidateCertificateResponse resp;
         std::string error;
+        STACK_OF(X509) *peerCerts = SSL_get_peer_cert_chain(ssl);
         if (reply_message.parse(reply, strlen(reply)) != Ssl::CrtdMessage::OK ||
-            !reply_message.parseResponse(resp, error) ) {
+            !reply_message.parseResponse(resp, peerCerts, error) ) {
             debugs(83, 5, HERE << "Reply from ssl_crtvd for " << request->GetHost() << " is incorrect");
+            validatorFailed = true;
         } else {
             if (reply_message.getCode() != "OK") {
                 debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " cannot be validated. ssl_crtvd response: " << reply_message.getBody());
+                validatorFailed = true;
             } else {
                 debugs(83, 5, HERE << "Certificate for " << request->GetHost() << " was successfully validated from ssl_crtvd");
-                // Copy the list of errors etc....
                 ACLFilledChecklist *check = NULL;
                 if (acl_access *acl = Config.ssl_client.cert_error) {
                     check = new ACLFilledChecklist(acl, request, dash_str);
                     for(std::vector<Ssl::ValidateCertificateResponse::ErrorItem>::const_iterator i = resp.errors.begin(); i != resp.errors.end(); ++i) {
-                        debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason << " " << i->certId);
+                        debugs(83, 7, "Error item: " << i->error_no << " " << i->error_reason);
 
                         if (i->error_no == SSL_ERROR_NONE)
                             continue; //ignore????
@@ -845,13 +857,7 @@ FwdState::sslCrtvdHandleReply(const char *reply)
                                 debugs(83, 3, "bypassing SSL error " << i->error_no << " in " << "buffer");
                             } else {
                                 debugs(83, 5, "confirming SSL error " << i->error_no);
-                                STACK_OF(X509) *peerCerts = SSL_get_peer_cert_chain(ssl);
-                                //if i->certID is not correct sk_X509_value returns NULL
-                                X509 *brokenCert = NULL;
-                                if (i->cert != NULL)
-                                    brokenCert = i->cert;
-                                else
-                                    brokenCert = sk_X509_value(peerCerts, i->certId);
+                                X509 *brokenCert = (i->cert ? i->cert : NULL);
                                 X509 *peerCert = SSL_get_peer_certificate(ssl);
                                 const char *aReason = i->error_reason.empty() ? NULL : i->error_reason.c_str();
                                 errDetails = new Ssl::ErrorDetail(i->error_no, peerCert, brokenCert, aReason);
@@ -877,22 +883,30 @@ FwdState::sslCrtvdHandleReply(const char *reply)
             }
         }
     }
-    // Check the list error with
-    if (errDetails && request->clientConnectionManager.valid()) {
-        // remember the server certificate from the ErrorDetail object
-        if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
-            // remember validation errors, if any
-            if (errs) {
-                if (serverBump->sslErrors)
-                    cbdataReference(serverBump->sslErrors);
-                serverBump->sslErrors = cbdataReference(errs);
+
+    ErrorState *anErr = NULL;
+    if (validatorFailed) {
+        anErr = new ErrorState(ERR_GATEWAY_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request);
+    }  else {
+
+        // Check the list error with
+        if (errDetails && request->clientConnectionManager.valid()) {
+            // remember the server certificate from the ErrorDetail object
+            if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
+                // remember validation errors, if any
+                if (errs) {
+                    if (serverBump->sslErrors)
+                        cbdataReference(serverBump->sslErrors);
+                    serverBump->sslErrors = cbdataReference(errs);
+                }
             }
         }
+
+        anErr = makeConnectingError(ERR_SECURE_CONNECT_FAIL);
+        anErr->detail = errDetails;
+        /*anErr->xerrno= Should preserved*/
     }
 
-    ErrorState *const anErr = makeConnectingError(ERR_SECURE_CONNECT_FAIL);
-    anErr->detail = errDetails;
-    /*anErr->xerrno= Should preserved*/
     fail(anErr);
     if (serverConnection()->getPeer()) {
         peerConnectFailed(serverConnection()->getPeer());

diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc
index b33645ce6dd2ba4f3125aa8dbe79f9bc11b9adf5..4a5adc00edc405f329539e4d1e368c0851688f40 100644 (file)
--- a/src/ssl/cert_validate_message.cc
+++ b/src/ssl/cert_validate_message.cc
@@ -1,4 +1,5 @@
 #include "squid.h"
+#include "acl/FilledChecklist.h"
 #include "ssl/support.h"
 #include "ssl/cert_validate_message.h"
 #include "ssl/ErrorDetail.h"
@@ -47,7 +48,7 @@ static int get_error_id(const char *label, size_t len)
     return strtol(e, 0 , 10);
 }
 
-bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, std::string &error)
+bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error)
 {
     int current_errorId = -1;
     std::vector<Ssl::ValidateCertificateResponse::CertItem> certs;
@@ -118,8 +119,10 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp,
                 }
             }
             if (!currentItem.cert) {
-                currentItem.certId = get_error_id(v.c_str(), v.length());
-                debugs(83, 6, HERE << "Cert ID read:" << currentItem.certId);
+                int certId = get_error_id(v.c_str(), v.length());
+                //if certId is not correct sk_X509_value returns NULL
+                currentItem.setCert(sk_X509_value(peerCerts, certId));
+                debugs(83, 6, HERE << "Cert ID read:" << certId);
             }
         } 
 
@@ -135,7 +138,6 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp,
 Ssl::ValidateCertificateResponse::ErrorItem::ErrorItem(const ErrorItem &old) {
     error_no = old.error_no;
     error_reason = old.error_reason;
-    certId = old.certId;
     cert = NULL;
     setCert(old.cert);
 }
@@ -148,7 +150,6 @@ Ssl::ValidateCertificateResponse::ErrorItem::~ErrorItem() {
 Ssl::ValidateCertificateResponse::ErrorItem & Ssl::ValidateCertificateResponse::ErrorItem::operator = (const ErrorItem &old) {
     error_no = old.error_no;
     error_reason = old.error_reason;
-    certId = old.certId;
     setCert(old.cert);
     return *this;
 }
@@ -168,7 +169,6 @@ void
 Ssl::ValidateCertificateResponse::ErrorItem::clear() {
     error_no = SSL_ERROR_NONE;
     error_reason = "";
-    certId = 0;
     if (cert)
         X509_free(cert);
     cert = NULL;

diff --git a/src/ssl/cert_validate_message.h b/src/ssl/cert_validate_message.h
index 1b0af4e1dd96d7b47a67d666b1c7ea20755e4818..8b4a408251185b09473a77498e00b06cdac67b97 100644 (file)
--- a/src/ssl/cert_validate_message.h
+++ b/src/ssl/cert_validate_message.h
@@ -25,7 +25,7 @@ class ValidateCertificateResponse {
 public:
     class  ErrorItem{
     public:
-        ErrorItem(): error_no(SSL_ERROR_NONE), certId(0), cert(NULL) {}
+        ErrorItem(): error_no(SSL_ERROR_NONE), cert(NULL) {}
         ErrorItem(const ErrorItem &);
         ~ErrorItem();
         ErrorItem & operator = (const ErrorItem &);
@@ -33,7 +33,6 @@ public:
         void clear();
         ssl_error_t error_no;
         std::string error_reason;
-        int certId;
         X509 *cert;
     };
 
@@ -57,7 +56,7 @@ class CertValidateMessage: public CrtdMessage {
 public:
     CertValidateMessage(): CrtdMessage() {}
     void composeRequest(ValidateCertificate const &vcert);
-    bool parseResponse(ValidateCertificateResponse &resp, std::string &error);
+    bool parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error);
 
     /// String code for "cert_validate" messages
     static const std::string code_cert_validate;