Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
Thanks will squash and merge. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-379719270___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
OK, thanks for all those details! I guess this can be merged if nobody else has comments. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-379706828___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
* About the question:
"The files tls_map.{c,h} seems to be imported from external source, being
under MIT license. tls
module seems to be under BSD, anyone knows if there is any conflict between
the two or
something needs to be mentioned in the README of the tls module?"
According to www.dwheeler.com/essays/floss-license-slide.html - the MIT licence
is compatible with the (new) BSD licence, and the result can be still licenced
under BSD licence. So no additional information should be necessary.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1484#issuecomment-378677038___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
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. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-378572496___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
Just to confirm I haven't missed something -- the private keys stored in
worker-local memory refer to keeping them in the map structure you introduced
with the new files tls_map.{c,h}. They are not referenced from old structures
of the tls module, right?
I see that the define conditions are on `#ifndef OPENSSL_NO_ENGINE`,
understanding that `OPENSSL_NO_ENGINE` is defined if libssl is compiled
without this engine feature. But is this feature depending on some version, or
is in libssl for very long time and makes no sense to check for a version that
doesn't have support for it at all?
The files tls_map.{c,h} seems to be imported from external source, being under
MIT license. tls module seems to be under BSD, anyone knows if there is any
conflict between the two or something needs to be mentioned in the README of
the tls module?
Some cosmetic things I would like to have for a safety future:
* define guards inside tls_map.h should rely on the name of the file, like in
the other cases. right now is `MAP_H`, exposing a risk of a conflict in the
future someone adds a map.h somewhere in kamailio code that will be included in
the same file with tls_map.h
* the global variable `engine` has a rather common name, should be renamed
like `ksr_tls_engine`, to make it more specific for kamailio context -- this
should avoid unexpected behaviour if one opens the shared objects with
RTLD_GLOBAL when there will be an overlap with such common name
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1484#issuecomment-378523224___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@miconda - do you had time to do a review as well? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-377367096___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
Packaging is here: stretch: https://packages.debian.org/stretch/softhsm2 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375935623___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
https://www.opendnssec.org/softhsm/ is a software based HSM -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375887257___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
Thank you for the detailed explanation, I understand the problem now. Then indeed you need a solution like you implemented. With regards to testing, is there a way to test it also without a HSM module, or exists something like a software "HSM" for testing? @miconda Understood - this is a quite important module, of course its better to review it from different sides. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375886970___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
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.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1484#issuecomment-375671973___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@aalba6675 pushed 1 commit. c802024 tls: use pkg_* functions -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1484/files/956d0f72a970ce7c826e394c9d1431da6f167b36..c802024442fd8c3ec5190382e84430d4dd4260a0 ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@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. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375572364___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
Sorry for the late reply, was yesterday pretty busy as well. Generally speaking, these are the different approaches that modules use for data access in kamailio child processes: * all children needs to access to a one shared data structure Create one global structure in mod_init in shm_memory, and then access it from different children. Protect the (possible) concurrent access with locks, if you support run-time modification of this data. You find examples in the carrierroute module, mod_init() -> init_route_data() and its reload functions and other modules. * All children needs access to a individual local data set Just allocate it in pkg_memory and access it from the children individually. You can find an example in the auth_diameter module, auth_diameter.c mod_child_init() function. * All children needs access to all local sets Then you need to implement a solution like you choose with a shared data structure, e.g. in a hash table that you write in child_init to. I did not fully understand why you need this here, maybe you can elaborate a bit on the requirements of the HSM child_init. I have another remark about the hash table you' added. The hash table uses system malloc() to allocate memory. Please change this to pkg_memory, if you need per-process individual memory, or shm_memory for shared memory. If you have more detailed questions, feel free to contact me per e-mail as well (hw at kamailio dot org). @miconda - as you currently traveling, I can do the further processing of this patch, no need to hurry from your side. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375566763___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@aalba6675 pushed 1 commit. 064689c tls: add documentation for engine params -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1484/files/5d5aae2826db9635d29a5db5be688fc8caf02e5e..064689c7d255791177875d9bc6d13f7943e99b59 ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
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. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/1484#issuecomment-375511220___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@aalba6675 pushed 1 commit. 5d5aae2 tls/tls_server.c: add HSM key support in outbound connections -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1484/files/4c5d1e6cb7d55c4f2f7f61cc95ca9c8a66aee059..5d5aae2826db9635d29a5db5be688fc8caf02e5e ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] [WIP] tls: add support for OpenSSL engine and private keys in HSM (#1484)
@aalba6675 pushed 1 commit. 6966c9f proof-of-concept: implement process-local storage for HSM keys -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/1484/files/b9b3a3247a312f5f406b40b637fbafed8b25..6966c9f20a444818cc3028afd448f3c5f46a9add ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
