From 925764922402ed4b3b281dc4ca8bb0f9863dc95e Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 15 Aug 2025 12:08:47 +1000 Subject: [PATCH 1/3] ctdb-pmda: Do not directly support CTDB_SOCKET environment variable Always use whatever CTDB uses in the current environment. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15921 Signed-off-by: Martin Schwenke Reviewed-by: Volker Lendecke (cherry picked from commit c4794e40529c63c696ecc3f8f27c810c22dd63a5) --- ctdb/utils/pmda/pmda_ctdb.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ctdb/utils/pmda/pmda_ctdb.c b/ctdb/utils/pmda/pmda_ctdb.c index 5c09aca7b84..edf6ffc136b 100644 --- a/ctdb/utils/pmda/pmda_ctdb.c +++ b/ctdb/utils/pmda/pmda_ctdb.c @@ -28,6 +28,8 @@ #include "lib/util/time.h" #include "lib/util/blocking.h" +#include "common/path.h" + #include "client/client.h" #include "client/client_sync.h" @@ -58,9 +60,7 @@ * CTDB PMDA * * This PMDA connects to the locally running ctdbd daemon and pulls - * statistics for export via PCP. The ctdbd Unix domain socket path can be - * specified with the CTDB_SOCKET environment variable, otherwise the default - * path is used. + * statistics for export via PCP. */ /* @@ -200,7 +200,7 @@ pmda_ctdb_disconnected(void *args) static int pmda_ctdb_daemon_connect(void) { - const char *socket_name; + char *socket_name = NULL; int ret; ev = tevent_context_init(NULL); @@ -209,9 +209,9 @@ pmda_ctdb_daemon_connect(void) return -1; } - socket_name = getenv("CTDB_SOCKET"); + socket_name = path_socket(ev, "ctdbd"); if (socket_name == NULL) { - socket_name = CTDB_SOCKET; + goto err_ev; } ret = ctdb_client_init(ev, ev, socket_name, &client); @@ -224,6 +224,7 @@ pmda_ctdb_daemon_connect(void) ctdb_client_set_disconnect_callback(client, pmda_ctdb_disconnected, NULL); + talloc_free(socket_name); return 0; err_ev: -- 2.47.3 From 027d8260a0a9ec7615b584a0bdd2379b1fae9327 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 15 Aug 2025 14:59:49 +1000 Subject: [PATCH 2/3] ctdb-common: Factor out checking of CTDB_TEST_MODE For use elsewhere. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15921 Signed-off-by: Martin Schwenke (cherry picked from commit 4c12a36eb5b44fb08d0461e6fa77fcdb4a128433) --- ctdb/common/path.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ctdb/common/path.c b/ctdb/common/path.c index ea3b08f4b2e..0673bace531 100644 --- a/ctdb/common/path.c +++ b/ctdb/common/path.c @@ -45,16 +45,24 @@ struct { .vardir = CTDB_VARDIR, }; -static void path_set_basedir(void) +static void path_set_test_mode(void) { - const char *t; + const char *t = NULL; t = getenv("CTDB_TEST_MODE"); if (t == NULL) { - goto done; + return; } ctdb_paths.test_mode = true; +} + +static void path_set_basedir(void) +{ + path_set_test_mode(); + if (!ctdb_paths.test_mode) { + goto done; + } ctdb_paths.basedir = getenv("CTDB_BASE"); if (ctdb_paths.basedir == NULL) { -- 2.47.3 From a3a9178e3de79ded7af212d0b87b484da92a44cb Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Fri, 15 Aug 2025 15:01:58 +1000 Subject: [PATCH 3/3] ctdb-common: Only respect CTDB_SOCKET in CTDB_TEST_MODE At the moment CTDB_SOCKET can be used outside of test mode even though nobody should do this. So, no longer allow this. This means ensuring CTDB_TEST_MODE is set in the in the "clusteredmember" selftest environment, so that CTDB_SOCKET is respected there.. Details... The associated use of chown(2) and chmod(2), used to secure the socket in ctdb_daemon.c:ux_socket_bind(), potentially enables a symlink race attack. However, the chown(2) is currently not done in test mode, so restricting the use of CTDB_SOCKET to test mode solves the potential security issue. Also, sprinkle warnings about use of CTDB_TEST_MODE in appropriate places, just to attempt to limit unwanted behaviour. An alternative could be to use the socket file descriptor with fchown(2) and fchmod(2). However, these system calls are not well defined on sockets. Still, this was previously done in CTDB's early days (using the poorly documented method where they are allowed in Linux (only?) before calling bind(2)). It was removed (due to portability issues, via commits cf1056df94943ddcc3d547d4533b4bc04f57f265 and 2da3fe1b175a468fdff4aa4f65627facd2c28394) and replaced with the current post-bind chown(2) and chmod(2). I would like to remove the CTDB_SOCKET environment variable entirely, since setting CTDB_TEST_MODE and CTDB_BASE covers all reasonable test environments. However, I have a feeling that people use it for interactive testing, and that can still be done in CTDB_TEST_MODE. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15921 Signed-off-by: Martin Schwenke Reported-by: *GUIAR OQBA * Reviewed-by: Volker Lendecke Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Thu Sep 25 09:02:06 UTC 2025 on atb-devel-224 (cherry picked from commit 7e2358fcf7be177d6e5de6e26f9d7c5af4acbb0c) --- ctdb/common/path.c | 17 +++++++++++++---- ctdb/server/ctdbd.c | 7 +++++++ ctdb/tests/README | 10 +++++++++- selftest/target/Samba.pm | 1 + selftest/target/Samba3.pm | 1 + 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ctdb/common/path.c b/ctdb/common/path.c index 0673bace531..0d935429460 100644 --- a/ctdb/common/path.c +++ b/ctdb/common/path.c @@ -49,6 +49,12 @@ static void path_set_test_mode(void) { const char *t = NULL; + /* + * Do not use CTDB_TEST_MODE outside a test environment to + * attempt to (for example) improve installation flexibility. + * This is unsupported, may cause unwanted security issues and + * may break in future releases. + */ t = getenv("CTDB_TEST_MODE"); if (t == NULL) { return; @@ -196,11 +202,14 @@ char *path_config(TALLOC_CTX *mem_ctx) char *path_socket(TALLOC_CTX *mem_ctx, const char *daemon) { - if (strcmp(daemon, "ctdbd") == 0) { - const char *t = getenv("CTDB_SOCKET"); + path_set_test_mode(); + if (ctdb_paths.test_mode) { + if (strcmp(daemon, "ctdbd") == 0) { + const char *t = getenv("CTDB_SOCKET"); - if (t != NULL) { - return talloc_strdup(mem_ctx, t); + if (t != NULL) { + return talloc_strdup(mem_ctx, t); + } } } diff --git a/ctdb/server/ctdbd.c b/ctdb/server/ctdbd.c index 0c55ef50b0e..d4cfe341275 100644 --- a/ctdb/server/ctdbd.c +++ b/ctdb/server/ctdbd.c @@ -241,6 +241,13 @@ int main(int argc, const char *argv[]) * Logging setup/options */ + + /* + * Do not use CTDB_TEST_MODE outside a test environment to + * attempt to (for example) improve installation flexibility. + * This is unsupported, may cause unwanted security issues and + * may break in future releases. + */ test_mode = getenv("CTDB_TEST_MODE"); /* Log to stderr (ignoring configuration) when running as interactive */ diff --git a/ctdb/tests/README b/ctdb/tests/README index 80f3311b684..8a243c21703 100644 --- a/ctdb/tests/README +++ b/ctdb/tests/README @@ -98,7 +98,7 @@ Test and debugging variable options PID file relative to CTDB_BASE. When testing with multiple local daemons on a single - machine this does 3 extra things: + machine this does some extra things: * Disables checks related to public IP addresses @@ -107,6 +107,14 @@ Test and debugging variable options * Disables real-time scheduling + * Allows the CTDB_SOCKET environment variable to be used to + specify ctdbd's Unix domain socket location. + + Do not use this variable outside a test environment to + attempt to (for example) improve installation flexibility. + This is unsupported, may cause unwanted security issues and + may break in future releases. + CTDB_DEBUG_HUNG_SCRIPT_LOGFILE=FILENAME FILENAME specifies where log messages should go when debugging hung eventscripts. This is a testing option. See diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm index 5fc856f7886..7edac506560 100644 --- a/selftest/target/Samba.pm +++ b/selftest/target/Samba.pm @@ -1017,6 +1017,7 @@ my @exported_envvars = ( "RESOLV_WRAPPER_HOSTS", # ctdb stuff + "CTDB_TEST_MODE", "CTDB_PREFIX", "NUM_NODES", "CTDB_BASE", diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index a9efd07b0e4..6ea64ff3525 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -4322,6 +4322,7 @@ sub provision_ctdb($$$$) $ret{"CTDB_IFACE_IP_NODE${i}"} = $ip; } + $ret{CTDB_TEST_MODE} = "yes"; $ret{CTDB_BASE} = $ret{CTDB_BASE_NODE0}; $ret{CTDB_SOCKET} = $ret{CTDB_SOCKET_NODE0}; $ret{CTDB_SERVER_NAME} = $ret{CTDB_SERVER_NAME_NODE0}; -- 2.47.3