The branch, master has been updated
via b5c0745 s3: torture: Adds regression test case for
se_access_check() owner rights issue.
via 29b02cf lib: security: se_access_check() incorrectly processes
owner rights (S-1-3-4) DENY ace entries
from 44a01a2 util: use SCOPE_DELIMITER for the IPv6 scope delimiter
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit b5c0745b0c99d6cef21b5e7eb695e15aae5d4e38
Author: Jeremy Allison <j...@samba.org>
Date: Thu Dec 8 10:40:27 2016 -0800
s3: torture: Adds regression test case for se_access_check() owner rights
issue.
This test passes against Win2K12 but fails against smbd
without the previous commit.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12466
Signed-off-by: Jeremy Allison <j...@samba.org>
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Ralph Boehme <s...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Sat Dec 10 10:11:10 CET 2016 on sn-devel-144
commit 29b02cf22f3c0f2d556408e9e768d68c1efc3b96
Author: Jeremy Allison <j...@samba.org>
Date: Thu Dec 8 10:40:18 2016 -0800
lib: security: se_access_check() incorrectly processes owner rights
(S-1-3-4) DENY ace entries
Reported and proposed fix by Shilpa K <shilpa.krishnare...@gmail.com>.
When processing DENY ACE entries for owner rights SIDs (S-1-3-4) the
code OR's in the deny access mask bits without taking into account if
they were being requested in the requested access mask.
E.g. The current logic has:
An ACL containining:
[0] SID: S-1-3-4
TYPE: DENY
MASK: WRITE_DATA
[1] SID: S-1-3-4
TYPE: ALLOW
MASK: ALLOW_ALL
prohibits an open request by the owner for READ_DATA - even though this
is explicitly allowed.
Furthermore a non-canonical ACL containing:
[0] SID: User SID 1-5-21-something
TYPE: ALLOW
MASK: READ_DATA
[1] SID: S-1-3-4
TYPE: DENY
MASK: READ_DATA
[2] SID: User SID 1-5-21-something
TYPE: ALLOW
MASK: WRITE_DATA
prohibits an open request by the owner for READ_DATA|WRITE_DATA - even
though READ_DATA is explicitly allowed in ACE no 0 and is thus already
filtered out of the "access-still-needed" mask when the deny ACE no 1 is
evaluated.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12466
Signed-off-by: Jeremy Allison <j...@samba.org>
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Ralph Boehme <s...@samba.org>
-----------------------------------------------------------------------
Summary of changes:
libcli/security/access_check.c | 2 +-
selftest/skip | 1 +
source3/selftest/tests.py | 2 +-
source3/torture/torture.c | 386 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 389 insertions(+), 2 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index 2be5928..b4c850b 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -220,7 +220,7 @@ NTSTATUS se_access_check(const struct security_descriptor
*sd,
owner_rights_allowed |= ace->access_mask;
owner_rights_default = false;
} else if (ace->type == SEC_ACE_TYPE_ACCESS_DENIED) {
- owner_rights_denied |= ace->access_mask;
+ owner_rights_denied |= (bits_remaining &
ace->access_mask);
owner_rights_default = false;
}
continue;
diff --git a/selftest/skip b/selftest/skip
index 0893962..1e6d311 100644
--- a/selftest/skip
+++ b/selftest/skip
@@ -49,6 +49,7 @@
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-OFD-LOCK # Fails against the
s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-STREAM-DELETE # Fails against
the s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).RENAME-ACCESS # Fails against the
s4 ntvfs server
+^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).OWNER-RIGHTS # Don't test against
the s4 ntvfs server anymore
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).PIDHIGH # Fails against the s4
ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the
s4 ntvfs server
^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4
ntvfs server
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index a678c77..d9d32cc 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -49,7 +49,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4",
"LOCK5", "LOCK6", "LOCK7"
"OPLOCK1", "OPLOCK2", "OPLOCK4", "STREAMERROR",
"DIR", "DIR1", "DIR-CREATETIME", "TCON", "TCONDEV", "RW1", "RW2",
"RW3", "LARGE_READX", "RW-SIGNING",
"OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "WILDDELETE",
"PROPERTIES", "W2K",
- "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2",
+ "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2",
"OWNER-RIGHTS",
"CHAIN3", "PIDHIGH",
"GETADDRINFO", "UID-REGRESSION-TEST", "SHORTNAME-TEST",
"CASE-INSENSITIVE-CREATE", "SMB2-BASIC", "NTTRANS-FSCTL",
"SMB2-NEGPROT",
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 09e8b93..8f0f136 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -5092,6 +5092,391 @@ static bool run_rename_access(int dummy)
return false;
}
+/*
+ Test owner rights ACE.
+ */
+static bool run_owner_rights(int dummy)
+{
+ static struct cli_state *cli = NULL;
+ const char *fname = "owner_rights.txt";
+ uint16_t fnum = (uint16_t)-1;
+ struct security_descriptor *sd = NULL;
+ struct security_descriptor *newsd = NULL;
+ NTSTATUS status;
+ TALLOC_CTX *frame = NULL;
+
+ frame = talloc_stackframe();
+ printf("starting owner rights test\n");
+
+ /* Windows connection. */
+ if (!torture_open_connection(&cli, 0)) {
+ goto fail;
+ }
+
+ smbXcli_conn_set_sockopt(cli->conn, sockops);
+
+ /* Start with a clean slate. */
+ cli_unlink(cli, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ /* Create the test file. */
+ /* Now try and open for read and write-dac. */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ GENERIC_ALL_ACCESS,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_CREATE,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Create of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Get the original SD. */
+ status = cli_query_secdesc(cli,
+ fnum,
+ frame,
+ &sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_query_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /*
+ * Add an "owner-rights" ACE denying WRITE_DATA,
+ * and an "owner-rights" ACE allowing READ_DATA.
+ */
+
+ newsd = security_descriptor_dacl_create(frame,
+ 0,
+ NULL,
+ NULL,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ FILE_WRITE_DATA,
+ 0,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ FILE_READ_DATA,
+ 0,
+ NULL);
+ if (newsd == NULL) {
+ goto fail;
+ }
+ sd->dacl = security_acl_concatenate(frame,
+ newsd->dacl,
+ sd->dacl);
+ if (sd->dacl == NULL) {
+ goto fail;
+ }
+ status = cli_set_secdesc(cli, fnum, sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_set_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ fnum = (uint16_t)-1;
+
+ /* Try and open for FILE_WRITE_DATA */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_WRITE_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+ printf("Open of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Now try and open for FILE_READ_DATA */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_READ_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Open of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Restore clean slate. */
+ TALLOC_FREE(sd);
+ cli_unlink(cli, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ /* Create the test file. */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ GENERIC_ALL_ACCESS,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_CREATE,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Create of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Get the original SD. */
+ status = cli_query_secdesc(cli,
+ fnum,
+ frame,
+ &sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_query_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /*
+ * Add an "owner-rights ACE denying WRITE_DATA,
+ * and an "owner-rights ACE allowing READ_DATA|WRITE_DATA.
+ */
+
+ newsd = security_descriptor_dacl_create(frame,
+ 0,
+ NULL,
+ NULL,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ FILE_WRITE_DATA,
+ 0,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ FILE_READ_DATA|FILE_WRITE_DATA,
+ 0,
+ NULL);
+ if (newsd == NULL) {
+ goto fail;
+ }
+ sd->dacl = security_acl_concatenate(frame,
+ newsd->dacl,
+ sd->dacl);
+ if (sd->dacl == NULL) {
+ goto fail;
+ }
+ status = cli_set_secdesc(cli, fnum, sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_set_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ fnum = (uint16_t)-1;
+
+ /* Try and open for FILE_WRITE_DATA */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_WRITE_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+ printf("Open of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Now try and open for FILE_READ_DATA */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_READ_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Open of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Restore clean slate. */
+ TALLOC_FREE(sd);
+ cli_unlink(cli, fname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+
+ /* Create the test file. */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ GENERIC_ALL_ACCESS,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_CREATE,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Create of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /* Get the original SD. */
+ status = cli_query_secdesc(cli,
+ fnum,
+ frame,
+ &sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_query_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ /*
+ * Add an "authenticated users" ACE allowing READ_DATA,
+ * add an "owner-rights" denying READ_DATA,
+ * and an "authenticated users" ACE allowing WRITE_DATA.
+ */
+
+ newsd = security_descriptor_dacl_create(frame,
+ 0,
+ NULL,
+ NULL,
+ SID_NT_AUTHENTICATED_USERS,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ FILE_READ_DATA,
+ 0,
+ SID_OWNER_RIGHTS,
+ SEC_ACE_TYPE_ACCESS_DENIED,
+ FILE_READ_DATA,
+ 0,
+ SID_NT_AUTHENTICATED_USERS,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ FILE_WRITE_DATA,
+ 0,
+ NULL);
+ if (newsd == NULL) {
+ printf("newsd == NULL\n");
+ goto fail;
+ }
+ sd->dacl = security_acl_concatenate(frame,
+ newsd->dacl,
+ sd->dacl);
+ if (sd->dacl == NULL) {
+ printf("sd->dacl == NULL\n");
+ goto fail;
+ }
+ status = cli_set_secdesc(cli, fnum, sd);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("cli_set_secdesc failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+ fnum = (uint16_t)-1;
+
+ /* Now try and open for FILE_READ_DATA|FILE_WRITE_DATA */
+ status = cli_ntcreate(cli,
+ fname,
+ 0,
+ FILE_READ_DATA|FILE_WRITE_DATA,
+ FILE_ATTRIBUTE_NORMAL,
+ FILE_SHARE_READ|FILE_SHARE_WRITE|
+ FILE_SHARE_DELETE,
+ FILE_OPEN,
+ 0,
+ 0,
+ &fnum,
+ NULL);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("Open of %s - %s\n", fname, nt_errstr(status));
+ goto fail;
+ }
+
+ status = cli_close(cli, fnum);
+ if (!NT_STATUS_IS_OK(status)) {
+ printf("close failed for %s (%s)\n",
+ fname, nt_errstr(status));
+ goto fail;
+ }
+
+ cli_unlink(cli, fname,
+ FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+
+ TALLOC_FREE(frame);
+ return true;
+
+ fail:
+
+ if (cli) {
+ if (fnum != -1) {
+ cli_close(cli, fnum);
+ }
+ cli_unlink(cli, fname,
+ FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
+ torture_close_connection(cli);
+ }
+
+ TALLOC_FREE(frame);
+ return false;
+}
+
static bool run_pipe_number(int dummy)
{
struct cli_state *cli1;
@@ -10650,6 +11035,7 @@ static struct {
{"XCOPY", run_xcopy, 0},
{"RENAME", run_rename, 0},
{"RENAME-ACCESS", run_rename_access, 0},
+ {"OWNER-RIGHTS", run_owner_rights, 0},
{"DELETE", run_deletetest, 0},
{"WILDDELETE", run_wild_deletetest, 0},
{"DELETE-LN", run_deletetest_ln, 0},
--
Samba Shared Repository
