Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenSSL host verification + hostname in certificate CN only seems broken in 7.82.0 #8559

Closed
kristofg opened this issue Mar 8, 2022 · 6 comments
Closed
Assignees

Comments

@kristofg
Copy link

@kristofg kristofg commented Mar 8, 2022

I did this

I have an HTTPS server where the hostname is only in subject CN of the certificate, not in the SAN list. This is deprecated, but as far as I can tell still expected to work. Using a newly built 7.82.0 and the public server certificate placed in /tmp/curltest/cacerts I see this:

$ /tmp/curltest/curl-7.82.0/bin/curl --version
curl 7.82.0-DEV (x86_64-pc-linux-gnu) libcurl/7.82.0-DEV OpenSSL/1.1.1l-fips zlib/1.2.11
Release-Date: [unreleased]
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS HSTS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets
$ /tmp/curltest/curl-7.82.0/bin/curl --capath /tmp/curltest/cacerts -o /dev/null -v --no-progress-meter https://servername.test/web/signin
*   Trying 10.0.0.51:443...
* Connected to servername.test (10.0.0.51) port 443 (#0)
* ALPN, offering http/1.1
*  CAfile: /etc/pki/tls/certs/ca-bundle.crt
*  CApath: /tmp/curltest/cacerts
} [5 bytes data]
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.3 (IN), TLS handshake, Server hello (2):
{ [112 bytes data]
* TLSv1.2 (IN), TLS handshake, Certificate (11):
{ [887 bytes data]
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
{ [300 bytes data]
* TLSv1.2 (IN), TLS handshake, Server finished (14):
{ [4 bytes data]
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
} [37 bytes data]
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
} [1 bytes data]
* TLSv1.2 (OUT), TLS handshake, Finished (20):
} [16 bytes data]
* TLSv1.2 (IN), TLS handshake, Finished (20):
{ [16 bytes data]
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: O=TemporaryDefaultCertificate; CN=servername.test
*  start date: Mar  8 09:56:04 2022 GMT
*  expire date: Mar  8 09:56:04 2023 GMT
* Closing connection 0
} [5 bytes data]
* TLSv1.2 (OUT), TLS alert, close notify (256):
} [2 bytes data]
curl: (27) Out of memory
$

I expected the following

Using the same CA dir and HTTPS server, my OS installed version works fine:

