aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Stenberg <daniel@haxx.se>2009-08-01 21:56:59 +0000
committerDaniel Stenberg <daniel@haxx.se>2009-08-01 21:56:59 +0000
commitc0e8bed5bf7a7e56897e492a4dcc399621939995 (patch)
tree5c5262804a2a51a53d69e1b66411a03612e5a929
parent0dce2ff8a09065b2be2a3531f498006906c81db5 (diff)
- 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.
-rw-r--r--CHANGES15
-rw-r--r--RELEASE-NOTES3
-rw-r--r--lib/ssluse.c33
3 files changed, 39 insertions, 12 deletions
diff --git a/CHANGES b/CHANGES
index d2b194ffb..e03f92c88 100644
--- a/CHANGES
+++ b/CHANGES
@@ -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);