[PATCH] Fix bug 10252 - make our access-based enumeration work exactly like Windows.
Jeremy Allison
jra at samba.org
Wed Oct 14 00:10:47 UTC 2015
This took much longer to fix than it should have
- fighting the test suite to access a specific
share, and discovering that Windows only does
access-based enumeration on wildcard directory
listings, not specific filenames :-).
Please review and push if happy !
Cheers,
Jeremy.
-------------- next part --------------
From 2c9f36128a781d32e16e25528eb9ca293469ba43 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Oct 2015 16:49:41 -0700
Subject: [PATCH 1/3] s3: smbd: Fix our access-based enumeration on "hide
unreadable" to match Windows.
Torture test to follow.
https://bugzilla.samba.org/show_bug.cgi?id=10252
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/dir.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 61 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index eebf9b1..3f99f88 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -1343,6 +1343,15 @@ bool get_dir_entry(TALLOC_CTX *ctx,
static bool user_can_read_file(connection_struct *conn,
struct smb_filename *smb_fname)
{
+ NTSTATUS status;
+ uint32_t rejected_share_access = 0;
+ uint32_t rejected_mask = 0;
+ struct security_descriptor *sd = NULL;
+ uint32_t access_mask = FILE_READ_DATA|
+ FILE_READ_EA|
+ FILE_READ_ATTRIBUTES|
+ SEC_STD_READ_CONTROL;
+
/*
* Never hide files from the root user.
* We use (uid_t)0 here not sec_initial_uid()
@@ -1353,10 +1362,59 @@ static bool user_can_read_file(connection_struct *conn,
return True;
}
- return NT_STATUS_IS_OK(smbd_check_access_rights(conn,
- smb_fname,
+ /*
+ * We can't directly use smbd_check_access_rights()
+ * here, as this implicitly grants FILE_READ_ATTRIBUTES
+ * which the Windows access-based-enumeration code
+ * explicitly checks for on the file security descriptor.
+ * See bug:
+ *
+ * https://bugzilla.samba.org/show_bug.cgi?id=10252
+ *
+ * and the smb2.acl2.ACCESSBASED test for details.
+ */
+
+ rejected_share_access = access_mask & ~(conn->share_access);
+ if (rejected_share_access) {
+ DEBUG(10, ("rejected share access 0x%x "
+ "on %s (0x%x)\n",
+ (unsigned int)access_mask,
+ smb_fname_str_dbg(smb_fname),
+ (unsigned int)rejected_share_access ));
+ return false;
+ }
+
+ status = SMB_VFS_GET_NT_ACL(conn,
+ smb_fname->base_name,
+ (SECINFO_OWNER |
+ SECINFO_GROUP |
+ SECINFO_DACL),
+ talloc_tos(),
+ &sd);
+
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(10, ("Could not get acl "
+ "on %s: %s\n",
+ smb_fname_str_dbg(smb_fname),
+ nt_errstr(status)));
+ return false;
+ }
+
+ status = se_file_access_check(sd,
+ get_current_nttok(conn),
false,
- FILE_READ_DATA));
+ access_mask,
+ &rejected_mask);
+
+ TALLOC_FREE(sd);
+
+ if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+ DEBUG(10,("rejected bits 0x%x read access for %s\n",
+ (unsigned int)rejected_mask,
+ smb_fname_str_dbg(smb_fname) ));
+ return false;
+ }
+ return true;
}
/*******************************************************************
--
2.6.0.rc2.230.g3dd15c0
From b84a37dee050101ab9b1b7bcc920aa4845ddd0bf Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 9 Oct 2015 15:08:05 -0700
Subject: [PATCH 2/3] lib: cli: Add accessor function smb2cli_tcon_flags() to
get tcon flags.
We need this to see if a share supports access-based enumeration.
https://bugzilla.samba.org/show_bug.cgi?id=10252
Signed-off-by: Jeremy Allison <jra at samba.org>
---
libcli/smb/smbXcli_base.c | 5 +++++
libcli/smb/smbXcli_base.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/libcli/smb/smbXcli_base.c b/libcli/smb/smbXcli_base.c
index c1e9e58..6fe4816 100644
--- a/libcli/smb/smbXcli_base.c
+++ b/libcli/smb/smbXcli_base.c
@@ -5991,6 +5991,11 @@ uint32_t smb2cli_tcon_capabilities(struct smbXcli_tcon *tcon)
return tcon->smb2.capabilities;
}
+uint32_t smb2cli_tcon_flags(struct smbXcli_tcon *tcon)
+{
+ return tcon->smb2.flags;
+}
+
void smb2cli_tcon_set_values(struct smbXcli_tcon *tcon,
struct smbXcli_session *session,
uint32_t tcon_id,
diff --git a/libcli/smb/smbXcli_base.h b/libcli/smb/smbXcli_base.h
index cf93135..e4cfb10 100644
--- a/libcli/smb/smbXcli_base.h
+++ b/libcli/smb/smbXcli_base.h
@@ -442,6 +442,7 @@ bool smb1cli_tcon_set_values(struct smbXcli_tcon *tcon,
const char *fs_type);
uint32_t smb2cli_tcon_current_id(struct smbXcli_tcon *tcon);
uint32_t smb2cli_tcon_capabilities(struct smbXcli_tcon *tcon);
+uint32_t smb2cli_tcon_flags(struct smbXcli_tcon *tcon);
void smb2cli_tcon_set_values(struct smbXcli_tcon *tcon,
struct smbXcli_session *session,
uint32_t tcon_id,
--
2.6.0.rc2.230.g3dd15c0
From 6ef937cb864ebd6a63038fa3eb573ba35c5306f2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 13 Oct 2015 15:33:47 -0700
Subject: [PATCH 3/3] s4: torture: Add SMB2 access-based enumeration test.
Passes against Win2k12R2.
https://bugzilla.samba.org/show_bug.cgi?id=10252
Signed-off-by: Jeremy Allison <jra at samba.org>
---
selftest/knownfail | 1 +
source4/torture/smb2/acls.c | 230 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 231 insertions(+)
diff --git a/selftest/knownfail b/selftest/knownfail
index bf73176..0d74933 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -147,6 +147,7 @@
^samba4.smb2.acls.*.generic
^samba4.smb2.acls.*.inheritflags
^samba4.smb2.acls.*.owner
+^samba4.smb2.acls.*.ACCESSBASED
^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.test_dirsync_deleted_items
#^samba4.ldap.dirsync.python.ad_dc_ntvfs..__main__.ExtendedDirsyncTests.*
^samba4.libsmbclient.opendir.opendir # This requires netbios browsing
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index 37052c6..8066bc9 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -20,13 +20,17 @@
*/
#include "includes.h"
+#include "lib/cmdline/popt_common.h"
#include "libcli/smb2/smb2.h"
#include "libcli/smb2/smb2_calls.h"
+#include "libcli/smb/smbXcli_base.h"
#include "torture/torture.h"
+#include "libcli/resolve/resolve.h"
#include "torture/util.h"
#include "torture/smb2/proto.h"
#include "libcli/security/security.h"
#include "librpc/gen_ndr/ndr_security.h"
+#include "lib/param/param.h"
#define CHECK_STATUS(status, correct) do { \
if (!NT_STATUS_EQUAL(status, correct)) { \
@@ -1855,6 +1859,231 @@ done:
}
#endif
+/**
+ * SMB2 connect with explicit share
+ **/
+static bool torture_smb2_con_share(struct torture_context *tctx,
+ const char *share,
+ struct smb2_tree **tree)
+{
+ struct smbcli_options options;
+ NTSTATUS status;
+ const char *host = torture_setting_string(tctx, "host", NULL);
+ struct cli_credentials *credentials = cmdline_credentials;
+
+ lpcfg_smbcli_options(tctx->lp_ctx, &options);
+
+ status = smb2_connect_ext(tctx,
+ host,
+ lpcfg_smb_ports(tctx->lp_ctx),
+ share,
+ lpcfg_resolve_context(tctx->lp_ctx),
+ credentials,
+ 0,
+ tree,
+ tctx->ev,
+ &options,
+ lpcfg_socket_options(tctx->lp_ctx),
+ lpcfg_gensec_settings(tctx, tctx->lp_ctx)
+ );
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Failed to connect to SMB2 share \\\\%s\\%s - %s\n",
+ host, share, nt_errstr(status));
+ return false;
+ }
+ return true;
+}
+
+static bool test_access_based(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ struct smb2_tree *tree1 = NULL;
+ NTSTATUS status;
+ struct smb2_create io;
+ const char *fname = BASEDIR "\\testfile";
+ bool ret = true;
+ struct smb2_handle fhandle, dhandle;
+ union smb_fileinfo q;
+ union smb_setfileinfo set;
+ struct security_descriptor *sd, *sd_orig=NULL;
+ const char *owner_sid;
+ uint32_t flags = 0;
+ /*
+ * Can't test without SEC_STD_READ_CONTROL as we
+ * own the file and implicitly have SEC_STD_READ_CONTROL.
+ */
+ uint32_t access_masks[] = {
+ /* Full READ access. */
+ SEC_STD_READ_CONTROL|FILE_READ_DATA|
+ FILE_READ_ATTRIBUTES|FILE_READ_EA,
+
+ /* Missing FILE_READ_EA. */
+ SEC_STD_READ_CONTROL|FILE_READ_DATA|
+ FILE_READ_ATTRIBUTES,
+
+ /* Missing FILE_READ_ATTRIBUTES. */
+ SEC_STD_READ_CONTROL|FILE_READ_DATA|
+ FILE_READ_EA,
+
+ /* Missing FILE_READ_DATA. */
+ SEC_STD_READ_CONTROL|
+ FILE_READ_ATTRIBUTES|FILE_READ_EA,
+ };
+ unsigned int i;
+ unsigned int count;
+ struct smb2_find f;
+ union smb_search_data *d;
+
+ ZERO_STRUCT(fhandle);
+ ZERO_STRUCT(dhandle);
+
+ if (!torture_smb2_con_share(tctx, "hideunread", &tree1)) {
+ torture_result(tctx, TORTURE_FAIL, "(%s) Unable to connect "
+ "to share 'hideunread'\n",
+ __location__);
+ ret = false;
+ goto done;
+ }
+
+ flags = smb2cli_tcon_flags(tree1->smbXcli);
+
+ smb2_util_unlink(tree1, fname);
+ smb2_deltree(tree1, BASEDIR);
+
+ torture_comment(tctx, "TESTING ACCESS BASED ENUMERATION\n");
+
+ if ((flags & SMB2_SHAREFLAG_ACCESS_BASED_DIRECTORY_ENUM)==0) {
+ torture_result(tctx, TORTURE_FAIL, "(%s) No access enumeration "
+ "on share 'hideunread'\n",
+ __location__);
+ ret = false;
+ goto done;
+ }
+
+ if (!smb2_util_setup_dir(tctx, tree1, BASEDIR)) {
+ torture_result(tctx, TORTURE_FAIL, "(%s) Unable to setup %s\n",
+ __location__, BASEDIR);
+ ret = false;
+ goto done;
+ }
+
+ /* Get a handle to the BASEDIR directory. */
+ status = torture_smb2_testdir(tree1, BASEDIR, &dhandle);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ smb2_util_close(tree1, dhandle);
+ ZERO_STRUCT(dhandle);
+
+ ZERO_STRUCT(io);
+ io.level = RAW_OPEN_SMB2;
+ io.in.create_flags = 0;
+ io.in.desired_access = SEC_RIGHTS_FILE_ALL;
+ io.in.create_options = 0;
+ io.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io.in.share_access = 0;
+ io.in.alloc_size = 0;
+ io.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS;
+ io.in.security_flags = 0;
+ io.in.fname = fname;
+
+ status = smb2_create(tree1, tctx, &io);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ fhandle = io.out.file.handle;
+
+ torture_comment(tctx, "get the original sd\n");
+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+ q.query_secdesc.in.file.handle = fhandle;
+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
+ status = smb2_getinfo_file(tree1, tctx, &q);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ sd_orig = q.query_secdesc.out.sd;
+
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+ torture_comment(tctx, "owner_sid is %s\n", owner_sid);
+
+ /* Setup for the search. */
+ ZERO_STRUCT(f);
+ f.in.pattern = "*";
+ f.in.continue_flags = SMB2_CONTINUE_FLAG_REOPEN;
+ f.in.max_response_size = 0x1000;
+ f.in.level = SMB2_FIND_DIRECTORY_INFO;
+
+ for (i = 0; i < ARRAY_SIZE(access_masks); i++) {
+
+ sd = security_descriptor_dacl_create(tctx,
+ 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ access_masks[i]|SEC_STD_SYNCHRONIZE,
+ 0,
+ NULL);
+
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+ set.set_secdesc.in.file.handle = fhandle;
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+ set.set_secdesc.in.sd = sd;
+ status = smb2_setinfo_file(tree1, &set);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ /* Now see if we can see the file in a directory listing. */
+
+ /* Re-open dhandle. */
+ status = torture_smb2_testdir(tree1, BASEDIR, &dhandle);
+ CHECK_STATUS(status, NT_STATUS_OK);
+ f.in.file.handle = dhandle;
+
+ count = 0;
+ d = NULL;
+ status = smb2_find_level(tree1, tree1, &f, &count, &d);
+ TALLOC_FREE(d);
+
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ smb2_util_close(tree1, dhandle);
+ ZERO_STRUCT(dhandle);
+
+ if (i == 0) {
+ /* We should see the first sd. */
+ if (count != 3) {
+ torture_result(tctx, TORTURE_FAIL,
+ "(%s) Normal SD - Unable "
+ "to see file %s\n",
+ __location__,
+ BASEDIR);
+ ret = false;
+ goto done;
+ }
+ } else {
+ /* But no others. */
+ if (count != 2) {
+ torture_result(tctx, TORTURE_FAIL,
+ "(%s) SD 0x%x - can "
+ "see file %s\n",
+ __location__,
+ access_masks[i],
+ BASEDIR);
+ ret = false;
+ goto done;
+ }
+ }
+ }
+
+done:
+
+ if (tree1) {
+ smb2_util_close(tree1, fhandle);
+ smb2_util_close(tree1, dhandle);
+ smb2_util_unlink(tree1, fname);
+ smb2_deltree(tree1, BASEDIR);
+ smb2_tdis(tree1);
+ smb2_logoff(tree1->session);
+ }
+ smb2_tdis(tree);
+ smb2_logoff(tree->session);
+ return ret;
+}
+
/*
basic testing of SMB2 ACLs
*/
@@ -1872,6 +2101,7 @@ struct torture_suite *torture_smb2_acls_init(void)
/* XXX This test does not work against XP or Vista. */
torture_suite_add_1smb2_test(suite, "GETSET", test_sd_get_set);
#endif
+ torture_suite_add_1smb2_test(suite, "ACCESSBASED", test_access_based);
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
--
2.6.0.rc2.230.g3dd15c0
More information about the samba-technical
mailing list