[PATCH] Fix SMB2 issues found by Microsoft plugfest test tool.

Jeremy Allison jra at samba.org
Tue Jun 17 12:06:37 MDT 2014


Hi Ira,

If you could review these fixes that
fix issues the Microsoft Interop Test
suite has found with Samba that would
be great !

The Microsoft team has confirmed they
fix the issues their tool reported.

Cheers,

	Jeremy.
-------------- next part --------------
>From 86c37c3db4c92ef81b1e6ee62370461e6852d2b6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 14:05:18 -0700
Subject: [PATCH 1/3] s3: smb2 - Negprot should return INVALID_PARAMETER if
 flags2 signed bit is set.

MS-SMB2: 3.3.5.2.4 Verifying the Signature.
If the SMB2 header of the SMB2 NEGOTIATE
request has the SMB2_FLAGS_SIGNED bit set in the
Flags field, the server MUST fail the request
with STATUS_INVALID_PARAMETER.

Found and fix confirmed by Microsoft test tool.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/smb2_server.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 69fe9e4..166fb6b 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2034,8 +2034,23 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req)
 		DATA_BLOB signing_key;
 
 		if (x == NULL) {
-			return smbd_smb2_request_error(
-				req, NT_STATUS_USER_SESSION_DELETED);
+			/*
+			 * MS-SMB2: 3.3.5.2.4 Verifying the Signature.
+			 * If the SMB2 header of the SMB2 NEGOTIATE
+			 * request has the SMB2_FLAGS_SIGNED bit set in the
+			 * Flags field, the server MUST fail the request
+			 * with STATUS_INVALID_PARAMETER.
+			 *
+			 * Microsoft test tool checks this.
+			 */
+
+			if ((opcode == SMB2_OP_NEGPROT) &&
+					(flags & SMB2_HDR_FLAG_SIGNED)) {
+				status = NT_STATUS_INVALID_PARAMETER;
+			} else {
+				status = NT_STATUS_USER_SESSION_DELETED;
+			}
+			return smbd_smb2_request_error(req, status);
 		}
 
 		signing_key = x->global->channels[0].signing_key;
-- 
1.9.1


>From 769ba07e2d1daf4632d16c85b5cdb61c8e4aba90 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 14:13:02 -0700
Subject: [PATCH 2/3] s3: smb2 - Check supplied impersonation level on
 SMB2_CREATE.

MS-SMB2: 2.2.13 SMB2 CREATE Request
ImpersonationLevel ... MUST contain one of the following values.
The server MUST validate this field, but otherwise ignore it.

Found and fix confirmed by Microsoft test tool.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/smb2_create.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
index 4e82e2c..06fd84a 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -217,6 +217,17 @@ NTSTATUS smbd_smb2_request_process_create(struct smbd_smb2_request *smb2req)
 		return smbd_smb2_request_error(smb2req, NT_STATUS_OBJECT_NAME_INVALID);
 	}
 
+	/*
+	 * MS-SMB2: 2.2.13 SMB2 CREATE Request
+	 * ImpersonationLevel ... MUST contain one of the following values.
+	 * The server MUST validate this field, but otherwise ignore it.
+	 */
+
+	if (in_impersonation_level > SMB2_IMPERSONATION_DELEGATE) {
+		return smbd_smb2_request_error(smb2req,
+			NT_STATUS_BAD_IMPERSONATION_LEVEL);
+	}
+
 	ZERO_STRUCT(in_context_blobs);
 	status = smb2_create_blob_parse(smb2req, in_context_buffer, &in_context_blobs);
 	if (!NT_STATUS_IS_OK(status)) {
-- 
1.9.1


>From 4b3f11957d4217e7d28368ee9a72a4fc1dc93603 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 16 Jun 2014 14:28:11 -0700
Subject: [PATCH 3/3] s3: smb2 - strictly obey file name restrictions w.r.t.
 the SMB2 protocol spec.

MS-SMB2: 3.3.5.9 - Receiving an SMB2 CREATE Request
If the file name length is greater than zero and the
first character is a path separator character, the
server MUST fail the request with
STATUS_INVALID_PARAMETER.

Found and fix confirmed by Microsoft test tool.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/smb2_create.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
index 06fd84a..5f14d7d 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -915,6 +915,22 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx,
 				return tevent_req_post(req, ev);
 			}
 
+			/*
+			 * We know we're going to do a local open, so now
+			 * we must be protocol strict. JRA.
+			 *
+			 * MS-SMB2: 3.3.5.9 - Receiving an SMB2 CREATE Request
+			 * If the file name length is greater than zero and the
+			 * first character is a path separator character, the
+			 * server MUST fail the request with
+			 * STATUS_INVALID_PARAMETER.
+			 */
+			if (in_name[0] == '\\' || in_name[0] == '/') {
+				tevent_req_nterror(req,
+					NT_STATUS_INVALID_PARAMETER);
+				return tevent_req_post(req, ev);
+			}
+
 			status = SMB_VFS_CREATE_FILE(smb1req->conn,
 						     smb1req,
 						     0, /* root_dir_fid */
-- 
1.9.1



More information about the samba-technical mailing list