Use v3 for fake certificate if we add _any_ certificate extension.

We used to force v3 version only when adding the subjectAltName extension.
That broke sites that did not have subjectAltName but used other mimicked x509
extensions, when accessed through Firefox 31 (at least):
  https://bugzilla.mozilla.org/show_bug.cgi?id=1045973

diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc
index a740cf53fb0a79a4ad50f8fa7c4eafe783288181..17b6c371982992594b39ffdbfe5c08224d5d8ea2 100644 (file)
--- a/src/ssl/gadgets.cc
+++ b/src/ssl/gadgets.cc
@@ -249,11 +249,12 @@ std::string & Ssl::CertificateProperties::dbKey() const
     return certKey;
 }
 
-// Copy certificate extensions from cert to mimicCert.
+/// Copy certificate extensions from cert to mimicCert.
+/// Returns the number of extensions copied.
 // Currently only extensions which are reported by the users that required are
 // mimicked. More safe to mimic extensions would be added here if users request
 // them.
-static void
+static int
 mimicExtensions(Ssl::X509_Pointer & cert, Ssl::X509_Pointer const & mimicCert)
 {
     static int extensions[]= {
@@ -278,12 +279,14 @@ mimicExtensions(Ssl::X509_Pointer & cert, Ssl::X509_Pointer const & mimicCert)
 
     int mimicAlgo = OBJ_obj2nid(mimicCert.get()->cert_info->key->algor->algorithm);
 
+    int added = 0;
     int nid;
     for (int i = 0; (nid = extensions[i]) != 0; ++i) {
         const int pos = X509_get_ext_by_NID(mimicCert.get(), nid, -1);
         if (X509_EXTENSION *ext = X509_get_ext(mimicCert.get(), pos)) {
             // Mimic extension exactly.
-            X509_add_ext(cert.get(), ext, -1);
+            if (X509_add_ext(cert.get(), ext, -1))
+                ++added;
             if ( nid == NID_key_usage && mimicAlgo != NID_rsaEncryption ) {
                 // NSS does not requre the KeyEncipherment flag on EC keys
                 // but it does require it for RSA keys.  Since ssl-bump
@@ -303,6 +306,8 @@ mimicExtensions(Ssl::X509_Pointer & cert, Ssl::X509_Pointer const & mimicCert)
 
     // We could also restrict mimicking of the CA extension to CA:FALSE
     // because Squid does not generate valid fake CA certificates.
+
+    return added;
 }
 
 static bool buildCertificate(Ssl::X509_Pointer & cert, Ssl::CertificateProperties const &properties)
@@ -362,22 +367,24 @@ static bool buildCertificate(Ssl::X509_Pointer & cert, Ssl::CertificatePropertie
             X509_alias_set1(cert.get(), alStr, alLen);
         }
 
+        int addedExtensions = 0;
+
         // Mimic subjectAltName unless we used a configured CN: browsers reject
         // certificates with CN unrelated to subjectAltNames.
         if (!properties.setCommonName) {
             int pos=X509_get_ext_by_NID (properties.mimicCert.get(), OBJ_sn2nid("subjectAltName"), -1);
             X509_EXTENSION *ext=X509_get_ext(properties.mimicCert.get(), pos);
             if (ext) {
-                X509_add_ext(cert.get(), ext, -1);
-                /* According the RFC 5280 using extensions requires version 3
-                   certificate.
-                   Set version value to 2 for version 3 certificates.
-                 */
-                X509_set_version(cert.get(), 2);
+                if (X509_add_ext(cert.get(), ext, -1))
+                    ++addedExtensions;
             }
         }
 
-        mimicExtensions(cert, properties.mimicCert);
+        addedExtensions += mimicExtensions(cert, properties.mimicCert);
+
+        // According to RFC 5280, using extensions requires v3 certificate.
+        if (addedExtensions)
+            X509_set_version(cert.get(), 2); // value 2 means v3
     }
 
     return true;