From 7f39d8577550f695327dc29b806b29373d92c615 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 3 Mar 2020 12:09:09 +0100 Subject: [PATCH 1/5] s4/torture: add a comprehensive "non-oplock-break-trigger" access mask test case BUG: https://bugzilla.samba.org/show_bug.cgi?id=14357 Signed-off-by: Ralph Boehme --- selftest/knownfail | 1 + source4/torture/smb2/oplock.c | 155 ++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 1e2deab5f79..ecee59f0c08 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -169,6 +169,7 @@ ^samba4.smb2.oplock.batch20\(.*\)$ # samba 4 oplocks are a mess ^samba4.smb2.oplock.batch26\(.*\)$ ^samba4.smb2.oplock.stream1 # samba 4 oplocks are a mess +^samba4.smb2.oplock.statopen1\(ad_dc_ntvfs\)$ # fails with ACCESS_DENIED on a SYNCHRONIZE_ACCESS open ^samba4.smb2.getinfo.complex # streams on directories does not work ^samba4.smb2.getinfo.qfs_buffercheck # S4 does not do the INFO_LENGTH_MISMATCH/BUFFER_OVERFLOW thingy ^samba4.smb2.getinfo.qfile_buffercheck # S4 does not do the INFO_LENGTH_MISMATCH/BUFFER_OVERFLOW thingy diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c index 30bbd92e7ec..72c63353533 100644 --- a/source4/torture/smb2/oplock.c +++ b/source4/torture/smb2/oplock.c @@ -38,6 +38,7 @@ #include "torture/smb2/proto.h" #include "lib/util/sys_rw.h" +#include "libcli/security/security.h" #define CHECK_RANGE(v, min, max) do { \ if ((v) < (min) || (v) > (max)) { \ @@ -3968,6 +3969,159 @@ static bool test_smb2_oplock_levelII502(struct torture_context *tctx, return true; } +static bool test_oplock_statopen1_do(struct torture_context *tctx, + struct smb2_tree *tree, + uint32_t access_mask, + bool expect_stat_open) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + struct smb2_create cr; + struct smb2_handle h1 = {{0}}; + struct smb2_handle h2 = {{0}}; + NTSTATUS status; + const char *fname = "oplock_statopen1.dat"; + bool ret = true; + + /* Open file with exclusive oplock. */ + cr = (struct smb2_create) { + .in.desired_access = SEC_FILE_ALL, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.create_disposition = NTCREATEX_DISP_OPEN_IF, + .in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION, + .in.fname = fname, + .in.oplock_level = SMB2_OPLOCK_LEVEL_BATCH, + }; + status = smb2_create(tree, mem_ctx, &cr); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h1 = cr.out.file.handle; + CHECK_VAL(cr.out.oplock_level, SMB2_OPLOCK_LEVEL_BATCH); + + /* Stat open */ + cr = (struct smb2_create) { + .in.desired_access = access_mask, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION, + .in.fname = fname, + }; + status = smb2_create(tree, mem_ctx, &cr); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h2 = cr.out.file.handle; + + if (expect_stat_open) { + torture_wait_for_oplock_break(tctx); + CHECK_VAL(break_info.count, 0); + CHECK_VAL(break_info.level, 0); + CHECK_VAL(break_info.failures, 0); + if (!ret) { + goto done; + } + } else { + CHECK_VAL(break_info.count, 1); + } + +done: + if (!smb2_util_handle_empty(h2)) { + smb2_util_close(tree, h2); + } + if (!smb2_util_handle_empty(h1)) { + smb2_util_close(tree, h1); + } + talloc_free(mem_ctx); + return ret; +} + +static bool test_smb2_oplock_statopen1(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "oplock_statopen1.dat"; + size_t i; + bool ret = true; + struct { + uint32_t access_mask; + bool expect_stat_open; + } tests[] = { + { + .access_mask = FILE_READ_DATA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_WRITE_DATA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_READ_EA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_WRITE_EA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_EXECUTE, + .expect_stat_open = false, + }, + { + .access_mask = FILE_READ_ATTRIBUTES, + .expect_stat_open = true, + }, + { + .access_mask = FILE_WRITE_ATTRIBUTES, + .expect_stat_open = true, + }, + { + .access_mask = DELETE_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = READ_CONTROL_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = WRITE_DAC_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = WRITE_OWNER_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = SYNCHRONIZE_ACCESS, + .expect_stat_open = true, + }, + }; + + tree->session->transport->oplock.handler = torture_oplock_handler; + tree->session->transport->oplock.private_data = tree; + + for (i = 0; i < ARRAY_SIZE(tests); i++) { + ZERO_STRUCT(break_info); + + ret = test_oplock_statopen1_do(tctx, + tree, + tests[i].access_mask, + tests[i].expect_stat_open); + if (ret == true) { + continue; + } + torture_result(tctx, TORTURE_FAIL, + "test %zu: access_mask: %s, " + "expect_stat_open: %s\n", + i, + get_sec_mask_str(tree, tests[i].access_mask), + tests[i].expect_stat_open ? "yes" : "no"); + goto done; + } + +done: + smb2_util_unlink(tree, fname); + return ret; +} + struct torture_suite *torture_smb2_oplocks_init(TALLOC_CTX *ctx) { struct torture_suite *suite = @@ -4016,6 +4170,7 @@ struct torture_suite *torture_smb2_oplocks_init(TALLOC_CTX *ctx) test_smb2_oplock_levelII501); torture_suite_add_2smb2_test(suite, "levelii502", test_smb2_oplock_levelII502); + torture_suite_add_1smb2_test(suite, "statopen1", test_smb2_oplock_statopen1); suite->description = talloc_strdup(suite, "SMB2-OPLOCK tests"); return suite; -- GitLab From 8937e02d2d20a46e295a292e0106de2f93e9653d Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 3 Mar 2020 12:09:09 +0100 Subject: [PATCH 2/5] s4/torture: add a comprehensive "non-lease-break-trigger" access mask test case BUG: https://bugzilla.samba.org/show_bug.cgi?id=14357 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.smb2.lease | 1 + source4/torture/smb2/lease.c | 185 +++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 selftest/knownfail.d/samba3.smb2.lease diff --git a/selftest/knownfail.d/samba3.smb2.lease b/selftest/knownfail.d/samba3.smb2.lease new file mode 100644 index 00000000000..079d4438b8c --- /dev/null +++ b/selftest/knownfail.d/samba3.smb2.lease @@ -0,0 +1 @@ +^samba3.smb2.lease.statopen4\(nt4_dc\) diff --git a/source4/torture/smb2/lease.c b/source4/torture/smb2/lease.c index 430fcd0a6e1..d3b8daea310 100644 --- a/source4/torture/smb2/lease.c +++ b/source4/torture/smb2/lease.c @@ -954,6 +954,190 @@ done: return ret; } +static bool test_lease_statopen4_do(struct torture_context *tctx, + struct smb2_tree *tree, + uint32_t access_mask, + bool expect_stat_open) +{ + 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}}; + struct smb2_handle h3 = {{0}}; + NTSTATUS status; + const char *fname = "lease_statopen2.dat"; + bool ret = true; + + /* Open file with RWH lease. */ + smb2_lease_create_share(&io, &ls, false, fname, + smb2_util_share_access("RWD"), + LEASE1, + smb2_util_lease_state("RWH")); + io.in.desired_access = SEC_FILE_ALL; + status = smb2_create(tree, mem_ctx, &io); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h1 = io.out.file.handle; + CHECK_LEASE(&io, "RWH", true, LEASE1, 0); + + /* Stat open */ + ZERO_STRUCT(io); + io.in.desired_access = access_mask; + io.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + io.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + io.in.create_disposition = NTCREATEX_DISP_OPEN; + io.in.fname = fname; + status = smb2_create(tree, mem_ctx, &io); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h2 = io.out.file.handle; + + if (expect_stat_open) { + CHECK_NO_BREAK(tctx); + if (!ret) { + goto done; + } + } else { + CHECK_VAL(lease_break_info.count, 1); + if (!ret) { + goto done; + } + /* + * Don't bother checking the lease state of an additional open + * below... + */ + goto done; + } + + /* Open file with RWH lease. */ + smb2_lease_create_share(&io, &ls, false, fname, + smb2_util_share_access("RWD"), + LEASE1, + smb2_util_lease_state("RWH")); + io.in.desired_access = SEC_FILE_WRITE_DATA; + status = smb2_create(tree, mem_ctx, &io); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + h3 = io.out.file.handle; + CHECK_LEASE(&io, "RWH", true, LEASE1, 0); + +done: + if (!smb2_util_handle_empty(h3)) { + smb2_util_close(tree, h3); + } + if (!smb2_util_handle_empty(h2)) { + smb2_util_close(tree, h2); + } + if (!smb2_util_handle_empty(h1)) { + smb2_util_close(tree, h1); + } + talloc_free(mem_ctx); + return ret; +} + +static bool test_lease_statopen4(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "lease_statopen4.dat"; + struct smb2_handle h1 = {{0}}; + uint32_t caps; + size_t i; + NTSTATUS status; + bool ret = true; + struct { + uint32_t access_mask; + bool expect_stat_open; + } tests[] = { + { + .access_mask = FILE_READ_DATA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_WRITE_DATA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_READ_EA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_WRITE_EA, + .expect_stat_open = false, + }, + { + .access_mask = FILE_EXECUTE, + .expect_stat_open = false, + }, + { + .access_mask = FILE_READ_ATTRIBUTES, + .expect_stat_open = true, + }, + { + .access_mask = FILE_WRITE_ATTRIBUTES, + .expect_stat_open = true, + }, + { + .access_mask = DELETE_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = READ_CONTROL_ACCESS, + .expect_stat_open = true, + }, + { + .access_mask = WRITE_DAC_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = WRITE_OWNER_ACCESS, + .expect_stat_open = false, + }, + { + .access_mask = SYNCHRONIZE_ACCESS, + .expect_stat_open = true, + }, + }; + + caps = smb2cli_conn_server_capabilities(tree->session->transport->conn); + if (!(caps & SMB2_CAP_LEASING)) { + torture_skip(tctx, "leases are not supported"); + } + + smb2_util_unlink(tree, fname); + tree->session->transport->lease.handler = torture_lease_handler; + tree->session->transport->lease.private_data = tree; + + status = torture_smb2_testfile(tree, fname, &h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + smb2_util_close(tree, h1); + ZERO_STRUCT(h1); + + for (i = 0; i < ARRAY_SIZE(tests); i++) { + ZERO_STRUCT(lease_break_info); + + ret = test_lease_statopen4_do(tctx, + tree, + tests[i].access_mask, + tests[i].expect_stat_open); + if (ret == true) { + continue; + } + torture_result(tctx, TORTURE_FAIL, + "test %zu: access_mask: %s, " + "expect_stat_open: %s\n", + i, + get_sec_mask_str(tree, tests[i].access_mask), + tests[i].expect_stat_open ? "yes" : "no"); + goto done; + } + +done: + smb2_util_unlink(tree, fname); + return ret; +} + static void torture_oplock_break_callback(struct smb2_request *req) { NTSTATUS status; @@ -3980,6 +4164,7 @@ struct torture_suite *torture_smb2_lease_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "statopen", test_lease_statopen); torture_suite_add_1smb2_test(suite, "statopen2", test_lease_statopen2); torture_suite_add_1smb2_test(suite, "statopen3", test_lease_statopen3); + torture_suite_add_1smb2_test(suite, "statopen4", test_lease_statopen4); torture_suite_add_1smb2_test(suite, "upgrade", test_lease_upgrade); torture_suite_add_1smb2_test(suite, "upgrade2", test_lease_upgrade2); torture_suite_add_1smb2_test(suite, "upgrade3", test_lease_upgrade3); -- GitLab From ccfeb02d1b3348937e42529f753251f9d12e5627 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 5 Mar 2020 15:12:20 +0100 Subject: [PATCH 3/5] smbd: rename is_stat_open() to is_oplock_stat_open() Testing stat opens with with leases reveals that that the access mask SYNCHRONIZE_ACCESS | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES is specific to oplocks. See also: https://lists.samba.org/archive/cifs-protocol/2020-March/003409.html BUG: https://bugzilla.samba.org/show_bug.cgi?id=14357 Signed-off-by: Ralph Boehme --- source3/smbd/open.c | 13 ++++++++----- source3/smbd/proto.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ecb46d75215..126cd5aaf6e 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1603,7 +1603,10 @@ static bool validate_my_share_entries_fn( } #endif -bool is_stat_open(uint32_t access_mask) +/** + * Allowed access mask for stat opens relevant to oplocks + **/ +bool is_oplock_stat_open(uint32_t access_mask) { const uint32_t stat_open_bits = (SYNCHRONIZE_ACCESS| @@ -1811,7 +1814,7 @@ static NTSTATUS open_mode_check(connection_struct *conn, uint16_t new_flags; bool ok, conflict, have_share_entries; - if (is_stat_open(access_mask)) { + if (is_oplock_stat_open(access_mask)) { /* Stat open that doesn't trigger oplock breaks or share mode * checks... ! JRA. */ return NT_STATUS_OK; @@ -1968,7 +1971,7 @@ static bool validate_oplock_types_fn( return false; } - if (e->op_type == NO_OPLOCK && is_stat_open(e->access_mask)) { + if (e->op_type == NO_OPLOCK && is_oplock_stat_open(e->access_mask)) { /* * We ignore stat opens in the table - they always * have NO_OPLOCK and never get or cause breaks. JRA. @@ -2462,7 +2465,7 @@ static NTSTATUS delay_for_oplock(files_struct *fsp, NTSTATUS status; bool ok; - if (is_stat_open(fsp->access_mask)) { + if (is_oplock_stat_open(fsp->access_mask)) { goto grant; } @@ -3528,7 +3531,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, * FILE_OVERWRITE and FILE_OVERWRITE_IF add in O_TRUNC, * which adds FILE_WRITE_DATA to open_access_mask. */ - if (is_stat_open(open_access_mask) && lease == NULL) { + if (is_oplock_stat_open(open_access_mask) && lease == NULL) { oplock_request = NO_OPLOCK; } } diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index cb32ebaca5e..faba6e35918 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -703,7 +703,7 @@ NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, files_struct *fsp); -bool is_stat_open(uint32_t access_mask); +bool is_oplock_stat_open(uint32_t access_mask); NTSTATUS send_break_message(struct messaging_context *msg_ctx, const struct file_id *id, const struct share_mode_entry *exclusive, -- GitLab From fa7dc25620cbb6ee2e0a66d36d2caa476fa89dfa Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 5 Mar 2020 15:14:21 +0100 Subject: [PATCH 4/5] smbd: add is_lease_stat_open() This adds a leases specific stat opens access mask check function. See also: https://lists.samba.org/archive/cifs-protocol/2020-March/003409.html BUG: https://bugzilla.samba.org/show_bug.cgi?id=14357 Signed-off-by: Ralph Boehme --- source3/smbd/open.c | 15 +++++++++++++++ source3/smbd/proto.h | 1 + 2 files changed, 16 insertions(+) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 126cd5aaf6e..5b2e7fb7d0c 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1617,6 +1617,21 @@ bool is_oplock_stat_open(uint32_t access_mask) ((access_mask & ~stat_open_bits) == 0)); } +/** + * Allowed access mask for stat opens relevant to leases + **/ +bool is_lease_stat_open(uint32_t access_mask) +{ + const uint32_t stat_open_bits = + (SYNCHRONIZE_ACCESS| + FILE_READ_ATTRIBUTES| + FILE_WRITE_ATTRIBUTES| + READ_CONTROL_ACCESS); + + return (((access_mask & stat_open_bits) != 0) && + ((access_mask & ~stat_open_bits) == 0)); +} + struct has_delete_on_close_state { bool ret; }; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index faba6e35918..faf0a301feb 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -704,6 +704,7 @@ void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, files_struct *fsp); bool is_oplock_stat_open(uint32_t access_mask); +bool is_lease_stat_open(uint32_t access_mask); NTSTATUS send_break_message(struct messaging_context *msg_ctx, const struct file_id *id, const struct share_mode_entry *exclusive, -- GitLab From cf5a320bdf95db5ac469e354a46bd4c95f8cbf06 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 5 Mar 2020 15:16:07 +0100 Subject: [PATCH 5/5] smbd: use is_lease_stat_open() in delay_for_oplock() This allows READ_CONTROL_ACCESS in the access mask as stat open if a file has only leases. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14357 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.smb2.lease | 1 - source3/smbd/open.c | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/samba3.smb2.lease diff --git a/selftest/knownfail.d/samba3.smb2.lease b/selftest/knownfail.d/samba3.smb2.lease deleted file mode 100644 index 079d4438b8c..00000000000 --- a/selftest/knownfail.d/samba3.smb2.lease +++ /dev/null @@ -1 +0,0 @@ -^samba3.smb2.lease.statopen4\(nt4_dc\) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 5b2e7fb7d0c..1b934d73265 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2411,6 +2411,10 @@ static bool delay_for_oplock_fn( state->have_other_lease = true; } + if (e_is_lease && is_lease_stat_open(fsp->access_mask)) { + return false; + } + break_to = e_lease_type & ~state->delay_mask; if (state->will_overwrite) { -- GitLab