[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Tue Aug 30 17:11:01 UTC 2022


The branch, master has been updated
       via  e4929866610 s3: torture: Add a comprehensive SMB2 DFS path torture tester.
      from  772319412df smbd: fix opening a READ-ONLY file with SEC_FLAG_MAXIMUM_ALLOWED

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


- Log -----------------------------------------------------------------
commit e492986661039f2e8a1000529e21dc5b2061d5f6
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Aug 29 14:37:35 2022 -0700

    s3: torture: Add a comprehensive SMB2 DFS path torture tester.
    
    Passes fully against Windows.
    
    This shows that DFS paths on Windows on SMB2 must
    be of the form:
    
    SERVER\SHARE\PATH
    
    but the actual contents of the strings SERVER and
    SHARE don't need to match the given server or share.
    
    The algorithm the Windows server uses is the following:
    
    Look for a '\\' character, and assign anything before
    that to the SERVER component. The characters in this
    component are not checked for validity.
    
    Look for a second '\\' character and assign anything
    between the first and second '\\' characters to the
    SHARE component. The characters in the share component
    are checked for validity, but only ':' is flagged as
    an illegal sharename character despite what:
    
    [MS-FSCC] https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/dc9978d7-6299-4c5a-a22d-a039cdc716ea
    
    says.
    
    Anything after the second '\\' character is assigned
    to the PATH component and becomes the share-relative
    path.
    
    If there aren't two '\\' characters it removes
    everything and ends up with the empty string as
    the share relative path.
    
    To give some examples, the following pathnames all map
    to the directory at the root of the DFS share:
    
    SERVER\SHARE
    SERVER
    ""
    ANY\NAME
    ANY
    ::::\NAME
    
    the name:
    
    SERVER\:
    
    is illegal (sharename contains ':') and the name:
    
    ANY\NAME\file
    
    maps to a share-relative pathname of "file",
    despite "ANY" not being the server name, and
    "NAME" not being the DFS share name we are
    connected to.
    
    Adds a knownfail for smbd as our current code
    in parse_dfs_path() is completely incorrect
    here and tries to map "incorrect" DFS names
    into local paths. I will work on fixing this
    later, but we should be able to remove parse_dfs_path()
    entirely and move the DFS pathname logic before
    the call to filename_convert_dirfsp() in the
    same way Volker suggested and was able to achieve
    for extract_snapshot_token() and the @GMT pathname
    processing.
    
    Also proves the "target" paths for SMB2_SETINFO
    rename and hardlink must *not* be DFS-paths.
    
    Next I will work on a torture tester for SMB1
    DFS paths.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reivewed-by: Noel Power <npower at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Tue Aug 30 17:10:33 UTC 2022 on sn-devel-184

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

Summary of changes:
 selftest/knownfail.d/dfs_paths |   1 +
 selftest/target/Samba3.pm      |   7 +
 source3/selftest/tests.py      |  16 +
 source3/torture/proto.h        |   1 +
 source3/torture/test_smb2.c    | 727 +++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c      |   4 +
 6 files changed, 756 insertions(+)
 create mode 100644 selftest/knownfail.d/dfs_paths


Changeset truncated at 500 lines:

diff --git a/selftest/knownfail.d/dfs_paths b/selftest/knownfail.d/dfs_paths
new file mode 100644
index 00000000000..a65e114eade
--- /dev/null
+++ b/selftest/knownfail.d/dfs_paths
@@ -0,0 +1 @@
+^samba3.smbtorture_s3.smb2.SMB2-DFS-PATHS.smbtorture\(fileserver\)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index d413f14bacd..7c7735f6f4d 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2534,6 +2534,9 @@ sub provision($$)
 	my $msdfs_shrdir2="$shrdir/msdfsshare2";
 	push(@dirs,$msdfs_shrdir2);
 
+	my $msdfs_pathname_share="$shrdir/msdfs_pathname_share";
+	push(@dirs,$msdfs_pathname_share);
+
 	my $msdfs_deeppath="$msdfs_shrdir/deeppath";
 	push(@dirs,$msdfs_deeppath);
 
@@ -2961,6 +2964,10 @@ sub provision($$)
 	path = $msdfs_shrdir2
 	msdfs root = yes
 	guest ok = yes
+[msdfs-pathname-share]
+	path = $msdfs_pathname_share
+	msdfs root = yes
+	guest ok = yes
 [hideunread]
 	copy = tmp
 	hide unreadable = yes
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index afb326029dc..a11b6d1d10b 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -229,6 +229,22 @@ plantestsuite("samba3.smbtorture_s3.smb1.MSDFS-ATTRIBUTE",
                 "-mNT1",
                 "-f msdfs-src1"])
 