$ curl --version
curl 7.79.1 (x86_64-redhat-linux-gnu) libcurl/7.79.1 OpenSSL/1.1.1l-fips zlib/1.2.11 brotli/1.0.9 libidn2/2.3.2 libpsl/0.21.1 (+libidn2/2.3.2) libssh/0.9.6/openssl/zlib nghttp2/1.45.1 OpenLDAP/2.4.59
Release-Date: 2021-09-22
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets
$ curl --capath /tmp/curltest/cacerts -o /dev/null -vs https://servername.test/web/signin
*   Trying 10.0.0.51:443...
* Connected to servername.test (10.0.0.51) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: /etc/pki/tls/certs/ca-bundle.crt
*  CApath: /tmp/curltest/cacerts
} [5 bytes data]
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
} [512 bytes data]
* TLSv1.3 (IN), TLS handshake, Server hello (2):
{ [112 bytes data]
* TLSv1.2 (IN), TLS handshake, Certificate (11):
{ [887 bytes data]
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
{ [300 bytes data]
* TLSv1.2 (IN), TLS handshake, Server finished (14):
{ [4 bytes data]
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
} [37 bytes data]
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
} [1 bytes data]
* TLSv1.2 (OUT), TLS handshake, Finished (20):
} [16 bytes data]
* TLSv1.2 (IN), TLS handshake, Finished (20):
{ [16 bytes data]
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server accepted to use http/1.1
* Server certificate:
*  subject: O=TemporaryDefaultCertificate; CN=servername.test
*  start date: Mar  8 09:56:04 2022 GMT
*  expire date: Mar  8 09:56:04 2023 GMT
*  common name: servername.test (matched)
*  issuer: O=TemporaryDefaultCertificate; CN=servername.test
*  SSL certificate verify ok.
} [5 bytes data]
> GET /web/signin HTTP/1.1
> Host: servername.test
> User-Agent: curl/7.79.1
> Accept: */*
> 
{ [5 bytes data]
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Server: nginx
[...]
{ [1284 bytes data]
* Connection #0 to host servername.test left intact
$ 

Discussion

I suspect the problem is the result = CURLE_OUT_OF_MEMORY; introduced in commit d15692ebb. I am guessing that this line is meant to deal with the OPENSSL_malloc() just above failing? This patch makes 7.82.0 work as expected for me:

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index 616a510b0..1bafe9613 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -1808,7 +1808,8 @@ CURLcode Curl_ossl_verifyhost(struct Curl_easy *data, struct connectdata *conn,
               memcpy(peer_CN, ASN1_STRING_get0_data(tmp), peerlen);
               peer_CN[peerlen] = '\0';
             }
-            result = CURLE_OUT_OF_MEMORY;
+            else
+              result = CURLE_OUT_OF_MEMORY;
           }
         }
         else /* not a UTF8 name */

curl/libcurl version

See above

operating system

$ uname -a 
Linux hostname 5.16.12-200.fc35.x86_64 #1 SMP PREEMPT Wed Mar 2 19:06:17 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
$ cat /etc/fedora-release 
Fedora release 35 (Thirty Five)
@bagder bagder self-assigned this Mar 8, 2022
@bagder bagder added the TLS label Mar 8, 2022
@bagder
Copy link
Member

@bagder bagder commented Mar 8, 2022

Gah, you're absolutely correct.

bagder added a commit that referenced this issue Mar 8, 2022
Due to a missing 'else' this returns error too easily.

Regressed in: d15692e

Reported-by: Kristoffer Gleditsch
Fixes #8559
@tawmoto
Copy link

@tawmoto tawmoto commented Mar 8, 2022

@bagder any chance for a quick release with this fix?
Thanks

@bagder
Copy link
Member

@bagder bagder commented Mar 8, 2022

I don't think this is a serious enough bug to warrant a patch release. Work-arounds:

  1. stick to previous curl version until next release (planned to happen on April 27th)
  2. apply the patch and build 7.82.0+patched
  3. make your certs use SAN fields

Publicly trusted CA:s have required the presence of a subjectAltName since 2012, so CN-only certs are only present in private CA:s, According to telemetry from Chrome in 2017, only 1.57% of privately-trusted CA certificates rely on this behavior (using CN only).

@kristofg
Copy link
Author

@kristofg kristofg commented Mar 8, 2022

Thanks for the impressively quick confirmation and fix!

fabricereix added a commit to Orange-OpenSource/hurl that referenced this issue Mar 11, 2022
jcamiel pushed a commit to Orange-OpenSource/hurl that referenced this issue Mar 11, 2022
jleverenz added a commit to jleverenz/hurl that referenced this issue Mar 19, 2022
See curl/curl#8559

* #[ignore] related unittest
* Use ".ignore" suffix on integration test to disable
* Re-enable other tests for arch linux
@Karlson2k
Copy link
Contributor

@Karlson2k Karlson2k commented Apr 20, 2022

Actually this breaks tests for GNU libmicrohttpd, so many package builders are broken. It would be nice to make a new release to fix it. Any ETA?

@tawmoto
Copy link

@tawmoto tawmoto commented Apr 20, 2022

Actually this breaks tests for GNU libmicrohttpd, so many package builders are broken. It would be nice to make a new release to fix it. Any ETA?

It was mentioned here, 27 April
#8559 (comment)

armcc added a commit to lgirdk/openembedded-core that referenced this issue Apr 20, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
armcc added a commit to lgirdk/openembedded-core that referenced this issue Apr 20, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
algitbot pushed a commit to alpinelinux/aports that referenced this issue Apr 21, 2022
armcc added a commit to lgirdk/openembedded-core that referenced this issue Apr 22, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
armcc added a commit to lgirdk/openembedded-core that referenced this issue Apr 24, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
armcc added a commit to lgirdk/openembedded-core that referenced this issue Apr 27, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
armcc added a commit to lgirdk/openembedded-core that referenced this issue May 19, 2022
…en in 7.82.0

Backport upstream fix for:

  curl/curl#8559

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
gerrit-photon pushed a commit to vmware/photon that referenced this issue May 20, 2022
Fixes curl/curl#8559

Change-Id: Ib5e9da27d7804c3af1af057a7b187bb1306c2f69
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/16066
Reviewed-by: Harinadh Dommaraju <hdommaraju@vmware.com>
Reviewed-by: Dweep Advani <dadvani@vmware.com>
Tested-by: Tapas Kundu <tkundu@vmware.com>
gerrit-photon pushed a commit to vmware/photon that referenced this issue May 20, 2022
Fixes curl/curl#8559

Change-Id: I98b0e5f5cb6e4ad876f3c5c330ed93697d8b09e8
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/16071
Reviewed-by: Tapas Kundu <tkundu@vmware.com>
Tested-by: Tapas Kundu <tkundu@vmware.com>
gerrit-photon pushed a commit to vmware/photon that referenced this issue May 20, 2022
Fixes curl/curl#8559

Change-Id: I7acef71fb77b9e2056f9cca4bb63d95cc3bd9295
Reviewed-on: http://photon-jenkins.eng.vmware.com:8082/16070
Tested-by: gerrit-photon <photon-checkins@vmware.com>
Reviewed-by: Shreenidhi Shedi <sshedi@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants