-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Description
There's a lot of API which is secretly a bunch of macros over SSL_CTX_ctrl
or SSL_ctrl
. Some time ago, we did a pass in BoringSSL and replaced them all with straightforward functions. We actually got rid of the SSL_CTX_ctrl
and SSL_ctrl
entry-points altogether. With SSLv2 gone, it doesn't really do much good.
It seems SSL_CTX_ctrl is even documented to not be public API:
These functions should never be called directly. All functionalities needed are made available via other functions or macros.
https://www.openssl.org/docs/manmaster/ssl/SSL_CTX_ctrl.html
Benefits of promoting to proper functions:
- Callers are actually type-checked. The current macros aren't type-checked. There's quite a few mistakes that go unnoticed. const-correctness usually goes out the window.
- No more
SSL_CTX_ctrl
in public API. - More idiomatic API in general. The header file describes the type.
- Adding a new API involves less effort (no adding ctrl value, then entries in both switch-cases and deciding if this is
s3
-only or universal even though they're the same.
Note: there exists code that does things like:
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
SSL_set_tlsext_host_name(ssl, hostname);
#endif
or:
#ifdef SSL_set_tlsext_host_name
SSL_set_tlsext_host_name(ssl, hostname);
#endif
or:
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
SSL_ctrl(ssl, SSL_CTRL_SET_TLSEXT_HOSTNAME, TLSEXT_NAMETYPE_host_name, hostname);
#endif
It's not that common, but I was worried about silent miscompiles, so we added a "Pre-processor compatibility section" to our header which contains a bunch of lines like:
#define SSL_CTRL_GET_SESS_CACHE_MODE doesnt_exist
#define SSL_CTRL_GET_SESS_CACHE_SIZE doesnt_exist
#define SSL_CTRL_GET_TLSEXT_TICKET_KEYS doesnt_exist
and
#define SSL_CTX_add0_chain_cert SSL_CTX_add0_chain_cert
#define SSL_CTX_add1_chain_cert SSL_CTX_add1_chain_cert
#define SSL_CTX_add_extra_chain_cert SSL_CTX_add_extra_chain_cert
The other option is to say you're okay with breaking those.
Note that the doesnt_exist
define will still break the third example (intentionally since we got rid of SSL_CTX_ctrl
), but the first two will keep working. Alternatively, you could even keep a compatibility SSL_CTX_ctrl
around which does a switch-case and calls the now real functions (so invert everything), but we haven't found it to be necessary.