[kamailio/kamailio] load_crl takes 90 seconds to complete it execution with crl file size of 4MB !!! (#2312)
<!-- Kamailio Project uses GitHub Issues only for bugs in the code or feature requests. Please use this template only for feature requests.
If you have questions about using Kamailio or related to its configuration file, ask on sr-users mailing list:
* http://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-users
If you have questions about developing extensions to Kamailio or its existing C code, ask on sr-dev mailing list:
* http://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Please try to fill this template as much as possible for any issue. It helps the developers to troubleshoot the issue.
If you submit a feature request (or enhancement) add the description of what you would like to be added.
If there is no content to be filled in a section, the entire section can be removed.
You can delete the comments from the template sections when filling.
You can delete next line and everything above before submitting (it is a comment). -->
### Description
Am using Kamailio 5.1.9 version, In my tls.cfg i have one client and server profile, along with default client and server profile.
I have crl enabled for the non default client and server profile , the crl file size is 4 MB in my case.
I have 22 child tcp process.
With this what i observe is load_crl is taking close to 90 seconds to finish its execution and return.
### Expected behavior
load_Crl function should not take 90 seconds to complete its execution. probably it should take in the range of 10-15 seconds to complete its execution or even lesser.
#### Actual observed behavior
load_Crl function is taking 90 seconds to complete its execution.
#### Debugging Data
It is very clear from the code, its because of this for loop. time taken to complete load_Crl execution is 90 seconds
procs_no=get_max_procs(); for(i = 0; i < procs_no; i++) { if (SSL_CTX_load_verify_locations(d->ctx[i], d->crl_file.s, 0) != 1) { ERR("%s: Unable to load certificate revocation list '%s'\n", tls_domain_str(d), d->crl_file.s); TLS_ERR("load_crl:"); return -1; } store = SSL_CTX_get_cert_store(d->ctx[i]); X509_STORE_set_flags(store, X509_V_FLAG_CRL_CHECK | X509_V_FLAG_CRL_CHECK_ALL); }
Is there a way this can be enhanced or as per the current kamailio design this is a must to do for each and every profile and its ssl context array list for each process and for every profile.
The same logic is seen in other load functions as well, for example load_cert, load_ca_list, load_crl, set_cipher_list, set_verification, set_ssl_options, set_session_cache, ksr_tls_fix_domain,
#### Log Messages
<!-- Check the syslog file and if there are relevant log messages printed by Kamailio, add them next, or attach to issue, or provide a link to download them (e.g., to a pastebin site). -->
``` (paste your log messages here) ```
#### SIP Traffic
<!-- If the issue is exposed by processing specific SIP messages, grab them with ngrep or save in a pcap file, then add them next, or attach to issue, or provide a link to download them (e.g., to a pastebin site). -->
``` (paste your sip traffic here) ```
### Possible Solutions
Reply from Henning Westerholt on posting this problem to Users Mailing list
"But the code could be probably also improved, maybe it is possible to parallelize it. You can open a feature request about it,"
### Additional Information
Kamailio 5.1.9 version
* **Operating System**:
<!-- Details about the operating system, the type: Linux (e.g.,: Debian 8.4, Ubuntu 16.04, CentOS 7.1, ...), MacOS, xBSD, Solaris, ...; Kernel details (output of `uname -a`) -->
``` Linux Kernel version : 3.10.0-693.el7.x86_64
Centos version : CentOS Linux release 7.4.1708 (Core)
CPU : 2 cores with model name : Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz
[root@miv5000 ~]# cat /proc/meminfo MemTotal: 3882076 kB MemFree: 811244 kB MemAvailable: 2320356 kB
Openssl verison : OpenSSL 1.0.2k-fips 26 Jan 2017 ```
From the code, each profile inside tls.cfg maps to this data structure tls_domain_t, which contains SSL_CTX** ctx; and this is an array of the child tcp process list.
What was the reason to keep it an array list, if am populating the same data to each and every member of the array list ? only one context would have been sufficient instead of an array of context inside tls_domain_t ?
Am just trying to understand what prompted to keep it as an array list of ssl_ctx, instead of a single ssl_ctx. So that could get to know the bigger picture of it.
Hi Henning, Any approximate timeline, on when this will be fixed.
Regards, Mahesh.B
@maheshb2487 - just as generic remark: if no developer volunteered to do it, so no comment/update here about ongoing work, likely nobody had a need for it in their deployments so far and it is no exact (or even approximate) time lime to expect the feature to be implemented.
From my point of view, when I get some spare time, I start implementing from my internal todo list or from this tracker -- it can be this evening, next week, or quite far in the future. It is a matter of events out there, family and needs for deployments we maintain. These days when most of us have to work from home, therefore with slower interaction with colleagues and customers, with schools being closed, a.s.o., expect that spare time is even more limited for developers.
From that perspective, considering this is an open source project, a pull request implementing the feature request has higher chances to be quickly merged (still after the review of developers), rather than expecting someone else to work on it in short term.
Otherwise I do not know if @henningw started to work (or even has time to work on it in the near future) on this. He can provide updates from his perspective and availability.
@miconda - thanks you. I did not started to work on it, just was in communication on the list with @maheshb2487 about it so far.
Agreed @miconda , Your input and guidance on the below is needed
From the code, each profile inside tls.cfg maps to this data structure tls_domain_t, which contains SSL_CTX** ctx; and this is an array of the child tcp process list.
What was the reason to keep it an array list, if am populating the same data to each and every member of the array list ? only one context would have been sufficient instead of an array of context inside tls_domain_t ?
Am just trying to understand what prompted to keep it as an array list of ssl_ctx, instead of a single ssl_ctx. So that could get to know the bigger picture of it and then think of how to resolve it and test it on my setup.
The problem is clear, running the for loop for the tcp child process list for each profile , is consuming time.
why is it SSL_CTX** ctx array, and not just SSL_CTX *ctx , if all the array elements contain the same information.
If you say , SSL_CTX has to be an array though it contains the same value in each item of the array for some design reason - it needs a deeper study on the tls module from my side on how to solve the problem. (Just a guess, probably spawn threads to run them parallely, instead of a single thread doing it)
If you say, SSL_CTX can be just a pointer, it looks like a straight forward problem to resolve.
Regards, Mahesh.B
This issue is stale because it has been open 6 weeks with no activity. Remove stale label or comment or this will be closed in 2 weeks.
Closed #2312 as not planned.
-
Daniel-Constantin Mierla -
github-actions[bot]
-
Henning Westerholt
-
maheshb2487