SSL CN wildcard must only match a single domain component [fragment].

When comparing the requested domain name with a certificate Common Name,
Squid expanded wildcard to cover more than one domain name label (a.k.a
component), violating RFC 2818 requirement[1]. For example, Squid
thought that wrong.host.example.com matched a *.example.com CN.

    [1] "the wildcard character * ... is considered to match any single
    domain name component or component fragment. E.g., *.a.com matches
    foo.a.com but not bar.foo.a.com".

In other contexts (e.g., ACLs), wildcards expand to all components.
matchDomainName() now accepts a mdnRejectSubsubDomains flag that selects
the right behavior for CN match validation.

The old boolean honorWildcards parameter replaced with a flag, for clarity
and consistency sake.

This patch also handles the cases where the host name consists only from dots
(eg malformed Host header or SNI info). The old code has undefined behaniour
in these cases. Moreover it handles the cases a certificate contain zero length
string as CN or alternate name.

This is a Measurement Factory project.

diff --git a/src/URL.h b/src/URL.h
index 7076958b8755b1027629ac18767a32a3d3619877..9a6c0916bd0214cf060b04b684836d7f34ecd34e 100644 (file)
--- a/src/URL.h
+++ b/src/URL.h
@@ -73,23 +73,29 @@ char *urlMakeAbsolute(const HttpRequest *, const char *);
 char *urlRInternal(const char *host, unsigned short port, const char *dir, const char *name);
 char *urlInternal(const char *dir, const char *name);
 
+enum MatchDomainNameFlags {
+    mdnNone = 0,
+    mdnHonorWildcards = 1 << 0,
+    mdnRejectSubsubDomains = 1 << 1
+};
+
 /**
- * matchDomainName() compares a hostname (usually extracted from traffic)
- * with a domainname (usually from an ACL) according to the following rules:
+ * matchDomainName() matches a hostname (usually extracted from traffic)
+ * with a domainname when mdnNone or mdnRejectSubsubDomains flags are used
+ * according to the following rules:
  *
- *    HOST      |   DOMAIN    |   MATCH?
- * -------------|-------------|------
- *    foo.com   |   foo.com   |     YES
- *   .foo.com   |   foo.com   |     YES
- *  x.foo.com   |   foo.com   |     NO
- *    foo.com   |  .foo.com   |     YES
- *   .foo.com   |  .foo.com   |     YES
- *  x.foo.com   |  .foo.com   |     YES
+ *    HOST      |   DOMAIN    |   mdnNone | mdnRejectSubsubDomains
+ * -------------|-------------|-----------|-----------------------
+ *      foo.com |   foo.com   |     YES   |   YES
+ *     .foo.com |   foo.com   |     YES   |   YES
+ *    x.foo.com |   foo.com   |     NO    |   NO
+ *      foo.com |  .foo.com   |     YES   |   YES
+ *     .foo.com |  .foo.com   |     YES   |   YES
+ *    x.foo.com |  .foo.com   |     YES   |   YES
+ *   .x.foo.com |  .foo.com   |     YES   |   NO
+ *  y.x.foo.com |  .foo.com   |     YES   |   NO
  *
- *  We strip leading dots on hosts (but not domains!) so that
- *  ".foo.com" is always the same as "foo.com".
- *
- * if honorWildcards is true then the matchDomainName() also accepts
+ * if mdnHonorWildcards flag is set then the matchDomainName() also accepts
  * optional wildcards on hostname:
  *
  *    HOST      |    DOMAIN    |  MATCH?
@@ -99,11 +105,14 @@ char *urlInternal(const char *dir, const char *name);
  *    *.foo.com |    .foo.com  |   YES
  *    *.foo.com |     foo.com  |   NO
  *
+ * The combination of mdnHonorWildcards and mdnRejectSubsubDomains flags is
+ * supported.
+ *
  * \retval 0 means the host matches the domain
  * \retval 1 means the host is greater than the domain
  * \retval -1 means the host is less than the domain
  */
-int matchDomainName(const char *host, const char *domain, bool honorWildcards = false);
+int matchDomainName(const char *host, const char *domain, uint flags = mdnNone);
 int urlCheckRequest(const HttpRequest *);
 int urlDefaultPort(AnyP::ProtocolType p);
 char *urlHostname(const char *url);

diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc
index 9cc08b28542fe2dafc0750440cb6cad2762e6d14..f49209bdc08573074083baa8d9c03442da1600ba 100644 (file)
--- a/src/acl/ServerName.cc
+++ b/src/acl/ServerName.cc
@@ -30,7 +30,7 @@ aclHostDomainCompare( char *const &a, char * const &b)
     const char *h = static_cast<const char *>(a);
     const char *d = static_cast<const char *>(b);
     debugs(28, 7, "Match:" << h << " <>  " << d);
-    return matchDomainName(h, d, true);
+    return matchDomainName(h, d, mdnHonorWildcards);
 }
 
 bool

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 6f01a9431302ae507b719636087c7e9667a90d53..5e334473b223f39b33fb704d254ccd86c3d9c6ee 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -199,9 +199,12 @@ static int check_domain( void *check_data, ASN1_STRING *cn_data)
     char cn[1024];
     const char *server = (const char *)check_data;
 
