Police ssl_crtd generating/parsing request code

This patch:

Add the following methods:
- Ssl::CrtdMessage::parseRequest: Parse a ssl_crtd "new certificate" request and
  store the requested certificate properties to a CertificateProperties object

- Ssl::CrtdMessage::composeRequest: Generate a "new certificate" ssl_crtd
  request using the certificate properties given by a CertificateProperties
  object

- Ssl::CertificateProperties::dbKey:  Return a valid key for the generated
  certificate to be used with a (memory or disk) database, based on its
  properties

Others:
- The ssl_crtd.cc code simplified using the above methods
- The ConnStateData::buildSslCertGenerationParams and ConnStateData::getSslContextStart
  simplified using the above methods

diff --git a/src/client_side.cc b/src/client_side.cc
index f3a2530be84e9ed2b5eeb66c00a9bd28a7865936..b50330fac1c6aa32296a3c9c65c50cca0cfb27f7 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -3652,11 +3652,9 @@ ConnStateData::sslCrtdHandleReply(const char * reply)
     getSslContextDone(NULL);
 }
 
-void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties, Ssl::CrtdMessage::BodyParams &certAdaptParams)
+void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties)
 {
-    // Build a key to use for storing/retrieving certificates to cache
-    sslBumpCertKey = sslCommonName.defined() ? sslCommonName : sslConnectHostOrIp;
-    certProperties.commonName = sslBumpCertKey.termedBuf();
+    certProperties.commonName =  sslCommonName.defined() ? sslCommonName.termedBuf() : sslConnectHostOrIp.termedBuf();
 
     // fake certificate adaptation requires bump-server-first mode
     if (!bumpServerFirstErrorEntry())
@@ -3682,46 +3680,29 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
             const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg];
             const char *param = ca->param;
   
-            // if already set ignore
-            if (certAdaptParams.find(alg) != certAdaptParams.end())
-                continue;
-
             // if not param defined for Common Name adaptation use hostname from 
             // the CONNECT request
-            if (ca->alg == Ssl::algSetCommonName) {
+            if (ca->alg == Ssl::algSetCommonName && !certProperties.setCommonName) {
                 if (!param)
                     param = sslConnectHostOrIp.termedBuf();
                 certProperties.commonName = param;
                 certProperties.setCommonName = true;
             }
-            else if(ca->alg == Ssl::algSetValidAfter)
+            else if(ca->alg == Ssl::algSetValidAfter && !certProperties.setValidAfter)
                 certProperties.setValidAfter = true;
-            else if(ca->alg == Ssl::algSetValidBefore)
+            else if(ca->alg == Ssl::algSetValidBefore && !certProperties.setValidBefore)
                 certProperties.setValidBefore = true;
 
             assert(alg && param);
             debugs(33, 5, HERE << "Matches certificate adaptation aglorithm: " << 
                    alg << " param: " << param);
-
-            // append to the certificate adaptation parameters
-            certAdaptParams.insert( std::make_pair(alg, param));
-
-            // And also build the key used to store certificate to cache.
-            sslBumpCertKey.append("+");
-            sslBumpCertKey.append(alg);
-            sslBumpCertKey.append("=");
-            sslBumpCertKey.append(param);
         }
     }
 
     certProperties.signAlgorithm = Ssl::algSignEnd;
     for (sslproxy_cert_sign *sg = Config.ssl_client.cert_sign; sg != NULL; sg = sg->next) {
         if (sg->aclList && checklist.fastCheck(sg->aclList) == ACCESS_ALLOWED) {
-            const char *sgAlg = Ssl::CertSignAlgorithmStr[sg->alg];
             certProperties.signAlgorithm = (Ssl::CertSignAlgorithm)sg->alg;
-            certAdaptParams.insert( std::make_pair(Ssl::CrtdMessage::param_Sign, sgAlg));
-            sslBumpCertKey.append("+Sign=");
-            sslBumpCertKey.append(sgAlg);
             break;
         }
     }
