[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread space88man via sr-dev
Closed #3695 as completed.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#event-11390919712
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread space88man via sr-dev
Closing now with commits on master—reopen with separate issues for OpenSSL 
1.1.1/OpenSSL 3.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1877228488
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread space88man via sr-dev
The initial set of commits are in. Verification:
OpenSSL 3:
- after kamailio is started, gdb attach rank 0 PID, verify that

(gdb) p pthread_getspecific(err_thread_local)
# should be 0x0

OpenSSL 1.1.1:
- after kamailio is started, gdb attach rank 0 PID and a worker PID, verify that

# in rank 0 and worker rank these pointers should be different
(gdb) p ERR_get_state()
# in rank 0 these two values should be 0x0
(gdb) p pthread_getspecific(public_drbg)
(gdb) p pthread_getspecific(private_drbg)




-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1877195960
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread Daniel-Constantin Mierla via sr-dev
@space88man: one remark regarding the commit message format in the PR #3696, do 
not make first line like:

```
tls: POC for OpenSSL 3
```

Where I assume POC stands for `proof of concept`. Make it more explicit about 
what the commit does, maybe like:

```
tls: init libssl in separate thread for v3.+
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876939525
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread space88man via sr-dev
Ok thanks guys for your feedback: I am going to proceed with a series of 
commits to master. These can then be reverted easilyt. To each commit message I 
will also add the label thread-local 
to facilitate review and search.

@miconda you are correct regarding libssl-initialization threads in rank 0; 
they are run-and-done type are will complete before `fork()`.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876930942
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread Daniel-Constantin Mierla via sr-dev
The tls module initializes very early the libssl, which was ok for libssl 
0.9.x, 1.0.x. With libssl 1.1.x we had to import random number generator to go 
around some of the libssl-specific multi-threading. With libssl 3.x seems to be 
more impact, somehow related to what was introduced in libssl 1.1.x, but 
expanded to other globals.

In other words, I don't think that a wrapper library done long time ago, pre 
libssl 1.1.x/3.x can help nowadays. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876852310
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread Olle E. Johansson via sr-dev
I have mentioned this before as an issue and it was a driving factor behind 
creating the API to the curl module. Having multiple modules using Curl that 
initiates OpenSSL (or non-curl modules initiating OpenSSL) will lead to 
problems. I remember that Kevin Fleming while working with Asterisk wrote a 
wrapper library that initialised OpenSSL once only for all modules. I have no 
idea if that still exists, but if it does, it could be an inspiration.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876836964
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread Daniel-Constantin Mierla via sr-dev
@space88man: thanks for digging deep into this one!

I am fine to try the proposed approach in the PR #3696 and see how it goes, the 
only question would be about the impact of other libs that link behind with 
libssl, like libcurl or libmysqlclient. Does a similar approach needs for the 
modules linked with such libraries?

Regarding multi-threading in Kamailio, there are couple of modules already 
using threads, like `lwsc`, `secsipid/_proc`. Issues with multi-threading and 
multi-processing is when there are active threads at `fork()` time, but if I 
got right the PR #3696, the process with rank 0 waits for the created 
libssl-initialization thread to terminate. Thats why secsipid_proc was split 
out of secsipid, to bind to it after fork(), which then initialize the golang 
library (that creates internally threads for memory management). lwsc creates 
threads only inside child processes, which is after fork().

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876830555
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread space88man via sr-dev
> ...just don't do the TLS initialization in rank 0, right? If we need to touch 
> all openssl using modules anyway, maybe this is an easier and less intrusive 
> way?

One solution is to have each module declare a `mod_init_openssl()`;  then have 
a helper function to run `mod_init_openssl` in a transient thread

```
pthread_create(... mod_init_openssl, )
// do OpenSSL stuff here
pthread_join(...)
```

Then this thread will disappear after `mod_init` in rank 0—all the OpenSSL 
thread-local varables in rank 0(thread#1) will  be "clean".

BTW this study explains why even OpenSSL 1.1.1 is so odd - per child replicated 
`SSL_CTX*`, and RNG replacement with `RAND_set_rand_method`. The root cause is 
the same: there are thread-local variables in rank 0(thread#1) that are 
replicated in the workers—after `fork()` OpenSSL doesn't properly reinitialize 
these states.

I have also gone back to look at the OpenSSL 1.1.1 implementation - by putting 
all initialization (`SSL_CTX_new`, `tls_fix_domains_cfg` etc) into a transient 
thread none of the workarounds are necessary any more(!) - in particular the  
`tls_rand.c` stuff is not needed.

To be clear, the dlsym-pthreads stuff(`src/main.c`) is still needed to handle 
multi-process locks.


-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876703918
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org


[sr-dev] Re: [kamailio/kamailio] tls: OpenSSL 3/1.1.1 is using shared ERR_STATE in all workers (Issue #3695)

2024-01-04 Thread Henning Westerholt via sr-dev
Thank you for the detailed analysis and also the POC. I remember from a custom 
module that I maintained for some year that using threads in Kamailio can bring 
some challenges due to its multi-process architectures. So its probably need to 
be discussed more thoroughly. The other option you mentioned would be to just 
don't do the TLS initialization in rank 0, right? If we need to touch all 
openssl using modules anyway, maybe this is an easier and less intrusive way?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/3695#issuecomment-1876686160
You are receiving this because you are subscribed to this thread.

Message ID: ___
Kamailio (SER) - Development Mailing List
To unsubscribe send an email to sr-dev-le...@lists.kamailio.org