[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu Apr 30 21:11:03 UTC 2020


The branch, master has been updated
       via  62621bd686a smbd: use is_lease_stat_open() in delay_for_oplock()
       via  9b0a31af128 smbd: add is_lease_stat_open()
       via  00001286e1d smbd: rename is_stat_open() to is_oplock_stat_open()
       via  aa3b3e18a7b s4/torture: add a comprehensive "non-lease-break-trigger" access mask test case
       via  4371c62f2ce s4/torture: add a comprehensive "non-oplock-break-trigger" access mask test case
      from  d3be759aeea s3: smbd: Add dirfsp parameter to check_access().

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 62621bd686a91328ae378cd56f9876c66be8eac4
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Mar 5 15:16:07 2020 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu Apr 30 21:10:43 UTC 2020 on sn-devel-184

commit 9b0a31af128f9ec2551b6cca7200903d8e24a6f4
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Mar 5 15:14:21 2020 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 00001286e1d8bd583f676d380b9c335432b0f61d
Author: Ralph Boehme <slow at samba.org>
Date:   Thu Mar 5 15:12:20 2020 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit aa3b3e18a7b59da2e0498f1f38bffef25757ebb1
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 3 12:09:09 2020 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 4371c62f2ce4ca8f6a9ccbda1ed92a3676418d80
Author: Ralph Boehme <slow at samba.org>
Date:   Tue Mar 3 12:09:09 2020 +0100

    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 <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 selftest/knownfail            |   1 +
 source3/smbd/open.c           |  32 ++++++--
 source3/smbd/proto.h          |   3 +-
 source4/torture/smb2/lease.c  | 185 ++++++++++++++++++++++++++++++++++++++++++
 source4/torture/smb2/oplock.c | 155 +++++++++++++++++++++++++++++++++++
 5 files changed, 370 insertions(+), 6 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/knownfail b/selftest/knownfail
index 09276dd2261..6394a2957c3 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -170,6 +170,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/source3/smbd/open.c b/source3/smbd/open.c
index aa86cef944a..8383fe1df5c 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1563,7 +1563,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|
@@ -1574,6 +1577,21 @@ bool is_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;
 };
@@ -1771,7 +1789,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;
@@ -1928,7 +1946,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.
@@ -2353,6 +2371,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) {
@@ -2422,7 +2444,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;
 	}
 
@@ -3491,7 +3513,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 3d9ffaa6898..dadccf189bb 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -713,7 +713,8 @@ NTSTATUS fd_close(files_struct *fsp);
 void change_file_owner_to_parent(connection_struct *conn,
 				 struct smb_filename *inherit_from_dir,
 				 files_struct *fsp);
-bool is_stat_open(uint32_t access_mask);
+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,
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);
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;


-- 
Samba Shared Repository



More information about the samba-cvs mailing list