Skip to content

Commit

Permalink
- Scott Cantor posted the bug report #2829955
Browse files Browse the repository at this point in the history
  (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.
  • Loading branch information
bagder committed Aug 1, 2009
1 parent 0dce2ff commit c0e8bed
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 12 deletions.
15 changes: 15 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion RELEASE-NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand All @@ -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)
33 changes: 22 additions & 11 deletions lib/ssluse.c
Original file line number Diff line number Diff line change
Expand Up @@ -990,22 +990,27 @@ 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 */
return HOST_MATCH;

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;
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 */
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit c0e8bed

Please sign in to comment.