@@ -3747,9 +3728,9 @@ void
 ConnStateData::getSslContextStart()
 {
     if (port->generateHostCertificates) {
-        Ssl::CrtdMessage::BodyParams certAdaptParams;
         Ssl::CertificateProperties certProperties;
-        buildSslCertGenerationParams(certProperties, certAdaptParams);
+        buildSslCertGenerationParams(certProperties);
+        sslBumpCertKey = certProperties.dbKey().c_str();
         assert(sslBumpCertKey.defined() && sslBumpCertKey[0] != '\0');
 
         debugs(33, 5, HERE << "Finding SSL certificate for " << sslBumpCertKey << " in cache");
@@ -3773,17 +3754,7 @@ ConnStateData::getSslContextStart()
         debugs(33, 5, HERE << "Generating SSL certificate for " << certProperties.commonName << " using ssl_crtd.");
         Ssl::CrtdMessage request_message;
         request_message.setCode(Ssl::CrtdMessage::code_new_certificate);
-        Ssl::CrtdMessage::BodyParams map;
-        map.insert(std::make_pair(Ssl::CrtdMessage::param_host, certProperties.commonName));
-        /*Append parameters for cert adaptation*/
-        map.insert(certAdaptParams.begin(), certAdaptParams.end());
-        std::string bufferToWrite;
-        Ssl::writeCertAndPrivateKeyToMemory(certProperties.signWithX509, certProperties.signWithPkey, bufferToWrite);
-        if (certProperties.mimicCert.get()) {
-            Ssl::appendCertToMemory(certProperties.mimicCert, bufferToWrite);
-            debugs(33, 5, HERE << "Appended certificate to mimic: " << bufferToWrite);
-        }
-        request_message.composeBody(map, bufferToWrite);
+        request_message.composeRequest(certProperties);
         debugs(33, 5, HERE << "SSL crtd request: " << request_message.compose().c_str());
         Ssl::Helper::GetInstance()->sslSubmit(request_message, sslCrtdHandleReplyWrapper, this);
         return;

diff --git a/src/client_side.h b/src/client_side.h
index 60753dabaa3cec0157348e31eb13a6e7d6518486..065729d93aef55fbee5f33e94d8d0b7e96e31670 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -342,7 +342,7 @@ public:
     void setBumpSslErrorList(Ssl::Errors *errNoList) {bumpSslErrorNoList = cbdataReference(errNoList);}
     /// Fill the certAdaptParams with the required data for certificate adaptation
     /// and create the key for storing/retrieve the certificate to/from the cache
-    void buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties, Ssl::CrtdMessage::BodyParams &certAdaptParams);
+    void buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties);
     bool serveDelayedError(ClientSocketContext *context);
 #else
     bool switchedToHttps() const { return false; }

diff --git a/src/ssl/crtd_message.cc b/src/ssl/crtd_message.cc
index 6d870336c8274067bb8ab5e1d845e80853fcc87b..ddcda17560a496f9d178e3881676b0e42962d1ff 100644 (file)
--- a/src/ssl/crtd_message.cc
+++ b/src/ssl/crtd_message.cc
@@ -174,6 +174,83 @@ void Ssl::CrtdMessage::composeBody(CrtdMessage::BodyParams const & map, std::str
         body += '\n' + other_part;
 }
 
