ssl_openssl: Fix some CRL mixups

[davidben]

Hi all! I tried to send this to openvpn-devel but it looks like it might not have gotten through? (I'm probably confused or just did something wrong.) It sounds like you all do look at GitHub though, so I figure I'll start here and we can figure out the mailing list step afterwards?

I've tested this by making sure things build and that the tests pass. However, it looks like some tests were skipped because they require manually configuring some things in ways I hadn't figured out. It's also unclear to me how well-tested CRL support is. There's no mention of CRLs in the tests directory. I'm not sure how you all test CRL-related changes.

---

There are two ways to load CRLs in OpenSSL. They can be loaded at the X509_STORE, shared across verifications, or loaded per verification at the X509_STORE_CTX.

OpenVPN currently does the former. However, it also supports CRL reloading, and tries to reload the CRL file before each connection. OpenSSL does not really have a good way to unload objects from an X509_STORE. OpenVPN currently does it by grabbing the STACK_OF(X509_OBJECT) out of the X509_STORE and manually deleting all the CRLs from it.

This mutates an OpenSSL internal object which bumps into problems if OpenSSL ever switches to a more efficient representation. See openssl/openssl#28599

(It's also not thread-safe, though it doesn't look like that impacts OpenVPN? Actually even reading that list doesn't work. See CVE-2024-0397. This OpenSSL API was simply broken.)

Additionally, this seems to cause two OpenVPN features to not work together. I gather backend_tls_ctx_reload_crl is trying to clear the CRLs loaded from last time it ran. But tls_ctx_load_ca with a ca_file can also load CRLs. tls_ctx_load_ca with ca_path will also pick up CRLs and backend_tls_ctx_reload_crl actually ends up clobbering some state X509_LOOKUP_hash_dir internally maintains on the X509_STORE. Likewise, tls_verify_crl_missing can get confused between backend_tls_ctx_reload_crl's crl_file-based CRLs and CRLs from tls_ctx_load_ca.

Avoid all this by tracking the two CRLs separately. crl_file-based CRLs now go onto a STACK_OF(X509_CRL) tracked on the tls_root_ctx. Now this field can be freely reloaded by OpenVPN without reconfiguring OpenSSL. Instead, pass the current value into OpenSSL at verification time. To do so, we need to use SSL_CTX_set_cert_verify_callback, which allows swapping out the X509_verify_cert call, and also tweaking the X509_STORE_CTX configuration before starting certificate verification.

Context: SSL_CTX_set_cert_verify_callback and the existing verify_callback are not the same. SSL_CTX_set_cert_verify_callback wraps the verification while verify_callback is called multiple times throughout verification. It's too late to reconfigure X509_STORE_CTX in verify_callback. verify_callback is usually not what you want. Sometimes current_cert and error_depth don't quite line up, and cert_hash_remember may end up called multiple times for a single certificate.

I suspect some of the other verify_callback logic would also be better done in the new callback, but I've left it alone to keep this change minimal. verify_callback is really only usable for suppressing errors. Application bookkeeping is better done elsewhere.

[cron2]

The preferred way for patch submission (these days) is our gerrit, https://gerrit.openvpn.com.

The second best way is the mailing list, but it has been a bit unreliable in the last weeks. Also, you need to be subscribed to be allowed to post (but my colleagues should see the moderation request and permit it).

Github MRs will not be merged - they can serve as a starting point for the discussion (and we do use GH issues), but in the end the final patch needs to be taken to the list or gerrit - one of us can help with it, if needed.

Now, for the issue at hand, I defer to people who understand OpenSSL :-)

[davidben]

I'm guessing that was meant to be https://gerrit.openvpn.net. The other URL took me somewhere confusing. :-) If I followed the links right, looks Gerrit uses the wiki credentials and I need to ask IRC to get an account? I've gone and done that.

(There's no mention of the Gerrit at https://github.com/OpenVPN/openvpn/blob/master/CONTRIBUTING.rst by the way. Maybe worth updating?)

[davidben]

To close the loop, the conclusion from IRC seems to have been that Gerrit accounts are only given to long-time contributors and that, for a one-off fix, GitHub was probably best?

I'll defer to you all on how you wish to do this. For now, I'm assuming that this is the right place and that this PR is just awaiting review. Let me know if you all would prefer another venue change.