+#
+# SMB2-DFS-PATHS needs to run against a special share msdfs-pathname-share
+# This is an empty DFS share with no links, used merely to test
+# incoming DFS pathnames and how they map to local paths.
+#
+plantestsuite("samba3.smbtorture_s3.smb2.SMB2-DFS-PATHS",
+                "fileserver",
+                [os.path.join(samba3srcdir,
+                              "script/tests/test_smbtorture_s3.sh"),
+                'SMB2-DFS-PATHS',
+                '//$SERVER_IP/msdfs-pathname-share',
+                '$USERNAME',
+                '$PASSWORD',
+                smbtorture3,
+                "-mSMB2"])
+
 #
 # SMB2-STREAM-ACL needs to run against a special share - vfs_wo_fruit
 #
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 551c4ea80ac..bb47c7a43a0 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -120,6 +120,7 @@ bool run_smb2_path_slash(int dummy);
 bool run_smb2_sacl(int dummy);
 bool run_smb2_quota1(int dummy);
 bool run_smb2_stream_acl(int dummy);
+bool run_smb2_dfs_paths(int dummy);
 bool run_list_dir_async_test(int dummy);
 bool run_delete_on_close_non_empty(int dummy);
 bool run_delete_on_close_nonwrite_delete_yes_test(int dummy);
diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
index c3f014100d9..cdd66d4f449 100644
--- a/source3/torture/test_smb2.c
+++ b/source3/torture/test_smb2.c
@@ -3608,3 +3608,730 @@ bool run_delete_on_close_nonwrite_delete_no_test(int dummy)
 	}
 	return ret;
 }
