The branch, v4-12-test has been updated
via 552548ff00e libcli/smb: allow unexpected padding in SMB2 IOCTL
responses
via c0a7b8c7bd2 libcli/smb: split out smb2cli_ioctl_parse_buffer()
via a5efe544353 libcli/smb: Allow
smb2cli_validate_negotiate_info_done() to ignore NT_STATUS_INVALID_PARAMETER.
via 2f8b1fb8aec libcli/smb: Change some checks to SMB_ASSERTS
via f972e3a70c2 WHATSNEW: Remove superfluous line.
from 5d5ab7c50ed VERSION: Bump version up to 4.12.12...
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-12-test
- Log -----------------------------------------------------------------
commit 552548ff00e08ba4639b2a9b1ef978259cdb7c1a
Author: Stefan Metzmacher <me...@samba.org>
Date: Thu Jan 14 17:32:15 2021 +0100
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 <v...@samba.org>
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Volker Lendecke <v...@samba.org>
Autobuild-User(master): Volker Lendecke <v...@samba.org>
Autobuild-Date(master): Fri Jan 15 08:36:34 UTC 2021 on sn-devel-184
(cherry picked from commit 4c6c71e1378401d66bf2ed230544a75f7b04376f)
Autobuild-User(v4-12-test): Karolin Seeger <ksee...@samba.org>
Autobuild-Date(v4-12-test): Mon Jan 18 08:57:48 UTC 2021 on sn-devel-184
commit c0a7b8c7bd23f02129839aae5541669c86ee5506
Author: Stefan Metzmacher <me...@samba.org>
Date: Thu Jan 14 17:27:21 2021 +0100
libcli/smb: split out smb2cli_ioctl_parse_buffer()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14607
Pair-Programmed-With: Volker Lendecke <v...@samba.org>
Signed-off-by: Stefan Metzmacher <me...@samba.org>
Signed-off-by: Volker Lendecke <v...@samba.org>
(cherry picked from commit 508ed5b42c23f8b3d9730d838bd921cb73c61358)
commit a5efe54435337463bca6a6eccabf4792745ffdca
Author: Jeremy Allison <j...@samba.org>
Date: Wed Jan 6 09:03:05 2021 -0800
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 <j...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
Reviewed-by: Stefan Metzmacher <me...@samba.org>
(cherry picked from commit 0abb5ca6b96c843909dea56d5594e334547ae90f)
commit 2f8b1fb8aec7274b33969922e2a329c2c26aca1f
Author: Stefan Metzmacher <me...@samba.org>
Date: Mon Jan 11 10:01:39 2021 +0100
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 <me...@samba.org>
Reviewed-by: Volker Lendecke <v...@samba.org>
(cherry picked from commit fdcdfceefdd3186ef0b70bb6e83dddc8f4c073db)
commit f972e3a70c27fe229dc3ad41a181d2320c958d54
Author: Karolin Seeger <ksee...@samba.org>
Date: Mon Jan 18 08:55:29 2021 +0100
WHATSNEW: Remove superfluous line.
Signed-off-by: Karolin Seeger <ksee...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
WHATSNEW.txt | 1 -
libcli/smb/smb2_signing.c | 9 +-
libcli/smb/smb2cli_ioctl.c | 207 +++++++++++++++++++++++++++------------------
libcli/smb/smbXcli_base.c | 25 ++++++
4 files changed, 154 insertions(+), 88 deletions(-)
Changeset truncated at 500 lines:
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index a5de41e2c75..c8559260371 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -34,7 +34,6 @@ o Ralph Boehme <s...@samba.org>
* BUG 14587: vfs_zfsacl: add missing inherited flag on hidden "magic"
everyone@ ACE.
* BUG 14596: vfs_fruit may close wrong backend fd.
- * BUG 14596: TODO
o Günther Deschner <g...@samba.org>
* BUG 14486: s3-vfs_glusterfs: always disable write-behind translator.
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;
diff --git a/libcli/smb/smb2cli_ioctl.c b/libcli/smb/smb2cli_ioctl.c
index 2b572baeb23..f9abcc57bab 100644
--- a/libcli/smb/smb2cli_ioctl.c
+++ b/libcli/smb/smb2cli_ioctl.c
@@ -160,6 +160,97 @@ 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 =
@@ -169,15 +260,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 +342,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;
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;
}
--
Samba Shared Repository