+bool Ssl::CrtdMessage::parseRequest(Ssl::CertificateProperties &certProperties, std::string error)
+{
+    Ssl::CrtdMessage::BodyParams map;
+    std::string certs_part;
+    parseBody(map, certs_part);
+    Ssl::CrtdMessage::BodyParams::iterator i = map.find(Ssl::CrtdMessage::param_host);
+    if (i == map.end()) {
+        error = "Parse request: Cannot find \"host\" parameter in request message";
+        return false;
+    }
+    certProperties.commonName = i->second;
+    
+     i = map.find(Ssl::CrtdMessage::param_SetValidAfter);
+    if (i != map.end() && strcasecmp(i->second.c_str(), "on") == 0)
+        certProperties.setValidAfter = true;
+    
+    i = map.find(Ssl::CrtdMessage::param_SetValidBefore);
+    if (i != map.end() && strcasecmp(i->second.c_str(), "on") == 0)
+        certProperties.setValidBefore = true;
+
+    i = map.find(Ssl::CrtdMessage::param_SetCommonName);
+    if (i != map.end()) {
+        // use this as Common Name  instead of the hostname 
+        // defined with host or Common Name from mimic cert
+        certProperties.commonName = i->second;
+        certProperties.setCommonName = true;
+    }
+
+    i = map.find(Ssl::CrtdMessage::param_Sign);
+    if (i != map.end()) {
+        if ((certProperties.signAlgorithm = Ssl::certSignAlgorithmId(i->second.c_str())) == Ssl::algSignEnd) {
+            error = "Parse request: Wrong signing algoritm: " + i->second;
+            return false;
+        }
+    }
+    else
+        certProperties.signAlgorithm = Ssl::algSignTrusted;
+
+    if (certProperties.signAlgorithm != Ssl::algSignSelf && 
+        !Ssl::readCertAndPrivateKeyFromMemory(certProperties.signWithX509, certProperties.signWithPkey, certs_part.c_str())) {
+        error = "Parse request: Broken signing certificate!";
+        return false;
+    }
+
+    static const std::string CERT_BEGIN_STR("-----BEGIN CERTIFICATE");
+    size_t pos;
+    if ((pos = certs_part.find(CERT_BEGIN_STR)) != std::string::npos) {
+        pos += CERT_BEGIN_STR.length();
+        if ((pos= certs_part.find(CERT_BEGIN_STR, pos)) != std::string::npos)
+            Ssl::readCertFromMemory(certProperties.mimicCert, certs_part.c_str() + pos);
+    }
+    return true;
+}
+
+void Ssl::CrtdMessage::composeRequest(Ssl::CertificateProperties const &certProperties)
+{
+    body.clear();
+    body = Ssl::CrtdMessage::param_host + "=" + certProperties.commonName;
+    if (certProperties.setCommonName)
+        body +=  "\n" + Ssl::CrtdMessage::param_SetCommonName + "=" + certProperties.commonName;
+    if (certProperties.setValidAfter)
+        body +=  "\n" + Ssl::CrtdMessage::param_SetValidAfter + "=on";
+    if (certProperties.setValidBefore)
+        body +=  "\n" + Ssl::CrtdMessage::param_SetValidBefore + "=on";
+    if(certProperties.signAlgorithm != Ssl::algSignEnd)
+        body +=  "\n" +  Ssl::CrtdMessage::param_Sign + "=" +  certSignAlgorithm(certProperties.signAlgorithm);
+
+    std::string certsPart;
+    bool ret = Ssl::writeCertAndPrivateKeyToMemory(certProperties.signWithX509, certProperties.signWithPkey, certsPart);
+    assert(ret);
+    if (certProperties.mimicCert.get()) {
+        ret = Ssl::appendCertToMemory(certProperties.mimicCert, certsPart);
+        assert(ret);
+    }
+    body += "\n" + certsPart;
+}
+
 const std::string Ssl::CrtdMessage::code_new_certificate("new_certificate");
 const std::string Ssl::CrtdMessage::param_host("host");
 const std::string Ssl::CrtdMessage::param_SetValidAfter(Ssl::CertAdaptAlgorithmStr[algSetValidAfter]);

