Attachment 18735 Details for Bug 15921 – Patch for all stable versions

[patch] Patch for all stable versions

BZ15921.patch (text/plain), 8.71 KB, created by Martin Schwenke on 2025-09-26 00:03:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2025-09-26 00:03:15 UTC

Size: 8.71 KB

patch

obsolete

>From 925764922402ed4b3b281dc4ca8bb0f9863dc95e Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <mschwenke@ddn.com>
>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 <mschwenke@ddn.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(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 <mschwenke@ddn.com>
>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 <mschwenke@ddn.com>
>(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 <mschwenke@ddn.com>
>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 <mschwenke@ddn.com>
>Reported-by: *GUIAR OQBA * <techokba@gmail.com>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>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
>

Flags:

vl: review+

Actions: View

Attachments on bug 15921: 18735