[PATCH] Funny Durable Handle v2 reconnect failure with lease

[Ralph Böhme]

Hi!

Attached is a patch for a funny bug in smbd_smb2_create_durable_lease_check()
that causes DH2C with an associated lease to fail because of a path mismatch:

[10 2018/07/17 12:46:04.907479 ../source3/smbd/smb2_create.c:423 smbd_smb2_create_durable_lease_check]
  Lease requested for file dir\bigfile, reopened file is named dir/bigfile

Look closely: '\' vs '/'. :)

Looks like our testsuite didn't cover the case of a DHv2 with lease on a file
in a subdirectory. :)

Patch attached, fixes smbd_smb2_create_durable_lease_check() and tweaks a test
to use a file in a directory for the test.

Please review & push if happy. Thanks!

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 6b1946563ba477cbd5e93708d830f96cc801d73d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 17 Jul 2018 15:56:05 +0200
Subject: [PATCH 1/2] s4: torture: run test_durable_v2_open_reopen2_lease() in
 a subdirectory

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13535

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.smb2.durable-v2-open |  1 +
 source4/torture/smb2/durable_v2_open.c           | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
 create mode 100644 selftest/knownfail.d/samba3.smb2.durable-v2-open

diff --git a/selftest/knownfail.d/samba3.smb2.durable-v2-open b/selftest/knownfail.d/samba3.smb2.durable-v2-open
new file mode 100644
index 00000000000..facf1d0b543
--- /dev/null
+++ b/selftest/knownfail.d/samba3.smb2.durable-v2-open
@@ -0,0 +1 @@
+^samba3.smb2.durable-v2-open.reopen2-lease-v2\(nt4_dc\)$
diff --git a/source4/torture/smb2/durable_v2_open.c b/source4/torture/smb2/durable_v2_open.c
index 3a0e0707d2c..0a928ec8c26 100644
--- a/source4/torture/smb2/durable_v2_open.c
+++ b/source4/torture/smb2/durable_v2_open.c
@@ -1518,9 +1518,15 @@ bool test_durable_v2_open_reopen2_lease_v2(struct torture_context *tctx,
 
 	options = tree->session->transport->options;
 
+	smb2_deltree(tree, __func__);
+	status = torture_smb2_testdir(tree, __func__, &_h);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"torture_smb2_testdir failed\n");
+	smb2_util_close(tree, _h);
+
 	/* Choose a random name in case the state is left a little funky. */
-	snprintf(fname, 256, "durable_v2_open_reopen2_%s.dat",
-		 generate_random_str(tctx, 8));
+	snprintf(fname, 256, "%s\\durable_v2_open_reopen2_%s.dat",
+		 __func__, generate_random_str(tctx, 8));
 
 	smb2_util_unlink(tree, fname);
 
@@ -1726,6 +1732,7 @@ bool test_durable_v2_open_reopen2_lease_v2(struct torture_context *tctx,
 	}
 
 	smb2_util_unlink(tree, fname);
+	smb2_deltree(tree, __func__);
 
 	talloc_free(tree);
 
-- 
2.13.6


From fadac7433573e9cfc7656b725a018795d7de9fad Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 17 Jul 2018 15:40:04 +0200
Subject: [PATCH 2/2] s3: smbd: fix path check in
 smbd_smb2_create_durable_lease_check()

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13535

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.smb2.durable-v2-open |  1 -
 source3/smbd/smb2_create.c                       | 16 +++++++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)
 delete mode 100644 selftest/knownfail.d/samba3.smb2.durable-v2-open

diff --git a/selftest/knownfail.d/samba3.smb2.durable-v2-open b/selftest/knownfail.d/samba3.smb2.durable-v2-open
deleted file mode 100644
index facf1d0b543..00000000000
--- a/selftest/knownfail.d/samba3.smb2.durable-v2-open
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.smb2.durable-v2-open.reopen2-lease-v2\(nt4_dc\)$
diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
index 8fbea238698..7f80f6f8138 100644
--- a/source3/smbd/smb2_create.c
+++ b/source3/smbd/smb2_create.c
@@ -381,6 +381,7 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req
 	const char *requested_filename, const struct files_struct *fsp,
 	const struct smb2_lease *lease_ptr)
 {
+	char *filename = NULL;
 	struct smb_filename *smb_fname = NULL;
 	uint32_t ucf_flags;
 	NTSTATUS status;
@@ -407,10 +408,23 @@ static NTSTATUS smbd_smb2_create_durable_lease_check(struct smb_request *smb1req
 		return NT_STATUS_OBJECT_NAME_NOT_FOUND;
 	}
 
+	filename = talloc_strdup(talloc_tos(), requested_filename);
+	if (filename == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	/* This also converts '\' to '/' */
+	status = check_path_syntax(filename);
+	if (!NT_STATUS_IS_OK(status)) {
+		TALLOC_FREE(filename);
+		return status;
+	}
+
 	ucf_flags = filename_create_ucf_flags(smb1req, FILE_OPEN);
 	status = filename_convert(talloc_tos(), fsp->conn,
-				  requested_filename, ucf_flags,
+				  filename, ucf_flags,
 				  NULL, &smb_fname);
+	TALLOC_FREE(filename);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("filename_convert returned %s\n",
 			   nt_errstr(status)));
-- 
2.13.6