diff --git a/src/ssl/crtd_message.h b/src/ssl/crtd_message.h
index 86489b53d085ec8ed5ae2afd1f4feaaccdd9b8ba..6149cf722767696b2022bcb3c939880077cd33c8 100644 (file)
--- a/src/ssl/crtd_message.h
+++ b/src/ssl/crtd_message.h
@@ -14,6 +14,8 @@
 
 namespace Ssl
 {
+class CertificateProperties;
+
 /**
  * This class is responsible for composing and parsing messages destined to, or comming
  * from an ssl_crtd server. Format of these mesages is:
@@ -61,6 +63,10 @@ public:
          The other multistring part of body.  \endverbatim
     */
     void composeBody(BodyParams const & map, std::string const & other_part);
+
+    bool parseRequest(Ssl::CertificateProperties &, std::string error);
+    void composeRequest(Ssl::CertificateProperties const &);
+
     /// String code for "new_certificate" messages
     static const std::string code_new_certificate;
     /// Parameter name for passing hostname

diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc
index bbfd252311802cbfb0f69577abb1415578c65f53..e5a266f4df07c4244207c98a365cbb60ba3e1ccd 100644 (file)
--- a/src/ssl/gadgets.cc
+++ b/src/ssl/gadgets.cc
@@ -190,6 +190,37 @@ Ssl::CertificateProperties::CertificateProperties(): serial(NULL),
                                                      signAlgorithm(Ssl::algSignEnd)
 {}
 
+std::string & Ssl::CertificateProperties::dbKey() const
+{
+    static std::string certKey;
+    certKey.reserve(4096);
+    if (mimicCert.get()) {
+        char buf[1024];
+        certKey = X509_NAME_oneline(X509_get_subject_name(mimicCert.get()), buf, sizeof(buf));
+    }
+
+    if (certKey.empty())
+        certKey = "/CN=" + commonName;
+
+    if (setValidAfter)
+        certKey.append("+SetValidAfter=on", 17);
+
+    if (setValidBefore)
+        certKey.append("+SetValidBefore=on", 18);
+
+    if (setCommonName) {
+        certKey.append("+SetCommonName=", 15);
+        certKey.append(commonName);
+    }
+
+    if (signAlgorithm != Ssl::algSignEnd) {
+        certKey.append("+Sign=", 6);
+        certKey.append(certSignAlgorithm(signAlgorithm));
+    }
+
+    return certKey;
+}
+
 static bool buildCertificate(Ssl::X509_Pointer & cert, Ssl::CertificateProperties const &properties)
 { 
     // not an Ssl::X509_NAME_Pointer because X509_REQ_get_subject_name()

diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h
index 10d46188e03ef9c879596a9e818f2b76641bd834..5d7f032cc59a4689a6bb25145fff8b487a3942f1 100644 (file)
--- a/src/ssl/gadgets.h
+++ b/src/ssl/gadgets.h
@@ -210,6 +210,9 @@ public:
     bool setCommonName; ///< Replace the CN field of the mimicing subject with the given
     std::string commonName; ///< A CN to use for the generated certificate
     CertSignAlgorithm signAlgorithm; ///< The signing algorithm to use
+    /// Returns certificate database primary key. New fake certificates
+    /// purge old fake certificates with the same key.
+    std::string & dbKey() const;
 private:
     CertificateProperties(CertificateProperties &);
     CertificateProperties &operator =(CertificateProperties const &);

diff --git a/src/ssl/ssl_crtd.cc b/src/ssl/ssl_crtd.cc
index 2c3ae0c2a54a5a2d589ff89f638782c06118754b..0855b7046743aacc7c8b7ba11da7c5652228f630 100644 (file)
--- a/src/ssl/ssl_crtd.cc
+++ b/src/ssl/ssl_crtd.cc
@@ -83,7 +83,6 @@ usage: ssl_crtd -g -s ssl_store_path
  \endverbatim
  */
 
-#define CERT_BEGIN_STR "-----BEGIN CERTIFICATE"
 static const char *const B_KBYTES_STR = "KB";
 static const char *const B_MBYTES_STR = "MB";
 static const char *const B_GBYTES_STR = "GB";
@@ -210,71 +209,18 @@ static void usage()
  \ingroup ssl_crtd
  * Proccess new request message.
  */
-static bool proccessNewRequest(Ssl::CrtdMessage const & request_message, std::string const & db_path, size_t max_db_size, size_t fs_block_size)
+static bool proccessNewRequest(Ssl::CrtdMessage & request_message, std::string const & db_path, size_t max_db_size, size_t fs_block_size)
 {
-    Ssl::CrtdMessage::BodyParams map;
     Ssl::CertificateProperties certProperties;
-    std::string body_part;
-    request_message.parseBody(map, body_part);
-
-    Ssl::CrtdMessage::BodyParams::iterator i = map.find(Ssl::CrtdMessage::param_host);
-    if (i == map.end())
-        throw std::runtime_error("Cannot find \"" + Ssl::CrtdMessage::param_host + "\" parameter in request message.");
-    certProperties.commonName = i->second;
+    std::string error;
+    if (!request_message.parseRequest(certProperties, error))
+        throw std::runtime_error("Error while parsing the crtd request" + error);
 
     Ssl::CertificateDb db(db_path, max_db_size, fs_block_size);
-
     Ssl::X509_Pointer cert;
     Ssl::EVP_PKEY_Pointer pkey;
+    std::string &cert_subject = certProperties.dbKey();
     
-    const char *s;
-    std::string cert_subject;
-    if ((s = strstr(body_part.c_str(), CERT_BEGIN_STR))) {
-        s += strlen(CERT_BEGIN_STR);
-        if ((s = strstr(s, CERT_BEGIN_STR))) {
-            Ssl::readCertFromMemory(certProperties.mimicCert, s);
-            if (certProperties.mimicCert.get()) {
-                char buf[1024];
-                cert_subject = X509_NAME_oneline(X509_get_subject_name(certProperties.mimicCert.get()), buf, sizeof(buf));
-            }
-        }
-    }
-
-    if (cert_subject.empty())
-        cert_subject = "/CN=" + certProperties.commonName;
-
-    i = map.find(Ssl::CrtdMessage::param_SetValidAfter);
-    if (i != map.end() && strcasecmp(i->second.c_str(), "on") == 0) {
-        cert_subject.append("+SetValidAfter=on");
-        certProperties.setValidAfter = true;
-    }
-    
-    i = map.find(Ssl::CrtdMessage::param_SetValidBefore);
-    if (i != map.end() && strcasecmp(i->second.c_str(), "on") == 0) {
-        cert_subject.append("+SetValidBefore=on");
-        certProperties.setValidBefore = true;
-    }
-
-    i = map.find(Ssl::CrtdMessage::param_SetCommonName);
-    if (i != map.end()) {
-        cert_subject.append("+SetCommonName=");
-        cert_subject.append(i->second);
-        // use this as Common Name  instead of the hostname 
-        // defined with host or Common Name from mimic cert
-        certProperties.commonName = i->second;
-        certProperties.setCommonName = true;
-    }
-
-    i = map.find(Ssl::CrtdMessage::param_Sign);
-    if (i != map.end()) {
-        cert_subject.append("+Sign=");
-        cert_subject.append(i->second);
-        if ((certProperties.signAlgorithm = Ssl::certSignAlgorithmId(i->second.c_str())) == Ssl::algSignEnd)
-            throw std::runtime_error("Wrong signing algoritm:" + i->second);
-    }
-    else
-        certProperties.signAlgorithm = Ssl::algSignTrusted;
-
     db.find(cert_subject, cert, pkey);
 
     if (cert.get() && certProperties.mimicCert.get()) {
@@ -286,12 +232,7 @@ static bool proccessNewRequest(Ssl::CrtdMessage const & request_message, std::st
         }
     }
 
-    if (!cert || !pkey) {
-        if (certProperties.signAlgorithm != Ssl::algSignSelf) {
-            if (!Ssl::readCertAndPrivateKeyFromMemory(certProperties.signWithX509, certProperties.signWithPkey, body_part.c_str()))
-                throw std::runtime_error("Broken signing certificate!");
-        } /*else Squid did not send certificate to sign the generated certificate*/
-        
+    if (!cert || !pkey) {        
         certProperties.serial.reset(db.getCurrentSerialNumber());
 
         if (!Ssl::generateSslCertificate(cert, pkey, certProperties))