From 15ec6b6662747a4e8e79293a896c380550396151 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 17 Feb 2022 09:58:27 -0800 Subject: [PATCH 1/3] s4: torture: Add new SMB2 lease test test_lease_duplicate_create(). Checks we return INVALID_PARAMETER when trying to create a new file with a duplicate lease key on the same share. Checked against Windows10. Samba already passes this but we didn't have a test before. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737 Signed-off-by: Jeremy Allison Reviewed-by: David Mulder (cherry picked from commit bf22548d11fe67ea3f4ec10dff81773d626e4703) --- source4/torture/smb2/lease.c | 54 ++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 2da320483fe..30101c96de5 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -4436,6 +4436,58 @@ done: return ret; } +static bool test_lease_duplicate_create(struct torture_context *tctx, + struct smb2_tree *tree) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_create io; + struct smb2_lease ls; + struct smb2_handle h1 = {{0}}; + struct smb2_handle h2 = {{0}}; + NTSTATUS status; + const char *fname1 = "duplicate_create1.dat"; + const char *fname2 = "duplicate_create2.dat"; + bool ret = true; + uint32_t caps; + + caps = smb2cli_conn_server_capabilities( + tree->session->transport->conn); + if (!(caps & SMB2_CAP_LEASING)) { + torture_skip(tctx, "leases are not supported"); + } + + /* Ensure files don't exist. */ + smb2_util_unlink(tree, fname1); + smb2_util_unlink(tree, fname2); + + /* Create file1 - LEASE1 key. */ + smb2_lease_create(&io, &ls, false, fname1, LEASE1, + smb2_util_lease_state("RWH")); + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + h1 = io.out.file.handle; + CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE); + CHECK_LEASE(&io, "RWH", true, LEASE1, 0); + + /* + * Create file2 with the same LEASE1 key - this should fail with. + * INVALID_PARAMETER. + */ + smb2_lease_create(&io, &ls, false, fname2, LEASE1, + smb2_util_lease_state("RWH")); + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER); + smb2_util_close(tree, h1); + +done: + smb2_util_close(tree, h2); + smb2_util_close(tree, h1); + smb2_util_unlink(tree, fname1); + smb2_util_unlink(tree, fname2); + talloc_free(mem_ctx); + return ret; +} + struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) { struct torture_suite *suite = @@ -4480,6 +4532,8 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "timeout-disconnect", test_lease_timeout_disconnect); torture_suite_add_1smb2_test(suite, "rename_wait", test_lease_rename_wait); + torture_suite_add_1smb2_test(suite, "duplicate_create", + test_lease_duplicate_create); suite->description = talloc_strdup(suite, "SMB2-LEASE tests"); -- 2.32.0 From 7a313366d2ac0d4cfd1e912b1512f0fb716ea0b6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 17 Feb 2022 10:58:32 -0800 Subject: [PATCH 2/3] s4: torture: Add new SMB2 lease test test_lease_duplicate_open(). Checks we return INVALID_PARAMETER when trying to open a different file with a duplicate lease key on the same share. Checked against Windows10. Currently fails against smbd so add knownfail.d/smb2-lease-duplicateopen BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737 Signed-off-by: Jeremy Allison Reviewed-by: David Mulder (cherry picked from commit ca3896b6f8bbcad68f042720feceedfa29ddbd83) --- selftest/knownfail.d/smb2-lease-duplicateopen | 1 + source4/torture/smb2/lease.c | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 selftest/knownfail.d/smb2-lease-duplicateopen diff --git a/selftest/knownfail.d/smb2-lease-duplicateopen b/selftest/knownfail.d/smb2-lease-duplicateopen new file mode 100644 index 00000000000..1336b02d74c --- /dev/null +++ b/selftest/knownfail.d/smb2-lease-duplicateopen @@ -0,0 +1 @@ +^samba3.smb2.lease.duplicate_open\(nt4_dc\) diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 30101c96de5..43b418c5acf 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -4488,6 +4488,74 @@ done: return ret; } +static bool test_lease_duplicate_open(struct torture_context *tctx, + struct smb2_tree *tree) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_create io; + struct smb2_lease ls; + struct smb2_handle h1 = {{0}}; + struct smb2_handle h2 = {{0}}; + NTSTATUS status; + const char *fname1 = "duplicate_open1.dat"; + const char *fname2 = "duplicate_open2.dat"; + bool ret = true; + uint32_t caps; + + caps = smb2cli_conn_server_capabilities( + tree->session->transport->conn); + if (!(caps & SMB2_CAP_LEASING)) { + torture_skip(tctx, "leases are not supported"); + } + + /* Ensure files don't exist. */ + smb2_util_unlink(tree, fname1); + smb2_util_unlink(tree, fname2); + + /* Create file1 - LEASE1 key. */ + smb2_lease_create(&io, &ls, false, fname1, LEASE1, + smb2_util_lease_state("RWH")); + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + h1 = io.out.file.handle; + CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE); + CHECK_LEASE(&io, "RWH", true, LEASE1, 0); + + /* Leave file1 open and leased. */ + + /* Create file2 - no lease. */ + smb2_lease_create(&io, NULL, false, fname2, 0, + smb2_util_lease_state("RWH")); + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_OK); + h2 = io.out.file.handle; + CHECK_CREATED(&io, CREATED, FILE_ATTRIBUTE_ARCHIVE); + /* Close it. */ + smb2_util_close(tree, h2); + + /* + * Try and open file2 with the same LEASE1 key - this should fail with. + * INVALID_PARAMETER. + */ + smb2_lease_create(&io, &ls, false, fname2, LEASE1, + smb2_util_lease_state("RWH")); + status = smb2_create(tree, mem_ctx, &io); + CHECK_STATUS(status, NT_STATUS_INVALID_PARAMETER); + /* + * If we did open this is an error, but save off + * the handle so we close below. + */ + h2 = io.out.file.handle; + +done: + smb2_util_close(tree, h2); + smb2_util_close(tree, h1); + smb2_util_unlink(tree, fname1); + smb2_util_unlink(tree, fname2); + talloc_free(mem_ctx); + return ret; +} + struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) { struct torture_suite *suite = @@ -4534,6 +4602,8 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) test_lease_rename_wait); torture_suite_add_1smb2_test(suite, "duplicate_create", test_lease_duplicate_create); + torture_suite_add_1smb2_test(suite, "duplicate_open", + test_lease_duplicate_open); suite->description = talloc_strdup(suite, "SMB2-LEASE tests"); -- 2.32.0 From 5c38b1a2935f3e6bd361702ae8d355d65b9ebb4d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 17 Feb 2022 11:12:39 -0800 Subject: [PATCH 3/3] s3: smbd: Fix our leases code to return the correct error in the non-dynamic share case. We now return INVALID_PARAMETER when trying to open a different file with a duplicate lease key on the same (non-dynamic) share. This will enable us to pass another Windows test suite leases test. We now behave the same as Windows10. Remove knownfail.d/smb2-lease-duplicateopen BUG: https://bugzilla.samba.org/show_bug.cgi?id=14737 Signed-off-by: Jeremy Allison Reviewed-by: David Mulder Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Feb 18 20:12:12 UTC 2022 on sn-devel-184 (cherry picked from commit 408be54323861c24b6377b804be4428cf45b471e) --- selftest/knownfail.d/smb2-lease-duplicateopen | 1 - source3/smbd/open.c | 38 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/smb2-lease-duplicateopen diff --git a/selftest/knownfail.d/smb2-lease-duplicateopen b/selftest/knownfail.d/smb2-lease-duplicateopen deleted file mode 100644 index 1336b02d74c..00000000000 --- a/selftest/knownfail.d/smb2-lease-duplicateopen +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.lease.duplicate_open\(nt4_dc\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index a5664b319ad..5a3ac2c064a 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -5302,8 +5302,42 @@ static void lease_match_parser( /* Everything should be the same. */ if (!file_id_equal(&state->id, &f->id)) { - /* This should catch all dynamic share cases. */ - state->match_status = NT_STATUS_OPLOCK_NOT_GRANTED; + /* + * The client asked for a lease on a + * file that doesn't match the file_id + * in the database. + * + * Maybe this is a dynamic share, i.e. + * a share where the servicepath is + * different for different users (e.g. + * the [HOMES] share. + * + * If the servicepath is different, but the requested + * file name + stream name is the same then this is + * a dynamic share, the client is using the same share + * name and doesn't know that the underlying servicepath + * is different. It was expecting a lease on the + * same file. Return NT_STATUS_OPLOCK_NOT_GRANTED + * to break leases + * + * Otherwise the client has messed up, or is + * testing our error codes, so return + * NT_STATUS_INVALID_PARAMETER. + */ + if (!strequal(f->servicepath, state->servicepath) && + strequal(f->base_name, state->fname->base_name) && + strequal(f->stream_name, state->fname->stream_name)) + { + /* + * Name is the same but servicepath is + * different, dynamic share. Break leases. + */ + state->match_status = + NT_STATUS_OPLOCK_NOT_GRANTED; + } else { + state->match_status = + NT_STATUS_INVALID_PARAMETER; + } break; } if (!strequal(f->servicepath, state->servicepath)) { -- 2.32.0