[PATCHES] SMB3 lock sequence (replay detection)

Michael Adam obnox at samba.org
Tue Sep 18 08:23:54 UTC 2018


Hi all,

reviving this slightly older topic:

SMB2.1 (for resilient file handles) and SMB3.X implement
the so called lock sequence number and based on that,
the lock replay detection in the server.

This was done as part of the original efforts of
implementing multi-channel, and is now popping up
again as we are cleaning up the remaining bits and
finishing up the last parts of multi-channel.

The patches were originally not merged because
windows activates them only for resilient or
persistent file hanldes, but as Guenther points
out in one of his patches, the SMB2 doc actually
says, the server SHOULD actually implement them
for all of smb 3.x.

Hence re-proposing this patcheset to be merged
to prevent bit-rot and have the feature ready
and useable in the official samba master.

Cheers - Michael
-------------- next part --------------
From 8f287854da289a7a4c259ad9d21e7e81557deef7 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 24 Oct 2012 15:06:54 +0200
Subject: [PATCH 1/8] s3:smbXsrv.idl: add lock_sequence_array to
 smbXsrv_open_global0

This is needed for lock replay detection.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Guenther Deschner <gd at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/librpc/idl/smbXsrv.idl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/librpc/idl/smbXsrv.idl b/source3/librpc/idl/smbXsrv.idl
index 935c4084252..74cc2b0c264 100644
--- a/source3/librpc/idl/smbXsrv.idl
+++ b/source3/librpc/idl/smbXsrv.idl
@@ -431,6 +431,7 @@ interface smbXsrv
 		DATA_BLOB				backend_cookie;
 		uint16					channel_sequence;
 		hyper					channel_generation;
