[PATCHES] SMB3 lock sequence (replay detection)

Michael Adam obnox at samba.org
Thu Sep 20 17:04:22 UTC 2018


On 2018-09-20 at 13:29 +0200, Michael Adam via samba-technical wrote:
> On 2018-09-18 at 19:04 +0200, Michael Adam via samba-technical wrote:
> > On 2018-09-18 at 16:31 +0200, Stefan Metzmacher wrote:
> > > Hi Michael,
> > > 
> > > thanks for driving this forward again!
> > > 
> > > I'd like to see a few minor changes before this goes upstream.
> > > 
> > > - Can we have an additional test the requires SMB3
> > >   and tries to open a persistent handle, but allows a fallback
> > >   just a durable handle?
> > 
> > Good point, we'll do that!
> > 
> > > - Please only activate this when "server multi channel support" is
> > >   activated for now.
> > 
> > "This" being lock sequence number checking in the server?
> 
> If that's what you mean, then we do actually have a possible
> problem with enabling this in the current master: In selftest,
> multi-channel does not work fully right now, because the
> socket-wrapper work for supporting fd-passing is not quite
> complete yet. And since connection passing is not only
> triggered by session binds but also by negotiate requests
> with a client GUID that is already connected to the server,
> enabling this has side effects.
> 
> We can of course create a new test env that will only be
> used for this set of lock replay detection tests, if that's
> what you want...

Update:

As food for thought, attached is an updated patchset
that intoduces a new test env nt4_dc_multichannel
and runs the smb2.lock tests against this. Then
changes the server to only do lock sequence checking
if multi channel is enabled.

Also note the patch "torture: fix and improve the smb2.lock.replay
test": It occurred to me that the replay test was actually
not testing in all cases what it wanted to test (due to using
an invalid bucket number where it intended to use a valid one).

So I fixed the test itself and added a few comments to
explain the flow better. If desired, this could be proposed
and merged separately before this bigger patchset along
with Guenther's preceding
"s4-torture: make smb2.lock.replay test work against Windows 2012 R2."

The only thing that is not done yet is the separate new test
case that requres SMB3 and does a persistent handle.

> Additional thoughts would be appreciated!

Cheers - Michael


> Cheers - Michael
> 
> 
> 
> > Thanks for the feed-back!
> > 
> > Michael
> > 
> > 
> > 
> > > Thanks!
> > > metze
> > > 
> > > Am 18.09.2018 um 10:23 schrieb Michael Adam via samba-technical:
> > > > 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 296619b139e877d2d56817e54cecf821552dc5b6 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 01/16] 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 a98e30986c82e37342e387189fd866db1eaaf30d 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 02/16] 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 ca2d645225930d9f15d0d813ca3382fc03f745e1 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 03/16] 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 479dcca4cce9bfe6a7de0bbbf3b1310b900b8cfd 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 04/16] 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 6c112ba269903cb4fe7c3f153905aa4259620d78 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 05/16] 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 925e63e8cfbc4263e66ce2554a4244e66f7cadb9 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 12:45:17 +0200
Subject: [PATCH 06/16] torture: fix and improve the smb2.lock.replay test

The test was wrong in that it used an invalid
lock sequence bucket (65) where it actually wanted
to use a valid on (64), and hence the test results
(which were adapted to the real responses) were not
quite logical.

This patch fixes this and also improves some of
the comments so that the flow of the patch becomes
a little more obvious.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 67 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 5b8aefb2076..105005f92a2 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2913,7 +2913,7 @@ static bool test_replay(struct torture_context *torture,
 		.in.file.handle	= h
 	};
 
-	torture_comment(torture, "Testing Lock (ignored) Replay detection:\n");
+	torture_comment(torture, "Testing Lock Replay detection [ignored]:\n");
 	lck.in.lock_sequence = 0x010 + 0x1;
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
@@ -2942,8 +2942,12 @@ static bool test_replay(struct torture_context *torture,
 	status = smb2_ioctl(tree, torture, &ioctl);
 	CHECK_STATUS(status, NT_STATUS_OK);
 
+	/*
+	 * Test with an invalid bucket number (only 1..64 are valid).
+	 * With an invalid number, lock replay detection is not performed.
+	 */
 	torture_comment(torture, "Testing Lock (ignored) Replay detection "
-				 "(Bucket No: 0):\n");
+				 "(Bucket No: 0 (invalid)) [ignored]:\n");
 	lck.in.lock_sequence = 0x000 + 0x1;
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
@@ -2987,6 +2991,8 @@ static bool test_replay(struct torture_context *torture,
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_OK);
 
