------------------------------------------------------------
revno: 14162
revision-id: squid3@treenet.co.nz-20170529055234-790hfbazjwy0fmk4
parent: squid3@treenet.co.nz-20170529053359-xtbuev2zwmdfj9mp
fixes bug: http://bugs.squid-cache.org/show_bug.cgi?id=4711
author: Christos Tsantilas <chtsanti@users.sourceforge.net>
committer: Amos Jeffries <squid3@treenet.co.nz>
branch nick: 3.5
timestamp: Mon 2017-05-29 17:52:34 +1200
message:
  Bug 4711: SubjectAlternativeNames is missing in some generated certificates
  
  Squid may generate certificates which have a Common Name, but do not have
  a subjectAltName extension. For example when squid generated certificates
  do not mimic an origin certificate or when the certificate adaptation
  algorithm sslproxy_cert_adapt/setCommonName is used.
  
  This is causes problems to some browsers, which validates a certificate using
  the SubjectAlternativeNames but ignore the CommonName field.
  
  This patch fixes squid to always add a SubjectAlternativeNames extension in
  generated certificates which do not mimic an origin certificate.
  
  Squid still will not add a subjectAltName extension when mimicking an origin
  server certificate, even if that origin server certificate does not include
  the subjectAltName extension. Such origin server may have problems when
  talking directly to browsers, and patched Squid is not trying to fix those
  problems.
  
  This is a Measurement Factory project
------------------------------------------------------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: squid3@treenet.co.nz-20170529055234-790hfbazjwy0fmk4
# target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# testament_sha1: e3162152cf590c8126eb3d189ea1ab90ba9a5c37
# timestamp: 2017-05-29 05:54:13 +0000
# source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5
# base_revision_id: squid3@treenet.co.nz-20170529053359-\
#   xtbuev2zwmdfj9mp
# 
# Begin patch
=== modified file 'src/ssl/gadgets.cc'
--- src/ssl/gadgets.cc	2017-01-01 00:16:45 +0000
+++ src/ssl/gadgets.cc	2017-05-29 05:52:34 +0000
@@ -339,7 +339,40 @@
     return added;
 }
 
-static bool buildCertificate(Ssl::X509_Pointer & cert, Ssl::CertificateProperties const &properties)
+/// Adds a new subjectAltName extension contining Subject CN or returns false
+/// expects the caller to check for the existing subjectAltName extension
+static bool
+addAltNameWithSubjectCn(Ssl::X509_Pointer &cert)
+{
+    X509_NAME *name = X509_get_subject_name(cert.get());
+    if (!name)
+        return false;
+
+    const int loc = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
+    if (loc < 0)
+        return false;
+
+    ASN1_STRING *cn_data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, loc));
+    if (!cn_data)
+        return false;
+
+    char dnsName[1024]; // DNS names are limited to 256 characters
+    const int res = snprintf(dnsName, sizeof(dnsName), "DNS:%*s", cn_data->length, cn_data->data);
+    if (res <= 0 || res >= static_cast<int>(sizeof(dnsName)))
+        return false;
+
+    X509_EXTENSION *ext = X509V3_EXT_conf_nid(NULL, NULL, NID_subject_alt_name, dnsName);
+    if (!ext)
+        return false;
+
+    const bool result = X509_add_ext(cert.get(), ext, -1);
+
+    X509_EXTENSION_free(ext);
+    return result;
+}
+
+static bool
+buildCertificate(Ssl::X509_Pointer & cert, Ssl::CertificateProperties const &properties)
 {
     // not an Ssl::X509_NAME_Pointer because X509_REQ_get_subject_name()
     // returns a pointer to the existing subject name. Nothing to clean here.
@@ -387,6 +420,8 @@
     } else if (!X509_gmtime_adj(X509_get_notAfter(cert.get()), 60*60*24*356*3))
         return false;
 
+    int addedExtensions = 0;
+    bool useCommonNameAsAltName = true;
     // mimic the alias and possibly subjectAltName
     if (properties.mimicCert.get()) {
         unsigned char *alStr;
@@ -396,26 +431,29 @@
             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);
+            int pos = X509_get_ext_by_NID(properties.mimicCert.get(), NID_subject_alt_name, -1);
             X509_EXTENSION *ext=X509_get_ext(properties.mimicCert.get(), pos);
             if (ext) {
                 if (X509_add_ext(cert.get(), ext, -1))
                     ++addedExtensions;
             }
+            // We want to mimic the server-sent subjectAltName, not enhance it.
+            useCommonNameAsAltName = false;
         }
 
         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
     }
 
+    if (useCommonNameAsAltName && addAltNameWithSubjectCn(cert))
+        ++addedExtensions;
+
+    // According to RFC 5280, using extensions requires v3 certificate.
+    if (addedExtensions)
+        X509_set_version(cert.get(), 2); // value 2 means v3
+
     return true;
 }