Attachment 17055 Details for Bug 14788 – Patches for v4-14-test

[patch] Patches for v4-14-test

tmp414.diff.txt (text/plain), 23.99 KB, created by Stefan Metzmacher on 2021-12-08 16:21:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2021-12-08 16:21:50 UTC

Size: 23.99 KB

patch

obsolete

>From d834c875b2638daf8926f09656643063f1eb824b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 13 Jul 2021 16:37:42 +0200
>Subject: [PATCH 1/8] s3:smbd: remove dead code from
> smbd_smb2_request_dispatch()
>
>We have '} else if (signing_required || (flags & SMB2_HDR_FLAG_SIGNED)) {'
>before...
>
>Use 'git show -U52' to see the whole story...
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit f8f4a9faf099eb768eaa25f1e1a7d126b75291d0)
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>---
> source3/smbd/smb2_server.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index 1322cf3ba083..f071c1a5ff98 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -2717,9 +2717,6 @@ static void smb2srv_update_crypto_flags(struct smbd_smb2_request *req,
> 		/* Unencrypted packet, can be signed */
> 		if (req->do_signing) {
> 			sign_flag = SMBXSRV_PROCESSED_SIGNED_PACKET;
>-		} else if (opcode == SMB2_OP_CANCEL) {
>-			/* Cancel requests are allowed to skip signing */
>-			sign_flag &= ~SMBXSRV_PROCESSED_UNSIGNED_PACKET;
> 		}
> 	}
> 
>@@ -3080,8 +3077,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 		if (!NT_STATUS_IS_OK(session_status)) {
> 			return smbd_smb2_request_error(req, session_status);
> 		}
>-	} else if (opcode == SMB2_OP_CANCEL) {
>-		/* Cancel requests are allowed to skip the signing */
> 	} else if (opcode == SMB2_OP_IOCTL) {
> 		/*
> 		 * Some special IOCTL calls don't require
>@@ -3118,13 +3113,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 			call = &_root_ioctl_call;
> 			break;
> 		}
>-	} else if (signing_required) {
>-		/*
>-		 * If signing is required we try to sign
>-		 * a possible error response
>-		 */
>-		req->do_signing = true;
>-		return smbd_smb2_request_error(req, NT_STATUS_ACCESS_DENIED);
> 	}
> 
> 	if (flags & SMB2_HDR_FLAG_CHAINED) {
>-- 
>2.25.1
>
>
>From 439db797d562d022aac7d39cfadb31d453437c2f Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 16 Sep 2021 10:51:43 +0200
>Subject: [PATCH 2/8] libcli/smb: split out smb2cli_raw_tcon* from
> smb2cli_tcon*
>
>This will be used in tests in order to separate the tcon from
>validate_negotiate_info.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 04a79139a42cfd1b607317dec041618cbf629584)
>---
> libcli/smb/smb2cli_tcon.c | 183 ++++++++++++++++++++++++++++++--------
> libcli/smb/smbXcli_base.h |  20 +++++
> 2 files changed, 168 insertions(+), 35 deletions(-)
>
>diff --git a/libcli/smb/smb2cli_tcon.c b/libcli/smb/smb2cli_tcon.c
>index 8863bae0764c..7bbae8ea3b3b 100644
>--- a/libcli/smb/smb2cli_tcon.c
>+++ b/libcli/smb/smb2cli_tcon.c
>@@ -23,42 +23,38 @@
> #include "../libcli/smb/smb_common.h"
> #include "../libcli/smb/smbXcli_base.h"
> 
>-struct smb2cli_tcon_state {
>-	struct tevent_context *ev;
>-	struct smbXcli_conn *conn;
>-	uint32_t timeout_msec;
>+struct smb2cli_raw_tcon_state {
> 	struct smbXcli_session *session;
> 	struct smbXcli_tcon *tcon;
> 	uint8_t fixed[8];
> 	uint8_t dyn_pad[1];
> };
> 
>-static void smb2cli_tcon_done(struct tevent_req *subreq);
>-
>-struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
>-				     struct tevent_context *ev,
>-				     struct smbXcli_conn *conn,
>-				     uint32_t timeout_msec,
>-				     struct smbXcli_session *session,
>-				     struct smbXcli_tcon *tcon,
>-				     uint16_t flags,
>-				     const char *unc)
>+static void smb2cli_raw_tcon_done(struct tevent_req *subreq);
>+
>+struct tevent_req *smb2cli_raw_tcon_send(TALLOC_CTX *mem_ctx,
>+					 struct tevent_context *ev,
>+					 struct smbXcli_conn *conn,
>+					 uint32_t additional_flags,
>+					 uint32_t clear_flags,
>+					 uint32_t timeout_msec,
>+					 struct smbXcli_session *session,
>+					 struct smbXcli_tcon *tcon,
>+					 uint16_t tcon_flags,
>+					 const char *unc)
> {
>-	struct tevent_req *req, *subreq;
>-	struct smb2cli_tcon_state *state;
>-	uint8_t *fixed;
>-	uint8_t *dyn;
>+	struct tevent_req *req = NULL;
>+	struct smb2cli_raw_tcon_state *state = NULL;
>+	struct tevent_req *subreq = NULL;
>+	uint8_t *fixed = NULL;
>+	uint8_t *dyn = NULL;
> 	size_t dyn_len;
>-	uint32_t additional_flags = 0;
>-	uint32_t clear_flags = 0;
> 
>-	req = tevent_req_create(mem_ctx, &state, struct smb2cli_tcon_state);
>+	req = tevent_req_create(mem_ctx, &state,
>+				struct smb2cli_raw_tcon_state);
> 	if (req == NULL) {
> 		return NULL;
> 	}
>-	state->ev = ev;
>-	state->conn = conn;
>-	state->timeout_msec = timeout_msec;
> 	state->session = session;
> 	state->tcon = tcon;
> 
>@@ -77,7 +73,7 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
> 	fixed = state->fixed;
> 	SSVAL(fixed, 0, 9);
> 	if (smbXcli_conn_protocol(conn) >= PROTOCOL_SMB3_10) {
>-		SSVAL(fixed, 2, flags);
>+		SSVAL(fixed, 2, tcon_flags);
> 	} else {
> 		SSVAL(fixed, 2, 0); /* Reserved */
> 	}
>@@ -89,10 +85,6 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
> 		dyn_len = sizeof(state->dyn_pad);
> 	}
> 
>-	if (smbXcli_session_is_authenticated(state->session)) {
>-		additional_flags |= SMB2_HDR_FLAG_SIGNED;
>-	}
>-
> 	subreq = smb2cli_req_send(state, ev, conn, SMB2_OP_TCON,
> 				  additional_flags, clear_flags,
> 				  timeout_msec,
>@@ -104,19 +96,17 @@ struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
> 	if (tevent_req_nomem(subreq, req)) {
> 		return tevent_req_post(req, ev);
> 	}
>-	tevent_req_set_callback(subreq, smb2cli_tcon_done, req);
>+	tevent_req_set_callback(subreq, smb2cli_raw_tcon_done, req);
> 
> 	return req;
> }
> 
>-static void smb2cli_tcon_validate(struct tevent_req *subreq);
>-
>-static void smb2cli_tcon_done(struct tevent_req *subreq)
>+static void smb2cli_raw_tcon_done(struct tevent_req *subreq)
> {
> 	struct tevent_req *req = tevent_req_callback_data(
> 		subreq, struct tevent_req);
>-	struct smb2cli_tcon_state *state = tevent_req_data(
>-		req, struct smb2cli_tcon_state);
>+	struct smb2cli_raw_tcon_state *state = tevent_req_data(
>+		req, struct smb2cli_raw_tcon_state);
> 	NTSTATUS status;
> 	struct iovec *iov;
> 	uint8_t *body;
>@@ -156,6 +146,129 @@ static void smb2cli_tcon_done(struct tevent_req *subreq)
> 				share_capabilities,
> 				maximal_access);
> 
>+	tevent_req_done(req);
>+}
>+
>+NTSTATUS smb2cli_raw_tcon_recv(struct tevent_req *req)
>+{
>+	return tevent_req_simple_recv_ntstatus(req);
>+}
>+
>+NTSTATUS smb2cli_raw_tcon(struct smbXcli_conn *conn,
>+			  uint32_t additional_flags,
>+			  uint32_t clear_flags,
>+			  uint32_t timeout_msec,
>+			  struct smbXcli_session *session,
>+			  struct smbXcli_tcon *tcon,
>+			  uint16_t tcon_flags,
>+			  const char *unc)
>+{
>+	TALLOC_CTX *frame = talloc_stackframe();
>+	struct tevent_context *ev;
>+	struct tevent_req *req;
>+	NTSTATUS status = NT_STATUS_NO_MEMORY;
>+
>+	if (smbXcli_conn_has_async_calls(conn)) {
>+		/*
>+		 * Can't use sync call while an async call is in flight
>+		 */
>+		status = NT_STATUS_INVALID_PARAMETER;
>+		goto fail;
>+	}
>+	ev = samba_tevent_context_init(frame);
>+	if (ev == NULL) {
>+		goto fail;
>+	}
>+	req = smb2cli_raw_tcon_send(frame, ev, conn,
>+				    additional_flags, clear_flags,
>+				    timeout_msec, session, tcon,
>+				    tcon_flags, unc);
>+	if (req == NULL) {
>+		goto fail;
>+	}
>+	if (!tevent_req_poll_ntstatus(req, ev, &status)) {
>+		goto fail;
>+	}
>+	status = smb2cli_raw_tcon_recv(req);
>+ fail:
>+	TALLOC_FREE(frame);
>+	return status;
>+}
>+
>+struct smb2cli_tcon_state {
>+	struct tevent_context *ev;
>+	struct smbXcli_conn *conn;
>+	uint32_t timeout_msec;
>+	struct smbXcli_session *session;
>+	struct smbXcli_tcon *tcon;
>+	uint8_t fixed[8];
>+	uint8_t dyn_pad[1];
>+};
>+
>+static void smb2cli_tcon_done(struct tevent_req *subreq);
>+
>+struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
>+				     struct tevent_context *ev,
>+				     struct smbXcli_conn *conn,
>+				     uint32_t timeout_msec,
>+				     struct smbXcli_session *session,
>+				     struct smbXcli_tcon *tcon,
>+				     uint16_t flags,
>+				     const char *unc)
>+{
>+	struct tevent_req *req, *subreq;
>+	struct smb2cli_tcon_state *state;
>+	uint32_t additional_flags = 0;
>+	uint32_t clear_flags = 0;
>+
>+	req = tevent_req_create(mem_ctx, &state, struct smb2cli_tcon_state);
>+	if (req == NULL) {
>+		return NULL;
>+	}
>+	state->ev = ev;
>+	state->conn = conn;
>+	state->timeout_msec = timeout_msec;
>+	state->session = session;
>+	state->tcon = tcon;
>+
>+	if (smbXcli_session_is_authenticated(state->session)) {
>+		additional_flags |= SMB2_HDR_FLAG_SIGNED;
>+	}
>+
>+	subreq = smb2cli_raw_tcon_send(state,
>+				       state->ev,
>+				       state->conn,
>+				       additional_flags,
>+				       clear_flags,
>+				       state->timeout_msec,
>+				       state->session,
>+				       state->tcon,
>+				       flags,
>+				       unc);
>+	if (tevent_req_nomem(subreq, req)) {
>+		return tevent_req_post(req, ev);
>+	}
>+	tevent_req_set_callback(subreq, smb2cli_tcon_done, req);
>+
>+	return req;
>+}
>+
>+static void smb2cli_tcon_validate(struct tevent_req *subreq);
>+
>+static void smb2cli_tcon_done(struct tevent_req *subreq)
>+{
>+	struct tevent_req *req = tevent_req_callback_data(
>+		subreq, struct tevent_req);
>+	struct smb2cli_tcon_state *state = tevent_req_data(
>+		req, struct smb2cli_tcon_state);
>+	NTSTATUS status;
>+
>+	status = smb2cli_raw_tcon_recv(subreq);
>+	TALLOC_FREE(subreq);
>+	if (tevent_req_nterror(req, status)) {
>+		return;
>+	}
>+
> 	if (!smbXcli_session_is_authenticated(state->session)) {
> 		tevent_req_done(req);
> 		return;
>diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
>index ac382b2ae6af..75e758f5a847 100644
>--- a/libcli/smb/smbXcli_base.h
>+++ b/libcli/smb/smbXcli_base.h
>@@ -589,6 +589,26 @@ NTSTATUS smb2cli_logoff(struct smbXcli_conn *conn,
> 			uint32_t timeout_msec,
> 			struct smbXcli_session *session);
> 
>+/* smb2cli_raw_tcon* should only be used in tests! */
>+struct tevent_req *smb2cli_raw_tcon_send(TALLOC_CTX *mem_ctx,
>+					 struct tevent_context *ev,
>+					 struct smbXcli_conn *conn,
>+					 uint32_t additional_flags,
>+					 uint32_t clear_flags,
>+					 uint32_t timeout_msec,
>+					 struct smbXcli_session *session,
>+					 struct smbXcli_tcon *tcon,
>+					 uint16_t tcon_flags,
>+					 const char *unc);
>+NTSTATUS smb2cli_raw_tcon_recv(struct tevent_req *req);
>+NTSTATUS smb2cli_raw_tcon(struct smbXcli_conn *conn,
>+			  uint32_t additional_flags,
>+			  uint32_t clear_flags,
>+			  uint32_t timeout_msec,
>+			  struct smbXcli_session *session,
>+			  struct smbXcli_tcon *tcon,
>+			  uint16_t tcon_flags,
>+			  const char *unc);
> struct tevent_req *smb2cli_tcon_send(TALLOC_CTX *mem_ctx,
> 				     struct tevent_context *ev,
> 				     struct smbXcli_conn *conn,
>-- 
>2.25.1
>
>
>From a8ad71c82ff0f714b77cf371420e21070116a652 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 15 Sep 2021 18:31:06 +0200
>Subject: [PATCH 3/8] s4:torture/smb2: add
> smb2.ioctl.bug14788.VALIDATE_NEGOTIATE
>
>Demonstrate that smbd fails FSCTL_VALIDATE_NEGOTIATE_INFO
>only because the user doesn't have permissions on the share root.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 735fc34682c541056fd912d07c69f299f961983c)
>---
> selftest/knownfail.d/smb2.ioctl.bug14788 |   1 +
> source4/torture/smb2/ioctl.c             | 111 +++++++++++++++++++++++
> 2 files changed, 112 insertions(+)
> create mode 100644 selftest/knownfail.d/smb2.ioctl.bug14788
>
>diff --git a/selftest/knownfail.d/smb2.ioctl.bug14788 b/selftest/knownfail.d/smb2.ioctl.bug14788
>new file mode 100644
>index 000000000000..843e7d815176
>--- /dev/null
>+++ b/selftest/knownfail.d/smb2.ioctl.bug14788
>@@ -0,0 +1 @@
>+^samba3.smb2.ioctl.*.bug14788.VALIDATE_NEGOTIATE
>diff --git a/source4/torture/smb2/ioctl.c b/source4/torture/smb2/ioctl.c
>index 1de5179e336d..84b5e0927050 100644
>--- a/source4/torture/smb2/ioctl.c
>+++ b/source4/torture/smb2/ioctl.c
>@@ -27,6 +27,10 @@
> #include "torture/smb2/proto.h"
> #include "../libcli/smb/smbXcli_base.h"
> #include "librpc/gen_ndr/ndr_ioctl.h"
>+#include "lib/cmdline/popt_common.h"
>+#include "libcli/resolve/resolve.h"
>+#include "lib/param/param.h"
>+#include "lib/util/tevent_ntstatus.h"
> 
> #define FNAME	"testfsctl.dat"
> #define FNAME2	"testfsctl2.dat"
>@@ -6845,12 +6849,115 @@ static bool test_ioctl_bug14607(struct torture_context *torture,
> 	return true;
> }
> 
>+/*
>+   basic regression test for BUG 14788,
>+   with FSCTL_VALIDATE_NEGOTIATE_INFO
>+   https://bugzilla.samba.org/show_bug.cgi?id=14788
>+*/
>+static bool test_ioctl_bug14788_VALIDATE_NEGOTIATE(struct torture_context *torture,
>+				struct smb2_tree *tree0)
>+{
>+	const char *host = torture_setting_string(torture, "host", NULL);
>+	const char *share = torture_setting_string(torture, "share", NULL);
>+	const char *noperm_share = torture_setting_string(torture, "noperm_share", "noperm");
>+	struct smb2_transport *transport0 = tree0->session->transport;
>+	struct smbcli_options options;
>+	struct smb2_transport *transport = NULL;
>+	struct smb2_tree *tree = NULL;
>+	struct smb2_session *session = NULL;
>+	uint16_t noperm_flags = 0;
>+	const char *noperm_unc = NULL;
>+	struct smb2_tree *noperm_tree = NULL;
>+	uint32_t timeout_msec;
>+	struct tevent_req *subreq = NULL;
>+	struct cli_credentials *credentials = popt_get_cmdline_credentials();
>+	NTSTATUS status;
>+
>+	if (smbXcli_conn_protocol(transport0->conn) < PROTOCOL_SMB3_00) {
>+		torture_skip(torture, "Can't test without SMB 3 support");
>+	}
>+
>+	options = transport0->options;
>+	options.client_guid = GUID_random();
>+	options.min_protocol = PROTOCOL_SMB3_00;
>+	options.max_protocol = PROTOCOL_SMB3_02;
>+
>+	status = smb2_connect(torture,
>+			      host,
>+			      lpcfg_smb_ports(torture->lp_ctx),
>+			      share,
>+			      lpcfg_resolve_context(torture->lp_ctx),
>+			      credentials,
>+			      &tree,
>+			      torture->ev,
>+			      &options,
>+			      lpcfg_socket_options(torture->lp_ctx),
>+			      lpcfg_gensec_settings(torture, torture->lp_ctx)
>+			      );
>+	torture_assert_ntstatus_ok(torture, status, "smb2_connect options failed");
>+	session = tree->session;
>+	transport = session->transport;
>+
>+	timeout_msec = tree->session->transport->options.request_timeout * 1000;
>+
>+	subreq = smb2cli_validate_negotiate_info_send(torture,
>+						      torture->ev,
>+						      transport->conn,
>+						      timeout_msec,
>+						      session->smbXcli,
>+						      tree->smbXcli);
>+	torture_assert(torture,
>+		       tevent_req_poll_ntstatus(subreq, torture->ev, &status),
>+		       "tevent_req_poll_ntstatus");
>+	status = smb2cli_validate_negotiate_info_recv(subreq);
>+	torture_assert_ntstatus_ok(torture, status, "smb2cli_validate_negotiate_info");
>+
>+	noperm_unc = talloc_asprintf(torture, "\\\\%s\\%s", host, noperm_share);
>+	torture_assert(torture, noperm_unc != NULL, "talloc_asprintf");
>+
>+	noperm_tree = smb2_tree_init(session, torture, false);
>+	torture_assert(torture, noperm_tree != NULL, "smb2_tree_init");
>+
>+	status = smb2cli_raw_tcon(transport->conn,
>+				  SMB2_HDR_FLAG_SIGNED,
>+				  0, /* clear_flags */
>+				  timeout_msec,
>+				  session->smbXcli,
>+				  noperm_tree->smbXcli,
>+				  noperm_flags,
>+				  noperm_unc);
>+	if (NT_STATUS_EQUAL(status, NT_STATUS_BAD_NETWORK_NAME)) {
>+		torture_skip(torture, talloc_asprintf(torture,
>+			     "noperm_unc[%s] %s",
>+			     noperm_unc, nt_errstr(status)));
>+	}
>+	torture_assert_ntstatus_ok(torture, status,
>+				   talloc_asprintf(torture,
>+				   "smb2cli_tcon(%s)",
>+				   noperm_unc));
>+
>+	subreq = smb2cli_validate_negotiate_info_send(torture,
>+						      torture->ev,
>+						      transport->conn,
>+						      timeout_msec,
>+						      session->smbXcli,
>+						      noperm_tree->smbXcli);
>+	torture_assert(torture,
>+		       tevent_req_poll_ntstatus(subreq, torture->ev, &status),
>+		       "tevent_req_poll_ntstatus");
>+	status = smb2cli_validate_negotiate_info_recv(subreq);
>+	torture_assert_ntstatus_ok(torture, status, "smb2cli_validate_negotiate_info noperm");
>+
>+	return true;
>+}
>+
> /*
>  * testing of SMB2 ioctls
>  */
> struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx)
> {
> 	struct torture_suite *suite = torture_suite_create(ctx, "ioctl");
>+	struct torture_suite *bug14788 = torture_suite_create(ctx, "bug14788");
> 
> 	torture_suite_add_1smb2_test(suite, "shadow_copy",
> 				     test_ioctl_get_shadow_copy);
>@@ -6993,6 +7100,10 @@ struct torture_suite *torture_smb2_ioctl_init(TALLOC_CTX *ctx)
> 	torture_suite_add_1smb2_test(suite, "bug14607",
> 				     test_ioctl_bug14607);
> 
>+	torture_suite_add_1smb2_test(bug14788, "VALIDATE_NEGOTIATE",
>+				     test_ioctl_bug14788_VALIDATE_NEGOTIATE);
>+	torture_suite_add_suite(suite, bug14788);
>+
> 	suite->description = talloc_strdup(suite, "SMB2-IOCTL tests");
> 
> 	return suite;
>-- 
>2.25.1
>
>
>From e5738bea9bbe2bd4f4f18a7746f9b6ceb925e4ac Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 15 Sep 2021 17:22:39 +0200
>Subject: [PATCH 4/8] smb2_server: make sure in_ctl_code = IVAL(body, 0x04);
> reads valid bytes
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 1cd948d8520fd41a4e2f0cc6ee787c1e20211e33)
>---
> source3/smbd/smb2_server.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index f071c1a5ff98..f5c28fde3025 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -3097,7 +3097,7 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 		const uint8_t *body = SMBD_SMB2_IN_BODY_PTR(req);
> 		size_t body_size = SMBD_SMB2_IN_BODY_LEN(req);
> 		uint32_t in_ctl_code;
>-		size_t needed = 4;
>+		size_t needed = 8;
> 
> 		if (needed > body_size) {
> 			return smbd_smb2_request_error(req,
>-- 
>2.25.1
>
>
>From 45ff4d7878a02f759d90f25168972ba33e1d5a5d Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 15 Sep 2021 17:25:53 +0200
>Subject: [PATCH 5/8] smb2_server: decouple IOCTL check from signing/encryption
> states
>
>There's no reason to handle FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT
>differently if signing/encryption is used.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit bd3ba3c96e6ba811afd5898ff5470188557a6e33)
>---
> source3/smbd/smb2_server.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index f5c28fde3025..ce9a1e001433 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -3077,7 +3077,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 		if (!NT_STATUS_IS_OK(session_status)) {
> 			return smbd_smb2_request_error(req, session_status);
> 		}
>-	} else if (opcode == SMB2_OP_IOCTL) {
>+	}
>+
>+	if (opcode == SMB2_OP_IOCTL) {
> 		/*
> 		 * Some special IOCTL calls don't require
> 		 * file, tcon nor session.
>-- 
>2.25.1
>
>
>From 948c1ceb5b47bac201af5773165e4384d4be0f77 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 16 Aug 2021 17:28:05 +0200
>Subject: [PATCH 6/8] smb2_server: skip tcon check and chdir_current_service()
> for FSCTL_VALIDATE_NEGOTIATE_INFO
>
>We should not fail this just because the user doesn't have permissions
>on the share root.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit c850ce96fd32ea91d8a31223bb09dd5b8b98d99e)
>---
> selftest/knownfail.d/smb2.ioctl.bug14788 | 1 -
> source3/smbd/smb2_server.c               | 3 +++
> 2 files changed, 3 insertions(+), 1 deletion(-)
> delete mode 100644 selftest/knownfail.d/smb2.ioctl.bug14788
>
>diff --git a/selftest/knownfail.d/smb2.ioctl.bug14788 b/selftest/knownfail.d/smb2.ioctl.bug14788
>deleted file mode 100644
>index 843e7d815176..000000000000
>--- a/selftest/knownfail.d/smb2.ioctl.bug14788
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smb2.ioctl.*.bug14788.VALIDATE_NEGOTIATE
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index ce9a1e001433..f1c615b2240a 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -3114,6 +3114,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 		case FSCTL_SMBTORTURE_FORCE_UNACKED_TIMEOUT:
> 			call = &_root_ioctl_call;
> 			break;
>+		case FSCTL_VALIDATE_NEGOTIATE_INFO:
>+			call = &_root_ioctl_call;
>+			break;
> 		}
> 	}
> 
>-- 
>2.25.1
>
>
>From 7c3672ac1ce02ae91df78ed4289a8c4b0da8f28a Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 15 Sep 2021 20:26:58 +0200
>Subject: [PATCH 7/8] smb2_ioctl: return BUFFER_TOO_SMALL in
> smbd_smb2_request_ioctl_done()
>
>We should not send more data than the client requested.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit aab540503434817cc6b2de1d9c507f9d0b3ad980)
>---
> source3/smbd/smb2_ioctl.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/source3/smbd/smb2_ioctl.c b/source3/smbd/smb2_ioctl.c
>index 3d65a96368c5..7ca15a233203 100644
>--- a/source3/smbd/smb2_ioctl.c
>+++ b/source3/smbd/smb2_ioctl.c
>@@ -296,6 +296,7 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq)
> 	uint32_t in_ctl_code;
> 	uint64_t in_file_id_persistent;
> 	uint64_t in_file_id_volatile;
>+	uint32_t in_max_output_length;
> 	uint32_t out_input_offset;
> 	uint32_t out_output_offset;
> 	DATA_BLOB out_output_buffer = data_blob_null;
>@@ -328,6 +329,24 @@ static void smbd_smb2_request_ioctl_done(struct tevent_req *subreq)
> 	in_ctl_code		= IVAL(inbody, 0x04);
> 	in_file_id_persistent	= BVAL(inbody, 0x08);
> 	in_file_id_volatile	= BVAL(inbody, 0x10);
>+	in_max_output_length	= IVAL(inbody, 0x2C);
>+
>+	if (out_output_buffer.length > in_max_output_length) {
>+		/*
>+		 * Return NT_STATUS_BUFFER_TOO_SMALL by
>+		 * default if the provided buffer doesn't
>+		 * fit.
>+		 *
>+		 * If someone wants truncated data
>+		 * together with STATUS_BUFFER_OVERFLOW
>+		 * it needs to be done explicitly in
>+		 * the backends. We currently do that
>+		 * in:
>+		 * - fsctl_dfs_get_refers()
>+		 * - smbd_smb2_ioctl_pipe_read_done()
>+		 */
>+		status = NT_STATUS_BUFFER_TOO_SMALL;
>+	}
> 
> 	if (smbd_smb2_ioctl_is_failure(in_ctl_code, status,
> 				       out_output_buffer.length)) {
>-- 
>2.25.1
>
>
>From 1b71e95fc9c8dae312356a664be53db1f27d434b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 15 Sep 2021 20:27:12 +0200
>Subject: [PATCH 8/8] smb2_server: don't let SMB2_OP_IOCTL force FILE_CLOSED
> for invalid file ids
>
>smbd_smb2_request_process_ioctl() already detailed checks for file_ids,
>which not reached before.
>
>.allow_invalid_fileid = true was only used for SMB2_OP_IOCTL.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14788
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 1744dd8c5bc342a74e397951506468636275fe45)
>---
> source3/smbd/smb2_server.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
>diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
>index f1c615b2240a..b5a831f6df0b 100644
>--- a/source3/smbd/smb2_server.c
>+++ b/source3/smbd/smb2_server.c
>@@ -64,7 +64,6 @@ static const struct smbd_smb2_dispatch_table {
> 	bool need_tcon;
> 	bool as_root;
> 	uint16_t fileid_ofs;
>-	bool allow_invalid_fileid;
> 	bool modify;
> } smbd_smb2_table[] = {
> #define _OP(o) .opcode = o, .name = #o
>@@ -129,7 +128,6 @@ static const struct smbd_smb2_dispatch_table {
> 		.need_session = true,
> 		.need_tcon = true,
> 		.fileid_ofs = 0x08,
>-		.allow_invalid_fileid = true,
> 		.modify = true,
> 	},{
> 		_OP(SMB2_OP_CANCEL),
>@@ -3218,16 +3216,12 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
> 
> 		fsp = file_fsp_smb2(req, file_id_persistent, file_id_volatile);
> 		if (fsp == NULL) {
>-			if (!call->allow_invalid_fileid) {
>-				return smbd_smb2_request_error(req,
>-						NT_STATUS_FILE_CLOSED);
>-			}
>-
>-			if (file_id_persistent != UINT64_MAX) {
>-				return smbd_smb2_request_error(req,
>-						NT_STATUS_FILE_CLOSED);
>-			}
>-			if (file_id_volatile != UINT64_MAX) {
>+			/*
>+			 * smbd_smb2_request_process_ioctl()
>+			 * has more checks in order to return more
>+			 * detailed error codes...
>+			 */
>+			if (opcode != SMB2_OP_IOCTL) {
> 				return smbd_smb2_request_error(req,
> 						NT_STATUS_FILE_CLOSED);
> 			}
>-- 
>2.25.1
>

Flags:

metze: review? (slow)

Actions: View

Attachments on bug 14788: 16739 | 16740 | 17035 | 17036 | 17044 | 17054 | 17055