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 9495a7f62d3dfbadabc5321dab857fc657792506..c6a4717d827ed15288757526c67ea428d7b7097d 100644 (file)
--- a/src/URL.h
+++ b/src/URL.h
@@ -174,23 +174,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?
@@ -200,11 +206,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 *);
 char *urlHostname(const char *url);
 void urlExtMethodConfigure(void);

diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc
index 9eb1754887212bbaaf75dffe3932315c26566190..0ddd77ccaee585ad63ca59dc0e11d39509543812 100644 (file)
--- a/src/acl/ServerName.cc
+++ b/src/acl/ServerName.cc
@@ -31,7 +31,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 3ba143a72d822a2c661e6d0aefc322a5d5d12566..7c8f5c1bab781e14bf70ae16bb307a48c96b59a9 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -202,9 +202,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) {
@@ -214,7 +217,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 6b37a84b45af4f603c36980f856ae94149fb6f6f..b516a2f469d5927a5b74a69d1b7b58dee4a97fdc 100644 (file)
--- a/src/url.cc
+++ b/src/url.cc
@@ -97,6 +97,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"));
@@ -109,6 +110,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? */
 }
 
@@ -667,16 +679,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);
 
     /*
@@ -714,9 +730,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;
         }
     }
@@ -728,7 +755,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;
 
     /*