Documentation updates will followed after feedback on this PR.

Thanks for the comments, I summarize actionable items at the bottom as the conversation develops. I can push further commits, and do the final squash when it can be accepted.

1. Preprocessor defines `OPENSSL_NO_ENGINE` - followed nginx and HAProxy where they use this to omit compile-time code that uses OpenSSL `ENGINE_xxxx` functions. Frankly I doubt any modern OpenSSL actually defines this. Same purpose as `OPENSSL_NO_ECDH` in existing `tls.c`.

At runtime it might be difficult as the symbol won't be in the users `libcrypto.so`. If we include these symbols, then the users `libcrypto.so` is required to have engine support (even if they don't use it)

Currently the runtime use is controlled by the proposed modparam `engine`, but ENGINE symbols are still UND in `tls.so`.

1. whitespace - added to TODO list below: it was a code editor setting, my bad 1. documentation - added to TODO list

Sample nginx code (because of `ENGINE_*` symbols). HAProxy has similar constructs: ``` #ifndef OPENSSL_NO_ENGINE u_char *p, *last; ENGINE *engine; EVP_PKEY *pkey; p = key->data + sizeof("engine:") - 1; last = (u_char *) ngx_strchr(p, ':'); if (last == NULL) { ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid syntax in "%V"", key); return NGX_ERROR; } *last = '\0'; engine = ENGINE_by_id((char *) p); ```

TODO list: * revert code editor gratuitous whitespace changes * documentation updates for new configuration directives

* I understand the reasoning behind the pre-processor defines now, if this is an existing patter used in other projects its probably a good idea to use it in our code as well. * As for the private key loading procedure related to the fork(), I know this issue from other modules, good that you take care of that. I will have a closer look to the code, and comment in-line there.

Are the new config parameters needed only in the global scope of the module level, or it can be something needed per tls config profile (client/server) inside tls.cfg?

@aalba6675 pushed 1 commit.

6966c9f proof-of-concept: implement process-local storage for HSM keys

The feature set is generally complete now with the last commit. Just leaving the documentation of the directives TODO

* support for OpenSSL engine and HSM keys for TLS server and client domains * HSM private keys are stored in worker-local memory - probably this is the most intrusive change; the rest is just related to initialization.

It happens that I am traveling right now and don't have much spare time, but I will review the code in the next few days and then report if I find something, or squash+merge the commits in this PR if all ok and no other objection meanwhile.

@henningw - thanks for your review and work here! I wrote it more from the perspective that I want to do also a deep review, because tls has some complexity in handling all those per server attributes and I would prefer not to break (if possible!!!). Somehow it was triggered by the reference in the comments about using private memory in the context that most of data for tls is in shared memory. I didn't want to start questioning that, preferring a look at the code before. As it could take time in my side, I made the last remark that people should not wait for me, if they need to do something else.

Thanks for the comments - I have replaced malloc/free in the mapping utilities with `pkg_malloc()/pkg_free()`. Re: "I did not fully understand why you need this here, maybe you can elaborate a bit on the requirements of the HSM child_init."

Background: For soft keys, we initialize the SSL_CTX `d->ctx[i]` in PROC_INIT, then fork(). All SSL_CTX objects are now completely initialized and can be used as-is by SSL* objects.

For the HSM case, the private key handle from `ENGINE_load_private_key()` is a proxy for the private key in the HSM. This proxy object is not guaranteed to be valid in a different process whether by fork() or shared memory.

During `mod_child()` we also cannot load the private key into `SSL_CTX *d->ctx[i]`, as the next child will simply overwrite the value since these are in shared memory. In my first implementation I forgot about shared memory so the keys are what the final child loaded. These keys would only work for the last child and the handle was invalid when other children tried to use them.

In my current implementation, I leave the SSL_CTXs as keyless. Instead loading the private key into a local set, keyed by the SSL_CTX pointer. Therefore SSL_CTX for HSM domains are keyless and incomplete for SSL_accept(). At runtime, just before

SSL_accept(ssl)

I retrieve the SSL_CTX* from ssl, and check if it is indeed keyless, i.e, it exists in the local key set. Then we retrieve the HSM key in just-in-time mode and load into the `SSL* ssl` object.

Summary: Soft key: d->ctx[i] fully initialized with private key from PEM file

HSM key: d->ctx[i] keyless, the privatekey is stored in a local set: as hash table { domain0->ctx[0]: EVP_PKEY* domain0->ctx[1]: EVP_PKEY* .... domain1->ctx[0]: EVP_PKEY* domain1->ctx[2]: EVP_PKEY* ... }.

Notes:

1. Most engines that deal with HSM private keys are wrappers around a vendor or OpenSC PKCS 11 library. 1. AWS CloudHSM engine (`libgem.so`) a wrapper around SafeNet Luna HSM `libCryptoki2_64.so` actually produces private keys that work even in a different process. If the last child/master loads its private key into `d->ctx[i]` all the other children can use this `SSL_CTX`. This type of behaviour is due to their special care in implementaion and not mandated. 1. OpenSC/libp11: this engine can wrap any PKCS11 library into an engine. When wrapping `libCryptoki2_64.so` its private keys didn't work in another process. That is why I chose to use a local set. 1. Interestingly the NGINX developers refused to accept HSM private key handling to the child. They state that is the role of the PKCS 11 library to make sure its objects are valid across a fork(). Sadly, this principle does not accord with reality. 1. Although HAProxy has engine support, they do not use engine private keys yet, so have not addressed this issue. 1. GNUTLS recommends that all PKCS 11 objects be used only in one process; i.e don't try to leak handles across fork() or shared memory.

https://www.opendnssec.org/softhsm/ is a software based HSM

@miconda - do you had time to do a review as well?

1. Yes - HSM private keys are stored in worker local memory and are not referenced in old structures during SIP connections. We make one reference during mod_child: we install it into the shmem SSL_CTX structure once (proc_no == 0) just to check the the private key corresponds to the cert; subsequently this reference is not used at connection time.

Later at connection time, even when we use SSL_CTX for proc_no == 0, we load the worker-local HSM private key JIT into the SSL *object and don't use the (probably invalid) private key reference in SSL_CTX.

2. All main distros debian/RHEL/ubuntu build OpenSSL with engine support. We can skip this check and just assume that kamailio is being built with a reasonable OpenSSL prerequisite if you prefer.

3. License - comments from the community?

4. A few commits for better naming and guards: use better module/filename-specificsymbol names; also make a few more symbols static to avoid accidental leakage with common names.

OK, thanks for all those details!

I guess this can be merged if nobody else has comments.

Merged #1484.