+	/* status: still locked */
+
 	/*
 	 * Server will not grant same Byte Range using a different Bucket Seq
 	 */
@@ -2998,11 +3004,25 @@ static bool test_replay(struct torture_context *torture,
 	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
 
 	torture_comment(torture, "Testing Lock Replay detection "
-				 "(Bucket No: 64):\n");
+				 "(Bucket No: 2):\n");
 
 	/*
 	 * Server will not grant same Byte Range using a different Bucket Num
 	 */
+	lck.in.lock_sequence = 0x020 + 0x1;
+	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
+	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
+	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
+
+	/* status: still locked */
+
+	/* test with invalid bucket when file is locked */
+
+	torture_comment(torture, "Testing Lock Replay detection "
+				 "(Bucket No: 65 (invalid)) [ignored]:\n");
+
 	lck.in.lock_sequence = 0x410 + 0x1;
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
@@ -3010,39 +3030,56 @@ static bool test_replay(struct torture_context *torture,
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
 
-	/*
-	 * Test Unlock replay detection
-	 */
-	lck.in.lock_sequence = 0x410 + 0x2;
 	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);
+
+	/* status: unlocked */
+
+	/*
+	 * Lock again for the unlock replay test
+	 */
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_OK);
 
-	lck.in.lock_sequence = 0x410 + 0x3;
-	el.flags = SMB2_LOCK_FLAG_UNLOCK;
-	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_OK);
+	torture_comment(torture, "Testing Lock Replay detection "
+				 "(Bucket No: 64):\n");
 
-	torture_comment(torture, "Testing Lock (ignored) Replay detection "
-				 "(Bucket No: 65):\n");
-	lck.in.lock_sequence = 0x420 + 0x1;
+	/*
+	 * Server will not grant same Byte Range using a different Bucket Num
+	 */
+	lck.in.lock_sequence = 0x400 + 0x1;
 	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
 	status = smb2_lock(tree, &lck);
-	CHECK_STATUS(status, NT_STATUS_OK);
+	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_LOCK_NOT_GRANTED);
 
+	/*
+	 * Test Unlock replay detection
+	 */
+	lck.in.lock_sequence = 0x400 + 0x2;
 	el.flags = SMB2_LOCK_FLAG_UNLOCK;
 	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_OK); /* new seq num ==> unlocked */
+	status = smb2_lock(tree, &lck);
+	CHECK_STATUS(status, NT_STATUS_OK); /* replay detected ==> ignored */
+
+	el.flags = SMB2_LOCK_FLAG_EXCLUSIVE | SMB2_LOCK_FLAG_FAIL_IMMEDIATELY;
+	status = smb2_lock(tree, &lck);     /* same seq num ==> ignored */
 	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* verify it's unlocked: */
+	lck.in.lock_sequence = 0x400 + 0x3;
+	el.flags = SMB2_LOCK_FLAG_UNLOCK;
 	status = smb2_lock(tree, &lck);
 	CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
 
+	/* status: not locked */
+
 done:
 	smb2_util_close(tree, h);
 	smb2_deltree(tree, BASEDIR);
-- 
2.14.4


