[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Thu Apr 16 20:43:03 UTC 2020
The branch, master has been updated
via 72a57d377e4 s4: torture: SMB2. Fix smb2.winattr to actually read the SD from the server and check it.
via 3dd78d2d407 s3: smbd: Ensure we don't try and read the on-disk security descriptor if no bits are requested.
via cb59b75bee3 s4: torture: SMB2. Add a new test that exposes interesting SD query behavior.
from 5c73a2b3c1f docs: Update smbclient manpage that four digit years are also allowed
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 72a57d377e451599fb19d51e08feb0facbf77409
Author: Jeremy Allison <jra at samba.org>
Date: Wed Apr 15 12:07:57 2020 -0700
s4: torture: SMB2. Fix smb2.winattr to actually read the SD from the server and check it.
We need READ_CONTROL, and actually have to ask for
the OWNER|GROUP|DACL bits if we're going to properly
check the SD.
Tested against Windows 10.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Thu Apr 16 20:42:58 UTC 2020 on sn-devel-184
commit 3dd78d2d407d88b797f8fdb0a9bfedfa830df206
Author: Jeremy Allison <jra at samba.org>
Date: Wed Apr 15 13:33:43 2020 -0700
s3: smbd: Ensure we don't try and read the on-disk security descriptor if no bits are requested.
The sdread test just added shows that a client
can open with READ_ATTRIBUTES and still issue
a query security descriptor. smbd passed that
test as it read the on-disk sd, but then threw
the information away and returned the NULL sd
the client expects.
Make sure that we don't try and read the on-disk
sd if the client doesn't request any bits.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
commit cb59b75bee361fd4c98a6d732854a029ce3cb91d
Author: Jeremy Allison <jra at samba.org>
Date: Wed Apr 15 11:59:17 2020 -0700
s4: torture: SMB2. Add a new test that exposes interesting SD query behavior.
If we open a file without READ_CONTROL, requesting a security
descriptor fails with ACCESS_DENIED if any of the requested
bits OWNER|GROUP|DACL are set.
However, if we send zero as the requested bits then a
security descriptor is returned containing no data,
even though reading an SD should fail based on the
access permissions we have on the handle.
This has been tested against Windows 10, and also
passes on Samba - although in smbd we actually
read the SD off disk first, before nulling out
all the data we read. We shouldn't (we have
no rights to do so) and a subsequent commit
will fix this.
This was discovered when investigating the
smb2.winattr test, which currently relies
on exactly this behavior. It shouldn't
and the next commit will fix that.
I wanted to preserve the current smb2.winattr
behavior in a test though.
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
-----------------------------------------------------------------------
Summary of changes:
selftest/skip | 1 +
source3/smbd/nttrans.c | 22 ++++--
source4/torture/smb2/attr.c | 163 +++++++++++++++++++++++++++++++++++++++++++-
source4/torture/smb2/smb2.c | 1 +
4 files changed, 178 insertions(+), 9 deletions(-)
Changeset truncated at 500 lines:
diff --git a/selftest/skip b/selftest/skip
index 549ba202021..f54a23c9235 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -152,6 +152,7 @@ bench # don't run benchmarks in our selftest
^samba4.*.base.birthtime
^samba4.*base.defer_open
^samba4.smb2.acls # new test which doesn't pass yet
+^samba4.smb2.sdread
# ktutil might not be installed or from mit...
# we should build a samba4ktutil and use that instead
^samba4.blackbox.ktpass # this test isn't portable ...
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index affedf5f527..f7e313d6edf 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -1996,6 +1996,7 @@ NTSTATUS smbd_do_query_security_desc(connection_struct *conn,
NTSTATUS status;
struct security_descriptor *psd = NULL;
TALLOC_CTX *frame = talloc_stackframe();
+ bool need_to_read_sd = false;
/*
* Get the permissions to return.
@@ -2027,17 +2028,26 @@ NTSTATUS smbd_do_query_security_desc(connection_struct *conn,
/* Don't return SECINFO_LABEL if anything else was
requested. See bug #8458. */
security_info_wanted &= ~SECINFO_LABEL;
+
+ /*
+ * Only query the file system SD if the caller asks
+ * for any bits. This allows a caller to open without
+ * READ_CONTROL but still issue a query sd. See
+ * smb2.sdread test.
+ */
+ need_to_read_sd = true;
}
- if (!lp_nt_acl_support(SNUM(conn))) {
- status = get_null_nt_acl(frame, &psd);
- } else if (security_info_wanted & SECINFO_LABEL) {
- /* Like W2K3 return a null object. */
- status = get_null_nt_acl(frame, &psd);
- } else {
+ if (lp_nt_acl_support(SNUM(conn)) &&
+ ((security_info_wanted & SECINFO_LABEL) == 0) &&
+ need_to_read_sd)
+ {
status = SMB_VFS_FGET_NT_ACL(
fsp, security_info_wanted, frame, &psd);
+ } else {
+ status = get_null_nt_acl(frame, &psd);
}
+
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
diff --git a/source4/torture/smb2/attr.c b/source4/torture/smb2/attr.c
index 5947997c05f..60068971d4b 100644
--- a/source4/torture/smb2/attr.c
+++ b/source4/torture/smb2/attr.c
@@ -255,7 +255,8 @@ bool torture_smb2_winattrtest(struct torture_context *tctx,
/* Open a file*/
create_io.in.create_flags = 0;
- create_io.in.desired_access = SEC_FILE_READ_DATA | SEC_FILE_WRITE_DATA;
+ create_io.in.desired_access = SEC_FILE_READ_DATA | SEC_FILE_WRITE_DATA |
+ SEC_STD_READ_CONTROL;
create_io.in.file_attributes = 0;
create_io.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
create_io.in.create_disposition = FILE_SUPERSEDE;
@@ -270,7 +271,10 @@ bool torture_smb2_winattrtest(struct torture_context *tctx,
/* Get security descriptor and store it*/
query_org.generic.level = RAW_FILEINFO_SEC_DESC;
query_org.generic.in.file.handle = create_io.out.file.handle;
- status = smb2_getinfo_file(tree, NULL, &query_org);
+ query_org.query_secdesc.in.secinfo_flags = SECINFO_OWNER|
+ SECINFO_GROUP|
+ SECINFO_DACL;
+ status = smb2_getinfo_file(tree, tctx, &query_org);
if(!NT_STATUS_IS_OK(status)){
NTSTATUS s = smb2_util_close(tree, create_io.out.file.handle);
torture_assert_ntstatus_ok_goto(tctx, s, ret, error_exit,
@@ -313,7 +317,8 @@ bool torture_smb2_winattrtest(struct torture_context *tctx,
create_io = (struct smb2_create){0};
create_io.in.create_flags = 0;
- create_io.in.desired_access = SEC_FILE_READ_ATTRIBUTE;
+ create_io.in.desired_access = SEC_FILE_READ_ATTRIBUTE|
+ SEC_STD_READ_CONTROL;
create_io.in.file_attributes = 0;
create_io.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
create_io.in.create_disposition = FILE_OPEN_IF;
@@ -328,6 +333,9 @@ bool torture_smb2_winattrtest(struct torture_context *tctx,
/*Get security descriptor */
query.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
query.query_secdesc.in.file.handle = create_io.out.file.handle;
+ query.query_secdesc.in.secinfo_flags = SECINFO_OWNER|
+ SECINFO_GROUP|
+ SECINFO_DACL;
status = smb2_getinfo_file(tree, tctx, &query);
if(!NT_STATUS_IS_OK(status)){
NTSTATUS s = smb2_util_close(tree, create_io.out.file.handle);
@@ -494,3 +502,152 @@ error_exit:
return ret;
}
+
+bool torture_smb2_sdreadtest(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = "sdread.file";
+ bool ret = true;
+ union smb_fileinfo query;
+ NTSTATUS status;
+ struct security_descriptor *sd = NULL;
+ struct smb2_create create_io = {0};
+ uint32_t sd_bits[] = { SECINFO_OWNER,
+ SECINFO_GROUP,
+ SECINFO_DACL };
+ size_t i;
+
+ ZERO_STRUCT(query);
+
+ smb2_util_unlink(tree, fname);
+
+ /* Create then close a file*/
+ create_io.in.create_flags = 0;
+ create_io.in.desired_access = SEC_FILE_READ_DATA | SEC_FILE_WRITE_DATA;
+ create_io.in.file_attributes = 0;
+ create_io.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+ create_io.in.create_disposition = FILE_SUPERSEDE;
+ create_io.in.create_options = 0;
+ create_io.in.security_flags = 0;
+ create_io.in.fname = fname;
+ status = smb2_create(tree, tctx, &create_io);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, error_exit,
+ talloc_asprintf(tctx, "open(1) of %s failed (%s)\n",
+ fname, nt_errstr(status)));
+ status = smb2_util_close(tree, create_io.out.file.handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, error_exit,
+ talloc_asprintf(tctx, "close(1) of %s failed (%s)\n",
+ fname, nt_errstr(status)));
+
+ /*
+ * Open the file with READ_ATTRIBUTES *only*,
+ * no READ_CONTROL.
+ *
+ * This should deny access for any attempt to
+ * get a security descriptor if we ask for
+ * any of OWNER|GROUP|DACL, but if
+ * we ask for *NO* info but still ask for
+ * the security descriptor, then Windows
+ * returns an ACL but with zero entries
+ * for OWNER|GROUP|DACL.
+ */
+
+ create_io = (struct smb2_create){0};
+ create_io.in.create_flags = 0;
+ create_io.in.desired_access = SEC_FILE_READ_ATTRIBUTE;
+ create_io.in.file_attributes = 0;
+ create_io.in.share_access = NTCREATEX_SHARE_ACCESS_NONE;
+ create_io.in.create_disposition = FILE_OPEN;
+ create_io.in.create_options = 0;
+ create_io.in.security_flags = 0;
+ create_io.in.fname = fname;
+ status = smb2_create(tree, tctx, &create_io);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret,
+ error_exit,
+ talloc_asprintf(tctx, "open(2) of %s failed (%s)\n",
+ fname, nt_errstr(status)));
+
+ /* Check asking for SD fails ACCESS_DENIED with actual bits set. */
+ for (i = 0; i < ARRAY_SIZE(sd_bits); i++) {
+ query.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+ query.query_secdesc.in.file.handle = create_io.out.file.handle;
+ query.query_secdesc.in.secinfo_flags = sd_bits[i];
+
+ status = smb2_getinfo_file(tree, tctx, &query);
+
+ /* Must return ACESS_DENIED. */
+ if(!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)){
+ NTSTATUS s = smb2_util_close(tree,
+ create_io.out.file.handle);
+ torture_assert_ntstatus_ok_goto(tctx, s, ret,
+ error_exit,
+ talloc_asprintf(tctx,
+ "close(2) of %s failed (%s)\n",
+ fname, nt_errstr(s)));
+ ret = false;
+ torture_fail_goto(tctx, error_exit,
+ talloc_asprintf(tctx,
+ "smb2_getinfo_file(2) of %s failed (%s)\n",
+ fname, nt_errstr(status)));
+ }
+ }
+
+ /*
+ * Get security descriptor whilst asking for *NO* bits.
+ * This succeeds even though we don't have READ_CONTROL
+ * access but returns an SD with zero data.
+ */
+ query.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+ query.query_secdesc.in.file.handle = create_io.out.file.handle;
+ query.query_secdesc.in.secinfo_flags = 0;
+
+ status = smb2_getinfo_file(tree, tctx, &query);
+ if(!NT_STATUS_IS_OK(status)){
+ NTSTATUS s = smb2_util_close(tree, create_io.out.file.handle);
+ torture_assert_ntstatus_ok_goto(tctx, s, ret, error_exit,
+ talloc_asprintf(tctx,
+ "close(3) of %s failed (%s)\n",
+ fname, nt_errstr(s)));
+ ret = false;
+ torture_fail_goto(tctx, error_exit, talloc_asprintf(tctx,
+ "smb2_getinfo_file(3) of %s failed (%s)\n",
+ fname, nt_errstr(status)));
+ }
+
+ sd = query.query_secdesc.out.sd;
+
+ /* Check it's empty. */
+ torture_assert_goto(tctx,
+ (sd->owner_sid == NULL),
+ ret,
+ error_exit,
+ "sd->owner_sid != NULL\n");
+
+ torture_assert_goto(tctx,
+ (sd->group_sid == NULL),
+ ret,
+ error_exit,
+ "sd->group_sid != NULL\n");
+
+ torture_assert_goto(tctx,
+ (sd->dacl == NULL),
+ ret,
+ error_exit,
+ "sd->dacl != NULL\n");
+
+ status = smb2_util_close(tree, create_io.out.file.handle);
+ torture_assert_ntstatus_ok_goto(tctx,
+ status,
+ ret,
+ error_exit,
+ talloc_asprintf(tctx, "close(4) of %s failed (%s)\n",
+ fname,
+ nt_errstr(status)));
+
+error_exit:
+
+ smb2_setatr(tree, fname, FILE_ATTRIBUTE_NORMAL);
+ smb2_util_unlink(tree, fname);
+
+ return ret;
+}
diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
index c860d03102e..8c87f4d0d20 100644
--- a/source4/torture/smb2/smb2.c
+++ b/source4/torture/smb2/smb2.c
@@ -201,6 +201,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx)
torture_suite_add_suite(suite, torture_smb2_timestamp_resolution_init(suite));
torture_suite_add_1smb2_test(suite, "openattr", torture_smb2_openattrtest);
torture_suite_add_1smb2_test(suite, "winattr", torture_smb2_winattrtest);
+ torture_suite_add_1smb2_test(suite, "sdread", torture_smb2_sdreadtest);
torture_suite_add_suite(suite, torture_smb2_readwrite_init(suite));
torture_suite_add_suite(suite, torture_smb2_charset(suite));
--
Samba Shared Repository
More information about the samba-cvs
mailing list