diff options
| -rw-r--r-- | CHANGES | 15 | ||||
| -rw-r--r-- | RELEASE-NOTES | 3 | ||||
| -rw-r--r-- | lib/ssluse.c | 33 |
3 files changed, 39 insertions, 12 deletions
@@ -7,6 +7,21 @@ Changelog Daniel Stenberg (1 Aug 2009) +- Scott Cantor posted the bug report #2829955 + (http://curl.haxx.se/bug/view.cgi?id=2829955) mentioning the recent SSL cert + verification flaw found and exploited by Moxie Marlinspike. The presentation + he did at Black Hat is available here: + https://www.blackhat.com/html/bh-usa-09/bh-usa-09-archives.html#Marlinspike + + Apparently at least one CA allowed a subjectAltName or CN that contain a + zero byte, and thus clients that assumed they would never have zero bytes + were exploited to OK a certificate that didn't actually match the site. Like + if the name in the cert was "example.com\0theatualsite.com", libcurl would + happily verify that cert for example.com. + + libcurl now better use the length of the extracted name, not assuming it is + zero terminated. + - Tanguy Fautre pointed out that OpenSSL's function RAND_screen() (present only in some OpenSSL installs - like on Windows) isn't thread-safe and we agreed that moving it to the global_init() function is a decent way to deal diff --git a/RELEASE-NOTES b/RELEASE-NOTES index bd5700f77..b019bbc74 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -40,6 +40,7 @@ This release includes the following bugfixes: o missing algorithms in libcurl+OpenSSL o with noproxy set you could still get a proxy if a proxy env was set o rand seeding on libcurl on windows built with OpenSSL was not thread-safe + o fixed the zero byte inserted in cert name flaw in libcurl+OpenSSL This release includes the following known bugs: @@ -53,6 +54,6 @@ advice from friends like these: Aaron Oneal, Igor Novoseltsev, Eric Wong, Bill Hoffman, Daniel Steinberg, Fabian Keil, Michal Marek, Reuven Wachtfogel, Markus Koetter, Constantine Sapuntzakis, David Binderman, Johan van Selst, Alexander Beedie, - Tanguy Fautre + Tanguy Fautre, Scott Cantor Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/ssluse.c b/lib/ssluse.c index ffc1fbd96..324b05d47 100644 --- a/lib/ssluse.c +++ b/lib/ssluse.c @@ -990,14 +990,19 @@ static int asn1_output(const ASN1_UTCTIME *tm, #define HOST_NOMATCH 0 #define HOST_MATCH 1 -static int hostmatch(const char *hostname, const char *pattern) +static int hostmatch(const char *hostname, const char *pattern, size_t plen) { while(1) { char c = *pattern++; + plen--; - if(c == '\0') + if(!plen) return (*hostname ? HOST_NOMATCH : HOST_MATCH); + if(!c) + /* an embedded zero in the pattern can't match a host name */ + return HOST_NOMATCH; + if(c == '*') { c = *pattern; if(c == '\0') /* "*\0" matches anything remaining */ @@ -1005,7 +1010,7 @@ static int hostmatch(const char *hostname, const char *pattern) while(*hostname) { /* The only recursive function in libcurl! */ - if(hostmatch(hostname++,pattern) == HOST_MATCH) + if(hostmatch(hostname++, pattern, plen) == HOST_MATCH) return HOST_MATCH; } break; @@ -1018,17 +1023,20 @@ static int hostmatch(const char *hostname, const char *pattern) } static int -cert_hostcheck(const char *match_pattern, const char *hostname) +cert_hostcheck(const char *match_pattern, size_t mlen, const char *hostname) { + size_t hlen = strlen(hostname); if(!match_pattern || !*match_pattern || - !hostname || !*hostname) /* sanity check */ + !hostname || !*hostname) /* sanity check */ return 0; - if(Curl_raw_equal(hostname, match_pattern)) /* trivial case */ + if((hlen == mlen) && !memcmp(hostname, match_pattern, hlen)) + /* trivial case */ return 1; - if(hostmatch(hostname,match_pattern) == HOST_MATCH) + if(hostmatch(hostname, match_pattern, mlen) == HOST_MATCH) return 1; + return 0; } @@ -1101,7 +1109,7 @@ static CURLcode verifyhost(struct connectdata *conn, if(check->type == target) { /* get data and length */ const char *altptr = (char *)ASN1_STRING_data(check->d.ia5); - size_t altlen; + size_t altlen = (size_t) ASN1_STRING_length(check->d.ia5); switch(target) { case GEN_DNS: /* name/pattern comparison */ @@ -1114,15 +1122,17 @@ static CURLcode verifyhost(struct connectdata *conn, Gisle researched the OpenSSL sources: "I checked the 0.9.6 and 0.9.8 sources before my patch and it always 0-terminates an IA5String." + + To reduce the risk of an embedded zero before the final zero + causing us trouble, we use the length OpenSSL reports! */ - if(cert_hostcheck(altptr, conn->host.name)) + if(cert_hostcheck(altptr, altlen, conn->host.name)) matched = TRUE; break; case GEN_IPADD: /* IP address comparison */ /* compare alternative IP address if the data chunk is the same size our server IP address is */ - altlen = (size_t) ASN1_STRING_length(check->d.ia5); if((altlen == addrlen) && !memcmp(altptr, &addr, altlen)) matched = TRUE; break; @@ -1196,7 +1206,8 @@ static CURLcode verifyhost(struct connectdata *conn, "SSL: unable to obtain common name from peer certificate"); return CURLE_PEER_FAILED_VERIFICATION; } - else if(!cert_hostcheck((const char *)peer_CN, conn->host.name)) { + else if(!cert_hostcheck((const char *)peer_CN, strlen((char *)peer_CN), + conn->host.name)) { if(data->set.ssl.verifyhost > 1) { failf(data, "SSL: certificate subject name '%s' does not match " "target host name '%s'", peer_CN, conn->host.dispname); |