From 7e5fc54afd79bb34215dd924a499598c7eca65c3 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 07/16] 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 105005f92a2..183ee1c1169 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 Replay detection [ignored]:\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 Replay detection [ignored]:\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);
+	}
 
 	/*
 	 * Test with an invalid bucket number (only 1..64 are valid).
-- 
2.14.4


From 3bf7d5cb38d5a7f19104b35465ffcb5756ebb8f8 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 08/16] 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 2cd820c0d9a542218d7027858f7e8625727dc0e0 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:34:22 +0200
Subject: [PATCH 09/16] torture: exit early for samba3 with too old protocol in
 smb2.lock.replay

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 183ee1c1169..3681053ec57 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2892,6 +2892,14 @@ static bool test_replay(struct torture_context *torture,
 				required for Lock Replay tests\n");
 	}
 
+	if (TARGET_IS_SAMBA3(torture)) {
+		if (smbXcli_conn_protocol(transport->conn) < PROTOCOL_SMB2_22) {
+			torture_skip(torture, "SMB 2.22 Dialect family or above \
+					required for Lock Replay tests against \
+					Samba\n");
+		}
+	}
+
 	status = torture_smb2_testdir(tree, BASEDIR, &h);
 	CHECK_STATUS(status, NT_STATUS_OK);
 	smb2_util_close(tree, h);
@@ -2913,16 +2921,8 @@ static bool test_replay(struct torture_context *torture,
 		.in.file.handle	= h
 	};
 
-	if (TARGET_IS_SAMBA3(torture)) {
+	if (!TARGET_IS_SAMBA3(torture)) {
 
-		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 Replay detection [ignored]:\n");
 		lck.in.lock_sequence = 0x010 + 0x1;
-- 
2.14.4


From 3bc1b13642ce0c52bbe7d269d05416bfeda01ad6 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:35:26 +0200
Subject: [PATCH 10/16] torture: add explaining comment for non-samba block in
 smb2.lock.replay

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 3681053ec57..e91390f48e1 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2923,6 +2923,15 @@ static bool test_replay(struct torture_context *torture,
 
 	if (!TARGET_IS_SAMBA3(torture)) {
 
+		/*
+		 * Only do this initial check if the server is not Samba.
+		 * Samba does lock checking also for non-resilient and
+		 * non-persistent file handles.
+		 *
+		 * This verifies Windows behavior of NOT performing
+		 * lock sequence checks on non-resilient, non-persistent
+		 * handles.
+		 */
 
 		torture_comment(torture, "Testing Lock Replay detection [ignored]:\n");
 		lck.in.lock_sequence = 0x010 + 0x1;
-- 
2.14.4


From 9a42f9c04148498d2689e8eb751317dc5e42aa3f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:36:02 +0200
Subject: [PATCH 11/16] torture: make special treatment samba case more obvious

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

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index e91390f48e1..1359955d95b 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2961,10 +2961,9 @@ static bool test_replay(struct torture_context *torture,
 		.in.out.length = sizeof(res_req)
 	};
 	status = smb2_ioctl(tree, torture, &ioctl);
-	if (NT_STATUS_EQUAL(status, NT_STATUS_INVALID_DEVICE_REQUEST)) {
-		torture_warning(torture, "Server does not support "
-					 "Resilient File Handles");
-	} else {
+	if (!(TARGET_IS_SAMBA3(torture) &&
+	      NT_STATUS_EQUAL(status, NT_STATUS_INVALID_DEVICE_REQUEST)))
+	{
 		CHECK_STATUS(status, NT_STATUS_OK);
 	}
 
-- 
2.14.4


From 0ab376ddd6c4f5a4253f6abbc219a2b72b1f491c Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:54:35 +0200
Subject: [PATCH 12/16] selftest: Add a new test environment with multi-channel
 enabled

Signed-off-by: Michael Adam <obnox at samba.org>
---
 selftest/target/Samba.pm  |  1 +
 selftest/target/Samba3.pm | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index e09cf365fc1..e912b82dc4d 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -381,6 +381,7 @@ sub get_interface($)
     $interfaces{"localktest6"} = 7;
     $interfaces{"maptoguest"} = 8;
     $interfaces{"localnt4dc9"} = 9;
+    $interfaces{"localnt4mc"} = 10;
 
     # 11-16 used by selftest.pl for client interfaces
 
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 356b4d3f82a..a5efd8efd06 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -170,6 +170,7 @@ sub check_env($$)
 	# name              => [dep_1, dep_2, ...],
 	nt4_dc              => [],
 	nt4_dc_schannel     => [],
+	nt4_dc_multichannel => [],
 
 	simpleserver        => [],
 	fileserver          => [],
@@ -285,6 +286,55 @@ sub setup_nt4_dc_schannel
 	return $vars;
 }
 
