OpenSSL CURLOPT_SSL_VERIFYPEER and CURLOPT_CAINFO_BLOB setting interaction issue

[kyled-dell]

I did this

Run the test code ossl_custom_verify.c using libcurl 7.88.0 or newer. These files can be found in scripts.tar.gz. This code connects to www.google.com.

$ ./test_verify
libcurl/7.88.0 OpenSSL/3.0.2
HTTP/1.1 200 OK
$ ./ossl_custom_verify
0: /C=US/O=Google Trust Services LLC/CN=GTS Root R1
libcurl/7.88.0 OpenSSL/3.0.2
curl_easy_perform returned 60
SSL peer certificate or SSH remote key was not OK

I expected the following

When run against curl 7.86.0, the code produces the following output. This is the output that I expect.

$ ./test_verify
libcurl/7.86.0 OpenSSL/3.0.2
HTTP/1.1 200 OK
$ ./ossl_custom_verify
libcurl/7.86.0 OpenSSL/3.0.2
1: /C=US/O=Google Trust Services LLC/CN=GTS Root R1
1: /C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
1: /CN=www.google.com
HTTP/1.1 200 OK

Running the test_verify.c script against curl 7.87.0 produces an error. A fix for this behavior was first released in curl 7.88.0 after merging curl issue #10351.

$ ./test_verify
libcurl/7.87.0 OpenSSL/3.0.2
curl_easy_perform returned 77
Problem with the SSL CA cert (path? access rights?)
$ ./ossl_custom_verify
libcurl/7.87.0 OpenSSL/3.0.2
1: /C=US/O=Google Trust Services LLC/CN=GTS Root R1
1: /C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
1: /CN=www.google.com
HTTP/1.1 200 OK

When run against curl 7.88.0 or newer ossl_custom_verify.c produces the error seen in the "I did this" section. I would expect the code to behave the same in all versions of curl.

curl/libcurl version

This issue can be traced across curl versions 7.86.0, 7.87.0, 7.88.0 and newer.

# curl -V
curl 7.86.0 (x86_64-pc-linux-gnu) libcurl/7.86.0 OpenSSL/3.0.2
Release-Date: 2022-10-26
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 Debug HSTS HTTPS-proxy IPv6 Largefile NTLM NTLM_WB SSL threadsafe TLS-SRP TrackMemory UnixSockets

# curl -V
curl 7.87.0 (x86_64-pc-linux-gnu) libcurl/7.87.0 OpenSSL/3.0.2
Release-Date: 2022-12-21
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 Debug HSTS HTTPS-proxy IPv6 Largefile NTLM NTLM_WB SSL threadsafe TLS-SRP TrackMemory UnixSockets

# curl -V
WARNING: this libcurl is Debug-enabled, do not use in production

curl 7.88.0 (x86_64-pc-linux-gnu) libcurl/7.88.0 OpenSSL/3.0.2
Release-Date: 2023-02-15
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 Debug HSTS HTTPS-proxy IPv6 Largefile NTLM NTLM_WB SSL threadsafe TLS-SRP TrackMemory UnixSockets

operating system

Docker image ubuntu:latest.

Linux f96651d64b63 5.14.21-150400.24.66-default #1 SMP PREEMPT_DYNAMIC Tue Jun 6 10:18:38 UTC 2023 (98adc02) x86_64 x86_64 x86_64 GNU/Linux

Many more details follow:

This issue and associated PR aim to maintain behavior consistent with curl's 7.86.0 implementation of the interaction between the CURLOPT_SSL_VERIFYPEER and CURLOPT_CAINFO_BLOB settings. This change would restore curl's behavior to the behavior that existed in curl 7.86.0. This means that certificates provided to the CURLOPT_CAINFO_BLOB setting will be added to OpenSSL's X509_STORE structures even if certificate validation is disabled. This behavior can be useful for applications that make use of CURLOPT_SSL_CTX_FUNCTION, but don't want to manually populate OpenSSL's root certificate database themselves. The dependency between certificate verification and root certificate importing appears to have been introduced when curl issue #10351 merged.