+		[flag(NDR_PAHEX)] uint8			lock_sequence_array[64];
 	} smbXsrv_open_global0;
 
 	typedef union {
-- 
2.14.4


From c874f31739e950204a57071bd6deee6741985096 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 24 Oct 2012 15:17:56 +0200
Subject: [PATCH 2/8] s3:smbXsrv_open: initialize
 smbXsrv_open_global->lock_sequence_array with 0xFF

This does not match the current documentation, but is very likely the
right thing to do.

If we would match the documentation and initialize with 0x00,
we would return STATUS_SUCCESS without doing any locks.
If the client also follows the documentation and starts
with a lock_sequence of 0 for the first operation.

Pair-Programmed-With: Michael Adam <obnox at samba.org>

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Guenther Deschner <gd at samba.org>
---
 source3/smbd/smbXsrv_open.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/source3/smbd/smbXsrv_open.c b/source3/smbd/smbXsrv_open.c
index 23fd96ef0ef..ea76acac0e9 100644
--- a/source3/smbd/smbXsrv_open.c
+++ b/source3/smbd/smbXsrv_open.c
@@ -526,6 +526,9 @@ static NTSTATUS smbXsrv_open_global_allocate(struct db_context *db,
 	}
 	talloc_set_destructor(global, smbXsrv_open_global_destructor);
 
+	memset(global->lock_sequence_array, 0xFF,
+	       sizeof(global->lock_sequence_array));
+
 	/*
 	 * Here we just randomly try the whole 32-bit space
 	 *
-- 
2.14.4


From 7e06de95646b4784c021bcfb4839837ebfaeba4e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 24 Oct 2012 14:53:05 +0200
Subject: [PATCH 3/8] s3:smb2_lock: pass in_lock_sequence to
 smbd_smb2_lock_send()

Take the value from the client if the dialect is SMB2_10 or higher,
otherwise default to 0.

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Guenther Deschner <gd at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smb2_lock.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index da5a54df623..645c396cf3c 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -50,6 +50,7 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 						 struct tevent_context *ev,
 						 struct smbd_smb2_request *smb2req,
 						 struct files_struct *in_fsp,
+						 uint32_t in_lock_sequence,
 						 uint16_t in_lock_count,
 						 struct smbd_smb2_lock_element *in_locks);
 static NTSTATUS smbd_smb2_lock_recv(struct tevent_req *req);
@@ -59,6 +60,7 @@ NTSTATUS smbd_smb2_request_process_lock(struct smbd_smb2_request *req)
 {
 	const uint8_t *inbody;
 	uint16_t in_lock_count;
+	uint32_t in_lock_sequence;
 	uint64_t in_file_id_persistent;
 	uint64_t in_file_id_volatile;
 	struct files_struct *in_fsp;
@@ -75,7 +77,12 @@ NTSTATUS smbd_smb2_request_process_lock(struct smbd_smb2_request *req)
 	inbody = SMBD_SMB2_IN_BODY_PTR(req);
 
 	in_lock_count			= CVAL(inbody, 0x02);
-	/* 0x04 - 4 bytes reserved */
+	if (req->xconn->protocol >= PROTOCOL_SMB2_10) {
+		in_lock_sequence	= IVAL(inbody, 0x04);
+	} else {
+		/* 0x04 - 4 bytes reserved */
+		in_lock_sequence	= 0;
+	}
 	in_file_id_persistent		= BVAL(inbody, 0x08);
 	in_file_id_volatile		= BVAL(inbody, 0x10);
 
@@ -136,6 +143,7 @@ NTSTATUS smbd_smb2_request_process_lock(struct smbd_smb2_request *req)
 
 	subreq = smbd_smb2_lock_send(req, req->ev_ctx,
 				     req, in_fsp,
+				     in_lock_sequence,
 				     in_lock_count,
 				     in_locks);
 	if (subreq == NULL) {
@@ -192,6 +200,7 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 						 struct tevent_context *ev,
 						 struct smbd_smb2_request *smb2req,
 						 struct files_struct *fsp,
+						 uint32_t in_lock_sequence,
 						 uint16_t in_lock_count,
 						 struct smbd_smb2_lock_element *in_locks)
 {
-- 
2.14.4


From c658e5af5414172c5e3da6965038880de9bd71d5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Wed, 24 Oct 2012 15:55:20 +0200
Subject: [PATCH 4/8] s3:smb2_lock: implement lock_sequence replay detection

Pair-Programmed-With: Michael Adam <obnox at samba.org>

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Guenther Deschner <gd at samba.org>
---
 source3/smbd/smb2_lock.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index 645c396cf3c..75300979edb 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -41,6 +41,8 @@ struct smbd_smb2_lock_state {
 	struct blocking_lock_record *blr;
 	uint16_t lock_count;
 	struct smbd_lock_element *locks;
+	uint8_t lock_sequence_value;
+	uint8_t *lock_sequence_element;
 };
 
 static void remove_pending_lock(struct smbd_smb2_lock_state *state,
@@ -213,6 +215,8 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 	struct smbd_lock_element *locks;
 	NTSTATUS status;
 	bool async = false;
+	bool check_lock_sequence = false;
+	uint32_t lock_sequence_bucket = 0;
 
 	req = tevent_req_create(mem_ctx, &state,
 			struct smbd_smb2_lock_state);
@@ -231,6 +235,39 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 	DEBUG(10,("smbd_smb2_lock_send: %s - %s\n",
 		  fsp_str_dbg(fsp), fsp_fnum_dbg(fsp)));
 
+	/*
+	 * TODO: Windows sets check_lock_sequence = true
+	 * only for resilient and persistent handles
+	 *
+	 * What about other handles using multi-channel?
+	 */
+	if (check_lock_sequence) {
+		state->lock_sequence_value = in_lock_sequence & 0xF;
+		lock_sequence_bucket = in_lock_sequence >> 4;
+	}
+	if ((lock_sequence_bucket > 0) &&
+	    (lock_sequence_bucket <= sizeof(fsp->op->global->lock_sequence_array)))
+	{
+		uint32_t idx = lock_sequence_bucket - 1;
+		uint8_t *array = fsp->op->global->lock_sequence_array;
+
+		state->lock_sequence_element = &array[idx];
+	}
+
+	if (state->lock_sequence_element != NULL) {
+		if (*state->lock_sequence_element == state->lock_sequence_value)
+		{
+			DBG_INFO("replayed smb2 lock request detected: "
+				 "file %s, value %u, bucket %u\n",
+				 fsp_str_dbg(fsp),
+				 (unsigned)state->lock_sequence_value,
+				 (unsigned)lock_sequence_bucket);
+			tevent_req_done(req);
+			return tevent_req_post(req, ev);
+		}
+		*state->lock_sequence_element = 0xFF;
+	}
+
 	locks = talloc_array(state, struct smbd_lock_element, in_lock_count);
 	if (locks == NULL) {
 		tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
@@ -382,6 +419,10 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 		return req;
 	}
 
+	if (state->lock_sequence_element != NULL) {
+		*state->lock_sequence_element = state->lock_sequence_value;
+	}
+
 	tevent_req_done(req);
 	return tevent_req_post(req, ev);
 }
@@ -777,6 +818,10 @@ static void reprocess_blocked_smb2_lock(struct smbd_smb2_request *smb2req,
 			fsp_fnum_dbg(fsp),
 			(int)state->lock_count));
 
+		if (state->lock_sequence_element != NULL) {
+			*state->lock_sequence_element = state->lock_sequence_value;
+		}
+
 		remove_pending_lock(state, blr);
 		tevent_req_done(smb2req->subreq);
 		return;
-- 
2.14.4


From 0036d00d95bc64a409dfd8ffb2eb7f2f1c55e473 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
Date: Mon, 25 Jan 2016 14:54:30 +0100
Subject: [PATCH 5/8] s4-torture: make smb2.lock.replay test work against
 Windows 2012 R2.

Guenther

Signed-off-by: Guenther Deschner <gd at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index bf090ade8f3..5b8aefb2076 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -3018,7 +3018,7 @@ static bool test_replay(struct torture_context *torture,
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_OK);
 	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_OK);
