- Move the CertItem class from ValidateCertificateResponse class to

  CertValidateMessage class

- Add the Ssl::CertValidateMessage::getCertByName method to search
  an for a certificate in a std::vector<CertItem> list

- Implement the Ssl::ValidateCertificateResponse::getError to search in
  Ssl::ValidateCertificateResponse::errors list for a error

- Improve the Ssl::CertValidateMessage::parseResponse method

diff --git a/src/ssl/cert_validate_message.cc b/src/ssl/cert_validate_message.cc
index 4a5adc00edc405f329539e4d1e368c0851688f40..91637e265b990d348924b5be606c7446bcbfee86 100644 (file)
--- a/src/ssl/cert_validate_message.cc
+++ b/src/ssl/cert_validate_message.cc
@@ -5,7 +5,8 @@
 #include "ssl/ErrorDetail.h"
 
 
-void Ssl::CertValidateMessage::composeRequest(ValidateCertificate const &vcert)
+void
+Ssl::CertValidateMessage::composeRequest(ValidateCertificate const &vcert)
 {
     body.clear();
     body += Ssl::CertValidateMessage::param_host + "=" + vcert.domainName;
@@ -40,7 +41,8 @@ void Ssl::CertValidateMessage::composeRequest(ValidateCertificate const &vcert)
     }
 }
 
-static int get_error_id(const char *label, size_t len)
+static int
+get_error_id(const char *label, size_t len)
 {
     const char *e = label + len -1;
     while (e != label && xisdigit(*e)) --e;
@@ -48,12 +50,10 @@ static int get_error_id(const char *label, size_t len)
     return strtol(e, 0 , 10);
 }
 
-bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, 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;
-
-    Ssl::ValidateCertificateResponse::ErrorItem currentItem;
+    std::vector<CertItem> certs;
 
     const char *param = body.c_str();
     while(*param) {
@@ -63,14 +63,14 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp,
 
         size_t param_len = strcspn(param, "=\r\n");
         if (param[param_len] !=  '=') {
-            debugs(83, 2, "Error parsing: " << param);
+            debugs(83, 2, "Cert validator response parse error: " << param);
             return false;
         }
         const char *value=param+param_len+1;
 
         if (param_len > param_cert.length() && 
             strncmp(param, param_cert.c_str(), param_cert.length()) == 0) {
-            Ssl::ValidateCertificateResponse::CertItem ci;
+            CertItem ci;
             ci.name.assign(param, param_len); 
             X509_Pointer x509;
             readCertFromMemory(x509, value);
@@ -79,7 +79,7 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp,
 
             const char *b = strstr(value, "-----END CERTIFICATE-----");
             if (b == NULL) {
-                debugs(83, 2, "Parse error: Failed  to find certificate boundary " << value);
+                debugs(83, 2, "Cert Vailidator response parse error: Failed  to find certificate boundary " << value);
                 return false;
             }
             b += strlen("-----END CERTIFICATE-----");
@@ -90,51 +90,75 @@ bool Ssl::CertValidateMessage::parseResponse(ValidateCertificateResponse &resp,
         size_t value_len = strcspn(value, "\r\n");
         std::string v(value, value_len);
 
-        debugs(83, 5, "Returned value: " << std::string(param, param_len).c_str() << ": " << 
+        debugs(83, 5, HERE << "Returned value: " << std::string(param, param_len).c_str() << ": " << 
                v.c_str());
 
         int errorId = get_error_id(param, param_len);
-
-        if (current_errorId >=0 && errorId != current_errorId) {
-            resp.errors.push_back(currentItem);  
-            /*Reset current item:*/
-            currentItem.clear();
-        }
-        current_errorId = errorId;
+        Ssl::ValidateCertificateResponse::ErrorItem &currentItem = resp.getError(errorId);
 
         if (param_len > param_error_name.length() && 
             strncmp(param, param_error_name.c_str(), param_error_name.length()) == 0){
             currentItem.error_no = Ssl::GetErrorCode(v.c_str());
+            if (currentItem.error_no == SSL_ERROR_NONE) {
+                debugs(83, 2, "Cert validator response parse error: Unknown SSL Error: " << v);
+                return false;
+            }
         } else if (param_len > param_error_reason.length() && 
                    strncmp(param, param_error_reason.c_str(), param_error_reason.length()) == 0) {
             currentItem.error_reason = v;
         } else if (param_len > param_error_cert.length() &&
                    strncmp(param, param_error_cert.c_str(), param_error_cert.length()) == 0) {
 
-            for (std::vector<Ssl::ValidateCertificateResponse::CertItem>::const_iterator ci = certs.begin(); ci != certs.end(); ++ci) {
-                if (ci->name.compare(v) == 0) {
-                    currentItem.setCert(ci->cert);
-                    debugs(83, 6, HERE << "The custom cert \"" << ci->name << "\" used.");
-                    break;
-                }
-            }
-            if (!currentItem.cert) {
+            if (X509 *cert = getCertByName(certs, v)) {
+                debugs(83, 6, HERE << "The certificate with id \"" << v << "\" found.");
+                currentItem.setCert(cert);
+            } else {
+                //In this case we assume that the certID is one of the certificates sent
+                // to cert validator. The certificates sent to cert validator have names in
+                // form "cert_xx" where the "xx" is an integer represents the position of
+                // the certificate inside peer certificates list.
                 int certId = get_error_id(v.c_str(), v.length());
+                debugs(83, 6, HERE << "Cert index in peer certificates list:" << certId);
                 //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);
             }
-        } 
+        } else {
+            debugs(83, 2, "Cert validator response parse error: Unknown parameter name " << std::string(param, param_len).c_str());
+            return false;
+        }
+        
 
         param = value + value_len +1;
     }
 
-    if (currentItem.error_no != SSL_ERROR_NONE)
-        resp.errors.push_back(currentItem);
+    /*Run through parsed errors to check for errors*/
 
     return true;
 }
 
+X509 *
+Ssl::CertValidateMessage::getCertByName(std::vector<CertItem> const &certs, std::string const & name)
+{
+    for (std::vector<CertItem>::const_iterator ci = certs.begin(); ci != certs.end(); ++ci) {
+        if (ci->name.compare(name) == 0)
+            return ci->cert;
+    }
+    return NULL;
+}
+
+Ssl::ValidateCertificateResponse::ErrorItem &
+Ssl::ValidateCertificateResponse::getError(int errorId)
+{
+    for(Ssl::ValidateCertificateResponse::Errors::iterator i = errors.begin(); i != errors.end(); ++i){
+        if (i->id == errorId)
+            return *i;
+    }
+    Ssl::ValidateCertificateResponse::ErrorItem errItem;
+    errItem.id = errorId;
+    errors.push_back(errItem);
+    return errors.back();
+}
+
 Ssl::ValidateCertificateResponse::ErrorItem::ErrorItem(const ErrorItem &old) {
     error_no = old.error_no;
     error_reason = old.error_reason;
@@ -174,28 +198,28 @@ Ssl::ValidateCertificateResponse::ErrorItem::clear() {
     cert = NULL;
 }
 
-Ssl::ValidateCertificateResponse::CertItem::CertItem(const CertItem &old)
+Ssl::CertValidateMessage::CertItem::CertItem(const CertItem &old)
 {
     name = old.name;
     cert = NULL;
     setCert(old.cert);
 }
 
-Ssl::ValidateCertificateResponse::CertItem & Ssl::ValidateCertificateResponse::CertItem::operator = (const CertItem &old)
+Ssl::CertValidateMessage::CertItem & Ssl::CertValidateMessage::CertItem::operator = (const CertItem &old)
 {
     name = old.name;
     setCert(old.cert);
     return *this;
 }
 
-Ssl::ValidateCertificateResponse::CertItem::~CertItem()
+Ssl::CertValidateMessage::CertItem::~CertItem()
 {
     if (cert)
         X509_free(cert);
 }
 
 void
-Ssl::ValidateCertificateResponse::CertItem::setCert(X509 *aCert)
+Ssl::CertValidateMessage::CertItem::setCert(X509 *aCert)
 {
     if (cert)
         X509_free(cert);

diff --git a/src/ssl/cert_validate_message.h b/src/ssl/cert_validate_message.h
index 8b4a408251185b09473a77498e00b06cdac67b97..a2e4b80199ddcb4ca3b60ee894c9a51b1b2e1449 100644 (file)
--- a/src/ssl/cert_validate_message.h
+++ b/src/ssl/cert_validate_message.h
@@ -25,17 +25,29 @@ class ValidateCertificateResponse {
 public:
     class  ErrorItem{
     public:
-        ErrorItem(): error_no(SSL_ERROR_NONE), cert(NULL) {}
+        ErrorItem(): id(0), error_no(SSL_ERROR_NONE), cert(NULL) {}
         ErrorItem(const ErrorItem &);
         ~ErrorItem();
         ErrorItem & operator = (const ErrorItem &);
         void setCert(X509 *);
         void clear();
-        ssl_error_t error_no;
-        std::string error_reason;
-        X509 *cert;
+        int id; ///<  The id of the error
+        ssl_error_t error_no; ///< The SSL error code
+        std::string error_reason; ///< A string describing the error
+        X509 *cert; ///< The broken certificate
     };
 
+    typedef std::vector<ErrorItem> Errors;
+
+    ValidateCertificateResponse() {}
+    /// Search in errors list for an error with id=errorId
+    /// If know found a new ErrorItem added with the given id;
+    ErrorItem &getError(int errorId);
+    Errors errors; ///< The list of parsed errors
+};
+
+class CertValidateMessage: public CrtdMessage {
+private:
     class CertItem {
     public:
         std::string name;
@@ -46,17 +58,11 @@ public:
         ~CertItem();
         void setCert(X509 *);
     };
-
-    std::vector<ErrorItem> errors;
-    ValidateCertificateResponse() {}
-    ~ValidateCertificateResponse() {/*Maybe needs to release Errors*/};
-};
-
-class CertValidateMessage: public CrtdMessage {
 public:
     CertValidateMessage(): CrtdMessage() {}
     void composeRequest(ValidateCertificate const &vcert);
     bool parseResponse(ValidateCertificateResponse &resp, STACK_OF(X509) *peerCerts, std::string &error);
+    X509 *getCertByName(std::vector<CertItem> const &, std::string const & name); ///< search in a list of CertItems for a certificate
 
     /// String code for "cert_validate" messages
     static const std::string code_cert_validate;