Note: See below for more info about motivation of this feature.
#### Pre-Submission Checklist - [ * ] Commit message has the format required by CONTRIBUTING guide - [ * ] Commits are split per component (core, individual modules, libs, utils, ...) - [ * ] Each component has a single commit (if not, squash them into one commit) - [ * ] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [ * ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: - [ ] PR should be backported to stable branches - [ * ] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description
This feature aims to replace require_certificate and verify_certificate params with a single option, verify_client: * Provides flexibility: require_certificate and verify_certificate are both booleans, so there are only 4 max combinations of params, and only 3 of them make sense (require_certificate=1 and verify_certificate=0 does not). In contrast, verify_client is a list of enumerated values, which can be more gracefully expanded by adding additional behaviors to the enum. * Motivation for this feature is to enable optional_no_ca behavior, described in the docbook; Without this feature, that behavior cannot be represented by any combination of require_certificate and verify_certificate. I figured if I need to add another variable to support desired behavior, it may as well be one that can hold more than just boolean values. * This feature was inspired from a similar one in Nginx; that software has a similar "ssl_verify_client" option that takes the same "on", "off", "optional", and "optional_no_ca" values, which effectively implement the same feature. Note that there is no shared code between implementations, and that these behaviors are implemented (in both cases) via a very thin layer of glue code on top of the OpenSSL library. * Note that the only function definition in tls_verify.c, verify_callback(int pre_verify_ok, X509_STORE_CTX *ctx), has been compiled into the kamailio binary, but apparently not used. Rather than modify the existing function, I added a simple 1-line function (2 if you count the log message too) to enable this feature.
Please let me know if I can answer any questions. Thanks! You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/2166
-- Commit Summary --
* tls: add verify_client support
-- File Changes --
M src/modules/tls/doc/params.xml (52) M src/modules/tls/tls_cfg.c (3) M src/modules/tls/tls_cfg.h (1) M src/modules/tls/tls_config.c (31) M src/modules/tls/tls_config.h (4) M src/modules/tls/tls_domain.c (20) M src/modules/tls/tls_domain.h (14) M src/modules/tls/tls_mod.c (12) M src/modules/tls/tls_rpc.c (3) M src/modules/tls/tls_verify.c (6) M src/modules/tls/tls_verify.h (5)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/2166.patch https://github.com/kamailio/kamailio/pull/2166.diff
@armenb pushed 1 commit.
21dc1cee499fc7f58c6b68fc40f5ebf5aa1e14cb fix error in forward-port
Thanks! As I can see, this is an alternative, not replacing the old variants.
Seems ok after quick look. I will squash&merge soon once I get a bit more time to check it.
@armenb pushed 1 commit.
1793ab5f0eed6fa4538e42e0aa2ec65e940c8218 docbook update
Hi Daniel,
Ah indeed, this is an alternative way of controlling certificate require/verify behavior, not a strict replacement.
I updated the docbook to include a conversion table between verify_certificate+require_certificate parameters and verify_client parameters (and fixed a docbook bug in the process).
Armen
Merged #2166 into master.