Attachment 16686 Details for Bug 14607 – Additional patches for v4-12-test (just as reference)

Additional patches for v4-12-test (just as reference)

tmp412.diff.txt (text/plain), 11.08 KB, created by Stefan Metzmacher on 2021-07-16 13:44:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2021-07-16 13:44:44 UTC

Size: 11.08 KB

patch

obsolete

>From ca270e59cd5eb8dd4cf6784b94b9e280f3efb1f7 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 29 Jun 2021 15:24:13 +0200
>Subject: [PATCH 1/2] libcli/smb: make smb2cli_ioctl_parse_buffer() available
> as smb2cli_parse_dyn_buffer()
>
>It will be used in smb2cli_read.c soon...
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 1faf15b3d0f41fa8a94b76d1616a4460ce0c6fa4)
>---
> libcli/smb/smb2cli_ioctl.c | 123 +++++--------------------------------
> libcli/smb/smbXcli_base.c  |  91 +++++++++++++++++++++++++++
> libcli/smb/smbXcli_base.h  |   9 +++
> 3 files changed, 116 insertions(+), 107 deletions(-)
>
>diff --git a/libcli/smb/smb2cli_ioctl.c b/libcli/smb/smb2cli_ioctl.c
>index f9abcc57bab3..d638b2816780 100644
>--- a/libcli/smb/smb2cli_ioctl.c
>+++ b/libcli/smb/smb2cli_ioctl.c
>@@ -160,97 +160,6 @@ 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;
>-	}
>-
>-	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) {
>-		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 =
>@@ -352,14 +261,14 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
> 
> 	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);
>+	error = smb2cli_parse_dyn_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;
> 	}
>@@ -370,14 +279,14 @@ static void smb2cli_ioctl_done(struct tevent_req *subreq)
> 	 */
> 	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);
>+	error = smb2cli_parse_dyn_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;
> 	}
>diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
>index e71f82456b2e..2811f45ed311 100644
>--- a/libcli/smb/smbXcli_base.c
>+++ b/libcli/smb/smbXcli_base.c
>@@ -6668,3 +6668,94 @@ uint64_t smb2cli_conn_get_mid(struct smbXcli_conn *conn)
> {
> 	return conn->smb2.mid;
> }
>+
>+NTSTATUS smb2cli_parse_dyn_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;
>+	}
>+
>+	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) {
>+		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;
>+}
>diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
>index 2afc7165cd97..4452cd808ea1 100644
>--- a/libcli/smb/smbXcli_base.h
>+++ b/libcli/smb/smbXcli_base.h
>@@ -390,6 +390,15 @@ void smb2cli_conn_set_cc_max_chunks(struct smbXcli_conn *conn,
> void smb2cli_conn_set_mid(struct smbXcli_conn *conn, uint64_t mid);
> uint64_t smb2cli_conn_get_mid(struct smbXcli_conn *conn);
> 
>+NTSTATUS smb2cli_parse_dyn_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);
>+
> struct tevent_req *smb2cli_req_create(TALLOC_CTX *mem_ctx,
> 				      struct tevent_context *ev,
> 				      struct smbXcli_conn *conn,
>-- 
>2.25.1
>
>
>From 2a7725514d85848ed1a0552a25cc3a40106fe1b7 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 29 Jun 2021 15:42:56 +0200
>Subject: [PATCH 2/2] libcli/smb: allow unexpected padding in SMB2 READ
> responses
>
>Make use of smb2cli_parse_dyn_buffer() in smb2cli_read_done()
>as it was exactly introduced for a similar problem see:
>
>    commit 4c6c71e1378401d66bf2ed230544a75f7b04376f
>    Author:     Stefan Metzmacher <metze@samba.org>
>    AuthorDate: Thu Jan 14 17:32:15 2021 +0100
>    Commit:     Volker Lendecke <vl@samba.org>
>    CommitDate: Fri Jan 15 08:36:34 2021 +0000
>
>        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
>
>RN: Work around special SMB2 READ response behavior of NetApp Ontap 7.3.7
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
>
>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): Thu Jul 15 23:53:55 UTC 2021 on sn-devel-184
>
>(cherry picked from commit 155348cda65b441a6c4db1ed84dbf1682d02973c)
>---
> libcli/smb/smb2cli_read.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
>diff --git a/libcli/smb/smb2cli_read.c b/libcli/smb/smb2cli_read.c
>index 8110b65d4322..c7f48741b875 100644
>--- a/libcli/smb/smb2cli_read.c
>+++ b/libcli/smb/smb2cli_read.c
>@@ -90,8 +90,13 @@ static void smb2cli_read_done(struct tevent_req *subreq)
> 		tevent_req_data(req,
> 		struct smb2cli_read_state);
> 	NTSTATUS status;
>+	NTSTATUS error;
> 	struct iovec *iov;
>+	const uint8_t dyn_ofs = SMB2_HDR_BODY + 0x10;
>+	DATA_BLOB dyn_buffer = data_blob_null;
> 	uint8_t data_offset;
>+	DATA_BLOB data_buffer = data_blob_null;
>+	uint32_t next_offset = 0; /* this variable is completely ignored */
> 	static const struct smb2cli_req_expected_response expected[] = {
> 	{
> 		.status = STATUS_BUFFER_OVERFLOW,
>@@ -117,14 +122,23 @@ static void smb2cli_read_done(struct tevent_req *subreq)
> 	data_offset = CVAL(iov[1].iov_base, 2);
> 	state->data_length = IVAL(iov[1].iov_base, 4);
> 
>-	if ((data_offset != SMB2_HDR_BODY + 16) ||
>-	    (state->data_length > iov[2].iov_len)) {
>-		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
>+	dyn_buffer = data_blob_const((uint8_t *)iov[2].iov_base,
>+				     iov[2].iov_len);
>+
>+	error = smb2cli_parse_dyn_buffer(dyn_ofs,
>+					 dyn_buffer,
>+					 dyn_ofs, /* min_offset */
>+					 data_offset,
>+					 state->data_length,
>+					 dyn_buffer.length, /* max_length */
>+					 &next_offset,
>+					 &data_buffer);
>+	if (tevent_req_nterror(req, error)) {
> 		return;
> 	}
> 
> 	state->recv_iov = iov;
>-	state->data = (uint8_t *)iov[2].iov_base;
>+	state->data = data_buffer.data;
> 
> 	state->out_valid = true;
> 
>-- 
>2.25.1
>

Actions: View

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