+sub setup_nt4_dc_multichannel
+{
+	my ($self, $path) = @_;
+
+	print "PROVISIONING NT4 DC WITH MULTI-CHANNEL ...";
+
+	my $pdc_options = "
+	domain master = yes
+	domain logons = yes
+	lanman auth = yes
+
+	rpc_server:epmapper = external
+	rpc_server:spoolss = external
+	rpc_server:lsarpc = external
+	rpc_server:samr = external
+	rpc_server:netlogon = external
+	rpc_server:register_embedded_np = yes
+
+	rpc_daemon:epmd = fork
+	rpc_daemon:spoolssd = fork
+	rpc_daemon:lsasd = fork
+
+	server multi channel support = yes
+	server max protocol = SMB3
+";
+
+	my $vars = $self->provision($path, "NT4MC",
+				    "LOCALNT4MC",
+				    "localntmcpass",
+				    $pdc_options);
+
+	$vars or return undef;
+
+	if (not $self->check_or_start($vars, "yes", "yes", "yes")) {
+	       return undef;
+	}
+
+	$vars->{DOMSID} = $vars->{SAMSID};
+	$vars->{DC_SERVER} = $vars->{SERVER};
+	$vars->{DC_SERVER_IP} = $vars->{SERVER_IP};
+	$vars->{DC_SERVER_IPV6} = $vars->{SERVER_IPV6};
+	$vars->{DC_NETBIOSNAME} = $vars->{NETBIOSNAME};
+	$vars->{DC_USERNAME} = $vars->{USERNAME};
+	$vars->{DC_PASSWORD} = $vars->{PASSWORD};
+
+	return $vars;
+}
+
+
 sub setup_nt4_member
 {
 	my ($self, $prefix, $nt4_dc_vars) = @_;
-- 
2.14.4


From a450c728758c41b0e81771cb9a9f861f34561b08 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:55:10 +0200
Subject: [PATCH 13/16] selftest: run the smb2.lock tests against the
 nt4_dc_multichannel env

So the replay test can succeed.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/selftest/tests.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index e03c02c018f..8064982a610 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -523,8 +523,8 @@ for t in tests:
         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
     elif t == "smb2.lock":
-        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
-        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+        plansmbtorture4testsuite(t, "nt4_dc_multichannel", '//$SERVER_IP/aio -U$USERNAME%$PASSWORD', 'aio')
+        plansmbtorture4testsuite(t, "nt4_dc_multichannel", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
     elif t == "raw.lock" or t == "base.lock":
         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
     elif t == "raw.read":
-- 
2.14.4


From a88b8201782ac4b42072bc9110702c7711cf6457 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:23:44 +0200
Subject: [PATCH 14/16] torture: skip the smb2.lock.replay for samba if
 multi-channel is not enabled

The server currently only enables lock sequence checking when
multi channel is activated.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source4/torture/smb2/lock.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 1359955d95b..5f76c17a329 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -2893,11 +2893,22 @@ static bool test_replay(struct torture_context *torture,
 	}
 
 	if (TARGET_IS_SAMBA3(torture)) {
+		uint32_t server_capabilities;
+
 		if (smbXcli_conn_protocol(transport->conn) < PROTOCOL_SMB2_22) {
 			torture_skip(torture, "SMB 2.22 Dialect family or above \
 					required for Lock Replay tests against \
 					Samba\n");
 		}
+
+		server_capabilities = smb2cli_conn_server_capabilities(
+						tree->session->transport->conn);
+		if (!(server_capabilities & SMB2_CAP_MULTI_CHANNEL)) {
+			torture_skip(torture, "Samba currently only enables "
+				"lock sequence checking when multi channel "
+				"support is active in the server. Skipping.\n");
+		}
+
 	}
 
 	status = torture_smb2_testdir(tree, BASEDIR, &h);
-- 
2.14.4


From ccca82d387bc513f99552e411fb654e314aa108c Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Thu, 20 Sep 2018 18:17:35 +0200
Subject: [PATCH 15/16] smbd: enable lock sequence checking only if the server
 has multi-channel enabled

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smb2_lock.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index 0f1731708c6..d43d8d0c591 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -246,9 +246,14 @@ static struct tevent_req *smbd_smb2_lock_send(TALLOC_CTX *mem_ctx,
 	 * - protocol dialect 3.x.
 	 *
 	 * TODO: What about other handles using multi-channel?
+	 *
+	 * For now, we enable lock sequence checking for
+	 * SMB3 and newer when multi-channel is enabled.
 	 */
 	if (smb2req->xconn->protocol >= PROTOCOL_SMB2_22) {
-		check_lock_sequence = true;
+		if (lp_server_multi_channel_support()) {
+			check_lock_sequence = true;
+		}
 	}
 
 	if (check_lock_sequence) {
-- 
2.14.4


From 393ec7fd3dabfb363047ade944bb86258cb5443f 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 16/16] 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/20180920/95a7c33e/signature.sig>


More information about the samba-technical mailing list