8000 Consider hiding SSL_ctrl and promoting macros to functions. · Issue #1319 · openssl/openssl · GitHub
[go: up one dir, main page]

Skip to content
Consider hiding SSL_ctrl and promoting macros to functions. #1319
@davidben

Description

@davidben

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    branch: masterMerge to master branchtriaged: refactorThe issue/pr requests/implements refactoring

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0