From d15692ebbad5e9cfb871b0f7f51a73e43762cee2 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 9 Feb 2022 17:27:09 +0100 Subject: [PATCH] hostcheck: pass in pattern length too, to avoid a strlen call Removes one strlen() call per SAN name in a cert-check. Closes #8418 --- lib/vtls/hostcheck.c | 11 +++++---- lib/vtls/hostcheck.h | 3 ++- lib/vtls/openssl.c | 40 +++++++++++++++---------------- lib/vtls/schannel_verify.c | 3 ++- lib/vtls/x509asn1.c | 2 +- tests/unit/unit1397.c | 48 +++++++++++++++++++++----------------- 6 files changed, 57 insertions(+), 50 deletions(-) diff --git a/lib/vtls/hostcheck.c b/lib/vtls/hostcheck.c index eb3dfa3dc0..08a3470383 100644 --- a/lib/vtls/hostcheck.c +++ b/lib/vtls/hostcheck.c @@ -72,17 +72,17 @@ static bool pmatch(const char *hostname, size_t hostlen, * Return TRUE on a match. FALSE if not. */ -static bool hostmatch(const char *hostname, const char *pattern) +static bool hostmatch(const char *hostname, const char *pattern, + size_t patternlen) { const char *pattern_label_end, *wildcard, *hostname_label_end; size_t prefixlen, suffixlen; /* normalize pattern and hostname by stripping off trailing dots */ - size_t patternlen; size_t hostlen = strlen(hostname); + DEBUGASSERT(patternlen); if(hostname[hostlen-1]=='.') hostlen--; - patternlen = strlen(pattern); if(pattern[patternlen-1]=='.') patternlen--; @@ -128,10 +128,11 @@ static bool hostmatch(const char *hostname, const char *pattern) /* * Curl_cert_hostcheck() returns TRUE if a match and FALSE if not. */ -bool Curl_cert_hostcheck(const char *match, const char *hostname) +bool Curl_cert_hostcheck(const char *match, size_t matchlen, + const char *hostname) { if(match && *match && hostname && *hostname) - return hostmatch(hostname, match); + return hostmatch(hostname, match, matchlen); return FALSE; } diff --git a/lib/vtls/hostcheck.h b/lib/vtls/hostcheck.h index b8bbb8a991..c8d0507ddc 100644 --- a/lib/vtls/hostcheck.h +++ b/lib/vtls/hostcheck.h @@ -25,6 +25,7 @@ #include /* returns TRUE if there's a match */ -bool Curl_cert_hostcheck(const char *match_pattern, const char *hostname); +bool Curl_cert_hostcheck(const char *match_pattern, size_t matchlen, + const char *hostname); #endif /* HEADER_CURL_HOSTCHECK_H */ diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c index 806d3dcec7..a0a6730c12 100644 --- a/lib/vtls/openssl.c +++ b/lib/vtls/openssl.c @@ -1612,14 +1612,16 @@ static void ossl_close_all(struct Curl_easy *data) * Match subjectAltName against the host name. */ static bool subj_alt_hostcheck(struct Curl_easy *data, - const char *match_pattern, const char *hostname, + const char *match_pattern, + size_t matchlen, + const char *hostname, const char *dispname) { #ifdef CURL_DISABLE_VERBOSE_STRINGS (void)dispname; (void)data; #endif - if(Curl_cert_hostcheck(match_pattern, hostname)) { + if(Curl_cert_hostcheck(match_pattern, matchlen, hostname)) { infof(data, " subjectAltName: host \"%s\" matched cert's \"%s\"", dispname, match_pattern); return TRUE; @@ -1728,7 +1730,7 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn, if((altlen == strlen(altptr)) && /* if this isn't true, there was an embedded zero in the name string and we cannot match it. */ - subj_alt_hostcheck(data, altptr, hostname, dispname)) { + subj_alt_hostcheck(data, altptr, altlen, hostname, dispname)) { dnsmatched = TRUE; } break; @@ -1764,17 +1766,17 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn, else { /* we have to look to the last occurrence of a commonName in the distinguished one to get the most significant one. */ - int j, i = -1; + int i = -1; + unsigned char *peer_CN = NULL; + int peerlen = 0; /* The following is done because of a bug in 0.9.6b */ - - unsigned char *nulstr = (unsigned char *)""; - unsigned char *peer_CN = nulstr; - X509_NAME *name = X509_get_subject_name(server_cert); - if(name) + if(name) { + int j; while((j = X509_NAME_get_index_by_NID(name, NID_commonName, i)) >= 0) i = j; + } /* we have the name entry and we will now convert this to a string that we can use for comparison. Doing this we support BMPstring, @@ -1790,19 +1792,20 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn, conditional in the future when OpenSSL has been fixed. */ if(tmp) { if(ASN1_STRING_type(tmp) == V_ASN1_UTF8STRING) { - j = ASN1_STRING_length(tmp); - if(j >= 0) { - peer_CN = OPENSSL_malloc(j + 1); + peerlen = ASN1_STRING_length(tmp); + if(peerlen >= 0) { + peer_CN = OPENSSL_malloc(peerlen + 1); if(peer_CN) { - memcpy(peer_CN, ASN1_STRING_get0_data(tmp), j); - peer_CN[j] = '\0'; + memcpy(peer_CN, ASN1_STRING_get0_data(tmp), peerlen); + peer_CN[peerlen] = '\0'; } + result = CURLE_OUT_OF_MEMORY; } } else /* not a UTF8 name */ - j = ASN1_STRING_to_UTF8(&peer_CN, tmp); + peerlen = ASN1_STRING_to_UTF8(&peer_CN, tmp); - if(peer_CN && (curlx_uztosi(strlen((char *)peer_CN)) != j)) { + if(peer_CN && (curlx_uztosi(strlen((char *)peer_CN)) != peerlen)) { /* there was a terminating zero before the end of string, this cannot match and we return failure! */ failf(data, "SSL: illegal cert name field"); @@ -1811,9 +1814,6 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn, } } - if(peer_CN == nulstr) - peer_CN = NULL; - if(result) /* error already detected, pass through */ ; @@ -1822,7 +1822,7 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn, "SSL: unable to obtain common name from peer certificate"); result = CURLE_PEER_FAILED_VERIFICATION; } - else if(!Curl_cert_hostcheck((const char *)peer_CN, hostname)) { + else if(!Curl_cert_hostcheck((const char *)peer_CN, peerlen, hostname)) { failf(data, "SSL: certificate subject name '%s' does not match " "target host name '%s'", peer_CN, dispname); result = CURLE_PEER_FAILED_VERIFICATION; diff --git a/lib/vtls/schannel_verify.c b/lib/vtls/schannel_verify.c index f496a59cad..d2d3bf9233 100644 --- a/lib/vtls/schannel_verify.c +++ b/lib/vtls/schannel_verify.c @@ -520,7 +520,8 @@ static CURLcode verify_host(struct Curl_easy *data, result = CURLE_OUT_OF_MEMORY; } else { - if(Curl_cert_hostcheck(cert_hostname, conn_hostname)) { + if(Curl_cert_hostcheck(cert_hostname, strlen(cert_hostname), + conn_hostname)) { infof(data, "schannel: connection hostname (%s) validated " "against certificate name (%s)", diff --git a/lib/vtls/x509asn1.c b/lib/vtls/x509asn1.c index d44bdbdeca..13fa694f2a 100644 --- a/lib/vtls/x509asn1.c +++ b/lib/vtls/x509asn1.c @@ -1330,7 +1330,7 @@ CURLcode Curl_verifyhost(struct Curl_easy *data, struct connectdata *conn, len = utf8asn1str(&dnsname, CURL_ASN1_IA5_STRING, name.beg, name.end); if(len > 0 && (size_t)len == strlen(dnsname)) - matched = Curl_cert_hostcheck(dnsname, hostname); + matched = Curl_cert_hostcheck(dnsname, (size_t)len, hostname); else matched = 0; free(dnsname); diff --git a/tests/unit/unit1397.c b/tests/unit/unit1397.c index ecf4a3100a..1b47b286a0 100644 --- a/tests/unit/unit1397.c +++ b/tests/unit/unit1397.c @@ -40,35 +40,39 @@ UNITTEST_START /* here you start doing things and checking that the results are good */ -fail_unless(Curl_cert_hostcheck("www.example.com", "www.example.com"), - "good 1"); -fail_unless(Curl_cert_hostcheck("*.example.com", "www.example.com"), +fail_unless(Curl_cert_hostcheck(STRCONST("www.example.com"), + "www.example.com"), "good 1"); +fail_unless(Curl_cert_hostcheck(STRCONST("*.example.com"), "www.example.com"), "good 2"); -fail_unless(Curl_cert_hostcheck("xxx*.example.com", "xxxwww.example.com"), - "good 3"); -fail_unless(Curl_cert_hostcheck("f*.example.com", "foo.example.com"), +fail_unless(Curl_cert_hostcheck(STRCONST("xxx*.example.com"), + "xxxwww.example.com"), "good 3"); +fail_unless(Curl_cert_hostcheck(STRCONST("f*.example.com"), "foo.example.com"), "good 4"); -fail_unless(Curl_cert_hostcheck("192.168.0.0", "192.168.0.0"), +fail_unless(Curl_cert_hostcheck(STRCONST("192.168.0.0"), "192.168.0.0"), "good 5"); -fail_if(Curl_cert_hostcheck("xxx.example.com", "www.example.com"), "bad 1"); -fail_if(Curl_cert_hostcheck("*", "www.example.com"), "bad 2"); -fail_if(Curl_cert_hostcheck("*.*.com", "www.example.com"), "bad 3"); -fail_if(Curl_cert_hostcheck("*.example.com", "baa.foo.example.com"), "bad 4"); -fail_if(Curl_cert_hostcheck("f*.example.com", "baa.example.com"), "bad 5"); -fail_if(Curl_cert_hostcheck("*.com", "example.com"), "bad 6"); -fail_if(Curl_cert_hostcheck("*fail.com", "example.com"), "bad 7"); -fail_if(Curl_cert_hostcheck("*.example.", "www.example."), "bad 8"); -fail_if(Curl_cert_hostcheck("*.example.", "www.example"), "bad 9"); -fail_if(Curl_cert_hostcheck("", "www"), "bad 10"); -fail_if(Curl_cert_hostcheck("*", "www"), "bad 11"); -fail_if(Curl_cert_hostcheck("*.168.0.0", "192.168.0.0"), "bad 12"); -fail_if(Curl_cert_hostcheck("www.example.com", "192.168.0.0"), "bad 13"); +fail_if(Curl_cert_hostcheck(STRCONST("xxx.example.com"), + "www.example.com"), "bad 1"); +fail_if(Curl_cert_hostcheck(STRCONST("*"), "www.example.com"), "bad 2"); +fail_if(Curl_cert_hostcheck(STRCONST("*.*.com"), "www.example.com"), "bad 3"); +fail_if(Curl_cert_hostcheck(STRCONST("*.example.com"), + "baa.foo.example.com"), "bad 4"); +fail_if(Curl_cert_hostcheck(STRCONST("f*.example.com"), + "baa.example.com"), "bad 5"); +fail_if(Curl_cert_hostcheck(STRCONST("*.com"), "example.com"), "bad 6"); +fail_if(Curl_cert_hostcheck(STRCONST("*fail.com"), "example.com"), "bad 7"); +fail_if(Curl_cert_hostcheck(STRCONST("*.example."), "www.example."), "bad 8"); +fail_if(Curl_cert_hostcheck(STRCONST("*.example."), "www.example"), "bad 9"); +fail_if(Curl_cert_hostcheck(STRCONST(""), "www"), "bad 10"); +fail_if(Curl_cert_hostcheck(STRCONST("*"), "www"), "bad 11"); +fail_if(Curl_cert_hostcheck(STRCONST("*.168.0.0"), "192.168.0.0"), "bad 12"); +fail_if(Curl_cert_hostcheck(STRCONST("www.example.com"), + "192.168.0.0"), "bad 13"); #ifdef ENABLE_IPV6 -fail_if(Curl_cert_hostcheck("*::3285:a9ff:fe46:b619", +fail_if(Curl_cert_hostcheck(STRCONST("*::3285:a9ff:fe46:b619"), "fe80::3285:a9ff:fe46:b619"), "bad 14"); -fail_unless(Curl_cert_hostcheck("fe80::3285:a9ff:fe46:b619", +fail_unless(Curl_cert_hostcheck(STRCONST("fe80::3285:a9ff:fe46:b619"), "fe80::3285:a9ff:fe46:b619"), "good 6"); #endif -- 2.47.3