Attachment 18096 Details for Bug 15473 – Patch for master v1

[patch] Patch for master v1

busy-rpc-DoS-master-v1.patch (text/plain), 15.53 KB, created by Andrew Bartlett on 2023-09-12 09:37:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Bartlett

Created: 2023-09-12 09:37:40 UTC

Size: 15.53 KB

patch

obsolete

>From 31892690a7e03854fab182133f237c836cf7ecce Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 12 Sep 2023 12:28:49 +1200
>Subject: [PATCH 1/2] s3-rpc_server: Strictly refuse to start RPC servers in
> conflict with AD DC
>
>Just as we refuse to start NETLOGON except on the DC, we must refuse
>to start all of the RPC services that are provided by the AD DC.
>
>Most critically of course this applies to netlogon, lsa and samr.
>
>This avoids the supression of these services being the result of a
>runtime epmapper lookup, as if that fails these services can disrupt
>service to end users by listening on the same socket as the AD DC
>servers.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15473
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source3/rpc_server/rpcd_classic.c  | 40 +++++++++++++++++++++++++-----
> source3/rpc_server/rpcd_epmapper.c | 33 ++++++++++++++++++++++--
> source3/rpc_server/rpcd_lsad.c     | 16 ++++++++++++
> source3/rpc_server/rpcd_rpcecho.c  | 33 ++++++++++++++++++++++--
> 4 files changed, 112 insertions(+), 10 deletions(-)
>
>diff --git a/source3/rpc_server/rpcd_classic.c b/source3/rpc_server/rpcd_classic.c
>index 4f6164c814c..a5858a0cf1c 100644
>--- a/source3/rpc_server/rpcd_classic.c
>+++ b/source3/rpc_server/rpcd_classic.c
>@@ -42,14 +42,29 @@ static size_t classic_interfaces(
> 	static const struct ndr_interface_table *ifaces[] = {
> 		&ndr_table_srvsvc,
> 		&ndr_table_netdfs,
>-		&ndr_table_wkssvc,
>+		&ndr_table_initshutdown,
> 		&ndr_table_svcctl,
> 		&ndr_table_ntsvcs,
> 		&ndr_table_eventlog,
>-		&ndr_table_initshutdown,
>+		&ndr_table_wkssvc,
> 	};
>+	size_t num_ifaces = ARRAY_SIZE(ifaces);
>+
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC wkssvc is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_ifaces -= 1;
>+		break;
>+	default:
>+		break;
>+	}
>+
> 	*pifaces = ifaces;
>-	return ARRAY_SIZE(ifaces);
>+	return num_ifaces;
>+
> }
> 
> static size_t classic_servers(
>@@ -58,15 +73,28 @@ static size_t classic_servers(
> 	void *private_data)
> {
> 	static const struct dcesrv_endpoint_server *ep_servers[7] = { NULL };
>+	size_t num_servers = ARRAY_SIZE(ep_servers);
> 	bool ok;
> 
> 	ep_servers[0] = srvsvc_get_ep_server();
> 	ep_servers[1] = netdfs_get_ep_server();
>-	ep_servers[2] = wkssvc_get_ep_server();
>+	ep_servers[2] = initshutdown_get_ep_server();
> 	ep_servers[3] = svcctl_get_ep_server();
> 	ep_servers[4] = ntsvcs_get_ep_server();
> 	ep_servers[5] = eventlog_get_ep_server();
>-	ep_servers[6] = initshutdown_get_ep_server();
>+	ep_servers[6] = wkssvc_get_ep_server();
>+
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC wkssvc is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_servers -= 1;
>+		break;
>+	default:
>+		break;
>+	}
> 
> 	ok = secrets_init();
> 	if (!ok) {
>@@ -85,7 +113,7 @@ static size_t classic_servers(
> 	mangle_reset_cache();
> 
> 	*_ep_servers = ep_servers;
>-	return ARRAY_SIZE(ep_servers);
>+	return num_servers;
> }
> 
> int main(int argc, const char *argv[])
>diff --git a/source3/rpc_server/rpcd_epmapper.c b/source3/rpc_server/rpcd_epmapper.c
>index 950ba7ec12a..455179ccfba 100644
>--- a/source3/rpc_server/rpcd_epmapper.c
>+++ b/source3/rpc_server/rpcd_epmapper.c
>@@ -19,6 +19,8 @@
> #include "rpc_worker.h"
> #include "librpc/gen_ndr/ndr_epmapper.h"
> #include "librpc/gen_ndr/ndr_epmapper_scompat.h"
>+#include "param/loadparm.h"
>+#include "libds/common/roles.h"
> 
> static size_t epmapper_interfaces(
> 	const struct ndr_interface_table ***pifaces,
>@@ -27,8 +29,22 @@ static size_t epmapper_interfaces(
> 	static const struct ndr_interface_table *ifaces[] = {
> 		&ndr_table_epmapper,
> 	};
>+	size_t num_ifaces = ARRAY_SIZE(ifaces);
>+
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC epmapper is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_ifaces = 0;
>+		break;
>+	default:
>+		break;
>+	}
>+
> 	*pifaces = ifaces;
>-	return ARRAY_SIZE(ifaces);
>+	return num_ifaces;
> }
> 
> static size_t epmapper_servers(
>@@ -37,11 +53,24 @@ static size_t epmapper_servers(
> 	void *private_data)
> {
> 	static const struct dcesrv_endpoint_server *ep_servers[] = { NULL };
>+	size_t num_servers = ARRAY_SIZE(ep_servers);
> 
> 	ep_servers[0] = epmapper_get_ep_server();
> 
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC epmapper is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_servers = 0;
>+		break;
>+	default:
>+		break;
>+	}
>+
> 	*_ep_servers = ep_servers;
>-	return ARRAY_SIZE(ep_servers);
>+	return num_servers;
> }
> 
> int main(int argc, const char *argv[])
>diff --git a/source3/rpc_server/rpcd_lsad.c b/source3/rpc_server/rpcd_lsad.c
>index 3ca0ed43fdd..18d4380bd3e 100644
>--- a/source3/rpc_server/rpcd_lsad.c
>+++ b/source3/rpc_server/rpcd_lsad.c
>@@ -46,6 +46,14 @@ static size_t lsad_interfaces(
> 		/* no netlogon for non-dc */
> 		num_ifaces -= 1;
> 		break;
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * All these services are provided by the 'samba'
>+		 * binary from source4, not this code which is the
>+		 * source3 / NT4-like "classic" DC implementation
>+		 */
>+		num_ifaces = 0;
>+		break;
> 	default:
> 		break;
> 	}
>@@ -80,6 +88,14 @@ static size_t lsad_servers(
> 		/* no netlogon for non-dc */
> 		num_servers -= 1;
> 		break;
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * All these services are provided by the 'samba'
>+		 * binary from source4, not this code which is the
>+		 * source3 / NT4-like "classic" DC implementation
>+		 */
>+		num_servers = 0;
>+		break;
> 	default:
> 		break;
> 	}
>diff --git a/source3/rpc_server/rpcd_rpcecho.c b/source3/rpc_server/rpcd_rpcecho.c
>index 9176039819f..37391f563db 100644
>--- a/source3/rpc_server/rpcd_rpcecho.c
>+++ b/source3/rpc_server/rpcd_rpcecho.c
>@@ -19,6 +19,8 @@
> #include "rpc_worker.h"
> #include "librpc/gen_ndr/ndr_echo.h"
> #include "librpc/gen_ndr/ndr_echo_scompat.h"
>+#include "param/loadparm.h"
>+#include "libds/common/roles.h"
> 
> static size_t rpcecho_interfaces(
> 	const struct ndr_interface_table ***pifaces,
>@@ -27,8 +29,22 @@ static size_t rpcecho_interfaces(
> 	static const struct ndr_interface_table *ifaces[] = {
> 		&ndr_table_rpcecho,
> 	};
>+	size_t num_ifaces = ARRAY_SIZE(ifaces);
>+
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC rpcecho is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_ifaces = 0;
>+		break;
>+	default:
>+		break;
>+	}
>+
> 	*pifaces = ifaces;
>-	return ARRAY_SIZE(ifaces);
>+	return num_ifaces;
> }
> 
> static size_t rpcecho_servers(
>@@ -37,11 +53,24 @@ static size_t rpcecho_servers(
> 	void *private_data)
> {
> 	static const struct dcesrv_endpoint_server *ep_servers[1] = { NULL };
>+	size_t num_servers = ARRAY_SIZE(ep_servers);
> 
> 	ep_servers[0] = rpcecho_get_ep_server();
> 
>+	switch(lp_server_role()) {
>+	case ROLE_ACTIVE_DIRECTORY_DC:
>+		/*
>+		 * On the AD DC rpcecho is provided by the 'samba'
>+		 * binary from source4/
>+		 */
>+		num_servers = 0;
>+		break;
>+	default:
>+		break;
>+	}
>+
> 	*_ep_servers = ep_servers;
>-	return ARRAY_SIZE(ep_servers);
>+	return num_servers;
> }
> 
> int main(int argc, const char *argv[])
>-- 
>2.25.1
>
>
>From 7665ab202a1aeb622473db2e8e9d8c953f16a230 Mon Sep 17 00:00:00 2001
>From: Andrew Bartlett <abartlet@samba.org>
>Date: Tue, 12 Sep 2023 16:23:49 +1200
>Subject: [PATCH 2/2] s3-rpc_server: Remove cross-check with "samba" EPM lookup
>
>We now have ensured that no conflicting services attempt to start
>so we do not need the runtime lookup and so avoid the risk that
>the lookup may fail.
>
>This means that any duplicates will be noticed early not just
>in a race condition.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15473
>
>Signed-off-by: Andrew Bartlett <abartlet@samba.org>
>---
> source3/rpc_server/rpc_host.c | 154 +---------------------------------
> 1 file changed, 4 insertions(+), 150 deletions(-)
>
>diff --git a/source3/rpc_server/rpc_host.c b/source3/rpc_server/rpc_host.c
>index c9ee9a3aca2..8cdd21e8540 100644
>--- a/source3/rpc_server/rpc_host.c
>+++ b/source3/rpc_server/rpc_host.c
>@@ -214,7 +214,6 @@ struct rpc_server_get_endpoints_state {
> 	char **argl;
> 	char *ncalrpc_endpoint;
> 	enum dcerpc_transport_t only_transport;
>-	struct dcerpc_binding **existing_bindings;
> 
> 	struct rpc_host_iface_name *iface_names;
> 	struct rpc_host_endpoint **endpoints;
>@@ -235,7 +234,6 @@ static void rpc_server_get_endpoints_done(struct tevent_req *subreq);
>  * @param[in] ev Event context to run this on
>  * @param[in] rpc_server_exe Binary to ask with --list-interfaces
>  * @param[in] only_transport Filter out anything but this
>- * @param[in] existing_bindings Filter out endpoints served by "samba"
>  * @return The tevent_req representing this process
>  */
> 
>@@ -243,8 +241,7 @@ static struct tevent_req *rpc_server_get_endpoints_send(
> 	TALLOC_CTX *mem_ctx,
> 	struct tevent_context *ev,
> 	const char *rpc_server_exe,
>-	enum dcerpc_transport_t only_transport,
>-	struct dcerpc_binding **existing_bindings)
>+	enum dcerpc_transport_t only_transport)
> {
> 	struct tevent_req *req = NULL, *subreq = NULL;
> 	struct rpc_server_get_endpoints_state *state = NULL;
>@@ -256,7 +253,6 @@ static struct tevent_req *rpc_server_get_endpoints_send(
> 		return NULL;
> 	}
> 	state->only_transport = only_transport;
>-	state->existing_bindings = existing_bindings;
> 
> 	progname = strrchr(rpc_server_exe, '/');
> 	if (progname != NULL) {
>@@ -417,37 +413,17 @@ static bool dcerpc_binding_same_endpoint(
>  * In member mode, we only serve named pipes. Indicated by NCACN_NP
>  * passed in via "only_transport".
>  *
>- * In AD mode, the "samba" process already serves many endpoints,
>- * passed in via "existing_binding". Don't serve those from
>- * samba-dcerpcd.
>- *
>  * @param[in] binding Which binding is in question?
>  * @param[in] only_transport Exclusive transport to serve
>- * @param[in] existing_bindings Endpoints served by "samba" already
>  * @return Do we want to serve "binding" from samba-dcerpcd?
>  */
> 
> static bool rpc_host_serve_endpoint(
> 	struct dcerpc_binding *binding,
>-	enum dcerpc_transport_t only_transport,
>-	struct dcerpc_binding **existing_bindings)
>+	enum dcerpc_transport_t only_transport)
> {
> 	enum dcerpc_transport_t transport =
> 		dcerpc_binding_get_transport(binding);
>-	size_t i, num_existing_bindings;
>-
>-	num_existing_bindings = talloc_array_length(existing_bindings);
>-
>-	for (i=0; i<num_existing_bindings; i++) {
>-		bool same = dcerpc_binding_same_endpoint(
>-			binding, existing_bindings[i]);
>-		if (same) {
>-			DBG_DEBUG("%s served by samba\n",
>-				  dcerpc_binding_get_string_option(
>-					  binding, "endpoint"));
>-			return false;
>-		}
>-	}
> 
> 	if (only_transport == NCA_UNKNOWN) {
> 		/* no filter around */
>@@ -486,7 +462,7 @@ static struct rpc_host_endpoint *rpc_host_endpoint_find(
> 	}
> 
> 	serve_this = rpc_host_serve_endpoint(
>-		ep->binding, state->only_transport, state->existing_bindings);
>+		ep->binding, state->only_transport);
> 	if (!serve_this) {
> 		goto fail;
> 	}
>@@ -1607,7 +1583,6 @@ static struct tevent_req *rpc_server_setup_send(
> 	TALLOC_CTX *mem_ctx,
> 	struct tevent_context *ev,
> 	struct rpc_host *host,
>-	struct dcerpc_binding **existing_bindings,
> 	const char *rpc_server_exe)
> {
> 	struct tevent_req *req = NULL, *subreq = NULL;
>@@ -1639,8 +1614,7 @@ static struct tevent_req *rpc_server_setup_send(
> 		state,
> 		ev,
> 		rpc_server_exe,
>-		host->np_helper ? NCACN_NP : NCA_UNKNOWN,
>-		existing_bindings);
>+		host->np_helper ? NCACN_NP : NCA_UNKNOWN);
> 	if (tevent_req_nomem(subreq, req)) {
> 		return tevent_req_post(req, ev);
> 	}
>@@ -2344,7 +2318,6 @@ static struct tevent_req *rpc_host_send(
> 	TALLOC_CTX *mem_ctx,
> 	struct tevent_context *ev,
> 	struct messaging_context *msg_ctx,
>-	struct dcerpc_binding **existing_bindings,
> 	char *servers,
> 	int ready_signal_fd,
> 	const char *daemon_ready_progname,
>@@ -2465,7 +2438,6 @@ static struct tevent_req *rpc_host_send(
> 			state,
> 			ev,
> 			host,
>-			existing_bindings,
> 			exe);
> 		if (tevent_req_nomem(subreq, req)) {
> 			return tevent_req_post(req, ev);
>@@ -2648,117 +2620,6 @@ static int rpc_host_pidfile_create(
> 	return EAGAIN;
> }
> 
>-/*
>- * Find which interfaces are already being served by the samba AD
>- * DC so we know not to serve them. Some interfaces like netlogon
>- * are served by "samba", some like srvsvc will be served by the
>- * source3 based RPC servers.
>- */
>-static NTSTATUS rpc_host_epm_lookup(
>-	TALLOC_CTX *mem_ctx,
>-	struct dcerpc_binding ***pbindings)
>-{
>-	struct rpc_pipe_client *cli = NULL;
>-	struct pipe_auth_data *auth = NULL;
>-	struct policy_handle entry_handle = { .handle_type = 0 };
>-	struct dcerpc_binding **bindings = NULL;
>-	NTSTATUS status = NT_STATUS_UNSUCCESSFUL;
>-
>-	status = rpc_pipe_open_ncalrpc(mem_ctx, &ndr_table_epmapper, &cli);
>-	if (!NT_STATUS_IS_OK(status)) {
>-		DBG_DEBUG("rpc_pipe_open_ncalrpc failed: %s\n",
>-			  nt_errstr(status));
>-		goto fail;
>-	}
>-	status = rpccli_ncalrpc_bind_data(cli, &auth);
>-	if (!NT_STATUS_IS_OK(status)) {
>-		DBG_DEBUG("rpccli_ncalrpc_bind_data failed: %s\n",
>-			  nt_errstr(status));
>-		goto fail;
>-	}
>-	status = rpc_pipe_bind(cli, auth);
>-	if (!NT_STATUS_IS_OK(status)) {
>-		DBG_DEBUG("rpc_pipe_bind failed: %s\n", nt_errstr(status));
>-		goto fail;
>-	}
>-
>-	for (;;) {
>-		size_t num_bindings = talloc_array_length(bindings);
>-		struct dcerpc_binding **tmp = NULL;
>-		uint32_t num_entries = 0;
>-		struct epm_entry_t *entry = NULL;
>-		struct dcerpc_binding *binding = NULL;
>-		uint32_t result;
>-
>-		entry = talloc(cli, struct epm_entry_t);
>-		if (entry == NULL) {
>-			goto fail;
>-		}
>-
>-		status = dcerpc_epm_Lookup(
>-			cli->binding_handle, /* binding_handle */
>-			cli,		     /* mem_ctx */
>-			0,		     /* rpc_c_ep_all */
>-			NULL,		     /* object */
>-			NULL,		     /* interface id */
>-			0,		     /* rpc_c_vers_all */
>-			&entry_handle,	     /* entry_handle */
>-			1,		     /* max_ents */
>-			&num_entries,	     /* num_ents */
>-			entry,		     /* entries */
>-			&result);	     /* result */
>-		if (!NT_STATUS_IS_OK(status)) {
>-			DBG_DEBUG("dcerpc_epm_Lookup failed: %s\n",
>-				  nt_errstr(status));
>-			goto fail;
>-		}
>-
>-		if (result == EPMAPPER_STATUS_NO_MORE_ENTRIES) {
>-			break;
>-		}
>-
>-		if (result != EPMAPPER_STATUS_OK) {
>-			DBG_DEBUG("dcerpc_epm_Lookup returned %"PRIu32"\n",
>-				  result);
>-			break;
>-		}
>-
>-		if (num_entries != 1) {
>-			DBG_DEBUG("epm_Lookup returned %"PRIu32" "
>-				  "entries, expected one\n",
>-				  num_entries);
>-			break;
>-		}
>-
>-		status = dcerpc_binding_from_tower(
>-			mem_ctx, &entry->tower->tower, &binding);
>-		if (!NT_STATUS_IS_OK(status)) {
>-			break;
>-		}
>-
>-		tmp = talloc_realloc(
>-			mem_ctx,
>-			bindings,
>-			struct dcerpc_binding *,
>-			num_bindings+1);
>-		if (tmp == NULL) {
>-			status = NT_STATUS_NO_MEMORY;
>-			goto fail;
>-		}
>-		bindings = tmp;
>-
>-		bindings[num_bindings] = talloc_move(bindings, &binding);
>-
>-		TALLOC_FREE(entry);
>-	}
>-
>-	*pbindings = bindings;
>-	status = NT_STATUS_OK;
>-fail:
>-	TALLOC_FREE(cli);
>-	return status;
>-}
>-
> static void samba_dcerpcd_stdin_handler(
> 	struct tevent_context *ev,
> 	struct tevent_fd *fde,
>@@ -2788,7 +2649,6 @@ int main(int argc, const char *argv[])
> 	struct tevent_context *ev_ctx = NULL;
> 	struct messaging_context *msg_ctx = NULL;
> 	struct tevent_req *req = NULL;
>-	struct dcerpc_binding **existing_bindings = NULL;
> 	char *servers = NULL;
> 	const char *arg = NULL;
> 	size_t num_servers;
>@@ -2995,11 +2855,6 @@ int main(int argc, const char *argv[])
> 		exit(1);
> 	}
> 
>-	status = rpc_host_epm_lookup(frame, &existing_bindings);
>-	DBG_DEBUG("rpc_host_epm_lookup returned %s, %zu bindings\n",
>-		  nt_errstr(status),
>-		  talloc_array_length(existing_bindings));
>-
> 	ret = rpc_host_pidfile_create(msg_ctx, progname, ready_signal_fd);
> 	if (ret != 0) {
> 		DBG_DEBUG("rpc_host_pidfile_create failed: %s\n",
>@@ -3013,7 +2868,6 @@ int main(int argc, const char *argv[])
> 		ev_ctx,
> 		ev_ctx,
> 		msg_ctx,
>-		existing_bindings,
> 		servers,
> 		ready_signal_fd,
> 		cmdline_daemon_cfg->fork ? NULL : progname,
>-- 
>2.25.1
>

Actions: View

Attachments on bug 15473: 18096 | 18097 | 18118 | 18119 | 18133 | 18134 | 18139 | 18140 | 18141 | 18142 | 18143 | 18145 | 18147