@@ -3026,7 +3026,7 @@ static bool test_replay(struct torture_context *torture,
 	lck.in.lock_sequence = 0x410 + 0x3;
 	el.flags = SMB2_LOCK_FLAG_UNLOCK;
 	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
+	CHECK_STATUS(status, NT_STATUS_OK);
 
 	torture_comment(torture, "Testing Lock (ignored) Replay detection "
 				 "(Bucket No: 65):\n");
-- 
2.14.4


From 0a689f4851e50d3b8023795040bc10ad36937572 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
Date: Mon, 25 Jan 2016 18:55:37 +0100
Subject: [PATCH 6/8] s4-torture: in smb2.lock.replay try testing lock sequence
 checking against SMB3 samba servers.

Guenther

Signed-off-by: Guenther Deschner <gd at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 43 ++++++++++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 5b8aefb2076..9035da4349a 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2913,19 +2913,31 @@ static bool test_replay(struct torture_context *torture,
 		.in.file.handle	= h
 	};
 
-	torture_comment(torture, "Testing Lock (ignored) Replay detection:\n");
-	lck.in.lock_sequence = 0x010 + 0x1;
-	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
-	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_OK);
-	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
+	if (TARGET_IS_SAMBA3(torture)) {
 
-	el.flags = SMB2_LOCK_FLAG_UNLOCK;
-	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_OK);
-	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
+		if (smbXcli_conn_protocol(transport->conn) < PROTOCOL_SMB2_22) {
+			torture_warning(torture, "SMB 2.22 Dialect family or above \
+					required for Lock Replay tests against \
+					Samba\n");
+			goto done;
+		}
+
+	} else {
+
+		torture_comment(torture, "Testing Lock (ignored) Replay detection:\n");
+		lck.in.lock_sequence = 0x010 + 0x1;
+		el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
+		status = smb2_lock(tree, &lck);
+		CHECK_STATUS(status, NT_STATUS_OK);
+		status = smb2_lock(tree, &lck);
+		CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
+
+		el.flags = SMB2_LOCK_FLAG_UNLOCK;
+		status = smb2_lock(tree, &lck);
+		CHECK_STATUS(status, NT_STATUS_OK);
+		status = smb2_lock(tree, &lck);
+		CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
+	}
 
 	torture_comment(torture, "Testing Set Resiliency:\n");
 	SIVAL(res_req, 0, 1000); /* timeout */
