[SCM] Samba Shared Repository - branch master updated
Ralph Böhme
slow at samba.org
Tue Aug 23 12:55:02 UTC 2022
The branch, master has been updated
via 6d493a9d568 smbd: implement access checks for SMB2-GETINFO as per MS-SMB2 3.3.5.20.1
via 9b2d2815710 smbtorture: check required access for SMB2-GETINFO
via 66e40690bdd s4/libcli/smb2: avoid using smb2_composite_setpathinfo() in smb2_util_setatr()
from 339e78f2075 gitlab-ci: Add a shellcheck runner
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 6d493a9d568c08cfe5242821ccbd5a5ee1fe5284
Author: Ralph Boehme <slow at samba.org>
Date: Sun Aug 14 18:46:24 2022 +0200
smbd: implement access checks for SMB2-GETINFO as per MS-SMB2 3.3.5.20.1
The spec lists the following as requiring special access:
- for requiring FILE_READ_ATTRIBUTES:
FileBasicInformation
FileAllInformation
FileNetworkOpenInformation
FileAttributeTagInformation
- for requiring FILE_READ_EA:
FileFullEaInformation
All other infolevels are unrestricted.
We ignore the IPC related infolevels:
FilePipeInformation
FilePipeLocalInformation
FilePipeRemoteInformation
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15153
RN: Missing SMB2-GETINFO access checks from MS-SMB2 3.3.5.20.1
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
Autobuild-User(master): Ralph Böhme <slow at samba.org>
Autobuild-Date(master): Tue Aug 23 12:54:08 UTC 2022 on sn-devel-184
commit 9b2d28157107602fcbe659664cf9ca25f08bb30b
Author: Ralph Boehme <slow at samba.org>
Date: Fri Aug 19 17:29:55 2022 +0200
smbtorture: check required access for SMB2-GETINFO
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15153
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
commit 66e40690bdd41800a01333ce4243bd62ee2b1894
Author: Ralph Boehme <slow at samba.org>
Date: Sun Aug 14 18:51:30 2022 +0200
s4/libcli/smb2: avoid using smb2_composite_setpathinfo() in smb2_util_setatr()
smb2_composite_setpathinfo() uses SEC_FLAG_MAXIMUM_ALLOWED which can
have unwanted side effects like breaking oplocks if the effective access
includes [READ|WRITE]_DATA.
For changing the DOS attributes we only need SEC_FILE_WRITE_ATTRIBUTE. With this
change test_smb2_oplock_batch25() doesn't trigger an oplock break anymore.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15153
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
-----------------------------------------------------------------------
Summary of changes:
selftest/knownfail | 3 +-
source3/smbd/smb2_getinfo.c | 28 ++++++++
source4/libcli/smb2/util.c | 37 +++++++++--
source4/torture/smb2/getinfo.c | 147 +++++++++++++++++++++++++++++++++++++++++
source4/torture/smb2/oplock.c | 10 +--
5 files changed, 208 insertions(+), 17 deletions(-)
Changeset truncated at 500 lines:
diff --git a/selftest/knownfail b/selftest/knownfail
index 0b4c5a44a7f..82dd7e1e8b4 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -176,6 +176,7 @@
^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.getinfo_access\(ad_dc_ntvfs\) # Access checks not implemented
^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
^samba4.smb2.getinfo.qsec_buffercheck # S4 does not do the BUFFER_TOO_SMALL thingy
@@ -207,10 +208,8 @@
^samba3.smb2.oplock.stream1
^samba3.smb2.streams.rename
^samba3.smb2.streams.rename2
-^samba3.smb2.streams.attributes1\(.*\)
^samba3.smb2.streams streams_xattr.rename\(nt4_dc\)
^samba3.smb2.streams streams_xattr.rename2\(nt4_dc\)
-^samba3.smb2.streams streams_xattr.attributes1\(nt4_dc\)
^samba3.smb2.getinfo.complex
^samba3.smb2.getinfo.fsinfo # quotas don't work yet
^samba3.smb2.setinfo.setinfo
diff --git a/source3/smbd/smb2_getinfo.c b/source3/smbd/smb2_getinfo.c
index 0320dcc5fde..23322e7b85f 100644
--- a/source3/smbd/smb2_getinfo.c
+++ b/source3/smbd/smb2_getinfo.c
@@ -303,6 +303,34 @@ static struct tevent_req *smbd_smb2_getinfo_send(TALLOC_CTX *mem_ctx,
ZERO_STRUCT(write_time_ts);
+ /*
+ * MS-SMB2 3.3.5.20.1 "Handling SMB2_0_INFO_FILE"
+ *
+ * FileBasicInformation, FileAllInformation,
+ * FileNetworkOpenInformation, FileAttributeTagInformation
+ * require FILE_READ_ATTRIBUTES.
+ *
+ * FileFullEaInformation requires FILE_READ_EA.
+ */
+ switch (in_file_info_class) {
+ case FSCC_FILE_BASIC_INFORMATION:
+ case FSCC_FILE_ALL_INFORMATION:
+ case FSCC_FILE_NETWORK_OPEN_INFORMATION:
+ case FSCC_FILE_ATTRIBUTE_TAG_INFORMATION:
+ if (!(fsp->access_mask & SEC_FILE_READ_ATTRIBUTE)) {
+ tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return tevent_req_post(req, ev);
+ }
+ break;
+
+ case FSCC_FILE_FULL_EA_INFORMATION:
+ if (!(fsp->access_mask & SEC_FILE_READ_EA)) {
+ tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED);
+ return tevent_req_post(req, ev);
+ }
+ break;
+ }
+
switch (in_file_info_class) {
case FSCC_FILE_FULL_EA_INFORMATION:
file_info_level = SMB2_FILE_FULL_EA_INFORMATION;
diff --git a/source4/libcli/smb2/util.c b/source4/libcli/smb2/util.c
index b2efabb5a44..f86a149c646 100644
--- a/source4/libcli/smb2/util.c
+++ b/source4/libcli/smb2/util.c
@@ -88,14 +88,37 @@ NTSTATUS smb2_util_mkdir(struct smb2_tree *tree, const char *dname)
*/
NTSTATUS smb2_util_setatr(struct smb2_tree *tree, const char *name, uint32_t attrib)
{
- union smb_setfileinfo io;
-
- ZERO_STRUCT(io);
- io.basic_info.level = RAW_SFILEINFO_BASIC_INFORMATION;
- io.basic_info.in.file.path = name;
- io.basic_info.in.attrib = attrib;
+ struct smb2_create cr = {0};
+ struct smb2_handle h1 = {{0}};
+ union smb_setfileinfo setinfo;
+ NTSTATUS status;
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_FILE_WRITE_ATTRIBUTE,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = FILE_OPEN,
+ .in.fname = name,
+ };
+ status = smb2_create(tree, tree, &cr);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ h1 = cr.out.file.handle;
- return smb2_composite_setpathinfo(tree, &io);
+ setinfo = (union smb_setfileinfo) {
+ .basic_info.level = RAW_SFILEINFO_BASIC_INFORMATION,
+ .basic_info.in.file.handle = h1,
+ .basic_info.in.attrib = attrib,
+ };
+
+ status = smb2_setinfo_file(tree, &setinfo);
+ if (!NT_STATUS_IS_OK(status)) {
+ smb2_util_close(tree, h1);
+ return status;
+ }
+
+ smb2_util_close(tree, h1);
+ return NT_STATUS_OK;
}
diff --git a/source4/torture/smb2/getinfo.c b/source4/torture/smb2/getinfo.c
index 5c9097f4200..539090ac71a 100644
--- a/source4/torture/smb2/getinfo.c
+++ b/source4/torture/smb2/getinfo.c
@@ -783,6 +783,151 @@ static bool torture_smb2_getinfo(struct torture_context *tctx)
return ret;
}
+#undef LEVEL
+#define LEVEL(l, u, ua, ra) \
+ .name = #l, \
+ .level = l, \
+ .unrestricted = u, \
+ .unrestricted_access = ua, \
+ .required_access = ra
+
+static struct {
+ const char *name;
+ uint16_t level;
+ bool unrestricted;
+ uint32_t unrestricted_access;
+ uint32_t required_access;
+} file_levels_access[] = {
+ /*
+ * The following info levels are not checked:
+ * - FileFullEaInformation and FileIdInformation:
+ * not implemented by the s4/libcli/raw
+ * - all pipe infolevels: that should be tested elsewhere by RPC tests
+ */
+
+ /*
+ * The following allow unrestricted access, so requesting
+ * SEC_FILE_READ_ATTRIBUTE works, SEC_FILE_READ_ATTRIBUTE or
+ * SEC_FILE_READ_EA as well of course.
+ */
+ { LEVEL(RAW_FILEINFO_STANDARD_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_INTERNAL_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_ACCESS_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_POSITION_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_MODE_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_ALIGNMENT_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_ALT_NAME_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_STREAM_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_COMPRESSION_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_NORMALIZED_NAME_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_EA_INFORMATION, true, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_EA) },
+
+ /*
+ * The following require either SEC_FILE_READ_ATTRIBUTE or
+ * SEC_FILE_READ_EA.
+ */
+ { LEVEL(RAW_FILEINFO_BASIC_INFORMATION, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_ALL_INFORMATION, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_NETWORK_OPEN_INFORMATION, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_ATTRIBUTE_TAG_INFORMATION, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_SMB2_ALL_INFORMATION, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_ATTRIBUTE) },
+ { LEVEL(RAW_FILEINFO_SMB2_ALL_EAS, false, SEC_STD_SYNCHRONIZE, SEC_FILE_READ_EA) },
+ /* Also try SEC_FILE_READ_ATTRIBUTE to show that it is the wrong one */
+ { LEVEL(RAW_FILEINFO_SMB2_ALL_EAS, false, SEC_FILE_READ_ATTRIBUTE, SEC_FILE_READ_EA) },
+ { LEVEL(RAW_FILEINFO_SEC_DESC, false, SEC_STD_SYNCHRONIZE, SEC_STD_READ_CONTROL) },
+ /* Also try SEC_FILE_READ_ATTRIBUTE to show that it is the wrong one */
+ { LEVEL(RAW_FILEINFO_SEC_DESC, false, SEC_FILE_READ_ATTRIBUTE, SEC_STD_READ_CONTROL) }
+};
+
+
+/*
+ test fileinfo levels
+*/
+static bool torture_smb2_getfinfo_access(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = "torture_smb2_getfinfo_access";
+ struct smb2_handle hfile;
+ NTSTATUS status;
+ bool ret = true;
+ int i;
+
+ smb2_deltree(tree, fname);
+
+ torture_setup_complex_file(tctx, tree, fname);
+
+ for (i = 0; i < ARRAY_SIZE(file_levels_access); i++) {
+ union smb_fileinfo finfo;
+ NTSTATUS expected_status;
+
+ /*
+ * First open with unrestricted_access, SEC_STD_SYNCHRONIZE for
+ * most tests, info levels with unrestricted=true should allow
+ * this.
+ */
+ status = torture_smb2_testfile_access(
+ tree, fname, &hfile, file_levels_access[i].unrestricted_access);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Unable to open test file\n");
+
+ if (file_levels_access[i].level == RAW_FILEINFO_SEC_DESC) {
+ finfo.query_secdesc.in.secinfo_flags = 0x7;
+ }
+ if (file_levels_access[i].level == RAW_FILEINFO_SMB2_ALL_EAS) {
+ finfo.all_eas.in.continue_flags =
+ SMB2_CONTINUE_FLAG_RESTART;
+ }
+
+ finfo.generic.level = file_levels_access[i].level;
+ finfo.generic.in.file.handle = hfile;
+
+ if (file_levels_access[i].unrestricted) {
+ expected_status = NT_STATUS_OK;
+ } else {
+ expected_status = NT_STATUS_ACCESS_DENIED;
+ }
+
+ status = smb2_getinfo_file(tree, tree, &finfo);
+ torture_assert_ntstatus_equal_goto(
+ tctx, status, expected_status, ret, done,
+ talloc_asprintf(tctx, "Level %s failed\n",
+ file_levels_access[i].name));
+
+ smb2_util_close(tree, hfile);
+
+ /*
+ * Now open with expected access, getinfo should work.
+ */
+ status = torture_smb2_testfile_access(
+ tree, fname, &hfile, file_levels_access[i].required_access);
+ torture_assert_ntstatus_ok_goto(
+ tctx, status, ret, done,
+ "Unable to open test file\n");
+
+ if (file_levels_access[i].level == RAW_FILEINFO_SEC_DESC) {
+ finfo.query_secdesc.in.secinfo_flags = 0x7;
+ }
+ if (file_levels_access[i].level == RAW_FILEINFO_SMB2_ALL_EAS) {
+ finfo.all_eas.in.continue_flags =
+ SMB2_CONTINUE_FLAG_RESTART;
+ }
+ finfo.generic.level = file_levels_access[i].level;
+ finfo.generic.in.file.handle = hfile;
+
+ status = smb2_getinfo_file(tree, tree, &finfo);
+ torture_assert_ntstatus_ok_goto(
+ tctx, status, ret, done,
+ talloc_asprintf(tctx, "%s on file",
+ file_levels_access[i].name));
+
+ smb2_util_close(tree, hfile);
+ }
+
+done:
+ smb2_deltree(tree, fname);
+ return ret;
+}
+
struct torture_suite *torture_smb2_getinfo_init(TALLOC_CTX *ctx)
{
struct torture_suite *suite = torture_suite_create(
@@ -800,5 +945,7 @@ struct torture_suite *torture_smb2_getinfo_init(TALLOC_CTX *ctx)
torture_smb2_fileinfo_grant_read);
torture_suite_add_simple_test(suite, "normalized",
torture_smb2_fileinfo_normalized);
+ torture_suite_add_1smb2_test(suite, "getinfo_access",
+ torture_smb2_getfinfo_access);
return suite;
}
diff --git a/source4/torture/smb2/oplock.c b/source4/torture/smb2/oplock.c
index 6bf7567ddb1..8f275bafcdb 100644
--- a/source4/torture/smb2/oplock.c
+++ b/source4/torture/smb2/oplock.c
@@ -2957,15 +2957,9 @@ static bool test_smb2_oplock_batch25(struct torture_context *tctx,
h1 = io.smb2.out.file.handle;
CHECK_VAL(io.smb2.out.oplock_level, SMB2_OPLOCK_LEVEL_BATCH);
- torture_comment(tctx, "changing the file attribute info should trigger "
- "a break and a violation\n");
-
status = smb2_util_setatr(tree1, fname, FILE_ATTRIBUTE_HIDDEN);
- torture_assert_ntstatus_equal(tctx, status, NT_STATUS_SHARING_VIOLATION,
- "Incorrect status");
-
- torture_wait_for_oplock_break(tctx);
- CHECK_VAL(break_info.count, 1);
+ torture_assert_ntstatus_ok(tctx, status, "Setting attributes "
+ "shouldn't trigger an oplock break");
smb2_util_close(tree1, h1);
smb2_util_close(tree1, h);
--
Samba Shared Repository
More information about the samba-cvs
mailing list