@oej commented on this pull request.

+int to_ssl(SSL_CTX** tls_ctx_p, SSL** tls_conn_p, int tcp_sock, int method)

+{ + *tls_ctx_p = init_ssl_ctx(method); + if (!(*tls_ctx_p)) { + LM_ERR("to_ssl(): Error on initialising ssl context\n"); + return -1; + } + if (certificate.s && private_key.s) { + if (load_certificates(*tls_ctx_p, &certificate, &private_key) < 0) { + LM_ERR("to_ssl(): Error on loading certificates\n"); + return -1; + } + } + *tls_conn_p = init_ssl_conn(tcp_sock, *tls_ctx_p); + if (!(*tls_conn_p)) { + LM_ERR("to_ssl(): Error on negociating ssl connection\n");

TLS connection

@oej commented on this pull request.

•  </itemizedlist>
  

+ <para><emphasis> Default value is <quote>TLSv1.2</quote>. </emphasis></para> + + <example> + <title>Set <varname>tls_method</varname> parameter</title> + + <programlisting format="linespecific">... +modparam("tls", "tls_method", "TLSv1") +... + </programlisting> + </example> + </section> + <section> + <title>private_key (string)</title> + + <para>Sets the private key file name.</para>

Do you support both der and pem encoding? Please specify format.

Just for brainstorming: My general feeling is that a lot of this code - the parsing of the TLS variant, the default settings - should be general for all kamailio modules using TLS. If we decide to deprecate a version of TLS or add a new version, it would be simpler to have it in one place, not spread out over the code base.

Core Kamailio now has two TLS stacks supported. I have no clue, but would it be possible to use those as an interface to get the same multi-stack support here.

Thanks also from my side. One additional note, please execute "clang-format" on the file, as the syntax check is currently failing. Just do a force push after you changed it to update the PR commit.

I think the TLS implementation for SIP can't be used in this module, because the cdp module has its own internal design with dedicated transport workers. Reusing some common functions could be eventually done, but I am not sure how much code is shared in the current approach. The tls config parser is anyhow the ini-parser from core, it is not inside the tls module. At this moment I don't see a straightforward way that having/adding support for a TLS version (say TLS 1.3) in one module becomes available in the other one.

For the moment, the current implementation might have to be merged as it is, future reworking might do a better code sharing.

As a generic note, the name of the function does not have to be explicitly added to the log messages, it is done automatically for all log messages -- it might be that CDP module was not updated to remove function names from old log messages, but the new ones should avoid having function names, they will appear twice.

@lbalaceanu pushed 2 commits.

9c3d2117168dc3c992df5d18ff18385508082f29 cdp: fixing formatting 9bdbe7250d892cb2d871f7e283217f0af2274ea9 cdp: logging reworking

@lbalaceanu pushed 2 commits.

d0a81b44f6cc3a058fd5771d7080c4e5d052ac7d cdp: adding ca_list param 35a08f96a3919a58d53ca5ce25d74134de978e8d cdp: updated with ca_list param and clarifications

@lbalaceanu pushed 1 commit.

085bf21505a67e1404118d79f7e3288bdd2e4e51 cdp: adding TLSv1.3 configuration

@lbalaceanu pushed 2 commits.

e3dea073f93bccdc9353d2f1ab9fa0a29fca0030 cdp: adding TLS capability to peer connections a1e6bed57a1e84b4837cfc9a6d961b3c262155ee cdp: documenting TLS capability related parameters

Hi. Hope I took care of the punctual requests made by @oej and @henningw. Please comment. However, starting tomorrow I will not have access to my laptop for a week. Have a nice day!

I haven't looked much at the changes, but I am not much using these modules, so I guess the PR can be merged and adjustments can be done down the road as needed.

Merged #3548 into master.

@lbalaceanu this code is breaking nightly-builds I commented at https://github.com/kamailio/kamailio/commit/defae2551f8839c5397523f135293fb2... with the compile errors https://kamailio.sipwise.com/job/kamailiodev-nightly-binaries/2489/console