@@ -2940,7 +2952,12 @@ static bool test_replay(struct torture_context *torture,
 		.in.out.length = sizeof(res_req)
 	};
 	status = smb2_ioctl(tree, torture, &ioctl);
-	CHECK_STATUS(status, NT_STATUS_OK);
+	if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_DEVICE_REQUEST)) {
+		torture_warning(torture, "Server does not support "
+					 "Resilient File Handles");
+	} else {
+		CHECK_STATUS(status, NT_STATUS_OK);
+	}
 
 	torture_comment(torture, "Testing Lock (ignored) Replay detection "
 				 "(Bucket No: 0):\n");
-- 
2.14.4


From 13d990735e74005527a7f842f76c2f25613878f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
Date: Mon, 25 Jan 2016 18:56:47 +0100
Subject: [PATCH 7/8] s3-smbd: enable SMB2 lock sequence checking when SMB3 is
 enabled.

While windows enables it only for resilient and persistent handles a SMB server
SHOULD (according to MS-SMB2 section 3.3.5.14 ) activate processing of lock
sequence numbers for EITHER
- protocol dialect 2.1 and resilient handles OR
- protocol dialect 3.x.

Guenther

Signed-off-by: Guenther Deschner <gd at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smb2_lock.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index 75300979edb..0f1731708c6 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -236,11 +236,21 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 		  fsp_str_dbg(fsp), fsp_fnum_dbg(fsp)));
 
 	/*
-	 * TODO: Windows sets check_lock_sequence = true
-	 * only for resilient and persistent handles
+	 * Windows sets check_lock_sequence = true
+	 * only for resilient and persistent handles.
 	 *
-	 * What about other handles using multi-channel?
+	 * According to MS-SMB2 section 3.3.5.14 a server SHOULD activate
+	 * processing of lock sequence numbers for EITHER
+	 * - protocol dialect 2.1 and resilient handles
+	 * OR
+	 * - protocol dialect 3.x.
+	 *
+	 * TODO: What about other handles using multi-channel?
 	 */
+	if (smb2req->xconn->protocol >= PROTOCOL_SMB2_22) {
+		check_lock_sequence = true;
+	}
+
 	if (check_lock_sequence) {
 		state->lock_sequence_value = in_lock_sequence & 0xF;
 		lock_sequence_bucket = in_lock_sequence >> 4;
-- 
2.14.4


From dd3c153866de809743b6dc92d33e64d568def632 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 22 Feb 2016 17:41:52 +0100
Subject: [PATCH 8/8] selftest:knownfail: smb2.lock.*replay succeed now

Signed-off-by: Michael Adam <obnox at samba.org>
---
 selftest/knownfail | 1 -
 1 file changed, 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index d6feefd91b0..8d4b8420edb 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -189,7 +189,6 @@
 ^samba3.smb2.replay.channel-sequence
 ^samba3.smb2.replay.replay3
 ^samba3.smb2.replay.replay4
-^samba3.smb2.lock.*replay
 ^samba3.smb2.lease.statopen3
 ^samba3.smb2.lease.unlink # we currently do not downgrade RH lease to R after unlink
 ^samba4.smb2.ioctl.compress_notsup.*\(ad_dc_ntvfs\)
-- 
2.14.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180918/24ab2589/signature.sig>


More information about the samba-technical mailing list