Why this change?

If an application is using CURLOPT_SSL_CTX_FUNCTION to perform some custom validation, this application may want curl to not perform automatic certificate validation but would still want curl to load the supplied certificates into the SSL_CTX structure. This change will also ensure that the CURLOPT_CAINFO_BLOB and CURLOPT_SSL_VERIFYPEER options remain independent.

For example the ossl_custom_verify.c has a rudimentary example of an application directly making use of OpenSSL's verify callback to log the certificate chain for every request. However applications could implement additional custom validation if they want to perform additional non-standard certificate validation. An application upgrading to curl 7.88.0 or newer would experience failures if configured with CURLOPT_SSL_VERIFYPEER==0 and a certificate provided through CURLOPT_CAINFO_BLOB. However this application would work if CURLOPT_SSL_VERIFYPEER==1 despite there being no changes made to the actual certificate verification functionality.

I wrote a 2 examples and used Docker to test these examples with curl versions 7.86.0, 7.87.0, 7.88.0, and the behavior after merging my proposed fix.

7.86.0 Behavior

In 7.86.0, observe that the request from both test_verify.c and ossl_custom_verify.c code get 200 OK responses as expected.

Using GDB, the behavior around the ossl_connect_step1 and load_cacert_from_memory functions can be examined more closely. With ca_info_blob->len == 0, no certificates are imported and load_cacert_from_memory returns CURLE_SSL_CACERT_BADFILE. In ossl_connect_step1, the result is checked on line 3347 and again on line 3348. Since this is not one of the conditions that would execute the body of the if statement on line 3348, curl continues processing the remainder of the request.

