- 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.
This commit is contained in:
Daniel Stenberg 2009-08-01 21:56:59 +00:00
parent 0dce2ff8a0
commit c0e8bed5bf
3 changed files with 39 additions and 12 deletions

15
CHANGES
View File

@ -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

View File

@ -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)

View File

@ -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);