[kamailio/kamailio] Adding TLS support to db_redis (also password support) and ndb_redis (PR #3477)
<!-- Kamailio Pull Request Template -->
<!-- IMPORTANT: - for detailed contributing guidelines, read: https://github.com/kamailio/kamailio/blob/master/.github/CONTRIBUTING.md - pull requests must be done to master branch, unless they are backports of fixes from master branch to a stable branch - backports to stable branches must be done with 'git cherry-pick -x ...' - code is contributed under BSD for core and main components (tm, sl, auth, tls) - code is contributed GPLv2 or a compatible license for the other components - GPL code is contributed with OpenSSL licensing exception -->
#### Pre-Submission Checklist <!-- Go over all points below, and after creating the PR, tick all the checkboxes that apply --> <!-- All points should be verified, otherwise, read the CONTRIBUTING guidelines from above--> <!-- If you're unsure about any of these, don't hesitate to ask on sr-dev mailing list --> - [ /] 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: <!-- Go over all points below, and after creating the PR, tick the checkboxes that apply --> - [ ] PR should be backported to stable branches - [ /] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description A couple of commits into db_redis and ndb_redis adding TLS support and also password support to db_redis.
This mainly includes checking if proper parameter is provided (for ndb_redis is `tls` option in the DB URL and, for db_redis, a new `opt_tls` parameter) and creates a temporary SSL context that is used to initialise the redis context.
Also added `ca_path` parameter to both modules to be able to define a valid folder containing the root certificates used to validate TLS' certificate chain.
db_redis is also updated with a `db_pass` parameter to provide a DB access password.
TLS support is automatically enabled by checking libhiredis_ssl.so existence in each Makefile and defining a `WITH_SSL` flag that enables all the corresponding code lines.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3477
-- Commit Summary --
* db_redis: Adding TLS support * ndb_redis: Adding TLS support
-- File Changes --
M src/modules/db_redis/Makefile (24) M src/modules/db_redis/db_redis_mod.c (13) M src/modules/db_redis/doc/db_redis.xml (5) M src/modules/db_redis/doc/db_redis_admin.xml (59) M src/modules/db_redis/redis_connection.c (98) M src/modules/db_redis/redis_connection.h (6) M src/modules/ndb_redis/Makefile (15) M src/modules/ndb_redis/doc/ndb_redis.xml (5) M src/modules/ndb_redis/doc/ndb_redis_admin.xml (26) M src/modules/ndb_redis/ndb_redis_mod.c (10) M src/modules/ndb_redis/redis_client.c (69) M src/modules/ndb_redis/redis_client.h (7)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3477.patch https://github.com/kamailio/kamailio/pull/3477.diff
@henningw This is the new PR for adding TLS support to Redis. Once again, I'm sorry for all the nuisance. @oej Also fyi (as you commented on the original PR). Thank you very much in advance!
@joelbax pushed 4 commits.
5ab0b26dc9da5d8c7bd11daa0526238f2ded015e db_redis: Fixing format 3cdbe739d5e5eba8bfb55789df02abe7367e303e db_redis: Missing format fixes d909846ed261f8f3279bf040217969f89d5f02c3 ndb_redis: Format fixes e9a9ec6e768a839031dac15427a51d39b68a0af9 db_redis: Fixing build
@joelbax pushed 1 commit.
deb3a2b2b206314dd956cd02322d483c397d7d27 db_redis: Fixing Makefile
@miconda commented on this pull request.
@@ -29,6 +29,12 @@
#include "redis_dbase.h" #include "redis_table.h"
+#ifdef WITH_SSL +int db_redis_opt_tls = 0; +char *ca_path = 0; +#endif +char *db_pass = 0;
Add module name prefix to the global variables ca_path and db_pass, like it is db_redis_opt_tls. The are also declared with extern and being prefixed reduce the chances of conflicts with similar variables (e.g., ca_path can be quite common with some libs).
@joelbax pushed 1 commit.
29a0f05e2e4dca5419c17e7052547a8006136677 db_redis: Fixing global variables
@joelbax pushed 1 commit.
5cbd4e2f19c9c64e72dbb9026d6b1d420e21a79e ndb_redis: Fixing global variables names
@joelbax commented on this pull request.
@@ -29,6 +29,12 @@
#include "redis_dbase.h" #include "redis_table.h"
+#ifdef WITH_SSL +int db_redis_opt_tls = 0; +char *ca_path = 0; +#endif +char *db_pass = 0;
Very good point! Thank you very much.
Done.
I see that the Makefile does a `ls` on a path that is taken from the output of `pkg-config hiredis --libs-only-L`, but in my Ubuntu 22.04 the output is empty (the libhiredis-dev is installed from package, thus the libs are in the standard location).
Therefore I assume that your PR is going to work only when libhiredis is installed in a custom location.
Maybe you can leverage libraries paths in the output of `gcc -print-search-dirs` or `clang -print-search-dirs` (in Makefile it should be the `$(CC) -print-search-dirs` when `pkg-config hiredis --libs-only-L` returns empty string.
Or maybe just as other modules are doing (like ndb_redis), assume a few default paths if there is no pkg-config available.
@joelbax pushed 1 commit.
f5a25660f6d6053b93f0a6e55629a683e6757dec ndb_redis: Missing uprotected redisSSLContext
@henningw: your comment is out of context, read again my comment, it is about the case when `pkg-config hiredis --libs-only-L` returns empty string (so `pkg-config` exists).
@joelbax pushed 2 commits.
59cf9016a3fba507916eff6f7ae0a5ec17db7204 db_redis: Searching SSL support in gcc search library path 8ef8b386bbade2a0b6daf9943cc05d8ffed96183 ndb_redis: Searching SSL support in gcc search library path
@joelbax pushed 2 commits.
3f52d4704d0ac1ef77762b59a3d4f41a24c2728f db_redis: Format fixes c026257b38ed8112a0ad6d3c5fd257977b25010e ndb_redis: Fixing format
@miconda I've added logic to check gcc search paths for libraries in case `pkg-config` returns nothing.
I've also fixed som formatting issues but the check-format task seems to complain about some lines but , I'm really sorry but, I can't see the difference between the wrong line and the new one.
Thanks!
Merged #3477 into master.
@joelbax: I merged and then updated a bit the find command in the Makefile for n/db_redis modules to silent some warning about missing directories and use `-name` for finding file expression. Now compiles fine on ubuntu 22.04, before I got errors. Maybe you can test on your system to be sure it didn't break something else.
-
Daniel-Constantin Mierla -
Henning Westerholt
-
joelbax