Attachment 16399 Details for Bug 14607 – git-am fix for 4.12.next.

[patch] git-am fix for 4.12.next.

bug-14607-4.12 (text/plain), 11.78 KB, created by Jeremy Allison on 2021-01-15 20:32:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2021-01-15 20:32:10 UTC

Size: 11.78 KB

patch

obsolete

>From 715f7c7f9e561ce4063f8165207404f7accd55cc Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Mon, 11 Jan 2021 10:01:39 +0100
>Subject: [PATCH 1/4] libcli/smb: Change some checks to SMB_ASSERTS
>
>If we end up here, it's definitely a programming error in the basic
>parsing layer of the SMB2 packet.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit fdcdfceefdd3186ef0b70bb6e83dddc8f4c073db)
>---
> libcli/smb/smb2_signing.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/libcli/smb/smb2_signing.c b/libcli/smb/smb2_signing.c
>index cc03607d789..230475480c2 100644
>--- a/libcli/smb/smb2_signing.c
>+++ b/libcli/smb/smb2_signing.c
>@@ -189,13 +189,8 @@ NTSTATUS smb2_signing_check_pdu(struct smb2_signing_key *signing_key,
> 	static const uint8_t zero_sig[16] = { 0, };
> 	int i;
> 
>-	if (count < 2) {
>-		return NT_STATUS_INVALID_PARAMETER;
>-	}
>-
>-	if (vector[0].iov_len != SMB2_HDR_BODY) {
>-		return NT_STATUS_INVALID_PARAMETER;
>-	}
>+	SMB_ASSERT(count >= 2);
>+	SMB_ASSERT(vector[0].iov_len == SMB2_HDR_BODY);
> 
> 	hdr = (const uint8_t *)vector[0].iov_base;
> 
>-- 
>2.27.0
>
>
>From 49bfa47e4c10c4b52168b1400db59409be55ec45 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 6 Jan 2021 09:03:05 -0800
>Subject: [PATCH 2/4] libcli/smb: Allow smb2cli_validate_negotiate_info_done()
> to ignore NT_STATUS_INVALID_PARAMETER.
>
>This can be returned from NetApp Ontap 7.3.7 SMB server
>implementations. Now we have ensured smb2_signing_check_pdu()
>cannot return NT_STATUS_INVALID_PARAMETER on a signing error
>it's safe to check this error code here. Windows 10
>clients ignore this error from the NetApp.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>(cherry picked from commit 0abb5ca6b96c843909dea56d5594e334547ae90f)
>---
> libcli/smb/smbXcli_base.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index 0115cbbec18..e71f82456b2 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -5424,6 +5424,18 @@ static void smb2cli_validate_negotiate_info_done(struct tevent_req *subreq)
> 				    &state->out_input_buffer,
> 				    &state->out_output_buffer);
> 	TALLOC_FREE(subreq);
>+
>+	/*
>+	 * This response must be signed correctly for
>+	 * these "normal" error codes to be processed.
>+	 * If the packet wasn't signed correctly we will get
>+	 * NT_STATUS_ACCESS_DENIED or NT_STATUS_HMAC_NOT_SUPPORTED,
>+	 * or NT_STATUS_INVALID_NETWORK_RESPONSE
>+	 * from smb2_signing_check_pdu().
>+	 *
>+	 * We must never ignore the above errors here.
>+	 */
>+
> 	if (NT_STATUS_EQUAL(status, NT_STATUS_FILE_CLOSED)) {
> 		/*
> 		 * The response was signed, but not supported
>@@ -5469,6 +5481,19 @@ static void smb2cli_validate_negotiate_info_done(struct tevent_req *subreq)
> 		tevent_req_done(req);
> 		return;
> 	}
>+	if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_PARAMETER)) {
>+		/*
>+		 * The response was signed, but not supported
>+		 *
>+		 * This might be returned by NetApp Ontap 7.3.7 SMB server
>+		 * implementations.
>+		 *
>+		 * BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>+		 *
>+		 */
>+		tevent_req_done(req);
>+		return;
>+	}
> 	if (tevent_req_nterror(req, status)) {
> 		return;
> 	}
>-- 
>2.27.0
>
>
>From 7b20ed60a4e19c33bcfafb1fcdc5bd3fe970b6f8 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 14 Jan 2021 17:27:21 +0100
>Subject: [PATCH 3/4] libcli/smb: split out smb2cli_ioctl_parse_buffer()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>Pair-Programmed-With: Volker Lendecke <vl@samba.org>
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 508ed5b42c23f8b3d9730d838bd921cb73c61358)
>---
> libcli/smb/smb2cli_ioctl.c | 190 +++++++++++++++++++++----------------
> 1 file changed, 110 insertions(+), 80 deletions(-)
>
>diff --git a/libcli/smb/smb2cli_ioctl.c b/libcli/smb/smb2cli_ioctl.c
>index 2b572baeb23..6368bd31bc2 100644
>--- a/libcli/smb/smb2cli_ioctl.c
>+++ b/libcli/smb/smb2cli_ioctl.c
>@@ -160,6 +160,80 @@ struct tevent_req *smb2cli_ioctl_send(TALLOC_CTX *mem_ctx,
> 	return req;
> }
> 
>+static NTSTATUS smb2cli_ioctl_parse_buffer(uint32_t dyn_offset,
>+					   const DATA_BLOB dyn_buffer,
>+					   uint32_t min_offset,
>+					   uint32_t buffer_offset,
>+					   uint32_t buffer_length,
>+					   uint32_t max_length,
>+					   uint32_t *next_offset,
>+					   DATA_BLOB *buffer)
>+{
>+	uint32_t offset;
>+	bool oob;
>+
>+	*buffer = data_blob_null;
>+	*next_offset = dyn_offset;
>+
>+	if (buffer_offset == 0) {
>+		/*
>+		 * If the offset is 0, we better ignore
>+		 * the buffer_length field.
>+		 */
>+		return NT_STATUS_OK;
>+	}
>+
>+	if (buffer_length == 0) {
>+		/*
>+		 * If the length is 0, we better ignore
>+		 * the buffer_offset field.
>+		 */
>+		return NT_STATUS_OK;
>+	}
>+
>+	SMB_ASSERT(min_offset >= dyn_offset);
>+	if (buffer_offset != min_offset) {
>+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
>+	}
>+
>+	/*
>+	 * Make [input|output]_buffer_offset relative to "dyn_buffer"
>+	 */
>+	offset = buffer_offset - dyn_offset;
>+	oob = smb_buffer_oob(dyn_buffer.length, offset, buffer_length);
>+	if (oob) {
>+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
>+	}
>+
>+	/*
>+	 * Give the caller a hint what we consumed,
>+	 * the caller may need to add possible padding.
>+	 */
>+	*next_offset = buffer_offset + buffer_length;
>+
>+	if (max_length == 0) {
>+		/*
>+		 * If max_input_length is 0 we ignore the
>+		 * input_buffer_length, because Windows 2008 echos the
>+		 * DCERPC request from the requested input_buffer to
>+		 * the response input_buffer.
>+		 *
>+		 * We just use the same logic also for max_output_length...
>+		 */
>+		buffer_length = 0;
>+	}
>+
>+	if (buffer_length > max_length) {
>+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
>+	}
>+
>+	*buffer = (DATA_BLOB) {
>+		.data = dyn_buffer.data + offset,
>+		.length = buffer_length,
>+	};
>+	return NT_STATUS_OK;
>+}
>+
> static void smb2cli_ioctl_done(struct tevent_req *subreq)
> {
> 	struct tevent_req *req =
>@@ -169,15 +243,19 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
> 		tevent_req_data(req,
> 		struct smb2cli_ioctl_state);
> 	NTSTATUS status;
>+	NTSTATUS error;
> 	struct iovec *iov;
> 	uint8_t *fixed;
>-	uint8_t *dyn;
>-	size_t dyn_len;
>+	DATA_BLOB dyn_buffer = data_blob_null;
> 	uint32_t dyn_ofs = SMB2_HDR_BODY + 0x30;
>+	uint32_t input_min_offset;
> 	uint32_t input_buffer_offset;
> 	uint32_t input_buffer_length;
>+	uint32_t input_next_offset;
>+	uint32_t output_min_offset;
> 	uint32_t output_buffer_offset;
> 	uint32_t output_buffer_length;
>+	uint32_t output_next_offset;
> 	static const struct smb2cli_req_expected_response expected[] = {
> 	{
> 		.status = NT_STATUS_OK,
>@@ -247,92 +325,44 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
> 
> 	state->recv_iov = iov;
> 	fixed = (uint8_t *)iov[1].iov_base;
>-	dyn = (uint8_t *)iov[2].iov_base;
>-	dyn_len = iov[2].iov_len;
>+	dyn_buffer = data_blob_const((uint8_t *)iov[2].iov_base,
>+				     iov[2].iov_len);
> 
> 	input_buffer_offset = IVAL(fixed, 0x18);
> 	input_buffer_length = IVAL(fixed, 0x1C);
> 	output_buffer_offset = IVAL(fixed, 0x20);
> 	output_buffer_length = IVAL(fixed, 0x24);
> 
>-	if ((input_buffer_offset > 0) && (input_buffer_length > 0)) {
>-		uint32_t ofs;
>-
>-		if (input_buffer_offset != dyn_ofs) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		ofs = input_buffer_length;
>-		ofs = NDR_ROUND(ofs, 8);
>-
>-		if (state->max_input_length == 0) {
>-			/*
>-			 * If max_input_length is 0 we ignore
>-			 * the input_buffer_length, because
>-			 * Windows 2008 echos the DCERPC request
>-			 * from the requested input_buffer
>-			 * to the response input_buffer.
>-			 */
>-			input_buffer_length = 0;
>-		}
>-
>-		if (input_buffer_length > dyn_len) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		if (input_buffer_length > state->max_input_length) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		state->out_input_buffer.data = dyn;
>-		state->out_input_buffer.length = input_buffer_length;
>-
>-		if (ofs > dyn_len) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		dyn_ofs += ofs;
>-		dyn += ofs;
>-		dyn_len -= ofs;
>+	input_min_offset = dyn_ofs;
>+	input_next_offset = dyn_ofs;
>+	error = smb2cli_ioctl_parse_buffer(dyn_ofs,
>+					   dyn_buffer,
>+					   input_min_offset,
>+					   input_buffer_offset,
>+					   input_buffer_length,
>+					   state->max_input_length,
>+					   &input_next_offset,
>+					   &state->out_input_buffer);
>+	if (tevent_req_nterror(req, error)) {
>+		return;
> 	}
> 
>-	if ((output_buffer_offset > 0) && (output_buffer_length > 0)) {
>-		if (output_buffer_offset != dyn_ofs) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		if (state->max_output_length == 0) {
>-			/*
>-			 * We do the same logic as for
>-			 * max_input_length.
>-			 */
>-			output_buffer_length = 0;
>-		}
>-
>-		if (output_buffer_length > dyn_len) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		if (output_buffer_length > state->max_output_length) {
>-			tevent_req_nterror(
>-				req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>-			return;
>-		}
>-
>-		state->out_output_buffer.data = dyn;
>-		state->out_output_buffer.length = output_buffer_length;
>+	/*
>+	 * If output data is returned, the output offset MUST be set to
>+	 * InputOffset + InputCount rounded up to a multiple of 8.
>+	 */
>+	output_min_offset = NDR_ROUND(input_next_offset, 8);
>+	output_next_offset = 0; /* this variable is completely ignored */
>+	error = smb2cli_ioctl_parse_buffer(dyn_ofs,
>+					   dyn_buffer,
>+					   output_min_offset,
>+					   output_buffer_offset,
>+					   output_buffer_length,
>+					   state->max_output_length,
>+					   &output_next_offset,
>+					   &state->out_output_buffer);
>+	if (tevent_req_nterror(req, error)) {
>+		return;
> 	}
> 
> 	state->out_valid = true;
>-- 
>2.27.0
>
>
>From 4c4c68be82189267e1dd343c3ee5e37fb52dc926 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 14 Jan 2021 17:32:15 +0100
>Subject: [PATCH 4/4] libcli/smb: allow unexpected padding in SMB2 IOCTL
> responses
>
>A NetApp Ontap 7.3.7 SMB server add 8 padding bytes to an
>offset that's already 8 byte aligned.
>
>RN: Work around special SMB2 IOCTL response behavior of NetApp Ontap 7.3.7
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>Pair-Programmed-With: Volker Lendecke <vl@samba.org>
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Signed-off-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Fri Jan 15 08:36:34 UTC 2021 on sn-devel-184
>
>(cherry picked from commit 4c6c71e1378401d66bf2ed230544a75f7b04376f)
>---
> libcli/smb/smb2cli_ioctl.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
>diff --git a/libcli/smb/smb2cli_ioctl.c b/libcli/smb/smb2cli_ioctl.c
>index 6368bd31bc2..f9abcc57bab 100644
>--- a/libcli/smb/smb2cli_ioctl.c
>+++ b/libcli/smb/smb2cli_ioctl.c
>@@ -191,8 +191,25 @@ static NTSTATUS smb2cli_ioctl_parse_buffer(uint32_t dyn_offset,
> 		return NT_STATUS_OK;
> 	}
> 
>+	if ((buffer_offset % 8) != 0) {
>+		/*
>+		 * The offset needs to be 8 byte aligned.
>+		 */
>+		return NT_STATUS_INVALID_NETWORK_RESPONSE;
>+	}
>+
>+	/*
>+	 * We used to enforce buffer_offset to be
>+	 * an exact match of the expected minimum,
>+	 * but the NetApp Ontap 7.3.7 SMB server
>+	 * gets the padding wrong and aligns the
>+	 * input_buffer_offset by a value of 8.
>+	 *
>+	 * So we just enforce that the offset is
>+	 * not lower than the expected value.
>+	 */
> 	SMB_ASSERT(min_offset >= dyn_offset);
>-	if (buffer_offset != min_offset) {
>+	if (buffer_offset < min_offset) {
> 		return NT_STATUS_INVALID_NETWORK_RESPONSE;
> 	}
> 
>-- 
>2.27.0
>

Flags:

metze: review+

Actions: View

Attachments on bug 14607: 16374 | 16376 | 16377 | 16398 | 16399 | 16683 | 16684 | 16685 | 16686