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@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/selftest/skip b/selftest/skip
index 549ba202021c3d94c21e46602e951d3035d21aa9..f54a23c9235b6777c2bd2e59ebfa19b8b0041737 100644 (file)
--- 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/source4/torture/smb2/attr.c b/source4/torture/smb2/attr.c
index 5947997c05fc844a893a94294d78ce6e6bfcb485..58d77f1cb6dfe1b2de7f794b143fac633120c1b5 100644 (file)
--- a/source4/torture/smb2/attr.c
+++ b/source4/torture/smb2/attr.c
@@ -494,3 +494,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 c860d03102e4c545e237cf9de3c95925988741d6..8c87f4d0d20e8753e88e74eaeff9a0c0f5f5612c 100644 (file)
--- 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));