(gdb) b load_cacert_from_memory
Breakpoint 1 at 0x7ffff7f6d1af: file vtls/openssl.c, line 2837.
(gdb) p *ca_info_blob
$2 = {data = 0x55555556f540, len = 0, flags = 1}
(gdb) finish
Run till exit from #0  load_cacert_from_memory (ctx=0x5555555f63b0, ca_info_blob=0x55555556f528) at vtls/openssl.c:2891
0x00007ffff7f6e716 in ossl_connect_step1 (data=0x5555555733e8, conn=0x55555556cb48, sockindex=0) at vtls/openssl.c:3346
3346        result = load_cacert_from_memory(backend->ctx, ca_info_blob);
Value returned is $5 = CURLE_SSL_CACERT_BADFILE
(gdb) n
3347        if(result) {
(gdb) n
3348          if(result == CURLE_OUT_OF_MEMORY ||
(gdb) n
3354          infof(data, "error importing CA certificate blob, continuing anyway");
(gdb) finish
Run till exit from #0  ossl_connect_step1 (data=0x5555555733e8, conn=0x55555556cb48, sockindex=0) at vtls/openssl.c:3358
0x00007ffff7f7115e in ossl_connect_common (data=0x5555555733e8, conn=0x55555556cb48, sockindex=0, nonblocking=true, done=0x7fffffffe31d) at vtls/openssl.c:4059
4059        result = ossl_connect_step1(data, conn, sockindex);
Value returned is $6 = CURLE_OK

The result local variable is not checked again, and the ossl_connect_step1 function returns CURLE_OK. The request continues from here.

7.87.0 Behavior

Running ossl_custom_verify.c against 7.87.0 succeeds. However test_verify.c returns a CURLE_SSL_CACERT_BADFILE error before the request is sent. The behavior causing test_verify.c to fail was fixed when curl 7.88.0 released by curl issue #10351.

Curl version 7.87.0 released a major refactor of the code that loads certificates. This results in ossl_connect_step1 calling set_up_x509_store to call populate_x509_store to call load_cacert_from_memory. With a breakpoint set on line 3247 we can observe the behavior of these functions that leads to ossl_connect_step1 returning CURLE_SSL_CACERT_BADFILE.

Thread 1 "a.out" hit Breakpoint 1, populate_x509_store (cf=0x55555556fc18, data=0x5555555733e8, store=0x5555555f6e50) at vtls/openssl.c:3247
3247        if(result) {
(gdb) n
3248          if(result == CURLE_OUT_OF_MEMORY ||
(gdb) n
3254          infof(data, "error importing CA certificate blob, continuing anyway");
(gdb) until 3340
populate_x509_store (cf=0x55555556fc18, data=0x5555555733e8, store=0x5555555f6e50) at vtls/openssl.c:3340
3340      return result;
(gdb) p result
$1 = CURLE_SSL_CACERT_BADFILE

The populate_x509_store function returns the CURLE_SSL_CACERT_BADFILE error to set_up_x509_store. The set_up_x509_store function also returns CURLE_SSL_CACERT_BADFILE to its caller ossl_connect_step1. Since ossl_connect_step1 has received an error it will return this error to its caller and the request will fail. However this appears to conflict with the info message that would be logged on line 3254 of lib/vtls/openssl.c stating that the request is continuing anyway in spite of the error importing the CA blob.

7.88.0 and newer Behavior

To address the behavior emerging in 7.87.0, a change was made to populate_x509_store that will only attempt to load certificates if verifypeer is true. However this appears to introduce a new dependency between these settings rather than returning to the behavior that 7.86.0 exhibited. GDB confirms that the load_cacert_from_memory function is not called at all. This means that the behavior of the CURLOPT_CAINFO_BLOB option is now dependent on the CURLOPT_SSL_VERIFYPEER option. This change in behavior leads to the ossl_custom_verify.c code failing when run against libcurl 7.88.0 or newer. I believe it would be sufficient to ignore errors configuring certificates on OpenSSL if certificate verification is disabled.

An application that makes use of the CURLOPT_SSL_CTX_FUNCTION option to directly modify OpenSSL's SSL context now would have to directly import its own certificates into OpenSSL. If not modified, the change in curl behavior results in certificate failures in the application since the certificate chain cannot be verified.

Behavior after merging this PR

After merging this PR the code behaves as it originally did in 7.86.0. If non-memory related errors are returned from load_cacert_from_memory and verifypeer is false, the errors are ignored by setting result = CURLE_OK.

diff --git a/lib/vtls/openssl.c b/lib/vtls/openssl.c
index c6a1dd218..efd0c1cab 100644
--- a/lib/vtls/openssl.c
+++ b/lib/vtls/openssl.c
@@ -3171,6 +3171,7 @@ static CURLcode populate_x509_store(struct Curl_cfilter *cf,
         return result;
       }
       /* Only warn if no certificate verification is required. */
+      result = CURLE_OK;
       infof(data, "error importing CA certificate blob, continuing anyway");
     }
     else {

This change resets the result local variable to CURLE_OK if the if and return statement starting from line 3168 were not executed. This ignores the CURLE_SSL_CACERT_BADFILE error that was returned from load_cacert_from_memory if no certificates were imported. It is my understanding that this is the same behavior that curl 7.86.0 had in this situation.

The rest of the changes are required to make the certificate handling code independent of the value of verifypeer.

Running curl 7.88.0 with this patch applied results in identical behavior to curl 7.86.0, including adding the root certificate to OpenSSL.

$ ./test_verify
libcurl/7.88.0 OpenSSL/3.0.2
HTTP/1.1 200 OK
$ ./ossl_custom_verify
libcurl/7.88.0 OpenSSL/3.0.2
1: /C=US/O=Google Trust Services LLC/CN=GTS Root R1
1: /C=US/O=Google Trust Services LLC/CN=GTS CA 1C3
1: /CN=www.google.com
HTTP/1.1 200 OK

Thanks for following along with my very long winded explanation.

[jay]

GDB confirms that the load_cacert_from_memory function is not called at all. This means that the behavior of the CURLOPT_CAINFO_BLOB option is now dependent on the CURLOPT_SSL_VERIFYPEER option

Yes that's what was intended. Certificates are no longer loaded when peer verification is disabled. If you are doing it manually then you should also load the certificates.

[bagder]

If you are doing it manually then you should also load the certificates.

I think that sounds like a pretty good stance, yes.