This latest commit implements some of the TODO's in the curl module. - default client_cert, client_key, cacert and verifyserver modparams implemented - added a cipher_suites modparam so the default list provided by libcurl can be overridden (e.g. enabling elliptic-curve ciphers) - added per-connection value for verifyserver
Here are some followup questions for @oej or anyone else who may use the curl module. Should the [verify_host](http://curl.haxx.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html) parameter be configurable as well as verify_peer? Is the intention to make the client cert/key configurable per-connection?
If we add more parameters to the curlcon modparam, it will easily get unmanageable. I suggest two modparams, curlcon and curlcon_param. The first sets up the url and parameters (this is unchanged). The second will add parameters to a curlcon that is already defined. E.g. ``` modparam("curl", "curlcon", "apione=>https://myserver.example.com/url;timeout=12") modparam("curl", "curlcon_param", "apione=>verifyserver=1") modparam("curl", "curlcon_param", "apione=>clientkey=/var/certs/myserver.example.com.pem") ```
Any thoughts? Have I missed anything else in this area? You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/481
-- Commit Summary --
* curl: Add client key/certificate to curl_connect
-- File Changes --
M modules/curl/curl.c (2) M modules/curl/curl.h (6) M modules/curl/curlcon.c (24) M modules/curl/doc/curl_admin.xml (26) M modules/curl/functions.c (113)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/481.patch https://github.com/kamailio/kamailio/pull/481.diff
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481
Is 'verifyserver' the best name for this parameter? The curl options are verifypeer and verifyhost for checking the certificate chain and common name/SAN (respectively). Should the curl module preserve that naming in the modparams?
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481#issuecomment-173149396
Just my opinion, not specific for this case - maybe is good to keep the same name and mention that in the docs so people can dig into the curl/lib docs if then need additional info. Anyhow, renaming can be done afterwards, with another commit.
The feature is important for proper security, so I am all for merging.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481#issuecomment-173198490
The configuration issue seems like we need to redesign a bit. I'll think about it. At some point, as you say, the one-liner doesn't work any more and we should probably look into having a separate config file, like the TLS module one, when needed. That may give us other possibilities.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481#issuecomment-173485016
Client certs (and possibly cipher suites) needs to be per connection.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481#issuecomment-173485121
Merged #481.
--- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/481#event-535653137
-
Daniel-Constantin Mierla -
Hugh Waite
-
Olle E. Johansson