Attachment 17548 Details for Bug 15159 – Patches for v4-16-test

[patch] Patches for v4-16-test

bfixes-tmp416.txt (text/plain), 14.27 KB, created by Stefan Metzmacher on 2022-10-10 07:09:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-10-10 07:09:09 UTC

Size: 14.27 KB

patch

obsolete

>From a2a0dca168af5c6b9730833ddc10eaa186434cfb Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 30 Aug 2022 16:56:12 +0200
>Subject: [PATCH 1/2] smbXsrv_client: correctly check in
> negotiate_request.length smbXsrv_client_connection_pass[ed]_*
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15159
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 21ef01e7b8368caa050ed82b9d787d1679220b2b)
>---
> source3/smbd/smbXsrv_client.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
>diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c
>index 277ae1bab251..5c3f1597b16b 100644
>--- a/source3/smbd/smbXsrv_client.c
>+++ b/source3/smbd/smbXsrv_client.c
>@@ -572,10 +572,6 @@ static bool smb2srv_client_mc_negprot_filter(struct messaging_rec *rec, void *pr
> 		return false;
> 	}
> 
>-	if (rec->buf.length < SMB2_HDR_BODY) {
>-		return false;
>-	}
>-
> 	return true;
> }
> 
>@@ -665,6 +661,14 @@ static void smb2srv_client_mc_negprot_done(struct tevent_req *subreq)
> 		return;
> 	}
> 
>+	if (passed_info0->negotiate_request.length != 0) {
>+		DBG_ERR("negotiate_request.length[%zu]\n",
>+			passed_info0->negotiate_request.length);
>+		NDR_PRINT_DEBUG(smbXsrv_connection_passB, &passed_blob);
>+		tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
>+		return;
>+	}
>+
> 	tevent_req_nterror(req, NT_STATUS_MESSAGE_RETRIEVED);
> }
> 
>@@ -883,12 +887,6 @@ static bool smbXsrv_client_connection_pass_filter(struct messaging_rec *rec, voi
> 		return false;
> 	}
> 
>-	if (rec->buf.length < SMB2_HDR_BODY) {
>-		return false;
>-	}
>-
>-	/* TODO: verify client_guid...? */
>-
> 	return true;
> }
> 
>@@ -981,6 +979,15 @@ static void smbXsrv_client_connection_pass_loop(struct tevent_req *subreq)
> 		goto next;
> 	}
> 
>+	if (pass_info0->negotiate_request.length < SMB2_HDR_BODY) {
>+		DBG_WARNING("negotiate_request.length[%zu]\n",
>+			    pass_info0->negotiate_request.length);
>+		if (DEBUGLVL(DBGLVL_WARNING)) {
>+			NDR_PRINT_DEBUG(smbXsrv_connection_passB, &pass_blob);
>+		}
>+		goto next;
>+	}
>+
> 	status = smb2srv_client_connection_passed(client, pass_info0);
> 	if (!NT_STATUS_IS_OK(status)) {
> 		const char *r = "smb2srv_client_connection_passed() failed";
>-- 
>2.34.1
>
>
>From abb7566e5612139d7870a3a42040aaa16d421939 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 30 Aug 2022 20:45:50 +0200
>Subject: [PATCH 2/2] smbXsrv_client: notify a different node to drop a
> connection by client guid.
>
>If a client disconnected all its interfaces and reconnects when
>the come back, it will likely start from any ip address returned
>dns, which means it can try to connect to a different ctdb node.
>The old node may not have noticed the disconnect and still holds
>the client_guid based smbd.
>
>Up unil now the new node returned NT_STATUS_NOT_SUPPORTED to
>the SMB2 Negotiate request, as messaging_send_iov[_from]() will
>return -1/ENOSYS if a file descriptor os passed to a process on
>a different node.
>
>Now we tell the other node to teardown all client connections
>belonging to the client-guid.
>
>Note that this is not authenticated, but if an attacker can
>capture the client-guid, he can also inject TCP resets anyway,
>to get the same effect.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15159
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Fri Sep  2 20:59:15 UTC 2022 on sn-devel-184
>
>(cherry picked from commit 8591d9424371e173b079d5c8a267ea4c2cb266ad)
>---
> librpc/idl/messaging.idl       |   1 +
> source3/librpc/idl/smbXsrv.idl |  28 ++++
> source3/smbd/smbXsrv_client.c  | 239 +++++++++++++++++++++++++++++++--
> 3 files changed, 255 insertions(+), 13 deletions(-)
>
>diff --git a/librpc/idl/messaging.idl b/librpc/idl/messaging.idl
>index d6929c799adf..5d217c03f5be 100644
>--- a/librpc/idl/messaging.idl
>+++ b/librpc/idl/messaging.idl
>@@ -138,6 +138,7 @@ interface messaging
> 		MSG_SMBXSRV_SESSION_CLOSE	= 0x0600,
> 		MSG_SMBXSRV_CONNECTION_PASS	= 0x0601,
> 		MSG_SMBXSRV_CONNECTION_PASSED	= 0x0602,
>+		MSG_SMBXSRV_CONNECTION_DROP	= 0x0603,
> 
> 		/* source4 and NTVFS smb server messages */
> 		MSG_BRL_RETRY                   = 0x0700,
>diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
>index fc502009b3bd..ec65a5c1a613 100644
>--- a/source3/librpc/idl/smbXsrv.idl
>+++ b/source3/librpc/idl/smbXsrv.idl
>@@ -143,6 +143,7 @@ interface smbXsrv
> 		boolean8		server_multi_channel_enabled;
> 		hyper			next_channel_id;
> 		[ignore] struct tevent_req		*connection_pass_subreq;
>+		[ignore] struct tevent_req		*connection_drop_subreq;
> 
> 		/*
> 		 * A List of pending breaks.
>@@ -194,6 +195,33 @@ interface smbXsrv
> 		[in] smbXsrv_connection_passB blob
> 		);
> 
>+	/*
>+	 * smbXsrv_connection_drop is used in the MSG_SMBXSRV_CONNECTION_DROP
>+	 * message as reaction the record is deleted.
>+	 */
>+	typedef struct {
>+		GUID					client_guid;
>+		server_id				src_server_id;
>+		NTTIME					xconn_connect_time;
>+		server_id				dst_server_id;
>+		NTTIME					client_connect_time;
>+	} smbXsrv_connection_drop0;
>+
>+	typedef union {
>+		[case(0)] smbXsrv_connection_drop0	*info0;
>+		[default] hyper				*dummy;
>+	} smbXsrv_connection_dropU;
>+
>+	typedef [public] struct {
>+		smbXsrv_version_values			version;
>+		[value(0)] uint32			reserved;
>+		[switch_is(version)] smbXsrv_connection_dropU	info;
>+	} smbXsrv_connection_dropB;
>+
>+	void smbXsrv_connection_drop_decode(
>+		[in] smbXsrv_connection_dropB blob
>+		);
>+
> 	/* sessions */
> 
> 	typedef [public,bitmap8bit] bitmap {
>diff --git a/source3/smbd/smbXsrv_client.c b/source3/smbd/smbXsrv_client.c
>index 5c3f1597b16b..1ee4410c1cf5 100644
>--- a/source3/smbd/smbXsrv_client.c
>+++ b/source3/smbd/smbXsrv_client.c
>@@ -339,6 +339,55 @@ static NTSTATUS smb2srv_client_connection_pass(struct smbd_smb2_request *smb2req
> 	return NT_STATUS_OK;
> }
> 
>+static NTSTATUS smb2srv_client_connection_drop(struct smbd_smb2_request *smb2req,
>+					       struct smbXsrv_client_global0 *global)
>+{
>+	DATA_BLOB blob;
>+	enum ndr_err_code ndr_err;
>+	NTSTATUS status;
>+	struct smbXsrv_connection_drop0 drop_info0;
>+	struct smbXsrv_connection_dropB drop_blob;
>+	struct iovec iov;
>+
>+	drop_info0 = (struct smbXsrv_connection_drop0) {
>+		.client_guid = global->client_guid,
>+		.src_server_id = smb2req->xconn->client->global->server_id,
>+		.xconn_connect_time = smb2req->xconn->client->global->initial_connect_time,
>+		.dst_server_id = global->server_id,
>+		.client_connect_time = global->initial_connect_time,
>+	};
>+
>+	ZERO_STRUCT(drop_blob);
>+	drop_blob.version = smbXsrv_version_global_current();
>+	drop_blob.info.info0 = &drop_info0;
>+
>+	if (DEBUGLVL(DBGLVL_DEBUG)) {
>+		NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+	}
>+
>+	ndr_err = ndr_push_struct_blob(&blob, talloc_tos(), &drop_blob,
>+			(ndr_push_flags_fn_t)ndr_push_smbXsrv_connection_dropB);
>+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>+		status = ndr_map_error2ntstatus(ndr_err);
>+		return status;
>+	}
>+
>+	iov.iov_base = blob.data;
>+	iov.iov_len = blob.length;
>+
>+	status = messaging_send_iov(smb2req->xconn->client->msg_ctx,
>+				    global->server_id,
>+				    MSG_SMBXSRV_CONNECTION_DROP,
>+				    &iov, 1,
>+				    NULL, 0);
>+	data_blob_free(&blob);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		return status;
>+	}
>+
>+	return NT_STATUS_OK;
>+}
>+
> static NTSTATUS smbXsrv_client_global_store(struct smbXsrv_client_global0 *global)
> {
> 	struct smbXsrv_client_globalB global_blob;
>@@ -532,15 +581,17 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req)
> 		return;
> 	}
> 
>-	subreq = messaging_filtered_read_send(state,
>-					      state->ev,
>-					      client->msg_ctx,
>-					      smb2srv_client_mc_negprot_filter,
>-					      NULL);
>-	if (tevent_req_nomem(subreq, req)) {
>-		return;
>+	if (procid_is_local(&global->server_id)) {
>+		subreq = messaging_filtered_read_send(state,
>+						      state->ev,
>+						      client->msg_ctx,
>+						      smb2srv_client_mc_negprot_filter,
>+						      NULL);
>+		if (tevent_req_nomem(subreq, req)) {
>+			return;
>+		}
>+		tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req);
> 	}
>-	tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_done, req);
> 
> 	subreq = dbwrap_watched_watch_send(state,
> 					   state->ev,
>@@ -551,11 +602,20 @@ static void smb2srv_client_mc_negprot_next(struct tevent_req *req)
> 	}
> 	tevent_req_set_callback(subreq, smb2srv_client_mc_negprot_watched, req);
> 
>-	status = smb2srv_client_connection_pass(state->smb2req,
>-						global);
>-	TALLOC_FREE(global);
>-	if (tevent_req_nterror(req, status)) {
>-		return;
>+	if (procid_is_local(&global->server_id)) {
>+		status = smb2srv_client_connection_pass(state->smb2req,
>+							global);
>+		TALLOC_FREE(global);
>+		if (tevent_req_nterror(req, status)) {
>+			return;
>+		}
>+	} else {
>+		status = smb2srv_client_connection_drop(state->smb2req,
>+							global);
>+		TALLOC_FREE(global);
>+		if (tevent_req_nterror(req, status)) {
>+			return;
>+		}
> 	}
> 
> 	TALLOC_FREE(state->db_rec);
>@@ -744,6 +804,8 @@ static int smbXsrv_client_destructor(struct smbXsrv_client *client)
> 
> static bool smbXsrv_client_connection_pass_filter(struct messaging_rec *rec, void *private_data);
> static void smbXsrv_client_connection_pass_loop(struct tevent_req *subreq);
>+static bool smbXsrv_client_connection_drop_filter(struct messaging_rec *rec, void *private_data);
>+static void smbXsrv_client_connection_drop_loop(struct tevent_req *subreq);
> 
> NTSTATUS smbXsrv_client_create(TALLOC_CTX *mem_ctx,
> 			       struct tevent_context *ev_ctx,
>@@ -826,6 +888,18 @@ NTSTATUS smbXsrv_client_create(TALLOC_CTX *mem_ctx,
> 	tevent_req_set_callback(subreq, smbXsrv_client_connection_pass_loop, client);
> 	client->connection_pass_subreq = subreq;
> 
>+	subreq = messaging_filtered_read_send(client,
>+					client->raw_ev_ctx,
>+					client->msg_ctx,
>+					smbXsrv_client_connection_drop_filter,
>+					client);
>+	if (subreq == NULL) {
>+		TALLOC_FREE(client);
>+		return NT_STATUS_NO_MEMORY;
>+	}
>+	tevent_req_set_callback(subreq, smbXsrv_client_connection_drop_loop, client);
>+	client->connection_drop_subreq = subreq;
>+
> 	*_client = client;
> 	return NT_STATUS_OK;
> }
>@@ -1051,6 +1125,144 @@ next:
> 	client->connection_pass_subreq = subreq;
> }
> 
>+static bool smbXsrv_client_connection_drop_filter(struct messaging_rec *rec, void *private_data)
>+{
>+	if (rec->msg_type != MSG_SMBXSRV_CONNECTION_DROP) {
>+		return false;
>+	}
>+
>+	if (rec->num_fds != 0) {
>+		return false;
>+	}
>+
>+	return true;
>+}
>+
>+static void smbXsrv_client_connection_drop_loop(struct tevent_req *subreq)
>+{
>+	struct smbXsrv_client *client =
>+		tevent_req_callback_data(subreq,
>+		struct smbXsrv_client);
>+	int ret;
>+	struct messaging_rec *rec = NULL;
>+	struct smbXsrv_connection_dropB drop_blob;
>+	enum ndr_err_code ndr_err;
>+	struct smbXsrv_connection_drop0 *drop_info0 = NULL;
>+	struct server_id_buf src_server_id_buf = {};
>+	NTSTATUS status;
>+
>+	client->connection_drop_subreq = NULL;
>+
>+	ret = messaging_filtered_read_recv(subreq, talloc_tos(), &rec);
>+	TALLOC_FREE(subreq);
>+	if (ret != 0) {
>+		goto next;
>+	}
>+
>+	if (rec->num_fds != 0) {
>+		DBG_ERR("MSG_SMBXSRV_CONNECTION_DROP: num_fds[%u]\n",
>+			rec->num_fds);
>+		goto next;
>+	}
>+
>+	ndr_err = ndr_pull_struct_blob(&rec->buf, rec, &drop_blob,
>+			(ndr_pull_flags_fn_t)ndr_pull_smbXsrv_connection_dropB);
>+	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
>+		status = ndr_map_error2ntstatus(ndr_err);
>+		DBG_WARNING("ndr_pull_struct_blob - %s\n", nt_errstr(status));
>+		goto next;
>+	}
>+
>+	if (DEBUGLVL(DBGLVL_DEBUG)) {
>+		NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+	}
>+
>+	if (drop_blob.version != SMBXSRV_VERSION_0) {
>+		DBG_ERR("ignore invalid version %u\n", drop_blob.version);
>+		NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+		goto next;
>+	}
>+
>+	drop_info0 = drop_blob.info.info0;
>+	if (drop_info0 == NULL) {
>+		DBG_ERR("ignore NULL info %u\n", drop_blob.version);
>+		NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+		goto next;
>+	}
>+
>+	if (!GUID_equal(&client->global->client_guid, &drop_info0->client_guid))
>+	{
>+		struct GUID_txt_buf buf1, buf2;
>+
>+		DBG_WARNING("client's client_guid [%s] != droped guid [%s]\n",
>+			    GUID_buf_string(&client->global->client_guid,
>+					    &buf1),
>+			    GUID_buf_string(&drop_info0->client_guid,
>+					    &buf2));
>+		if (DEBUGLVL(DBGLVL_WARNING)) {
>+			NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+		}
>+		goto next;
>+	}
>+
>+	if (client->global->initial_connect_time !=
>+	    drop_info0->client_connect_time)
>+	{
>+		DBG_WARNING("client's initial connect time [%s] (%llu) != "
>+			"droped initial connect time [%s] (%llu)\n",
>+			nt_time_string(talloc_tos(),
>+				       client->global->initial_connect_time),
>+			(unsigned long long)client->global->initial_connect_time,
>+			nt_time_string(talloc_tos(),
>+				       drop_info0->client_connect_time),
>+			(unsigned long long)drop_info0->client_connect_time);
>+		if (DEBUGLVL(DBGLVL_WARNING)) {
>+			NDR_PRINT_DEBUG(smbXsrv_connection_dropB, &drop_blob);
>+		}
>+		goto next;
>+	}
>+
>+	/*
>+	 * Disconnect all client connections, which means we will tear down all
>+	 * sessions, tcons and non-durable opens. At the end we will remove our
>+	 * smbXsrv_client_global.tdb record, which will wake up the watcher on
>+	 * the other node in order to let it take over the client.
>+	 *
>+	 * The client will have to reopen all sessions, tcons and durable opens.
>+	 */
>+	smbd_server_disconnect_client(client,
>+		server_id_str_buf(drop_info0->src_server_id, &src_server_id_buf));
>+	return;
>+
>+next:
>+	if (rec != NULL) {
>+		int sock_fd;
>+		uint8_t fd_idx;
>+
>+		for (fd_idx = 0; fd_idx < rec->num_fds; fd_idx++) {
>+			sock_fd = rec->fds[fd_idx];
>+			close(sock_fd);
>+		}
>+		rec->num_fds = 0;
>+
>+		TALLOC_FREE(rec);
>+	}
>+
>+	subreq = messaging_filtered_read_send(client,
>+					client->raw_ev_ctx,
>+					client->msg_ctx,
>+					smbXsrv_client_connection_drop_filter,
>+					client);
>+	if (subreq == NULL) {
>+		const char *r;
>+		r = "messaging_read_send(MSG_SMBXSRV_CONNECTION_DROP failed";
>+		exit_server_cleanly(r);
>+		return;
>+	}
>+	tevent_req_set_callback(subreq, smbXsrv_client_connection_drop_loop, client);
>+	client->connection_drop_subreq = subreq;
>+}
>+
> NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client)
> {
> 	struct smbXsrv_client_table *table = client->table;
>@@ -1069,6 +1281,7 @@ NTSTATUS smbXsrv_client_remove(struct smbXsrv_client *client)
> 	}
> 
> 	TALLOC_FREE(client->connection_pass_subreq);
>+	TALLOC_FREE(client->connection_drop_subreq);
> 
> 	client->global->db_rec = smbXsrv_client_global_fetch_locked(
> 					table->global.db_ctx,
>-- 
>2.34.1
>

Flags:

slow: review+

Actions: View

Attachments on bug 15159: 17509 | 17510 | 17511 | 17548