-    if (cn_data->length > (int)sizeof(cn) - 1) {
+    if (cn_data->length == 0)
+        return 1; // zero length cn, ignore
+
+    if (cn_data->length > (int)sizeof(cn) - 1)
         return 1; //if does not fit our buffer just ignore
-    }
+
     char *s = reinterpret_cast<char*>(cn_data->data);
     char *d = cn;
     for (int i = 0; i < cn_data->length; ++i, ++d, ++s) {
@@ -211,7 +214,7 @@ static int check_domain( void *check_data, ASN1_STRING *cn_data)
     }
     cn[cn_data->length] = '\0';
     debugs(83, 4, "Verifying server domain " << server << " to certificate name/subjectAltName " << cn);
-    return matchDomainName(server, cn[0] == '*' ? cn + 1 : cn);
+    return matchDomainName(server, (cn[0] == '*' ? cn + 1 : cn), mdnRejectSubsubDomains);
 }
 
 bool Ssl::checkX509ServerValidity(X509 *cert, const char *server)

diff --git a/src/url.cc b/src/url.cc
index ebdecbf6bde23420aaae6b15ecac9c1246f6eb1f..c7ffa92cdc0b529d9c477bcfcb63503e31ffc3e0 100644 (file)
--- a/src/url.cc
+++ b/src/url.cc
@@ -54,6 +54,7 @@ urlInitialize(void)
     assert(0 == matchDomainName("foo.com", ".foo.com"));
     assert(0 == matchDomainName(".foo.com", ".foo.com"));
     assert(0 == matchDomainName("x.foo.com", ".foo.com"));
+    assert(0 == matchDomainName("y.x.foo.com", ".foo.com"));
     assert(0 != matchDomainName("x.foo.com", "foo.com"));
     assert(0 != matchDomainName("foo.com", "x.foo.com"));
     assert(0 != matchDomainName("bar.com", "foo.com"));
@@ -66,6 +67,17 @@ urlInitialize(void)
     assert(0 < matchDomainName("bfoo.com", "afoo.com"));
     assert(0 > matchDomainName("afoo.com", "bfoo.com"));
     assert(0 < matchDomainName("x-foo.com", ".foo.com"));
+
+    assert(0 == matchDomainName(".foo.com", ".foo.com", mdnRejectSubsubDomains));
+    assert(0 == matchDomainName("x.foo.com", ".foo.com", mdnRejectSubsubDomains));
+    assert(0 != matchDomainName("y.x.foo.com", ".foo.com", mdnRejectSubsubDomains));
+    assert(0 != matchDomainName(".x.foo.com", ".foo.com", mdnRejectSubsubDomains));
+
+    assert(0 == matchDomainName("*.foo.com", "x.foo.com", mdnHonorWildcards));
+    assert(0 == matchDomainName("*.foo.com", ".x.foo.com", mdnHonorWildcards));
+    assert(0 == matchDomainName("*.foo.com", ".foo.com", mdnHonorWildcards));
+    assert(0 != matchDomainName("*.foo.com", "foo.com", mdnHonorWildcards));
+
     /* more cases? */
 }
 
@@ -690,16 +702,20 @@ urlMakeAbsolute(const HttpRequest * req, const char *relUrl)
 }
 
 int
-matchDomainName(const char *h, const char *d, bool honorWildcards)
+matchDomainName(const char *h, const char *d, uint flags)
 {
     int dl;
     int hl;
 
+    const bool hostIncludesSubdomains = (*h == '.');
     while ('.' == *h)
         ++h;
 
     hl = strlen(h);
 
+    if (hl == 0)
+        return -1;
+
     dl = strlen(d);
 
     /*
@@ -737,9 +753,20 @@ matchDomainName(const char *h, const char *d, bool honorWildcards)
              * is a leading '.'.
              */
 
-            if ('.' == d[0])
-                return 0;
-            else
+            if ('.' == d[0]) {
+                if (flags & mdnRejectSubsubDomains) {
+                    // Check for sub-sub domain and reject
+                    while(--hl >= 0 && h[hl] != '.');
+                    if (hl < 0) {
+                        // No sub-sub domain found, but reject if there is a
+                        // leading dot in given host string (which is removed
+                        // before the check is started).
+                        return hostIncludesSubdomains ? 1 : 0;
+                    } else
+                        return 1; // sub-sub domain, reject
+                } else
+                    return 0;
+            } else
                 return 1;
         }
     }
@@ -751,7 +778,7 @@ matchDomainName(const char *h, const char *d, bool honorWildcards)
     // If the h has a form of "*.foo.com" and d has a form of "x.foo.com"
     // then the h[hl] points to '*', h[hl+1] to '.' and d[dl] to 'x'
     // The following checks are safe, the "h[hl + 1]" in the worst case is '\0'.
-    if (honorWildcards && h[hl] == '*' && h[hl + 1] == '.')
+    if ((flags & mdnHonorWildcards) && h[hl] == '*' && h[hl + 1] == '.')
         return 0;
 
     /*