------------------------------------------------------------ revno: 14079 revision-id: squid3@treenet.co.nz-20160908122706-2yt62k87gop2mfe5 parent: squidadm@squid-cache.org-20160907181433-zpuak0ab0rav5hrk author: Christos Tsantilas committer: Amos Jeffries branch nick: 3.5 timestamp: Fri 2016-09-09 00:27:06 +1200 message: 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. ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20160908122706-2yt62k87gop2mfe5 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 11fade6d29041e192c12ced93fdfb887c29ca4a8 # timestamp: 2016-09-08 12:50:59 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squidadm@squid-cache.org-20160907181433-\ # zpuak0ab0rav5hrk # # Begin patch === modified file 'src/URL.h' --- src/URL.h 2016-01-01 00:14:27 +0000 +++ src/URL.h 2016-09-08 12:27:06 +0000 @@ -73,23 +73,29 @@ 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: - * - * 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 - * - * 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 + * 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 | 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 + * + * if mdnHonorWildcards flag is set then the matchDomainName() also accepts * optional wildcards on hostname: * * HOST | DOMAIN | MATCH? @@ -99,11 +105,14 @@ * *.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); === modified file 'src/acl/ServerName.cc' --- src/acl/ServerName.cc 2016-01-01 00:14:27 +0000 +++ src/acl/ServerName.cc 2016-09-08 12:27:06 +0000 @@ -30,7 +30,7 @@ const char *h = static_cast(a); const char *d = static_cast(b); debugs(28, 7, "Match:" << h << " <> " << d); - return matchDomainName(h, d, true); + return matchDomainName(h, d, mdnHonorWildcards); } bool === modified file 'src/ssl/support.cc' --- src/ssl/support.cc 2016-06-15 22:08:16 +0000 +++ src/ssl/support.cc 2016-09-08 12:27:06 +0000 @@ -199,9 +199,12 @@ 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(cn_data->data); char *d = cn; for (int i = 0; i < cn_data->length; ++i, ++d, ++s) { @@ -211,7 +214,7 @@ } 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) === modified file 'src/url.cc' --- src/url.cc 2016-05-06 06:25:07 +0000 +++ src/url.cc 2016-09-08 12:27:06 +0000 @@ -54,6 +54,7 @@ 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 @@ 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 @@ } 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 @@ * 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 @@ // 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; /*