[PATCH] Follow-up patch for bug in dealing with "Owner Rights" ACEs when calculating maximum access
Ralph Böhme
slow at samba.org
Mon Mar 4 06:12:15 UTC 2019
On Fri, Mar 01, 2019 at 02:45:20PM -0800, Jeremy Allison wrote:
>Let me know when you're done and I'll review with
>a view to pushing to master !
final patchset with a few changes attached:
- futher refined variable names in libcli/security: fix handling of deny type
ACEs in access_check_max_allowed()
- fixed test to remove SEC_STD_DELETE from expected maximum access bits, as
Windows 2016 wasn't return that bit. I didn't want to open this can of worms,
so I simply remove the bit from the exepcted mask.
- additional test that does the same ALLOW+DENY ACE check as
test_owner_rights_deny1(), just not with Owner Rights but with a user SID
- additional test that verifies that the server honors all bit in a requested
access mask that contains SEC_FLAG_MAXIMUM_ALLOWED
- patchset also contains my smbcalcs extension (for master only), including
async versions of cli_smb2_query_mxac() as requested by Volker, a simple
test and a manpage update
CI: https://gitlab.com/samba-team/devel/samba/pipelines/50089003
All tests pass against WIndows 2016.
Please review&push if happy. Thanks!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG-Fingerprint FAE2C6088A24252051C559E4AA1E9B7126399E46
-------------- next part --------------
From fd0d90958f1ed689a0734838fd44464e784777a2 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 28 Feb 2019 13:55:31 -0800
Subject: [PATCH 01/11] s4:torture: Fix the test_owner_rights() test to show
permissions are additive.
Tested against Windows.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
source4/torture/smb2/acls.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index b02d74367e3..c45125b30dc 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -2419,6 +2419,14 @@ static bool test_owner_rights(struct torture_context *tctx,
sd_orig = gi.query_secdesc.out.sd;
owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+ /*
+ * Add a 2 element ACL
+ * SEC_RIGHTS_FILE_READ for the owner,
+ * SEC_FILE_WRITE_DATA for SID_OWNER_RIGHTS.
+ *
+ * Proves that the owner and SID_OWNER_RIGHTS
+ * ACE entries are additive.
+ */
sd = security_descriptor_dacl_create(tctx, 0, NULL, NULL,
owner_sid,
SEC_ACE_TYPE_ACCESS_ALLOWED,
@@ -2426,7 +2434,7 @@ static bool test_owner_rights(struct torture_context *tctx,
0,
SID_OWNER_RIGHTS,
SEC_ACE_TYPE_ACCESS_ALLOWED,
- SEC_RIGHTS_FILE_READ,
+ SEC_FILE_WRITE_DATA,
0,
NULL);
torture_assert_not_null_goto(tctx, sd, ret, done,
@@ -2467,10 +2475,14 @@ static bool test_owner_rights(struct torture_context *tctx,
torture_assert_ntstatus_ok_goto(tctx, mxac_status, ret, done,
"smb2_setinfo_file failed\n");
- /* SEC_STD_DELETE comes from the parent directory */
+ /*
+ * For some reasons Windows 2016 doesn't set SEC_STD_DELETE but we
+ * do. Mask it out so the test passes against Samba and Windows.
+ */
torture_assert_int_equal_goto(tctx,
- cr.out.maximal_access,
- SEC_RIGHTS_FILE_READ|SEC_STD_DELETE,
+ cr.out.maximal_access & ~SEC_STD_DELETE,
+ SEC_RIGHTS_FILE_READ |
+ SEC_FILE_WRITE_DATA,
ret, done,
"Wrong maximum access\n");
--
2.17.2
From eb33947a388f69b869b3ec16343c05958d4a6ea9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 28 Feb 2019 14:37:09 -0800
Subject: [PATCH 02/11] s4:torture: Add test_owner_rights_deny().
Shows that owner and SID_OWNER_RIGHTS ACE
entries interact in max permissions requests.
Tested against Windows.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
selftest/knownfail.d/smb2.acls | 2 +
source4/torture/smb2/acls.c | 137 +++++++++++++++++++++++++++++++++
2 files changed, 139 insertions(+)
create mode 100644 selftest/knownfail.d/smb2.acls
diff --git a/selftest/knownfail.d/smb2.acls b/selftest/knownfail.d/smb2.acls
new file mode 100644
index 00000000000..e1b98cec606
--- /dev/null
+++ b/selftest/knownfail.d/smb2.acls
@@ -0,0 +1,2 @@
+^samba3.smb2.acls.OWNER-RIGHTS-DENY\(ad_dc\)
+^samba3.smb2.acls.OWNER-RIGHTS-DENY\(nt4_dc\)
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index c45125b30dc..0f2d5345e72 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -2499,6 +2499,141 @@ static bool test_owner_rights(struct torture_context *tctx,
return ret;
}
+/*
+ * test Owner Rights with a leading DENY ACE, S-1-3-4
+ */
+static bool test_owner_rights_deny(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = BASEDIR "\\owner_right_deny.txt";
+ struct smb2_create cr;
+ struct smb2_handle handle = {{0}};
+ union smb_fileinfo gi;
+ union smb_setfileinfo si;
+ struct security_descriptor *sd_orig = NULL;
+ struct security_descriptor *sd = NULL;
+ const char *owner_sid = NULL;
+ NTSTATUS mxac_status;
+ NTSTATUS status;
+ bool ret = true;
+
+ smb2_deltree(tree, BASEDIR);
+
+ ret = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ret, ret, done,
+ "smb2_util_setup_dir failed\n");
+
+ torture_comment(tctx, "TESTING OWNER RIGHTS DENY\n");
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL |
+ SEC_STD_WRITE_DAC |SEC_STD_WRITE_OWNER,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+ handle = cr.out.file.handle;
+
+ torture_comment(tctx, "get the original sd\n");
+
+ gi = (union smb_fileinfo) {
+ .query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+ .query_secdesc.in.file.handle = handle,
+ .query_secdesc.in.secinfo_flags = SECINFO_DACL|SECINFO_OWNER,
+ };
+
+ status = smb2_getinfo_file(tree, tctx, &gi);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_getinfo_file failed\n");
+
+ sd_orig = gi.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+ /*
+ * Add a 2 element ACL
+ * DENY SEC_FILE_DATA_READ for SID_OWNER_RIGHTS
+ * SEC_FILE_READ_DATA for the owner.
+ *
+ * Proves that the owner and SID_OWNER_RIGHTS
+ * ACE entries are additive.
+ */
+ sd = security_descriptor_dacl_create(tctx, 0, NULL, NULL,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ SEC_FILE_READ_DATA,
+ 0,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_FILE_READ,
+ 0,
+ NULL);
+ torture_assert_not_null_goto(tctx, sd, ret, done,
+ "SD create failed\n");
+
+ si = (union smb_setfileinfo) {
+ .set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+ .set_secdesc.in.file.handle = handle,
+ .set_secdesc.in.secinfo_flags = SECINFO_DACL,
+ .set_secdesc.in.sd = sd,
+ };
+
+ status = smb2_setinfo_file(tree, &si);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.query_maximal_access = true,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+ handle = cr.out.file.handle;
+
+ mxac_status = NT_STATUS(cr.out.maximal_access_status);
+ torture_assert_ntstatus_ok_goto(tctx, mxac_status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ /*
+ * For some reasons Windows 2016 doesn't set SEC_STD_DELETE but we
+ * do. Mask it out so the test passes against Samba and Windows.
+ */
+ torture_assert_int_equal_goto(tctx,
+ cr.out.maximal_access & ~SEC_STD_DELETE,
+ SEC_RIGHTS_FILE_READ & ~SEC_FILE_READ_DATA,
+ ret, done,
+ "Wrong maximum access\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+done:
+ if (!smb2_util_handle_empty(handle)) {
+ smb2_util_close(tree, handle);
+ }
+ smb2_deltree(tree, BASEDIR);
+ return ret;
+}
+
/*
basic testing of SMB2 ACLs
*/
@@ -2519,6 +2654,8 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
#endif
torture_suite_add_1smb2_test(suite, "ACCESSBASED", test_access_based);
torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS", test_owner_rights);
+ torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS-DENY",
+ test_owner_rights_deny);
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
--
2.17.2
From a41a4f53a37ce4eb0d3203d099515252139b1e99 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 1 Mar 2019 18:20:35 +0100
Subject: [PATCH 03/11] libcli/security: correct access check and maximum
access calculation for Owner Rights ACEs
We basically must process the Owner Rights ACEs as any other ACE wrt to the
order of adding granted permissions and checking denied permissions. According
to MS-DTYP 2.5.3.2 Owner Rights ACEs must be evaluated in the main loop over
the ACEs in an ACL and the corresponding access_mask must be directly applied
to bits_remaining. We currently defer this to after the loop over the ACEs in
ACL, this is wrong.
We just have to do some initial magic to determine if an ACL contains and
Owner Rights ACEs, and in case it doesn't we grant SEC_STD_WRITE_DAC |
SEC_STD_READ_CONTROL at the *beginning*. MS-DTYP:
-- the owner of an object is always granted READ_CONTROL and WRITE_DAC.
CALL SidInToken(Token, SecurityDescriptor.Owner, PrincipalSelfSubst)
IF SidInToken returns True THEN
IF DACL does not contain ACEs from object owner THEN
Remove READ_CONTROL and WRITE_DAC from RemainingAccess
Set GrantedAccess to GrantedAccess or READ_CONTROL or WRITE_OWNER
END IF
END IF
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Ralph Boehme <slow at samba.org>
---
libcli/security/access_check.c | 140 +++++++++++++++++----------------
selftest/knownfail.d/smb2.acls | 2 -
2 files changed, 73 insertions(+), 69 deletions(-)
delete mode 100644 selftest/knownfail.d/smb2.acls
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index 5d49b718f0c..d1d57eecef2 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -109,10 +109,9 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
const struct security_token *token)
{
uint32_t denied = 0, granted = 0;
+ bool am_owner = false;
+ bool have_owner_rights_ace = false;
unsigned i;
- uint32_t owner_rights_allowed = 0;
- uint32_t owner_rights_denied = 0;
- bool owner_rights_default = true;
if (sd->dacl == NULL) {
if (security_token_has_sid(token, sd->owner_sid)) {
@@ -121,26 +120,50 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
return granted;
}
+ if (security_token_has_sid(token, sd->owner_sid)) {
+ /*
+ * Check for explicit owner rights: if there are none, we remove
+ * the default owner right SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL
+ * from remaining_access. Otherwise we just process the
+ * explicitly granted rights when processing the ACEs.
+ */
+ am_owner = true;
+
+ for (i=0; i < sd->dacl->num_aces; i++) {
+ struct security_ace *ace = &sd->dacl->aces[i];
+
+ if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
+ continue;
+ }
+
+ have_owner_rights_ace = dom_sid_equal(
+ &ace->trustee, &global_sid_Owner_Rights);
+ if (have_owner_rights_ace) {
+ break;
+ }
+ }
+ }
+
+ if (am_owner && !have_owner_rights_ace) {
+ granted |= SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL;
+ }
+
for (i = 0;i<sd->dacl->num_aces; i++) {
struct security_ace *ace = &sd->dacl->aces[i];
+ bool is_owner_rights_ace = false;
if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
continue;
}
- if (dom_sid_equal(&ace->trustee, &global_sid_Owner_Rights)) {
- if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED) {
- owner_rights_allowed |= ace->access_mask;
- owner_rights_default = false;
- } else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED) {
- owner_rights_denied |= (owner_rights_allowed &
- ace->access_mask);
- owner_rights_default = false;
- }
- continue;
+ if (am_owner) {
+ is_owner_rights_ace = dom_sid_equal(
+ &ace->trustee, &global_sid_Owner_Rights);
}
- if (!security_token_has_sid(token, &ace->trustee)) {
+ if (!is_owner_rights_ace &&
+ !security_token_has_sid(token, &ace->trustee))
+ {
continue;
}
@@ -157,15 +180,6 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
}
}
- if (security_token_has_sid(token, sd->owner_sid)) {
- if (owner_rights_default) {
- granted |= SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL;
- } else {
- granted |= owner_rights_allowed;
- granted &= ~owner_rights_denied;
- }
- }
-
return granted & ~denied;
}
@@ -182,16 +196,8 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
uint32_t i;
uint32_t bits_remaining;
uint32_t explicitly_denied_bits = 0;
- /*
- * Up until Windows Server 2008, owner always had these rights. Now
- * we have to use Owner Rights perms if they are on the file.
- *
- * In addition we have to accumulate these bits and apply them
- * correctly. See bug #8795
- */
- uint32_t owner_rights_allowed = 0;
- uint32_t owner_rights_denied = 0;
- bool owner_rights_default = true;
+ bool am_owner = false;
+ bool have_owner_rights_ace = false;
*access_granted = access_desired;
bits_remaining = access_desired;
@@ -221,35 +227,50 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
goto done;
}
+ if (security_token_has_sid(token, sd->owner_sid)) {
+ /*
+ * Check for explicit owner rights: if there are none, we remove
+ * the default owner right SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL
+ * from remaining_access. Otherwise we just process the
+ * explicitly granted rights when processing the ACEs.
+ */
+ am_owner = true;
+
+ for (i=0; i < sd->dacl->num_aces; i++) {
+ struct security_ace *ace = &sd->dacl->aces[i];
+
+ if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
+ continue;
+ }
+
+ have_owner_rights_ace = dom_sid_equal(
+ &ace->trustee, &global_sid_Owner_Rights);
+ if (have_owner_rights_ace) {
+ break;
+ }
+ }
+ }
+ if (am_owner && !have_owner_rights_ace) {
+ bits_remaining &= ~(SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL);
+ }
+
/* check each ace in turn. */
for (i=0; bits_remaining && i < sd->dacl->num_aces; i++) {
struct security_ace *ace = &sd->dacl->aces[i];
+ bool is_owner_rights_ace = false;
if (ace->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
continue;
}
- /*
- * We need the Owner Rights permissions to ensure we
- * give or deny the correct permissions to the owner. Replace
- * owner_rights with the perms here if it is present.
- *
- * We don't care if we are not the owner because that is taken
- * care of below when we check if our token has the owner SID.
- *
- */
- if (dom_sid_equal(&ace->trustee, &global_sid_Owner_Rights)) {
- if (ace->type == SEC_ACE_TYPE_ACCESS_ALLOWED) {
- owner_rights_allowed |= ace->access_mask;
- owner_rights_default = false;
- } else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED) {
- owner_rights_denied |= (bits_remaining & ace->access_mask);
- owner_rights_default = false;
- }
- continue;
+ if (am_owner) {
+ is_owner_rights_ace = dom_sid_equal(
+ &ace->trustee, &global_sid_Owner_Rights);
}
- if (!security_token_has_sid(token, &ace->trustee)) {
+ if (!is_owner_rights_ace &&
+ !security_token_has_sid(token, &ace->trustee))
+ {
continue;
}
@@ -269,21 +290,6 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
/* Explicitly denied bits always override */
bits_remaining |= explicitly_denied_bits;
- /* The owner always gets owner rights as defined above. */
- if (security_token_has_sid(token, sd->owner_sid)) {
- if (owner_rights_default) {
- /*
- * Just remove them, no need to check if they are
- * there.
- */
- bits_remaining &= ~(SEC_STD_WRITE_DAC |
- SEC_STD_READ_CONTROL);
- } else {
- bits_remaining &= ~owner_rights_allowed;
- bits_remaining |= owner_rights_denied;
- }
- }
-
/*
* We check privileges here because they override even DENY entries.
*/
diff --git a/selftest/knownfail.d/smb2.acls b/selftest/knownfail.d/smb2.acls
deleted file mode 100644
index e1b98cec606..00000000000
--- a/selftest/knownfail.d/smb2.acls
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba3.smb2.acls.OWNER-RIGHTS-DENY\(ad_dc\)
-^samba3.smb2.acls.OWNER-RIGHTS-DENY\(nt4_dc\)
--
2.17.2
From d761d5ffcf9741a34daa95d1d9afd3a7dbf232c6 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 28 Feb 2019 14:59:01 -0800
Subject: [PATCH 04/11] s4:torture: Add test_owner_rights_deny1().
Creates a 3-element ALLOW + ALLOW + DENY ACE showing that when
calculating maximum access already seen allow bits are not removed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Ralph Boehme <slow at samba.org>
---
selftest/knownfail.d/smb2.acls | 2 +
source4/torture/smb2/acls.c | 144 +++++++++++++++++++++++++++++++++
2 files changed, 146 insertions(+)
create mode 100644 selftest/knownfail.d/smb2.acls
diff --git a/selftest/knownfail.d/smb2.acls b/selftest/knownfail.d/smb2.acls
new file mode 100644
index 00000000000..f8541adc1b0
--- /dev/null
+++ b/selftest/knownfail.d/smb2.acls
@@ -0,0 +1,2 @@
+^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(ad_dc\)
+^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(nt4_dc\)
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index 0f2d5345e72..916a582eac5 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -2634,6 +2634,148 @@ static bool test_owner_rights_deny(struct torture_context *tctx,
return ret;
}
+/*
+ * test Owner Rights with a trailing DENY ACE, S-1-3-4
+ */
+static bool test_owner_rights_deny1(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = BASEDIR "\\owner_right_deny1.txt";
+ struct smb2_create cr;
+ struct smb2_handle handle = {{0}};
+ union smb_fileinfo gi;
+ union smb_setfileinfo si;
+ struct security_descriptor *sd_orig = NULL;
+ struct security_descriptor *sd = NULL;
+ const char *owner_sid = NULL;
+ NTSTATUS mxac_status;
+ NTSTATUS status;
+ bool ret = true;
+
+ smb2_deltree(tree, BASEDIR);
+
+ ret = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ret, ret, done,
+ "smb2_util_setup_dir failed\n");
+
+ torture_comment(tctx, "TESTING OWNER RIGHTS DENY1\n");
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL |
+ SEC_STD_WRITE_DAC |SEC_STD_WRITE_OWNER,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+ handle = cr.out.file.handle;
+
+ torture_comment(tctx, "get the original sd\n");
+
+ gi = (union smb_fileinfo) {
+ .query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+ .query_secdesc.in.file.handle = handle,
+ .query_secdesc.in.secinfo_flags = SECINFO_DACL|SECINFO_OWNER,
+ };
+
+ status = smb2_getinfo_file(tree, tctx, &gi);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_getinfo_file failed\n");
+
+ sd_orig = gi.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+ /*
+ * Add a 3 element ACL
+ *
+ * SEC_RIGHTS_FILE_READ allow for owner.
+ * SEC_FILE_WRITE_DATA allow for SID-OWNER-RIGHTS.
+ * SEC_FILE_WRITE_DATA|SEC_FILE_READ_DATA) deny for SID-OWNER-RIGHTS.
+ *
+ * Shows on Windows that trailing DENY entries don't
+ * override granted permissions in max access calculations.
+ */
+ sd = security_descriptor_dacl_create(tctx, 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_FILE_READ,
+ 0,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_FILE_WRITE_DATA,
+ 0,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ (SEC_FILE_WRITE_DATA|
+ SEC_FILE_READ_DATA),
+ 0,
+ NULL);
+ torture_assert_not_null_goto(tctx, sd, ret, done,
+ "SD create failed\n");
+
+ si = (union smb_setfileinfo) {
+ .set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+ .set_secdesc.in.file.handle = handle,
+ .set_secdesc.in.secinfo_flags = SECINFO_DACL,
+ .set_secdesc.in.sd = sd,
+ };
+
+ status = smb2_setinfo_file(tree, &si);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.query_maximal_access = true,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+ handle = cr.out.file.handle;
+
+ mxac_status = NT_STATUS(cr.out.maximal_access_status);
+ torture_assert_ntstatus_ok_goto(tctx, mxac_status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ /*
+ * For some reasons Windows 2016 doesn't set SEC_STD_DELETE but we
+ * do. Mask it out so the test passes against Samba and Windows.
+ */
+ torture_assert_int_equal_goto(tctx,
+ cr.out.maximal_access & ~SEC_STD_DELETE,
+ SEC_RIGHTS_FILE_READ | SEC_FILE_WRITE_DATA,
+ ret, done,
+ "Wrong maximum access\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+done:
+ if (!smb2_util_handle_empty(handle)) {
+ smb2_util_close(tree, handle);
+ }
+ smb2_deltree(tree, BASEDIR);
+ return ret;
+}
+
/*
basic testing of SMB2 ACLs
*/
@@ -2656,6 +2798,8 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS", test_owner_rights);
torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS-DENY",
test_owner_rights_deny);
+ torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS-DENY1",
+ test_owner_rights_deny1);
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
--
2.17.2
From ba17ba3b1d5a51f857fce917cfbc61401ebb2cd2 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 3 Mar 2019 08:33:51 +0100
Subject: [PATCH 05/11] s4:torture: Add test_deny1().
Creates a 2-element ALLOW + DENY ACE showing that when calculating
effective permissions and maximum access already seen allow bits are not
removed.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/knownfail.d/smb2.acls | 2 +
source4/torture/smb2/acls.c | 140 +++++++++++++++++++++++++++++++++
2 files changed, 142 insertions(+)
diff --git a/selftest/knownfail.d/smb2.acls b/selftest/knownfail.d/smb2.acls
index f8541adc1b0..b76a3c719ce 100644
--- a/selftest/knownfail.d/smb2.acls
+++ b/selftest/knownfail.d/smb2.acls
@@ -1,2 +1,4 @@
^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(ad_dc\)
^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(nt4_dc\)
+^samba3.smb2.acls.DENY1\(ad_dc\)
+^samba3.smb2.acls.DENY1\(nt4_dc\)
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index 916a582eac5..7bccce803f0 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -2776,6 +2776,144 @@ static bool test_owner_rights_deny1(struct torture_context *tctx,
return ret;
}
+/*
+ * test that shows that a DENY ACE doesn't remove rights granted
+ * by a previous ALLOW ACE.
+ */
+static bool test_deny1(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = BASEDIR "\\test_deny1.txt";
+ struct smb2_create cr;
+ struct smb2_handle handle = {{0}};
+ union smb_fileinfo gi;
+ union smb_setfileinfo si;
+ struct security_descriptor *sd_orig = NULL;
+ struct security_descriptor *sd = NULL;
+ const char *owner_sid = NULL;
+ NTSTATUS mxac_status;
+ NTSTATUS status;
+ bool ret = true;
+
+ smb2_deltree(tree, BASEDIR);
+
+ ret = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ret, ret, done,
+ "smb2_util_setup_dir failed\n");
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL |
+ SEC_STD_WRITE_DAC |SEC_STD_WRITE_OWNER,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+ handle = cr.out.file.handle;
+
+ torture_comment(tctx, "get the original sd\n");
+
+ gi = (union smb_fileinfo) {
+ .query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+ .query_secdesc.in.file.handle = handle,
+ .query_secdesc.in.secinfo_flags = SECINFO_DACL|SECINFO_OWNER,
+ };
+
+ status = smb2_getinfo_file(tree, tctx, &gi);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_getinfo_file failed\n");
+
+ sd_orig = gi.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+ /*
+ * Add a 2 element ACL
+ *
+ * SEC_RIGHTS_FILE_READ|SEC_FILE_WRITE_DATA allow for owner.
+ * SEC_FILE_WRITE_DATA deny for owner
+ *
+ * Shows on Windows that trailing DENY entries don't
+ * override granted permissions.
+ */
+ sd = security_descriptor_dacl_create(tctx, 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_FILE_READ|SEC_FILE_WRITE_DATA,
+ 0,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ SEC_FILE_WRITE_DATA,
+ 0,
+ NULL);
+ torture_assert_not_null_goto(tctx, sd, ret, done,
+ "SD create failed\n");
+
+ si = (union smb_setfileinfo) {
+ .set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+ .set_secdesc.in.file.handle = handle,
+ .set_secdesc.in.secinfo_flags = SECINFO_DACL,
+ .set_secdesc.in.sd = sd,
+ };
+
+ status = smb2_setinfo_file(tree, &si);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL | SEC_FILE_WRITE_DATA,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.query_maximal_access = true,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+ handle = cr.out.file.handle;
+
+ mxac_status = NT_STATUS(cr.out.maximal_access_status);
+ torture_assert_ntstatus_ok_goto(tctx, mxac_status, ret, done,
+ "Wrong maximum access status\n");
+
+ /*
+ * For some reasons Windows 2016 doesn't set SEC_STD_DELETE but we
+ * do. Mask it out so the test passes against Samba and Windows.
+ * SEC_STD_WRITE_DAC comes from being the owner.
+ */
+ torture_assert_int_equal_goto(tctx,
+ cr.out.maximal_access & ~SEC_STD_DELETE,
+ SEC_RIGHTS_FILE_READ |
+ SEC_FILE_WRITE_DATA |
+ SEC_STD_WRITE_DAC,
+ ret, done,
+ "Wrong maximum access\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+done:
+ if (!smb2_util_handle_empty(handle)) {
+ smb2_util_close(tree, handle);
+ }
+ smb2_deltree(tree, BASEDIR);
+ return ret;
+}
+
/*
basic testing of SMB2 ACLs
*/
@@ -2800,6 +2938,8 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
test_owner_rights_deny);
torture_suite_add_1smb2_test(suite, "OWNER-RIGHTS-DENY1",
test_owner_rights_deny1);
+ torture_suite_add_1smb2_test(suite, "DENY1",
+ test_deny1);
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
--
2.17.2
From d9c38c42a81d06ab6c4c4fe29c02e9caadd4dc26 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 1 Mar 2019 18:57:23 +0100
Subject: [PATCH 06/11] libcli/security: fix handling of deny type ACEs in
access_check_max_allowed()
Deny ACEs must always be evaluated against explicitly granted rights
from previous ACEs.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13812
Signed-off-by: Ralph Boehme <slow at samba.org>
---
libcli/security/access_check.c | 2 +-
selftest/knownfail.d/smb2.acls | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
delete mode 100644 selftest/knownfail.d/smb2.acls
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index d1d57eecef2..322f4fdb0c6 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -173,7 +173,7 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
break;
case SEC_ACE_TYPE_ACCESS_DENIED:
case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
- denied |= ace->access_mask;
+ denied |= ~granted & ace->access_mask;
break;
default: /* Other ACE types not handled/supported */
break;
diff --git a/selftest/knownfail.d/smb2.acls b/selftest/knownfail.d/smb2.acls
deleted file mode 100644
index b76a3c719ce..00000000000
--- a/selftest/knownfail.d/smb2.acls
+++ /dev/null
@@ -1,4 +0,0 @@
-^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(ad_dc\)
-^samba3.smb2.acls.OWNER-RIGHTS-DENY1\(nt4_dc\)
-^samba3.smb2.acls.DENY1\(ad_dc\)
-^samba3.smb2.acls.DENY1\(nt4_dc\)
--
2.17.2
From 8a78e191a7172c0378d79d61127b5e2d68cb996f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 1 Mar 2019 18:06:48 +0100
Subject: [PATCH 07/11] s4:torture: add a test with additional bits in
SEC_FLAG_MAXIMUM_ALLOWED
When access_mask contains SEC_FLAG_MAXIMUM_ALLOWED, the server must still
proces other bits from access_mask. Eg if access_mask contains a right that
the requester doesn't have, the function must validate that against the
effective permissions.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source4/torture/smb2/acls.c | 111 ++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index 7bccce803f0..4f4538b15e3 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -2914,6 +2914,115 @@ static bool test_deny1(struct torture_context *tctx,
return ret;
}
+/*
+ * test SEC_FLAG_MAXIMUM_ALLOWED with not-granted access
+ *
+ * When access_mask contains SEC_FLAG_MAXIMUM_ALLOWED, the server must still
+ * proces other bits from access_mask. Eg if access_mask contains a right that
+ * the requester doesn't have, the function must validate that against the
+ * effective permissions.
+ */
+static bool test_mxac_not_granted(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ const char *fname = BASEDIR "\\test_mxac_not_granted.txt";
+ struct smb2_create cr;
+ struct smb2_handle handle = {{0}};
+ union smb_fileinfo gi;
+ union smb_setfileinfo si;
+ struct security_descriptor *sd_orig = NULL;
+ struct security_descriptor *sd = NULL;
+ const char *owner_sid = NULL;
+ NTSTATUS status;
+ bool ret = true;
+
+ smb2_deltree(tree, BASEDIR);
+
+ ret = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ret, ret, done,
+ "smb2_util_setup_dir failed\n");
+
+ torture_comment(tctx, "TESTING OWNER RIGHTS DENY\n");
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_STD_READ_CONTROL |
+ SEC_STD_WRITE_DAC |SEC_STD_WRITE_OWNER,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+ handle = cr.out.file.handle;
+
+ torture_comment(tctx, "get the original sd\n");
+
+ gi = (union smb_fileinfo) {
+ .query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+ .query_secdesc.in.file.handle = handle,
+ .query_secdesc.in.secinfo_flags = SECINFO_DACL|SECINFO_OWNER,
+ };
+
+ status = smb2_getinfo_file(tree, tctx, &gi);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_getinfo_file failed\n");
+
+ sd_orig = gi.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd_orig->owner_sid);
+
+ sd = security_descriptor_dacl_create(tctx, 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_FILE_READ_DATA,
+ 0,
+ NULL);
+ torture_assert_not_null_goto(tctx, sd, ret, done,
+ "SD create failed\n");
+
+ si = (union smb_setfileinfo) {
+ .set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+ .set_secdesc.in.file.handle = handle,
+ .set_secdesc.in.secinfo_flags = SECINFO_DACL,
+ .set_secdesc.in.sd = sd,
+ };
+
+ status = smb2_setinfo_file(tree, &si);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_setinfo_file failed\n");
+
+ status = smb2_util_close(tree, handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_close failed\n");
+ ZERO_STRUCT(handle);
+
+ cr = (struct smb2_create) {
+ .in.desired_access = SEC_FLAG_MAXIMUM_ALLOWED |
+ SEC_FILE_WRITE_DATA,
+ .in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+ .in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+ .in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+ .in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+ .in.fname = fname,
+ };
+
+ status = smb2_create(tree, tctx, &cr);
+ torture_assert_ntstatus_equal_goto(tctx, status,
+ NT_STATUS_ACCESS_DENIED,
+ ret, done,
+ "Wrong smb2_create result\n");
+
+done:
+ if (!smb2_util_handle_empty(handle)) {
+ smb2_util_close(tree, handle);
+ }
+ smb2_deltree(tree, BASEDIR);
+ return ret;
+}
+
/*
basic testing of SMB2 ACLs
*/
@@ -2940,6 +3049,8 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
test_owner_rights_deny1);
torture_suite_add_1smb2_test(suite, "DENY1",
test_deny1);
+ torture_suite_add_1smb2_test(suite, "MXAC-NOT-GRANTED",
+ test_mxac_not_granted);
suite->description = talloc_strdup(suite, "SMB2-ACLS tests");
--
2.17.2
From 729035bb18574f7141424035c2ab0b7ee56b2a15 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 1 Mar 2019 09:48:25 +0100
Subject: [PATCH 08/11] s3:libsmb: add cli_smb2_query_mxac()
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/libsmb/cli_smb2_fnum.c | 190 +++++++++++++++++++++++++++++++++
source3/libsmb/cli_smb2_fnum.h | 9 ++
2 files changed, 199 insertions(+)
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 795dc557c45..ef36ba88ee1 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -2781,6 +2781,196 @@ NTSTATUS cli_smb2_set_security_descriptor(struct cli_state *cli,
return status;
}
+/***************************************************************
+ Wrapper that allows SMB2 to query a security descriptor.
+ Synchronous only.
+
+***************************************************************/
+
+struct cli_smb2_mxac_state {
+ struct tevent_context *ev;
+ struct cli_state *cli;
+ const char *fname;
+ struct smb2_create_blobs in_cblobs;
+ uint16_t fnum;
+ NTSTATUS status;
+ uint32_t mxac;
+};
+
+static void cli_smb2_mxac_opened(struct tevent_req *subreq);
+static void cli_smb2_mxac_closed(struct tevent_req *subreq);
+
+struct tevent_req *cli_smb2_query_mxac_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct cli_state *cli,
+ const char *fname)
+{
+ struct tevent_req *req = NULL, *subreq = NULL;
+ struct cli_smb2_mxac_state *state = NULL;
+ NTSTATUS status;
+
+ req = tevent_req_create(mem_ctx, &state, struct cli_smb2_mxac_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ *state = (struct cli_smb2_mxac_state) {
+ state->ev = ev,
+ state->cli = cli,
+ state->fname = fname,
+ };
+
+ if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) {
+ tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
+ return tevent_req_post(req, ev);
+ }
+
+ status = smb2_create_blob_add(state,
+ &state->in_cblobs,
+ SMB2_CREATE_TAG_MXAC,
+ data_blob(NULL, 0));
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
+ }
+
+ subreq = cli_smb2_create_fnum_send(
+ state,
+ state->ev,
+ state->cli,
+ state->fname,
+ 0, /* create_flags */
+ SMB2_IMPERSONATION_IMPERSONATION,
+ FILE_READ_ATTRIBUTES,
+ 0, /* file attributes */
+ FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0, /* create_options */
+ &state->in_cblobs);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, cli_smb2_mxac_opened, req);
+ return req;
+}
+
+static void cli_smb2_mxac_opened(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct cli_smb2_mxac_state *state = tevent_req_data(
+ req, struct cli_smb2_mxac_state);
+ struct smb2_create_blobs out_cblobs = {0};
+ DATA_BLOB *mxac_blob = NULL;
+ NTSTATUS status;
+ int i;
+
+ status = cli_smb2_create_fnum_recv(
+ subreq, &state->fnum, NULL, state, &out_cblobs);
+ TALLOC_FREE(subreq);
+
+ if (tevent_req_nterror(req, status)) {
+ return;
+ }
+
+ for (i = 0; i < out_cblobs.num_blobs; i++) {
+ if (strcmp(out_cblobs.blobs[i].tag, SMB2_CREATE_TAG_MXAC) != 0) {
+ continue;
+ }
+ mxac_blob = &out_cblobs.blobs[i].data;
+ break;
+ }
+ if (mxac_blob == NULL) {
+ state->status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+ goto close;
+ }
+ if (mxac_blob->length != 8) {
+ state->status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+ goto close;
+ }
+
+ state->status = NT_STATUS(IVAL(mxac_blob->data, 0));
+ state->mxac = IVAL(mxac_blob->data, 4);
+
+close:
+ subreq = cli_smb2_close_fnum_send(
+ state, state->ev, state->cli, state->fnum);
+ if (tevent_req_nomem(subreq, req)) {
+ return;
+ }
+ tevent_req_set_callback(subreq, cli_smb2_mxac_closed, req);
+
+ return;
+}
+
+static void cli_smb2_mxac_closed(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ NTSTATUS status;
+
+ status = cli_smb2_close_fnum_recv(subreq);
+ if (tevent_req_nterror(req, status)) {
+ return;
+ }
+
+ tevent_req_done(req);
+}
+
+NTSTATUS cli_smb2_query_mxac_recv(struct tevent_req *req, uint32_t *mxac)
+{
+ struct cli_smb2_mxac_state *state = tevent_req_data(
+ req, struct cli_smb2_mxac_state);
+ NTSTATUS status;
+
+ if (tevent_req_is_nterror(req, &status)) {
+ return status;
+ }
+
+ if (!NT_STATUS_IS_OK(state->status)) {
+ return state->status;
+ }
+
+ *mxac = state->mxac;
+ return NT_STATUS_OK;
+}
+
+NTSTATUS cli_smb2_query_mxac(struct cli_state *cli,
+ const char *fname,
+ uint32_t *_mxac)
+{
+ TALLOC_CTX *frame = talloc_stackframe();
+ struct tevent_context *ev = NULL;
+ struct tevent_req *req = NULL;
+ NTSTATUS status = NT_STATUS_INTERNAL_ERROR;
+ bool ok;
+
+ if (smbXcli_conn_has_async_calls(cli->conn)) {
+ /*
+ * Can't use sync call while an async call is in flight
+ */
+ status = NT_STATUS_INVALID_PARAMETER;
+ goto fail;
+ }
+
+ ev = samba_tevent_context_init(frame);
+ if (ev == NULL) {
+ goto fail;
+ }
+ req = cli_smb2_query_mxac_send(frame, ev, cli, fname);
+ if (req == NULL) {
+ goto fail;
+ }
+ ok = tevent_req_poll_ntstatus(req, ev, &status);
+ if (!ok) {
+ goto fail;
+ }
+ status = cli_smb2_query_mxac_recv(req, _mxac);
+
+fail:
+ cli->raw_status = status;
+ TALLOC_FREE(frame);
+ return status;
+}
+
/***************************************************************
Wrapper that allows SMB2 to rename a file.
Synchronous only.
diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h
index 62ae093862f..c4ca069ee96 100644
--- a/source3/libsmb/cli_smb2_fnum.h
+++ b/source3/libsmb/cli_smb2_fnum.h
@@ -173,6 +173,15 @@ NTSTATUS cli_smb2_set_security_descriptor(struct cli_state *cli,
uint16_t fnum,
uint32_t sec_info,
const struct security_descriptor *sd);
+struct tevent_req *cli_smb2_query_mxac_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct cli_state *cli,
+ const char *fname);
+NTSTATUS cli_smb2_query_mxac_recv(struct tevent_req *req,
+ uint32_t *mxac);
+NTSTATUS cli_smb2_query_mxac(struct cli_state *cli,
+ const char *fname,
+ uint32_t *mxac);
NTSTATUS cli_smb2_rename(struct cli_state *cli,
const char *fname_src,
const char *fname_dst,
--
2.17.2
From 009309b0765ca6d4d062ddaa408894e1ca2fa7fa Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 1 Mar 2019 09:49:17 +0100
Subject: [PATCH 09/11] s3:libsmb: add cli_query_mxac()
Works only for SMB2.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/libsmb/clisecdesc.c | 12 ++++++++++++
source3/libsmb/proto.h | 4 ++++
2 files changed, 16 insertions(+)
diff --git a/source3/libsmb/clisecdesc.c b/source3/libsmb/clisecdesc.c
index c11e4b322ef..f9a7a057ed6 100644
--- a/source3/libsmb/clisecdesc.c
+++ b/source3/libsmb/clisecdesc.c
@@ -91,6 +91,18 @@ NTSTATUS cli_query_secdesc(struct cli_state *cli, uint16_t fnum,
return cli_query_security_descriptor(cli, fnum, sec_info, mem_ctx, sd);
}
+NTSTATUS cli_query_mxac(struct cli_state *cli,
+ const char *filename,
+ uint32_t *mxac)
+{
+ if (smbXcli_conn_protocol(cli->conn) < PROTOCOL_SMB2_02) {
+ return NT_STATUS_NOT_SUPPORTED;
+ }
+
+ return cli_smb2_query_mxac(cli, filename, mxac);
+}
+
+
/****************************************************************************
set the security descriptor for a open file
****************************************************************************/
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index b0cfcb5aa90..e1a54ef75f9 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -927,6 +927,10 @@ NTSTATUS cli_set_security_descriptor(struct cli_state *cli,
NTSTATUS cli_set_secdesc(struct cli_state *cli, uint16_t fnum,
const struct security_descriptor *sd);
+NTSTATUS cli_query_mxac(struct cli_state *cli,
+ const char *filename,
+ uint32_t *mxac);
+
/* The following definitions come from libsmb/clistr.c */
size_t clistr_pull_talloc(TALLOC_CTX *ctx,
--
2.17.2
From 63e7df6c76ce443b36398d8794192f7624809d8b Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 27 Feb 2019 16:45:07 +0100
Subject: [PATCH 10/11] smbcacls: add -x argument, prints maximum access
Signed-off-by: Ralph Boehme <slow at samba.org>
---
docs-xml/manpages/smbcacls.1.xml | 9 +++++++
source3/utils/smbcacls.c | 40 ++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+)
diff --git a/docs-xml/manpages/smbcacls.1.xml b/docs-xml/manpages/smbcacls.1.xml
index 6071047682d..7f87da80329 100644
--- a/docs-xml/manpages/smbcacls.1.xml
+++ b/docs-xml/manpages/smbcacls.1.xml
@@ -38,6 +38,7 @@
<arg choice="opt">--set-security-info FLAGS</arg>
<arg choice="opt">--sddl</arg>
<arg choice="opt">--domain-sid SID</arg>
+ <arg choice="opt">-x|--maximum-access</arg>
</cmdsynopsis>
</refsynopsisdiv>
@@ -189,6 +190,14 @@
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term>-x|--maximum-access</term>
+ <listitem><para>When displaying an ACL additionally query
+ the server for effective maximum permissions. Note that this
+ is only supported with SMB protocol version 2 or higher.
+ </para></listitem>
+ </varlistentry>
+
&stdarg.server.debug;
&popt.common.samba;
&popt.common.credentials;
diff --git a/source3/utils/smbcacls.c b/source3/utils/smbcacls.c
index a3a40e9eeb9..b61d11df860 100644
--- a/source3/utils/smbcacls.c
+++ b/source3/utils/smbcacls.c
@@ -40,6 +40,7 @@ static int test_args;
static int sddl;
static int query_sec_info = -1;
static int set_sec_info = -1;
+static bool want_mxac;
static const char *domain_sid = NULL;
@@ -359,12 +360,33 @@ static bool set_secdesc(struct cli_state *cli, const char *filename,
return result;
}
+/*****************************************************
+get maximum access for a file
+*******************************************************/
+static int cacl_mxac(struct cli_state *cli, const char *filename)
+{
+ NTSTATUS status;
+ uint32_t mxac;
+
+ status = cli_query_mxac(cli, filename, &mxac);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Failed to get mxac: %s\n", nt_errstr(status));
+ return EXIT_FAILED;
+ }
+
+ printf("Maximum access: 0x%x\n", mxac);
+
+ return EXIT_OK;
+}
+
+
/*****************************************************
dump the acls for a file
*******************************************************/
static int cacl_dump(struct cli_state *cli, const char *filename, bool numeric)
{
struct security_descriptor *sd;
+ int ret;
if (test_args) {
return EXIT_OK;
@@ -386,6 +408,13 @@ static int cacl_dump(struct cli_state *cli, const char *filename, bool numeric)
sec_desc_print(cli, stdout, sd, numeric);
}
+ if (want_mxac) {
+ ret = cacl_mxac(cli, filename);
+ if (ret != EXIT_OK) {
+ return ret;
+ }
+ }
+
return EXIT_OK;
}
@@ -910,6 +939,14 @@ int main(int argc, char *argv[])
.descrip = "Set the max protocol level",
.argDescrip = "LEVEL",
},
+ {
+ .longName = "maximum-access",
+ .shortName = 'x',
+ .argInfo = POPT_ARG_NONE,
+ .arg = NULL,
+ .val = 'x',
+ .descrip = "Query maximum persmissions",
+ },
POPT_COMMON_SAMBA
POPT_COMMON_CONNECTION
POPT_COMMON_CREDENTIALS
@@ -975,6 +1012,9 @@ int main(int argc, char *argv[])
case 'm':
lp_set_cmdline("client max protocol", poptGetOptArg(pc));
break;
+ case 'x':
+ want_mxac = true;
+ break;
}
}
--
2.17.2
From 396ef9b47087ead591d893c398f764775b464981 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 2 Mar 2019 15:37:38 +0100
Subject: [PATCH 11/11] tests: add a simple test for smbcacls -x
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/script/tests/test_acl_xattr.sh | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/source3/script/tests/test_acl_xattr.sh b/source3/script/tests/test_acl_xattr.sh
index cba79122045..f134ff79c91 100755
--- a/source3/script/tests/test_acl_xattr.sh
+++ b/source3/script/tests/test_acl_xattr.sh
@@ -33,6 +33,22 @@ setup_remote_file() {
$SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "lcd $PREFIX; put $fname" || exit 1
}
+smbcacls_x() {
+ local share=$1
+ local fname="$share.$$"
+
+ # skip with SMB1
+ echo "$ADDARGS" | grep mNT1 && exit 0
+
+ $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD "$fname" -x || exit 1
+ mxac=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD "$fname" -x | awk '/Maximum access/ {print $3}')
+
+ echo "mxac: $mxac"
+ if test "$mxac" != "0x1f01ff" ; then
+ exit 1
+ fi
+}
+
nt_affects_posix() {
local share=$1
local expected=$2
@@ -122,6 +138,7 @@ nt_affects_chgrp() {
testit "setup remote file tmp" setup_remote_file tmp
testit "setup remote file ign_sysacls" setup_remote_file ign_sysacls
+testit "smbcacls -x" smbcacls_x tmp
testit "nt_affects_posix tmp" nt_affects_posix tmp "true"
testit "nt_affects_posix ign_sysacls" nt_affects_posix ign_sysacls "false"
testit "setup remote file tmp" setup_remote_file tmp
--
2.17.2
More information about the samba-technical
mailing list