+
+/*
+ * Open an SMB2 file readonly and return the inode number.
+ */
+static NTSTATUS get_smb2_inode(struct cli_state *cli,
+				const char *pathname,
+				uint64_t *ino_ret)
+{
+	NTSTATUS status;
+	uint64_t fid_persistent = 0;
+	uint64_t fid_volatile = 0;
+	DATA_BLOB outbuf = data_blob_null;
+	/*
+	 * Open the file.
+	 */
+	status = smb2cli_create(cli->conn,
+				cli->timeout,
+				cli->smb2.session,
+				cli->smb2.tcon,
+				pathname,
+				SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */
+				SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */
+				SEC_STD_SYNCHRONIZE|
+					SEC_FILE_READ_DATA|
+					SEC_FILE_READ_ATTRIBUTE, /* desired_access, */
+				FILE_ATTRIBUTE_NORMAL, /* file_attributes, */
+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */
+				FILE_OPEN, /* create_disposition, */
+				0, /* create_options, */
+				NULL, /* smb2_create_blobs *blobs */
+				&fid_persistent,
+				&fid_volatile,
+				NULL, /* struct smb_create_returns * */
+				talloc_tos(), /* mem_ctx. */
+				NULL); /* struct smb2_create_blobs * */
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	/*
+	 * Get the inode.
+	 */
+	status = smb2cli_query_info(cli->conn,
+				    cli->timeout,
+				    cli->smb2.session,
+				    cli->smb2.tcon,
+				    SMB2_0_INFO_FILE,
+				    (SMB_FILE_ALL_INFORMATION - 1000), /* in_file_info_class */
+				    1024, /* in_max_output_length */
+				    NULL, /* in_input_buffer */
+				    0, /* in_additional_info */
+				    0, /* in_flags */
+				    fid_persistent,
+				    fid_volatile,
+				    talloc_tos(),
+				    &outbuf);
+
+	if (NT_STATUS_IS_OK(status)) {
+		*ino_ret = PULL_LE_U64(outbuf.data, 0x40);
+	}
+
+	(void)smb2cli_close(cli->conn,
+			    cli->timeout,
+			    cli->smb2.session,
+			    cli->smb2.tcon,
+			    0,
+			    fid_persistent,
+			    fid_volatile);
+	return status;
+}
+
+/*
+ * Check an inode matches a given SMB2 path.
+ */
+static bool smb2_inode_matches(struct cli_state *cli,
+				const char *match_pathname,
+				uint64_t ino_tomatch,
+				const char *test_pathname)
+{
+	uint64_t test_ino = 0;
+	NTSTATUS status;
+
+	status = get_smb2_inode(cli,
+				test_pathname,
+				&test_ino);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("%s: Failed to get ino "
+			"number for %s, (%s)\n",
+			__func__,
+			test_pathname,
+			nt_errstr(status));
+		return false;
+	}
+	if (test_ino != ino_tomatch) {
+		printf("%s: Inode missmatch, ino_tomatch (%s) "
+			"ino=%"PRIu64" test (%s) "
+			"ino=%"PRIu64"\n",
+			__func__,
+			match_pathname,
+			ino_tomatch,
+			test_pathname,
+			test_ino);
+		return false;
+	}
+	return true;
+}
+
+/*
+ * Delete an SMB2 file on a DFS share.
+ */
+static NTSTATUS smb2_dfs_delete(struct cli_state *cli,
+				const char *pathname)
+{
+	NTSTATUS status;
+	uint64_t fid_persistent = 0;
+	uint64_t fid_volatile = 0;
+	uint8_t data[1];
+	DATA_BLOB inbuf;
+
+	/*
+	 * Open the file.
+	 */
+	status = smb2cli_create(cli->conn,
+				cli->timeout,
+				cli->smb2.session,
+				cli->smb2.tcon,
+				pathname,
+				SMB2_OPLOCK_LEVEL_NONE, /* oplock_level, */
+				SMB2_IMPERSONATION_IMPERSONATION, /* impersonation_level, */
+				SEC_STD_SYNCHRONIZE|
+					SEC_STD_DELETE, /* desired_access, */
+				FILE_ATTRIBUTE_NORMAL, /* file_attributes, */
+				FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access, */
+				FILE_OPEN, /* create_disposition, */
+				0, /* create_options, */
+				NULL, /* smb2_create_blobs *blobs */
+				&fid_persistent,
+				&fid_volatile,
+				NULL, /* struct smb_create_returns * */
+				talloc_tos(), /* mem_ctx. */
+				NULL); /* struct smb2_create_blobs * */
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	/*
+	 * Set delete on close.
+	 */
+	PUSH_LE_U8(&data[0], 0, 1);
+	inbuf.data = &data[0];
+	inbuf.length = 1;
+
+	status = smb2cli_set_info(cli->conn,
+				  cli->timeout,
+				  cli->smb2.session,
+				  cli->smb2.tcon,
+				  SMB2_0_INFO_FILE, /* info_type. */
+				  SMB_FILE_DISPOSITION_INFORMATION - 1000, /* info_class */
+				  &inbuf,
+				  0, /* additional_info. */
+				  fid_persistent,
+				  fid_volatile);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+	status = smb2cli_close(cli->conn,
+			       cli->timeout,
+			       cli->smb2.session,
+			       cli->smb2.tcon,
+			       0,
+			       fid_persistent,
+			       fid_volatile);
+	return status;
+}
+
+/*
+ * Rename or hardlink an SMB2 file on a DFS share.
+ */
+static NTSTATUS smb2_dfs_setinfo_name(struct cli_state *cli,
+				      uint64_t fid_persistent,
+				      uint64_t fid_volatile,
+				      const char *newname,
+				      bool do_rename)
+{
+	NTSTATUS status;
+	DATA_BLOB inbuf;
+	smb_ucs2_t *converted_str = NULL;
+	size_t converted_size_bytes = 0;
+	size_t inbuf_size;
+	uint8_t info_class = 0;
+	bool ok;
+
+	ok = push_ucs2_talloc(talloc_tos(),
+			      &converted_str,
+			      newname,
+			      &converted_size_bytes);
+	if (!ok) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+	/*
+	 * W2K8 insists the dest name is not null terminated. Remove
+	 * the last 2 zero bytes and reduce the name length.
+	 */
+	if (converted_size_bytes < 2) {
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+	converted_size_bytes -= 2;
+	inbuf_size = 20 + converted_size_bytes;
+	if (inbuf_size < 20) {
+		/* Integer wrap check. */
+		return NT_STATUS_INVALID_PARAMETER;
+	}
+
+	/*
+	 * The Windows 10 SMB2 server has a minimum length
+	 * for a SMB2_FILE_RENAME_INFORMATION buffer of
+	 * 24 bytes. It returns NT_STATUS_INFO_LENGTH_MISMATCH
+	 * if the length is less.
+	 */
+	inbuf_size = MAX(inbuf_size, 24);
+	inbuf = data_blob_talloc_zero(talloc_tos(), inbuf_size);
+	if (inbuf.data == NULL) {
+		return NT_STATUS_NO_MEMORY;
+        }
+	PUSH_LE_U32(inbuf.data, 16, converted_size_bytes);
+	memcpy(inbuf.data + 20, converted_str, converted_size_bytes);
+	TALLOC_FREE(converted_str);
+
+	if (do_rename == true) {
+		info_class = SMB_FILE_RENAME_INFORMATION - 1000;
+	} else {
+		/* Hardlink. */
+		info_class = SMB_FILE_LINK_INFORMATION - 1000;
+	}
+
+	status = smb2cli_set_info(cli->conn,
+				  cli->timeout,
+				  cli->smb2.session,
+				  cli->smb2.tcon,
+				  SMB2_0_INFO_FILE, /* info_type. */
+				  info_class, /* info_class */
+				  &inbuf,
+				  0, /* additional_info. */
+				  fid_persistent,
+				  fid_volatile);
+	return status;
+}
+
+static NTSTATUS smb2_dfs_rename(struct cli_state *cli,
+				      uint64_t fid_persistent,
+				      uint64_t fid_volatile,
+				      const char *newname)
+{
+	return smb2_dfs_setinfo_name(cli,
+				     fid_persistent,
+				     fid_volatile,
+				     newname,
+				     true); /* do_rename */
+}
+
+static NTSTATUS smb2_dfs_hlink(struct cli_state *cli,
+			       uint64_t fid_persistent,
+			       uint64_t fid_volatile,
+			       const char *newname)
+{
+	return smb2_dfs_setinfo_name(cli,
+				     fid_persistent,
+				     fid_volatile,
+				     newname,
+				     false); /* do_rename */
+}
+
+/*
+ * According to:
+
+ * https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/dc9978d7-6299-4c5a-a22d-a039cdc716ea
+ *
+ *  (Characters " \ / [ ] : | < > + = ; , * ?,
+ *  and control characters in range 0x00 through
+ *  0x1F, inclusive, are illegal in a share name)
+ *
+ * But Windows server only checks in DFS sharenames ':'. All other
+ * share names are allowed.
+ */
+
+static bool test_smb2_dfs_sharenames(struct cli_state *cli,
+				     const char *dfs_root_share_name,
+				     uint64_t root_ino)
+{
+	char test_path[9];
+	const char *test_str = "/[]:|<>+=;,*?";
+	const char *p;
+	unsigned int i;
+	bool ino_matched = false;
+
+	/* Setup template pathname. */
+	memcpy(test_path, "SERVER\\X", 9);
+
+	/* Test invalid control characters. */
+	for (i = 1; i < 0x20; i++) {
+		test_path[7] = i;
+		ino_matched = smb2_inode_matches(cli,
+					 dfs_root_share_name,
+					 root_ino,
+					 test_path);
+		if (!ino_matched) {
+			return false;
+		}
+	}
+
+	/* Test explicit invalid characters. */
+	for (p = test_str; *p != '\0'; p++) {
+		test_path[7] = *p;
+		if (*p == ':') {
+			/*
+			 * Only ':' is treated as an INVALID sharename
+			 * for a DFS SERVER\\SHARE path.
+			 */
+			uint64_t test_ino = 0;
+			NTSTATUS status = get_smb2_inode(cli,
+							 test_path,
+							 &test_ino);
+			if (!NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_INVALID)) {
+				printf("%s:%d Open of %s should get "
+					"NT_STATUS_OBJECT_NAME_INVALID, got %s\n",
+					__FILE__,
+					__LINE__,
+					test_path,
+					nt_errstr(status));
+				return false;
+			}
+		} else {
+			ino_matched = smb2_inode_matches(cli,
+						 dfs_root_share_name,
+						 root_ino,
+						 test_path);
+			if (!ino_matched) {
+				return false;
+			}
+		}
+	}
+	return true;
+}
+
+/*
+ * "Raw" test of SMB2 paths to a DFS share.
+ * We must use the lower level smb2cli_XXXX() interfaces,
+ * not the cli_XXX() ones here as the ultimate goal is to fix our
+ * cli_XXX() interfaces to work transparently over DFS.
+ *
+ * So here, we're testing the server code, not the client code.
+ *
+ * Passes cleanly against Windows.
+ */
+
+bool run_smb2_dfs_paths(int dummy)
+{
+	struct cli_state *cli = NULL;
+	NTSTATUS status;
+	bool dfs_supported = false;
+	char *dfs_root_share_name = NULL;
+	uint64_t root_ino = 0;
+	uint64_t test_ino = 0;
+	bool ino_matched = false;
+	uint64_t fid_persistent = 0;
+	uint64_t fid_volatile = 0;
+	bool retval = false;
+	bool ok = false;
+
+	printf("Starting SMB2-DFS-PATHS\n");
+
+	if (!torture_init_connection(&cli)) {
+		return false;
+	}
+
+	status = smbXcli_negprot(cli->conn,
+				cli->timeout,
+				PROTOCOL_SMB2_02,
+				PROTOCOL_SMB3_11);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("smbXcli_negprot returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	status = cli_session_setup_creds(cli, torture_creds);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_session_setup returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	status = cli_tree_connect(cli, share, "?????", NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_tree_connect returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	/* Ensure this is a DFS share. */
+	dfs_supported = smbXcli_conn_dfs_supported(cli->conn);
+	if (!dfs_supported) {
+		printf("Server %s does not support DFS\n",
+			smbXcli_conn_remote_name(cli->conn));
+		return false;
+	}
+	dfs_supported = smbXcli_tcon_is_dfs_share(cli->smb2.tcon);
+	if (!dfs_supported) {
+		printf("Share %s does not support DFS\n",
+			cli->share);
+		return false;
+	}
+	/*
+	 * Create the "official" DFS share root name.
+	 * No SMB2 paths can start with '\\'.
+	 */
+	dfs_root_share_name = talloc_asprintf(talloc_tos(),
+					"%s\\%s",
+					smbXcli_conn_remote_name(cli->conn),
+					cli->share);
+	if (dfs_root_share_name == NULL) {
+		printf("Out of memory\n");
+		return false;
+	}


-- 
Samba Shared Repository



More information about the samba-cvs mailing list