[Samba] Issue with acl_xattr:ignore system acls in 4.5rc2
Ralph Böhme
slow at samba.org
Sat Aug 27 10:46:12 UTC 2016
On Sat, Aug 27, 2016 at 10:14:57AM +0200, Ralph Böhme via samba wrote:
> On Fri, Aug 26, 2016 at 04:03:49PM -0700, Jeremy Allison wrote:
> > On Fri, Aug 26, 2016 at 02:46:19PM -0700, Jeremy Allison via samba wrote:
> > > On Fri, Aug 26, 2016 at 06:44:05PM +0200, Ralph Böhme wrote:
> > > >
> > > > Cheerio!
> > > > -slow
> > >
> > > Still reviewing this - but a few things that will need changing:
> > >
> > > When adding the validate_nt_acl_blob() function in
> > > [PATCH 06/12] vfs_acl_common: move the ACL blob validation to a helper function
> > >
> > > this makes some of the existing function names in debug statements
> > > incorrect.
>
> thanks for spotting this!
>
> > >
> > > Eg. validate_nt_acl_blob() has debug statements:
> >
> > How about adding this on top - or squashing into it ?
>
> Updated patchset attached. My patches now (hopefully) update all DEBUG
> calls they touch. New patch on-top that does the rest of the file,
> using the new DBG_LEVEL macros in all places.
...and this one even has bug urls in all commit messages. Sorry for
forgetting this in the previous version.
Cheerio!
-slow
-------------- next part --------------
From 874b48593225e204e62c9b998a23b021c27dd023 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:04:24 +0200
Subject: [PATCH 01/13] Revert "vfs_acl_xattr: objects without NT ACL xattr"
This reverts commit 961c4b591bb102751079d9cc92d7aa1c37f1958c.
Subsequent commits will add the same functionality as an optional
feature.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 43 ++++++++++++++++++++++++++++++++++++----
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2fda938e..a287945 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -379,10 +379,12 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
gid_to_sid(&group_sid, psbuf->st_ex_gid);
/*
- * We provide 2 ACEs:
- * - Owner
- * - NT System
- */
+ We provide up to 4 ACEs
+ - Owner
+ - Group
+ - Everyone
+ - NT System
+ */
if (mode & S_IRUSR) {
if (mode & S_IWUSR) {
@@ -402,6 +404,39 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
0);
idx++;
+ access_mask = 0;
+ if (mode & S_IRGRP) {
+ access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+ }
+ if (mode & S_IWGRP) {
+ /* note that delete is not granted - this matches posix behaviour */
+ access_mask |= SEC_RIGHTS_FILE_WRITE;
+ }
+ if (access_mask) {
+ init_sec_ace(&aces[idx],
+ &group_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ access_mask,
+ 0);
+ idx++;
+ }
+
+ access_mask = 0;
+ if (mode & S_IROTH) {
+ access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+ }
+ if (mode & S_IWOTH) {
+ access_mask |= SEC_RIGHTS_FILE_WRITE;
+ }
+ if (access_mask) {
+ init_sec_ace(&aces[idx],
+ &global_sid_World,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ access_mask,
+ 0);
+ idx++;
+ }
+
init_sec_ace(&aces[idx],
&global_sid_System,
SEC_ACE_TYPE_ACCESS_ALLOWED,
--
2.7.4
From 55d74651c664d210a55b6833dee575dc1b101271 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:08:12 +0200
Subject: [PATCH 02/13] vfs_acl_common: rename psd to psd_blob in
get_nt_acl_internal()
This makes it explicit where the SD is originating from. No change in
behaviour.
This just paves the way for a later change that will simplify the whole
logic and talloc hierarchy, therefor this also strictly renames the
occurences after the out label.
Logically, behind the out label, we're dealing with a variable that
points to what we're going to return, so the name psd_blob is
misleading, but I'm desperately trying to avoid logic changes in this
commit and therefor I'm just strictly renaming.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 58 ++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index a287945..1adc875 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -488,7 +488,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE];
uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
- struct security_descriptor *psd = NULL;
+ struct security_descriptor *psd_blob = NULL;
struct security_descriptor *pdesc_next = NULL;
const struct smb_filename *smb_fname = NULL;
bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
@@ -509,25 +509,25 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
nt_errstr(status)));
- psd = NULL;
+ psd_blob = NULL;
goto out;
} else {
- status = parse_acl_blob(&blob, mem_ctx, &psd,
+ status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
&hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("parse_acl_blob returned %s\n",
nt_errstr(status)));
- psd = NULL;
+ psd_blob = NULL;
goto out;
}
}
- /* Ensure we don't leak psd if we don't choose it.
+ /* Ensure we don't leak psd_blob if we don't choose it.
*
* We don't allocate it onto frame as it is preferred not to
* steal from a talloc pool.
*/
- talloc_steal(frame, psd);
+ talloc_steal(frame, psd_blob);
/* determine which type of xattr we got */
switch (xattr_version) {
@@ -550,8 +550,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
"mismatch (%u) for file %s\n",
(unsigned int)hash_type,
smb_fname->base_name));
- TALLOC_FREE(psd);
- psd = NULL;
+ TALLOC_FREE(psd_blob);
+ psd_blob = NULL;
goto out;
}
@@ -561,8 +561,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
"(%u) unexpected for file %s\n",
(unsigned int)hash_type,
smb_fname->base_name));
- TALLOC_FREE(psd);
- psd = NULL;
+ TALLOC_FREE(psd_blob);
+ psd_blob = NULL;
goto out;
}
@@ -645,8 +645,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
status = hash_sd_sha256(pdesc_next, hash_tmp);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(psd);
- psd = pdesc_next;
+ TALLOC_FREE(psd_blob);
+ psd_blob = pdesc_next;
goto out;
}
@@ -670,12 +670,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
}
- TALLOC_FREE(psd);
- psd = pdesc_next;
+ TALLOC_FREE(psd_blob);
+ psd_blob = pdesc_next;
}
out:
- if (psd == NULL) {
+ if (psd_blob == NULL) {
/* Get the full underlying sd, as we failed to get the
* blob for the hash, or the revision/hash type wasn't
* known */
@@ -708,10 +708,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
* steal from a talloc pool.
*/
talloc_steal(frame, pdesc_next);
- psd = pdesc_next;
+ psd_blob = pdesc_next;
}
- if (psd != pdesc_next) {
+ if (psd_blob != pdesc_next) {
/* We're returning the blob, throw
* away the filesystem SD. */
TALLOC_FREE(pdesc_next);
@@ -764,20 +764,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
status = make_default_filesystem_acl(mem_ctx,
smb_fname->base_name,
psbuf,
- &psd);
+ &psd_blob);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
}
} else {
if (is_directory &&
- !sd_has_inheritable_components(psd,
+ !sd_has_inheritable_components(psd_blob,
true)) {
status = add_directory_inheritable_components(
handle,
smb_fname->base_name,
psbuf,
- psd);
+ psd_blob);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(frame);
return status;
@@ -787,23 +787,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
can't be inherited in this way under POSIX.
Remove it for Windows-style ACLs. */
- psd->type &= ~SEC_DESC_DACL_PROTECTED;
+ psd_blob->type &= ~SEC_DESC_DACL_PROTECTED;
}
}
if (!(security_info & SECINFO_OWNER)) {
- psd->owner_sid = NULL;
+ psd_blob->owner_sid = NULL;
}
if (!(security_info & SECINFO_GROUP)) {
- psd->group_sid = NULL;
+ psd_blob->group_sid = NULL;
}
if (!(security_info & SECINFO_DACL)) {
- psd->type &= ~SEC_DESC_DACL_PRESENT;
- psd->dacl = NULL;
+ psd_blob->type &= ~SEC_DESC_DACL_PRESENT;
+ psd_blob->dacl = NULL;
}
if (!(security_info & SECINFO_SACL)) {
- psd->type &= ~SEC_DESC_SACL_PRESENT;
- psd->sacl = NULL;
+ psd_blob->type &= ~SEC_DESC_SACL_PRESENT;
+ psd_blob->sacl = NULL;
}
TALLOC_FREE(blob.data);
@@ -811,11 +811,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (DEBUGLEVEL >= 10) {
DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
smb_fname->base_name ));
- NDR_PRINT_DEBUG(security_descriptor, psd);
+ NDR_PRINT_DEBUG(security_descriptor, psd_blob);
}
/* The VFS API is that the ACL is expected to be on mem_ctx */
- *ppdesc = talloc_move(mem_ctx, &psd);
+ *ppdesc = talloc_move(mem_ctx, &psd_blob);
TALLOC_FREE(frame);
return NT_STATUS_OK;
--
2.7.4
From b37829fdcc5dac4e289b3b6ccc5a05fad4e529a5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:11:24 +0200
Subject: [PATCH 03/13] vfs_acl_common: rename pdesc_next to psd_fs
In most realistic cases the "next" VFS op will return the permissions
from the filesystem. This rename makes it explicit where the SD is
originating from. No change in behaviour.
This just paves the way for a later change that will simplify the whole
logic and talloc hierarchy.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 1adc875..fb01bf4 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -489,7 +489,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
struct security_descriptor *psd_blob = NULL;
- struct security_descriptor *pdesc_next = NULL;
+ struct security_descriptor *psd_fs = NULL;
const struct smb_filename *smb_fname = NULL;
bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
ACL_MODULE_NAME,
@@ -618,13 +618,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
fsp,
HASH_SECURITY_INFO,
mem_ctx,
- &pdesc_next);
+ &psd_fs);
} else {
status = SMB_VFS_NEXT_GET_NT_ACL(handle,
smb_fname,
HASH_SECURITY_INFO,
mem_ctx,
- &pdesc_next);
+ &psd_fs);
}
if (!NT_STATUS_IS_OK(status)) {
@@ -636,17 +636,17 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
return status;
}
- /* Ensure we don't leak psd_next if we don't choose it.
+ /* Ensure we don't leak psd_fs if we don't choose it.
*
* We don't allocate it onto frame as it is preferred not to
* steal from a talloc pool.
*/
- talloc_steal(frame, pdesc_next);
+ talloc_steal(frame, psd_fs);
- status = hash_sd_sha256(pdesc_next, hash_tmp);
+ status = hash_sd_sha256(psd_fs, hash_tmp);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(psd_blob);
- psd_blob = pdesc_next;
+ psd_blob = psd_fs;
goto out;
}
@@ -667,11 +667,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (DEBUGLEVEL >= 10) {
DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n",
smb_fname->base_name ));
- NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
+ NDR_PRINT_DEBUG(security_descriptor, psd_fs);
}
TALLOC_FREE(psd_blob);
- psd_blob = pdesc_next;
+ psd_blob = psd_fs;
}
out:
@@ -684,13 +684,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
fsp,
security_info,
mem_ctx,
- &pdesc_next);
+ &psd_fs);
} else {
status = SMB_VFS_NEXT_GET_NT_ACL(handle,
smb_fname,
security_info,
mem_ctx,
- &pdesc_next);
+ &psd_fs);
}
if (!NT_STATUS_IS_OK(status)) {
@@ -702,19 +702,19 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
return status;
}
- /* Ensure we don't leak psd_next if we don't choose it.
+ /* Ensure we don't leak psd_fs if we don't choose it.
*
* We don't allocate it onto frame as it is preferred not to
* steal from a talloc pool.
*/
- talloc_steal(frame, pdesc_next);
- psd_blob = pdesc_next;
+ talloc_steal(frame, psd_fs);
+ psd_blob = psd_fs;
}
- if (psd_blob != pdesc_next) {
+ if (psd_blob != psd_fs) {
/* We're returning the blob, throw
* away the filesystem SD. */
- TALLOC_FREE(pdesc_next);
+ TALLOC_FREE(psd_fs);
} else {
SMB_STRUCT_STAT sbuf;
SMB_STRUCT_STAT *psbuf = &sbuf;
@@ -760,7 +760,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
is_directory = S_ISDIR(psbuf->st_ex_mode);
if (ignore_file_system_acl) {
- TALLOC_FREE(pdesc_next);
+ TALLOC_FREE(psd_fs);
status = make_default_filesystem_acl(mem_ctx,
smb_fname->base_name,
psbuf,
--
2.7.4
From 0d2d6afb990c382a508da1823631e1b76af57019 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:14:50 +0200
Subject: [PATCH 04/13] vfs_acl_common: remove redundant NULL assignment
The variables are already set to NULL by TALLOC_FREE.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index fb01bf4..0c40f37 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -551,7 +551,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
(unsigned int)hash_type,
smb_fname->base_name));
TALLOC_FREE(psd_blob);
- psd_blob = NULL;
goto out;
}
@@ -562,7 +561,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
(unsigned int)hash_type,
smb_fname->base_name));
TALLOC_FREE(psd_blob);
- psd_blob = NULL;
goto out;
}
--
2.7.4
From 832b1f982b3800c65b2031e4f2bb51c555997cdd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 17:07:20 +0200
Subject: [PATCH 05/13] vfs_acl_common: simplify ACL logic, cleanup and talloc
hierarchy
No change in behaviour (hopefully! :-). This paves the way for moving
the ACL blob validation to a helper function in the next commit.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 131 ++++++++++++++++++---------------------
1 file changed, 61 insertions(+), 70 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 0c40f37..66c58e7 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -488,6 +488,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE];
uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
+ struct security_descriptor *psd = NULL;
struct security_descriptor *psd_blob = NULL;
struct security_descriptor *psd_fs = NULL;
const struct smb_filename *smb_fname = NULL;
@@ -495,7 +496,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
ACL_MODULE_NAME,
"ignore system acls",
false);
- TALLOC_CTX *frame = talloc_stackframe();
+ char *sys_acl_blob_description = NULL;
+ DATA_BLOB sys_acl_blob = { 0 };
+ bool psd_is_from_fs = false;
if (fsp && smb_fname_in == NULL) {
smb_fname = fsp->fsp_name;
@@ -505,11 +508,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
- status = get_acl_blob(frame, handle, fsp, smb_fname, &blob);
+ status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
nt_errstr(status)));
- psd_blob = NULL;
goto out;
} else {
status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
@@ -517,17 +519,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10, ("parse_acl_blob returned %s\n",
nt_errstr(status)));
- psd_blob = NULL;
+ TALLOC_FREE(blob.data);
goto out;
}
}
- /* Ensure we don't leak psd_blob if we don't choose it.
- *
- * We don't allocate it onto frame as it is preferred not to
- * steal from a talloc pool.
- */
- talloc_steal(frame, psd_blob);
+ TALLOC_FREE(blob.data);
/* determine which type of xattr we got */
switch (xattr_version) {
@@ -537,10 +534,14 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
* require confirmation of the hash. In particular,
* the NTVFS file server uses version 1, but
* 'samba-tool ntacl' can set these as well */
+ psd = psd_blob;
+ psd_blob = NULL;
goto out;
case 3:
case 4:
if (ignore_file_system_acl) {
+ psd = psd_blob;
+ psd_blob = NULL;
goto out;
}
@@ -569,20 +570,18 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
case 4:
{
int ret;
- char *sys_acl_blob_description;
- DATA_BLOB sys_acl_blob;
if (fsp) {
/* Get the full underlying sd, then hash. */
ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FD(handle,
fsp,
- frame,
+ mem_ctx,
&sys_acl_blob_description,
&sys_acl_blob);
} else {
/* Get the full underlying sd, then hash. */
ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FILE(handle,
smb_fname->base_name,
- frame,
+ mem_ctx,
&sys_acl_blob_description,
&sys_acl_blob);
}
@@ -592,16 +591,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (ret == 0) {
status = hash_blob_sha256(sys_acl_blob, sys_acl_hash_tmp);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
+ TALLOC_FREE(sys_acl_blob_description);
+ TALLOC_FREE(sys_acl_blob.data);
+
if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0],
XATTR_SD_HASH_SIZE) == 0) {
/* Hash matches, return blob sd. */
DEBUG(10, ("get_nt_acl_internal: blob hash "
"matches for file %s\n",
smb_fname->base_name ));
+ psd = psd_blob;
+ psd_blob = NULL;
goto out;
}
}
@@ -630,21 +633,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
"returned %s\n",
smb_fname->base_name,
nt_errstr(status)));
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
- /* Ensure we don't leak psd_fs if we don't choose it.
- *
- * We don't allocate it onto frame as it is preferred not to
- * steal from a talloc pool.
- */
- talloc_steal(frame, psd_fs);
-
status = hash_sd_sha256(psd_fs, hash_tmp);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(psd_blob);
- psd_blob = psd_fs;
+ psd = psd_fs;
+ psd_fs = NULL;
+ psd_is_from_fs = true;
goto out;
}
@@ -653,6 +650,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
DEBUG(10, ("get_nt_acl_internal: blob hash "
"matches for file %s\n",
smb_fname->base_name ));
+ psd = psd_blob;
+ psd_blob = NULL;
goto out;
}
@@ -669,11 +668,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
}
TALLOC_FREE(psd_blob);
- psd_blob = psd_fs;
+ psd = psd_fs;
+ psd_fs = NULL;
+ psd_is_from_fs = true;
}
- out:
- if (psd_blob == NULL) {
+out:
+ if (psd == NULL) {
/* Get the full underlying sd, as we failed to get the
* blob for the hash, or the revision/hash type wasn't
* known */
@@ -682,13 +683,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
fsp,
security_info,
mem_ctx,
- &psd_fs);
+ &psd);
} else {
status = SMB_VFS_NEXT_GET_NT_ACL(handle,
smb_fname,
security_info,
mem_ctx,
- &psd_fs);
+ &psd);
}
if (!NT_STATUS_IS_OK(status)) {
@@ -696,24 +697,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
"returned %s\n",
smb_fname->base_name,
nt_errstr(status)));
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
- /* Ensure we don't leak psd_fs if we don't choose it.
- *
- * We don't allocate it onto frame as it is preferred not to
- * steal from a talloc pool.
- */
- talloc_steal(frame, psd_fs);
- psd_blob = psd_fs;
+ psd_is_from_fs = true;
}
- if (psd_blob != psd_fs) {
- /* We're returning the blob, throw
- * away the filesystem SD. */
- TALLOC_FREE(psd_fs);
- } else {
+ if (psd_is_from_fs) {
SMB_STRUCT_STAT sbuf;
SMB_STRUCT_STAT *psbuf = &sbuf;
bool is_directory = false;
@@ -725,8 +715,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (fsp) {
status = vfs_stat_fsp(fsp);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
psbuf = &fsp->fsp_name->st;
} else {
@@ -751,72 +740,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
smb_fname,
&sbuf);
if (ret == -1) {
- TALLOC_FREE(frame);
- return map_nt_error_from_unix(errno);
+ status = map_nt_error_from_unix(errno);
+ goto fail;
}
}
is_directory = S_ISDIR(psbuf->st_ex_mode);
if (ignore_file_system_acl) {
- TALLOC_FREE(psd_fs);
+ TALLOC_FREE(psd);
status = make_default_filesystem_acl(mem_ctx,
smb_fname->base_name,
psbuf,
- &psd_blob);
+ &psd);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
} else {
if (is_directory &&
- !sd_has_inheritable_components(psd_blob,
+ !sd_has_inheritable_components(psd,
true)) {
status = add_directory_inheritable_components(
handle,
smb_fname->base_name,
psbuf,
- psd_blob);
+ psd);
if (!NT_STATUS_IS_OK(status)) {
- TALLOC_FREE(frame);
- return status;
+ goto fail;
}
}
/* The underlying POSIX module always sets
the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
can't be inherited in this way under POSIX.
Remove it for Windows-style ACLs. */
- psd_blob->type &= ~SEC_DESC_DACL_PROTECTED;
+ psd->type &= ~SEC_DESC_DACL_PROTECTED;
}
}
if (!(security_info & SECINFO_OWNER)) {
- psd_blob->owner_sid = NULL;
+ psd->owner_sid = NULL;
}
if (!(security_info & SECINFO_GROUP)) {
- psd_blob->group_sid = NULL;
+ psd->group_sid = NULL;
}
if (!(security_info & SECINFO_DACL)) {
- psd_blob->type &= ~SEC_DESC_DACL_PRESENT;
- psd_blob->dacl = NULL;
+ psd->type &= ~SEC_DESC_DACL_PRESENT;
+ psd->dacl = NULL;
}
if (!(security_info & SECINFO_SACL)) {
- psd_blob->type &= ~SEC_DESC_SACL_PRESENT;
- psd_blob->sacl = NULL;
+ psd->type &= ~SEC_DESC_SACL_PRESENT;
+ psd->sacl = NULL;
}
- TALLOC_FREE(blob.data);
-
if (DEBUGLEVEL >= 10) {
DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
smb_fname->base_name ));
- NDR_PRINT_DEBUG(security_descriptor, psd_blob);
+ NDR_PRINT_DEBUG(security_descriptor, psd);
}
- /* The VFS API is that the ACL is expected to be on mem_ctx */
- *ppdesc = talloc_move(mem_ctx, &psd_blob);
+ *ppdesc = psd;
- TALLOC_FREE(frame);
return NT_STATUS_OK;
+
+fail:
+ TALLOC_FREE(psd);
+ TALLOC_FREE(psd_blob);
+ TALLOC_FREE(psd_fs);
+ TALLOC_FREE(sys_acl_blob_description);
+ TALLOC_FREE(sys_acl_blob.data);
+ return status;
}
/*********************************************************************
--
2.7.4
From 695256ac9846ee8671388c58b6ded2dbd9adbce9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 22:32:57 +0200
Subject: [PATCH 06/13] vfs_acl_common: move the ACL blob validation to a
helper function
No change in behaviour.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 216 +++++++++++++++++++++++----------------
1 file changed, 127 insertions(+), 89 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 66c58e7..2bc2499 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -467,20 +467,32 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
return NT_STATUS_OK;
}
-/*******************************************************************
- Pull a DATA_BLOB from an xattr given a pathname.
- If the hash doesn't match, or doesn't exist - return the underlying
- filesystem sd.
-*******************************************************************/
-
-static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
- files_struct *fsp,
- const struct smb_filename *smb_fname_in,
- uint32_t security_info,
- TALLOC_CTX *mem_ctx,
- struct security_descriptor **ppdesc)
+/**
+ * Validate an ACL blob
+ *
+ * This validates an ACL blob against the underlying filesystem ACL. If this
+ * function returns NT_STATUS_OK ppsd can be
+ *
+ * 1. the ACL from the blob (psd_from_fs=false), or
+ * 2. the ACL from the fs (psd_from_fs=true), or
+ * 3. NULL (!)
+ *
+ * If the return value is anything else then NT_STATUS_OK, ppsd is set to NULL
+ * and psd_from_fs set to false.
+ *
+ * Returning the underlying filesystem ACL in case no. 2 is really just an
+ * optimisation, because some validations have to fetch the filesytem ACL as
+ * part of the validation, so we already have it available and callers might
+ * need it as well.
+ **/
+static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
+ vfs_handle_struct *handle,
+ files_struct *fsp,
+ const struct smb_filename *smb_fname,
+ DATA_BLOB *blob,
+ struct security_descriptor **ppsd,
+ bool *psd_is_from_fs)
{
- DATA_BLOB blob = data_blob_null;
NTSTATUS status;
uint16_t hash_type = XATTR_SD_HASH_TYPE_NONE;
uint16_t xattr_version = 0;
@@ -491,41 +503,28 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
struct security_descriptor *psd = NULL;
struct security_descriptor *psd_blob = NULL;
struct security_descriptor *psd_fs = NULL;
- const struct smb_filename *smb_fname = NULL;
+ char *sys_acl_blob_description = NULL;
+ DATA_BLOB sys_acl_blob = { 0 };
bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
ACL_MODULE_NAME,
"ignore system acls",
false);
- char *sys_acl_blob_description = NULL;
- DATA_BLOB sys_acl_blob = { 0 };
- bool psd_is_from_fs = false;
- if (fsp && smb_fname_in == NULL) {
- smb_fname = fsp->fsp_name;
- } else {
- smb_fname = smb_fname_in;
- }
-
- DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
+ *ppsd = NULL;
+ *psd_is_from_fs = false;
- status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
+ status = parse_acl_blob(blob,
+ mem_ctx,
+ &psd_blob,
+ &hash_type,
+ &xattr_version,
+ &hash[0],
+ &sys_acl_hash[0]);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
- nt_errstr(status)));
- goto out;
- } else {
- status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
- &hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("parse_acl_blob returned %s\n",
- nt_errstr(status)));
- TALLOC_FREE(blob.data);
- goto out;
- }
+ DBG_DEBUG("parse_acl_blob returned %s\n", nt_errstr(status));
+ goto fail;
}
- TALLOC_FREE(blob.data);
-
/* determine which type of xattr we got */
switch (xattr_version) {
case 1:
@@ -534,35 +533,29 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
* require confirmation of the hash. In particular,
* the NTVFS file server uses version 1, but
* 'samba-tool ntacl' can set these as well */
- psd = psd_blob;
- psd_blob = NULL;
- goto out;
+ *ppsd = psd_blob;
+ return NT_STATUS_OK;
case 3:
case 4:
if (ignore_file_system_acl) {
- psd = psd_blob;
- psd_blob = NULL;
- goto out;
+ *ppsd = psd_blob;
+ return NT_STATUS_OK;
}
break;
default:
- DEBUG(10, ("get_nt_acl_internal: ACL blob revision "
- "mismatch (%u) for file %s\n",
- (unsigned int)hash_type,
- smb_fname->base_name));
+ DBG_DEBUG("ACL blob revision mismatch (%u) for file %s\n",
+ (unsigned int)hash_type, smb_fname->base_name);
TALLOC_FREE(psd_blob);
- goto out;
+ return NT_STATUS_OK;
}
/* determine which type of xattr we got */
if (hash_type != XATTR_SD_HASH_TYPE_SHA256) {
- DEBUG(10, ("get_nt_acl_internal: ACL blob hash type "
- "(%u) unexpected for file %s\n",
- (unsigned int)hash_type,
- smb_fname->base_name));
+ DBG_DEBUG("ACL blob hash type (%u) unexpected for file %s\n",
+ (unsigned int)hash_type, smb_fname->base_name);
TALLOC_FREE(psd_blob);
- goto out;
+ return NT_STATUS_OK;
}
/* determine which type of xattr we got */
@@ -600,12 +593,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0],
XATTR_SD_HASH_SIZE) == 0) {
/* Hash matches, return blob sd. */
- DEBUG(10, ("get_nt_acl_internal: blob hash "
- "matches for file %s\n",
- smb_fname->base_name ));
- psd = psd_blob;
- psd_blob = NULL;
- goto out;
+ DBG_DEBUG("blob hash matches for file %s\n",
+ smb_fname->base_name);
+ *ppsd = psd_blob;
+ return NT_STATUS_OK;
}
}
@@ -629,51 +620,102 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
}
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
- "returned %s\n",
- smb_fname->base_name,
- nt_errstr(status)));
+ DBG_DEBUG("get_next_acl for file %s returned %s\n",
+ smb_fname->base_name, nt_errstr(status));
goto fail;
}
status = hash_sd_sha256(psd_fs, hash_tmp);
if (!NT_STATUS_IS_OK(status)) {
TALLOC_FREE(psd_blob);
- psd = psd_fs;
- psd_fs = NULL;
- psd_is_from_fs = true;
- goto out;
+ *ppsd = psd_fs;
+ *psd_is_from_fs = true;
+ return NT_STATUS_OK;
}
if (memcmp(&hash[0], &hash_tmp[0], XATTR_SD_HASH_SIZE) == 0) {
/* Hash matches, return blob sd. */
- DEBUG(10, ("get_nt_acl_internal: blob hash "
- "matches for file %s\n",
- smb_fname->base_name ));
- psd = psd_blob;
- psd_blob = NULL;
- goto out;
+ DBG_DEBUG("blob hash matches for file %s\n",
+ smb_fname->base_name);
+ *ppsd = psd_blob;
+ return NT_STATUS_OK;
}
/* Hash doesn't match, return underlying sd. */
- DEBUG(10, ("get_nt_acl_internal: blob hash "
- "does not match for file %s - returning "
- "file system SD mapping.\n",
- smb_fname->base_name ));
+ DBG_DEBUG("blob hash does not match for file %s - returning "
+ "file system SD mapping.\n",
+ smb_fname->base_name);
if (DEBUGLEVEL >= 10) {
- DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n",
- smb_fname->base_name ));
+ DBG_DEBUG("acl for blob hash for %s is:\n",
+ smb_fname->base_name);
NDR_PRINT_DEBUG(security_descriptor, psd_fs);
}
TALLOC_FREE(psd_blob);
- psd = psd_fs;
- psd_fs = NULL;
- psd_is_from_fs = true;
+ *ppsd = psd_fs;
+ *psd_is_from_fs = true;
+ }
+
+ return NT_STATUS_OK;
+
+fail:
+ TALLOC_FREE(psd);
+ TALLOC_FREE(psd_blob);
+ TALLOC_FREE(psd_fs);
+ TALLOC_FREE(sys_acl_blob_description);
+ TALLOC_FREE(sys_acl_blob.data);
+ return status;
+}
+
+/*******************************************************************
+ Pull a DATA_BLOB from an xattr given a pathname.
+ If the hash doesn't match, or doesn't exist - return the underlying
+ filesystem sd.
+*******************************************************************/
+
+static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
+ files_struct *fsp,
+ const struct smb_filename *smb_fname_in,
+ uint32_t security_info,
+ TALLOC_CTX *mem_ctx,
+ struct security_descriptor **ppdesc)
+{
+ DATA_BLOB blob = data_blob_null;
+ NTSTATUS status;
+ struct security_descriptor *psd = NULL;
+ const struct smb_filename *smb_fname = NULL;
+ bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
+ ACL_MODULE_NAME,
+ "ignore system acls",
+ false);
+ bool psd_is_from_fs = false;
+
+ if (fsp && smb_fname_in == NULL) {
+ smb_fname = fsp->fsp_name;
+ } else {
+ smb_fname = smb_fname_in;
+ }
+
+ DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
+
+ status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
+ if (NT_STATUS_IS_OK(status)) {
+ status = validate_nt_acl_blob(mem_ctx,
+ handle,
+ fsp,
+ smb_fname,
+ &blob,
+ &psd,
+ &psd_is_from_fs);
+ TALLOC_FREE(blob.data);
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_DEBUG("ACL validation for [%s] failed\n",
+ smb_fname->base_name);
+ goto fail;
+ }
}
-out:
if (psd == NULL) {
/* Get the full underlying sd, as we failed to get the
* blob for the hash, or the revision/hash type wasn't
@@ -803,10 +845,6 @@ out:
fail:
TALLOC_FREE(psd);
- TALLOC_FREE(psd_blob);
- TALLOC_FREE(psd_fs);
- TALLOC_FREE(sys_acl_blob_description);
- TALLOC_FREE(sys_acl_blob.data);
return status;
}
--
2.7.4
From 69cd4f376d0a287719eadf96ca3324e76317ab66 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:01:17 +0200
Subject: [PATCH 07/13] vfs_acl_tdb|xattr: use a config handle
Better for performance and a subsequent commit will add one more option
where this will pay off.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 50 ++++++++++++++++++++++++++++++++--------
source3/modules/vfs_acl_tdb.c | 7 ++++++
source3/modules/vfs_acl_xattr.c | 7 ++++++
3 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2bc2499..fc72b98 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -46,6 +46,34 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle,
SECINFO_DACL | \
SECINFO_SACL)
+struct acl_common_config {
+ bool ignore_system_acls;
+};
+
+static bool init_acl_common_config(vfs_handle_struct *handle)
+{
+ struct acl_common_config *config = NULL;
+
+ config = talloc_zero(handle->conn, struct acl_common_config);
+ if (config == NULL) {
+ DBG_ERR("talloc_zero() failed\n");
+ errno = ENOMEM;
+ return false;
+ }
+
+ config->ignore_system_acls = lp_parm_bool(SNUM(handle->conn),
+ ACL_MODULE_NAME,
+ "ignore system acls",
+ false);
+
+ SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
+ struct acl_common_config,
+ return false);
+
+ return true;
+}
+
+
/*******************************************************************
Hash a security descriptor.
*******************************************************************/
@@ -505,14 +533,15 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
struct security_descriptor *psd_fs = NULL;
char *sys_acl_blob_description = NULL;
DATA_BLOB sys_acl_blob = { 0 };
- bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
- ACL_MODULE_NAME,
- "ignore system acls",
- false);
+ struct acl_common_config *config = NULL;
*ppsd = NULL;
*psd_is_from_fs = false;
+ SMB_VFS_HANDLE_GET_DATA(handle, config,
+ struct acl_common_config,
+ return NT_STATUS_UNSUCCESSFUL);
+
status = parse_acl_blob(blob,
mem_ctx,
&psd_blob,
@@ -537,7 +566,7 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
return NT_STATUS_OK;
case 3:
case 4:
- if (ignore_file_system_acl) {
+ if (config->ignore_system_acls) {
*ppsd = psd_blob;
return NT_STATUS_OK;
}
@@ -685,11 +714,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
NTSTATUS status;
struct security_descriptor *psd = NULL;
const struct smb_filename *smb_fname = NULL;
- bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
- ACL_MODULE_NAME,
- "ignore system acls",
- false);
bool psd_is_from_fs = false;
+ struct acl_common_config *config = NULL;
+
+ SMB_VFS_HANDLE_GET_DATA(handle, config,
+ struct acl_common_config,
+ return NT_STATUS_UNSUCCESSFUL);
if (fsp && smb_fname_in == NULL) {
smb_fname = fsp->fsp_name;
@@ -788,7 +818,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
}
is_directory = S_ISDIR(psbuf->st_ex_mode);
- if (ignore_file_system_acl) {
+ if (config->ignore_system_acls) {
TALLOC_FREE(psd);
status = make_default_filesystem_acl(mem_ctx,
smb_fname->base_name,
diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index e4c8462..0c92b72 100644
--- a/source3/modules/vfs_acl_tdb.c
+++ b/source3/modules/vfs_acl_tdb.c
@@ -308,6 +308,7 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
const char *user)
{
int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
+ bool ok;
if (ret < 0) {
return ret;
@@ -318,6 +319,12 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
return -1;
}
+ ok = init_acl_common_config(handle);
+ if (!ok) {
+ DBG_ERR("init_acl_common_config failed\n");
+ return -1;
+ }
+
/* Ensure we have the parameters correct if we're
* using this module. */
DEBUG(2,("connect_acl_tdb: setting 'inherit acls = true' "
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index d311c57..307ab6a 100644
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -180,11 +180,18 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
const char *user)
{
int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
+ bool ok;
if (ret < 0) {
return ret;
}
+ ok = init_acl_common_config(handle);
+ if (!ok) {
+ DBG_ERR("init_acl_common_config failed\n");
+ return -1;
+ }
+
/* Ensure we have the parameters correct if we're
* using this module. */
DEBUG(2,("connect_acl_xattr: setting 'inherit acls = true' "
--
2.7.4
From cdb2da35e9ddc394b9b4fef3080de4a1cae35000 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:30:15 +0200
Subject: [PATCH 08/13] vfs_acl_common: move stat stuff to a helper function
Will be reused in the next commit when moving the
make_default_filesystem_acl() stuff to a different place.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 79 ++++++++++++++++++++++++----------------
1 file changed, 48 insertions(+), 31 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index fc72b98..cf50689 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -697,6 +697,48 @@ fail:
return status;
}
+static NTSTATUS stat_fsp_or_smb_fname(vfs_handle_struct *handle,
+ files_struct *fsp,
+ const struct smb_filename *smb_fname,
+ SMB_STRUCT_STAT *sbuf,
+ SMB_STRUCT_STAT **psbuf)
+{
+ NTSTATUS status;
+ int ret;
+
+ if (fsp) {
+ status = vfs_stat_fsp(fsp);
+ if (!NT_STATUS_IS_OK(status)) {
+ return status;
+ }
+ *psbuf = &fsp->fsp_name->st;
+ } else {
+ /*
+ * https://bugzilla.samba.org/show_bug.cgi?id=11249
+ *
+ * We are currently guaranteed that 'name' here is a
+ * smb_fname->base_name, which *cannot* contain a stream name
+ * (':'). vfs_stat_smb_fname() splits a name into a base name +
+ * stream name, which when we get here we know we've already
+ * done. So we have to call the stat or lstat VFS calls
+ * directly here. Else, a base_name that contains a ':' (from a
+ * demangled name) will get split again.
+ *
+ * FIXME.
+ * This uglyness will go away once smb_fname is fully plumbed
+ * through the VFS.
+ */
+ ret = vfs_stat_smb_basename(handle->conn,
+ smb_fname,
+ sbuf);
+ if (ret == -1) {
+ return map_nt_error_from_unix(errno);
+ }
+ }
+
+ return NT_STATUS_OK;
+}
+
/*******************************************************************
Pull a DATA_BLOB from an xattr given a pathname.
If the hash doesn't match, or doesn't exist - return the underlying
@@ -784,38 +826,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
* filesystem. If it's a directory, and has no
* inheritable ACE entries we have to fake them.
*/
- if (fsp) {
- status = vfs_stat_fsp(fsp);
- if (!NT_STATUS_IS_OK(status)) {
- goto fail;
- }
- psbuf = &fsp->fsp_name->st;
- } else {
- /*
- * https://bugzilla.samba.org/show_bug.cgi?id=11249
- *
- * We are currently guaranteed that 'name' here is
- * a smb_fname->base_name, which *cannot* contain
- * a stream name (':'). vfs_stat_smb_fname() splits
- * a name into a base name + stream name, which
- * when we get here we know we've already done.
- * So we have to call the stat or lstat VFS
- * calls directly here. Else, a base_name that
- * contains a ':' (from a demangled name) will
- * get split again.
- *
- * FIXME.
- * This uglyness will go away once smb_fname
- * is fully plumbed through the VFS.
- */
- int ret = vfs_stat_smb_basename(handle->conn,
- smb_fname,
- &sbuf);
- if (ret == -1) {
- status = map_nt_error_from_unix(errno);
- goto fail;
- }
+
+ status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
+ &sbuf, &psbuf);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
}
+
is_directory = S_ISDIR(psbuf->st_ex_mode);
if (config->ignore_system_acls) {
--
2.7.4
From 7c9c463cdf2204ab7d4cdb7c8bfb8164765615fa Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:43:47 +0200
Subject: [PATCH 09/13] vfs_acl_common: check for ignore_system_acls before
fetching filesystem ACL
If ignore_system_acls is set and we're synthesizing a default ACL, we
were fetching the filesystem ACL just to free it again. This change
avoids this.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 99 ++++++++++++++++++++++------------------
1 file changed, 55 insertions(+), 44 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index cf50689..93b4e39 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -792,35 +792,57 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
/* Get the full underlying sd, as we failed to get the
* blob for the hash, or the revision/hash type wasn't
* known */
- if (fsp) {
- status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
- fsp,
- security_info,
- mem_ctx,
- &psd);
+
+ if (config->ignore_system_acls) {
+ SMB_STRUCT_STAT sbuf;
+ SMB_STRUCT_STAT *psbuf = &sbuf;
+
+ status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
+ &sbuf, &psbuf);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
+
+ status = make_default_filesystem_acl(
+ mem_ctx,
+ smb_fname->base_name,
+ psbuf,
+ &psd);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto fail;
+ }
} else {
- status = SMB_VFS_NEXT_GET_NT_ACL(handle,
- smb_fname,
- security_info,
- mem_ctx,
- &psd);
- }
+ if (fsp) {
+ status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
+ fsp,
+ security_info,
+ mem_ctx,
+ &psd);
+ } else {
+ status = SMB_VFS_NEXT_GET_NT_ACL(handle,
+ smb_fname,
+ security_info,
+ mem_ctx,
+ &psd);
+ }
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
- "returned %s\n",
- smb_fname->base_name,
- nt_errstr(status)));
- goto fail;
- }
+ if (!NT_STATUS_IS_OK(status)) {
+ DBG_DEBUG("get_next_acl for file %s "
+ "returned %s\n",
+ smb_fname->base_name,
+ nt_errstr(status));
+ goto fail;
+ }
- psd_is_from_fs = true;
+ psd_is_from_fs = true;
+ }
}
if (psd_is_from_fs) {
SMB_STRUCT_STAT sbuf;
SMB_STRUCT_STAT *psbuf = &sbuf;
bool is_directory = false;
+
/*
* We're returning the underlying ACL from the
* filesystem. If it's a directory, and has no
@@ -835,34 +857,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
is_directory = S_ISDIR(psbuf->st_ex_mode);
- if (config->ignore_system_acls) {
- TALLOC_FREE(psd);
- status = make_default_filesystem_acl(mem_ctx,
- smb_fname->base_name,
- psbuf,
- &psd);
+ if (is_directory && !sd_has_inheritable_components(psd, true)) {
+ status = add_directory_inheritable_components(
+ handle,
+ smb_fname->base_name,
+ psbuf,
+ psd);
if (!NT_STATUS_IS_OK(status)) {
goto fail;
}
- } else {
- if (is_directory &&
- !sd_has_inheritable_components(psd,
- true)) {
- status = add_directory_inheritable_components(
- handle,
- smb_fname->base_name,
- psbuf,
- psd);
- if (!NT_STATUS_IS_OK(status)) {
- goto fail;
- }
- }
- /* The underlying POSIX module always sets
- the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
- can't be inherited in this way under POSIX.
- Remove it for Windows-style ACLs. */
- psd->type &= ~SEC_DESC_DACL_PROTECTED;
}
+
+ /*
+ * The underlying POSIX module always sets the
+ * ~SEC_DESC_DACL_PROTECTED bit, as ACLs can't be inherited in
+ * this way under POSIX. Remove it for Windows-style ACLs.
+ */
+ psd->type &= ~SEC_DESC_DACL_PROTECTED;
}
if (!(security_info & SECINFO_OWNER)) {
--
2.7.4
From ec7c255a37b72df31f859343eaeb07065aa4653c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 20:31:00 +0200
Subject: [PATCH 10/13] vfs_acl_xattr|tdb: add option to control default ACL
style
Existing behaviour is "posix" style. Next commit will (re)add the
"windows" style. This commit doesn't change behaviour in any way.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
docs-xml/manpages/vfs_acl_tdb.8.xml | 25 ++++++++++++++++++
docs-xml/manpages/vfs_acl_xattr.8.xml | 25 ++++++++++++++++++
source3/modules/vfs_acl_common.c | 48 ++++++++++++++++++++++++++++++-----
3 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml b/docs-xml/manpages/vfs_acl_tdb.8.xml
index 640cec0..607e344 100644
--- a/docs-xml/manpages/vfs_acl_tdb.8.xml
+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
@@ -63,6 +63,31 @@
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>acl_tdb:default acl style = [posix|windows]</term>
+ <listitem>
+ <para>
+ This parameter determines the type of ACL that is synthesized in
+ case a file or directory lacks an
+ <emphasis>security.NTACL</emphasis> xattr.
+ </para>
+ <para>
+ When set to <emphasis>posix</emphasis>, an ACL will be
+ synthesized based on the POSIX mode permissions for user, group
+ and others, with an additional ACE for <emphasis>NT
+ Authority\SYSTEM</emphasis> will full rights.
+ </para>
+ <para>
+ When set to <emphasis>windows</emphasis>, an ACL is synthesized
+ the same way Windows does it, only including permissions for the
+ owner and <emphasis>NT Authority\SYSTEM</emphasis>.
+ </para>
+ <para>
+ The default for this option is <emphasis>posix</emphasis>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml
index 60a1c2d..8da73e0 100644
--- a/docs-xml/manpages/vfs_acl_xattr.8.xml
+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
@@ -67,6 +67,31 @@
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term>acl_xattr:default acl style = [posix|windows]</term>
+ <listitem>
+ <para>
+ This parameter determines the type of ACL that is synthesized in
+ case a file or directory lacks an
+ <emphasis>security.NTACL</emphasis> xattr.
+ </para>
+ <para>
+ When set to <emphasis>posix</emphasis>, an ACL will be
+ synthesized based on the POSIX mode permissions for user, group
+ and others, with an additional ACE for <emphasis>NT
+ Authority\SYSTEM</emphasis> will full rights.
+ </para>
+ <para>
+ When set to <emphasis>windows</emphasis>, an ACL is synthesized
+ the same way Windows does it, only including permissions for the
+ owner and <emphasis>NT Authority\SYSTEM</emphasis>.
+ </para>
+ <para>
+ The default for this option is <emphasis>posix</emphasis>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect1>
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 93b4e39..45ce532 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -46,8 +46,16 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle,
SECINFO_DACL | \
SECINFO_SACL)
+enum default_acl_style {DEFAULT_ACL_POSIX, DEFAULT_ACL_WINDOWS};
+
+static const struct enum_list default_acl_style[] = {
+ {DEFAULT_ACL_POSIX, "posix"},
+ {DEFAULT_ACL_WINDOWS, "windows"}
+};
+
struct acl_common_config {
bool ignore_system_acls;
+ enum default_acl_style default_acl_style;
};
static bool init_acl_common_config(vfs_handle_struct *handle)
@@ -65,6 +73,11 @@ static bool init_acl_common_config(vfs_handle_struct *handle)
ACL_MODULE_NAME,
"ignore system acls",
false);
+ config->default_acl_style = lp_parm_enum(SNUM(handle->conn),
+ ACL_MODULE_NAME,
+ "default acl style",
+ default_acl_style,
+ DEFAULT_ACL_POSIX);
SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
struct acl_common_config,
@@ -387,10 +400,10 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle,
return NT_STATUS_OK;
}
-static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
- const char *name,
- SMB_STRUCT_STAT *psbuf,
- struct security_descriptor **ppdesc)
+static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx,
+ const char *name,
+ SMB_STRUCT_STAT *psbuf,
+ struct security_descriptor **ppdesc)
{
struct dom_sid owner_sid, group_sid;
size_t size = 0;
@@ -400,8 +413,7 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
struct security_acl *new_dacl = NULL;
int idx = 0;
- DEBUG(10,("make_default_filesystem_acl: file %s mode = 0%o\n",
- name, (int)mode ));
+ DBG_DEBUG("file %s mode = 0%o\n",name, (int)mode);
uid_to_sid(&owner_sid, psbuf->st_ex_uid);
gid_to_sid(&group_sid, psbuf->st_ex_gid);
@@ -495,6 +507,29 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
return NT_STATUS_OK;
}
+static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
+ struct acl_common_config *config,
+ const char *name,
+ SMB_STRUCT_STAT *psbuf,
+ struct security_descriptor **ppdesc)
+{
+ NTSTATUS status;
+
+ switch (config->default_acl_style) {
+
+ case DEFAULT_ACL_POSIX:
+ status = make_default_acl_posix(ctx, name, psbuf, ppdesc);
+ break;
+
+ default:
+ DBG_ERR("unknown acl style %d", config->default_acl_style);
+ status = NT_STATUS_INTERNAL_ERROR;
+ break;
+ }
+
+ return status;
+}
+
/**
* Validate an ACL blob
*
@@ -805,6 +840,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
status = make_default_filesystem_acl(
mem_ctx,
+ config,
smb_fname->base_name,
psbuf,
&psd);
--
2.7.4
From 7afcd4f2164b83a3aaf3b651b69e4a5ac03b8d24 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Aug 2016 07:45:34 +0200
Subject: [PATCH 11/13] vfs_acl_common: Windows style default ACL
Reintroduce Windows style default ACL, but this time as an optional
feature, not changing default behaviour.
Original bugreport that got reverted because it changed the default
behaviour: https://bugzilla.samba.org/show_bug.cgi?id=12028
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 76 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 45ce532..9a5b9bd 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -507,6 +507,78 @@ static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx,
return NT_STATUS_OK;
}
+static NTSTATUS make_default_acl_windows(TALLOC_CTX *ctx,
+ const char *name,
+ SMB_STRUCT_STAT *psbuf,
+ struct security_descriptor **ppdesc)
+{
+ struct dom_sid owner_sid, group_sid;
+ size_t size = 0;
+ struct security_ace aces[4];
+ uint32_t access_mask = 0;
+ mode_t mode = psbuf->st_ex_mode;
+ struct security_acl *new_dacl = NULL;
+ int idx = 0;
+
+ DBG_DEBUG("file [%s] mode [0%o]\n", name, (int)mode);
+
+ uid_to_sid(&owner_sid, psbuf->st_ex_uid);
+ gid_to_sid(&group_sid, psbuf->st_ex_gid);
+
+ /*
+ * We provide 2 ACEs:
+ * - Owner
+ * - NT System
+ */
+
+ if (mode & S_IRUSR) {
+ if (mode & S_IWUSR) {
+ access_mask |= SEC_RIGHTS_FILE_ALL;
+ } else {
+ access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+ }
+ }
+ if (mode & S_IWUSR) {
+ access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
+ }
+
+ init_sec_ace(&aces[idx],
+ &owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ access_mask,
+ 0);
+ idx++;
+
+ init_sec_ace(&aces[idx],
+ &global_sid_System,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_FILE_ALL,
+ 0);
+ idx++;
+
+ new_dacl = make_sec_acl(ctx,
+ NT4_ACL_REVISION,
+ idx,
+ aces);
+
+ if (!new_dacl) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ *ppdesc = make_sec_desc(ctx,
+ SECURITY_DESCRIPTOR_REVISION_1,
+ SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT,
+ &owner_sid,
+ &group_sid,
+ NULL,
+ new_dacl,
+ &size);
+ if (!*ppdesc) {
+ return NT_STATUS_NO_MEMORY;
+ }
+ return NT_STATUS_OK;
+}
+
static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
struct acl_common_config *config,
const char *name,
@@ -521,6 +593,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
status = make_default_acl_posix(ctx, name, psbuf, ppdesc);
break;
+ case DEFAULT_ACL_WINDOWS:
+ status = make_default_acl_windows(ctx, name, psbuf, ppdesc);
+ break;
+
default:
DBG_ERR("unknown acl style %d", config->default_acl_style);
status = NT_STATUS_INTERNAL_ERROR;
--
2.7.4
From cb873d7d6707d55c5d4a2cc5438d09c1d979c53e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Aug 2016 16:30:24 +0200
Subject: [PATCH 12/13] s4/torture: tests for vfs_acl_xattr default ACL styles
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
selftest/target/Samba3.pm | 8 +
source3/selftest/tests.py | 4 +-
source4/torture/vfs/acl_xattr.c | 314 ++++++++++++++++++++++++++++++++++++++++
source4/torture/vfs/vfs.c | 1 +
source4/torture/wscript_build | 2 +-
5 files changed, 327 insertions(+), 2 deletions(-)
create mode 100644 source4/torture/vfs/acl_xattr.c
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 8fc3204..f68d7de 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1792,6 +1792,14 @@ sub provision($$$$$$$$)
vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
inherit owner = yes
include = $dfqconffile
+[acl_xattr_ign_sysacl_posix]
+ copy = tmp
+ acl_xattr:ignore system acls = yes
+ acl_xattr:default acl style = posix
+[acl_xattr_ign_sysacl_windows]
+ copy = tmp
+ acl_xattr:ignore system acls = yes
+ acl_xattr:default acl style = windows
";
close(CONF);
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 7d0dcc1..e4c31c6 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -322,7 +322,7 @@ nbt = ["nbt.dgram" ]
libsmbclient = ["libsmbclient"]
-vfs = ["vfs.fruit"]
+vfs = ["vfs.fruit", "vfs.acl_xattr"]
tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs
@@ -418,6 +418,8 @@ for t in tests:
plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --signing=required')
elif t == "smb2.dosmode":
plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/dosmode -U$USERNAME%$PASSWORD')
+ elif t == "vfs.acl_xattr":
+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
else:
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
diff --git a/source4/torture/vfs/acl_xattr.c b/source4/torture/vfs/acl_xattr.c
new file mode 100644
index 0000000..7fd10d0
--- /dev/null
+++ b/source4/torture/vfs/acl_xattr.c
@@ -0,0 +1,314 @@
+/*
+ Unix SMB/CIFS implementation.
+
+ Copyright (C) Ralph Boehme 2016
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#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 "torture/vfs/proto.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 BASEDIR "smb2-testsd"
+
+#define CHECK_SECURITY_DESCRIPTOR(_sd1, _sd2) do { \
+ if (!security_descriptor_equal(_sd1, _sd2)) { \
+ torture_warning(tctx, "security descriptors don't match!\n"); \
+ torture_warning(tctx, "got:\n"); \
+ NDR_PRINT_DEBUG(security_descriptor, _sd1); \
+ torture_warning(tctx, "expected:\n"); \
+ NDR_PRINT_DEBUG(security_descriptor, _sd2); \
+ torture_result(tctx, TORTURE_FAIL, \
+ "%s: security descriptors don't match!\n", \
+ __location__); \
+ ret = false; \
+ } \
+} while (0)
+
+/**
+ * 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_default_acl_posix(struct torture_context *tctx,
+ struct smb2_tree *tree_unused)
+{
+ struct smb2_tree *tree = NULL;
+ NTSTATUS status;
+ bool ok;
+ bool ret = true;
+ const char *dname = BASEDIR "\\testdir";
+ const char *fname = BASEDIR "\\testdir\\testfile";
+ struct smb2_handle fhandle, dhandle;
+ union smb_fileinfo q;
+ union smb_setfileinfo set;
+ struct security_descriptor *sd = NULL;
+ struct security_descriptor *exp_sd = NULL;
+ char *owner_sid = NULL;
+ char *group_sid = NULL;
+
+ ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_posix", &tree);
+ torture_assert_goto(tctx, ok == true, ret, done,
+ "Unable to connect to 'acl_xattr_ign_sysacl_posix'\n");
+
+ ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n");
+
+ ZERO_STRUCT(dhandle);
+ status = torture_smb2_testdir(tree, dname, &dhandle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n");
+
+ torture_comment(tctx, "Get the original sd\n");
+
+ ZERO_STRUCT(q);
+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+ q.query_secdesc.in.file.handle = dhandle;
+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+ status = smb2_getinfo_file(tree, tctx, &q);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+
+ sd = q.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd->owner_sid);
+ group_sid = dom_sid_string(tctx, sd->group_sid);
+ torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid);
+
+ torture_comment(tctx, "Set ACL with no inheritable ACE\n");
+
+ sd = security_descriptor_dacl_create(tctx,
+ 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_DIR_ALL,
+ 0,
+ NULL);
+
+ ZERO_STRUCT(set);
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+ set.set_secdesc.in.file.handle = dhandle;
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+ set.set_secdesc.in.sd = sd;
+ status = smb2_setinfo_file(tree, &set);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n");
+
+ TALLOC_FREE(sd);
+ smb2_util_close(tree, dhandle);
+
+ torture_comment(tctx, "Create file\n");
+
+ ZERO_STRUCT(fhandle);
+ status = torture_smb2_testfile(tree, fname, &fhandle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n");
+
+ torture_comment(tctx, "Query file SD\n");
+
+ ZERO_STRUCT(q);
+ 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 | SECINFO_GROUP;
+ status = smb2_getinfo_file(tree, tctx, &q);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+ sd = q.query_secdesc.out.sd;
+
+ smb2_util_close(tree, fhandle);
+ ZERO_STRUCT(fhandle);
+
+ torture_comment(tctx, "Checking actual file SD against expected SD\n");
+
+ exp_sd = security_descriptor_dacl_create(
+ tctx, 0, owner_sid, group_sid,
+ owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+ group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
+ SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
+ SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+ NULL);
+
+ CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
+
+done:
+ if (!smb2_util_handle_empty(fhandle)) {
+ smb2_util_close(tree, fhandle);
+ }
+ if (!smb2_util_handle_empty(dhandle)) {
+ smb2_util_close(tree, dhandle);
+ }
+ if (tree != NULL) {
+ smb2_deltree(tree, BASEDIR);
+ smb2_tdis(tree);
+ }
+
+ return ret;
+}
+
+static bool test_default_acl_win(struct torture_context *tctx,
+ struct smb2_tree *tree_unused)
+{
+ struct smb2_tree *tree = NULL;
+ NTSTATUS status;
+ bool ok;
+ bool ret = true;
+ const char *dname = BASEDIR "\\testdir";
+ const char *fname = BASEDIR "\\testdir\\testfile";
+ struct smb2_handle fhandle, dhandle;
+ union smb_fileinfo q;
+ union smb_setfileinfo set;
+ struct security_descriptor *sd = NULL;
+ struct security_descriptor *exp_sd = NULL;
+ char *owner_sid = NULL;
+ char *group_sid = NULL;
+
+ ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_windows", &tree);
+ torture_assert_goto(tctx, ok == true, ret, done,
+ "Unable to connect to 'acl_xattr_ign_sysacl_windows'\n");
+
+ ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
+ torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n");
+
+ ZERO_STRUCT(dhandle);
+ status = torture_smb2_testdir(tree, dname, &dhandle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n");
+
+ torture_comment(tctx, "Get the original sd\n");
+
+ ZERO_STRUCT(q);
+ q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+ q.query_secdesc.in.file.handle = dhandle;
+ q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+ status = smb2_getinfo_file(tree, tctx, &q);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+
+ sd = q.query_secdesc.out.sd;
+ owner_sid = dom_sid_string(tctx, sd->owner_sid);
+ group_sid = dom_sid_string(tctx, sd->group_sid);
+ torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid);
+
+ torture_comment(tctx, "Set ACL with no inheritable ACE\n");
+
+ sd = security_descriptor_dacl_create(tctx,
+ 0, NULL, NULL,
+ owner_sid,
+ SEC_ACE_TYPE_ACCESS_ALLOWED,
+ SEC_RIGHTS_DIR_ALL,
+ 0,
+ NULL);
+
+ ZERO_STRUCT(set);
+ set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+ set.set_secdesc.in.file.handle = dhandle;
+ set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+ set.set_secdesc.in.sd = sd;
+ status = smb2_setinfo_file(tree, &set);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n");
+
+ TALLOC_FREE(sd);
+ smb2_util_close(tree, dhandle);
+
+ torture_comment(tctx, "Create file\n");
+
+ ZERO_STRUCT(fhandle);
+ status = torture_smb2_testfile(tree, fname, &fhandle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n");
+
+ torture_comment(tctx, "Query file SD\n");
+
+ ZERO_STRUCT(q);
+ 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 | SECINFO_GROUP;
+ status = smb2_getinfo_file(tree, tctx, &q);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+ sd = q.query_secdesc.out.sd;
+
+ smb2_util_close(tree, fhandle);
+ ZERO_STRUCT(fhandle);
+
+ torture_comment(tctx, "Checking actual file SD against expected SD\n");
+
+ exp_sd = security_descriptor_dacl_create(
+ tctx, 0, owner_sid, group_sid,
+ owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+ SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+ NULL);
+
+ CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
+
+done:
+ if (!smb2_util_handle_empty(fhandle)) {
+ smb2_util_close(tree, fhandle);
+ }
+ if (!smb2_util_handle_empty(dhandle)) {
+ smb2_util_close(tree, dhandle);
+ }
+ if (tree != NULL) {
+ smb2_deltree(tree, BASEDIR);
+ smb2_tdis(tree);
+ }
+
+ return ret;
+}
+
+/*
+ basic testing of vfs_acl_xattr
+*/
+struct torture_suite *torture_acl_xattr(void)
+{
+ struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "acl_xattr");
+
+ torture_suite_add_1smb2_test(suite, "default-acl-style-posix", test_default_acl_posix);
+ torture_suite_add_1smb2_test(suite, "default-acl-style-windows", test_default_acl_win);
+
+ suite->description = talloc_strdup(suite, "vfs_acl_xattr tests");
+
+ return suite;
+}
diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c
index f3ce447..7f805f4 100644
--- a/source4/torture/vfs/vfs.c
+++ b/source4/torture/vfs/vfs.c
@@ -107,6 +107,7 @@ NTSTATUS torture_vfs_init(void)
suite->description = talloc_strdup(suite, "VFS modules tests");
torture_suite_add_suite(suite, torture_vfs_fruit());
+ torture_suite_add_suite(suite, torture_acl_xattr());
torture_register_suite(suite);
diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build
index 382e2c6..ff79c3d 100755
--- a/source4/torture/wscript_build
+++ b/source4/torture/wscript_build
@@ -271,7 +271,7 @@ bld.SAMBA_MODULE('TORTURE_NTP',
)
bld.SAMBA_MODULE('TORTURE_VFS',
- source='vfs/vfs.c vfs/fruit.c',
+ source='vfs/vfs.c vfs/fruit.c vfs/acl_xattr.c',
subsystem='smbtorture',
deps='LIBCLI_SMB POPT_CREDENTIALS TORTURE_UTIL smbclient-raw TORTURE_RAW',
internal_module=True,
--
2.7.4
From 4b5d7a5577094544aa52b65a22cc0c61fde97478 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 27 Aug 2016 10:11:14 +0200
Subject: [PATCH 13/13] vfs_acl_common: use DBG_LEVEL and remove function
prefixes in DEBUG statements
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12177
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_acl_common.c | 75 ++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 42 deletions(-)
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 9a5b9bd..2163a75 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -144,8 +144,8 @@ static NTSTATUS parse_acl_blob(const DATA_BLOB *pblob,
(ndr_pull_flags_fn_t)ndr_pull_xattr_NTACL);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
- DEBUG(5, ("parse_acl_blob: ndr_pull_xattr_NTACL failed: %s\n",
- ndr_errstr(ndr_err)));
+ DBG_INFO("ndr_pull_xattr_NTACL failed: %s\n",
+ ndr_errstr(ndr_err));
TALLOC_FREE(frame);
return ndr_map_error2ntstatus(ndr_err);
}
@@ -241,8 +241,8 @@ static NTSTATUS create_acl_blob(const struct security_descriptor *psd,
(ndr_push_flags_fn_t)ndr_push_xattr_NTACL);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
- DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed: %s\n",
- ndr_errstr(ndr_err)));
+ DBG_INFO("ndr_push_xattr_NTACL failed: %s\n",
+ ndr_errstr(ndr_err));
return ndr_map_error2ntstatus(ndr_err);
}
@@ -287,8 +287,8 @@ static NTSTATUS create_sys_acl_blob(const struct security_descriptor *psd,
(ndr_push_flags_fn_t)ndr_push_xattr_NTACL);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
- DEBUG(5, ("create_acl_blob: ndr_push_xattr_NTACL failed: %s\n",
- ndr_errstr(ndr_err)));
+ DBG_INFO("ndr_push_xattr_NTACL failed: %s\n",
+ ndr_errstr(ndr_err));
return ndr_map_error2ntstatus(ndr_err);
}
@@ -345,10 +345,7 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle,
mode = dir_mode | file_mode;
- DEBUG(10, ("add_directory_inheritable_components: directory %s, "
- "mode = 0%o\n",
- name,
- (unsigned int)mode ));
+ DBG_DEBUG("directory %s, mode = 0%o\n", name, (unsigned int)mode);
if (num_aces) {
memcpy(new_ace_list, psd->dacl->aces,
@@ -880,7 +877,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
smb_fname = smb_fname_in;
}
- DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
+ DBG_DEBUG("name=%s\n", smb_fname->base_name);
status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
if (NT_STATUS_IS_OK(status)) {
@@ -1004,8 +1001,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
}
if (DEBUGLEVEL >= 10) {
- DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
- smb_fname->base_name ));
+ DBG_DEBUG("returning acl for %s is:\n",
+ smb_fname->base_name);
NDR_PRINT_DEBUG(security_descriptor, psd);
}
@@ -1072,9 +1069,8 @@ static NTSTATUS set_underlying_acl(vfs_handle_struct *handle, files_struct *fsp,
return NT_STATUS_ACCESS_DENIED;
}
- DEBUG(10, ("fset_nt_acl_common: overriding chown on file %s "
- "for sid %s\n",
- fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid)));
+ DBG_DEBUG("overriding chown on file %s for sid %s\n",
+ fsp_str_dbg(fsp), sid_string_tos(psd->owner_sid));
/* Ok, we failed to chown and we have
SEC_STD_WRITE_OWNER access - override. */
@@ -1097,27 +1093,25 @@ static NTSTATUS store_v3_blob(vfs_handle_struct *handle, files_struct *fsp,
DATA_BLOB blob;
if (DEBUGLEVEL >= 10) {
- DEBUG(10, ("fset_nt_acl_xattr: storing xattr sd for file %s\n",
- fsp_str_dbg(fsp)));
+ DBG_DEBUG("storing xattr sd for file %s\n",
+ fsp_str_dbg(fsp));
NDR_PRINT_DEBUG(
security_descriptor,
discard_const_p(struct security_descriptor, psd));
if (pdesc_next != NULL) {
- DEBUG(10, ("fset_nt_acl_xattr: storing has in xattr sd "
- "based on \n"));
+ DBG_DEBUG("storing xattr sd based on \n");
NDR_PRINT_DEBUG(
security_descriptor,
discard_const_p(struct security_descriptor,
pdesc_next));
} else {
- DEBUG(10,
- ("fset_nt_acl_xattr: ignoring underlying sd\n"));
+ DBG_DEBUG("ignoring underlying sd\n");
}
}
status = create_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256, hash);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("fset_nt_acl_xattr: create_acl_blob failed\n"));
+ DBG_DEBUG("create_acl_blob failed\n");
return status;
}
@@ -1146,8 +1140,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp,
SNUM(handle->conn), ACL_MODULE_NAME, "ignore system acls", false);
if (DEBUGLEVEL >= 10) {
- DEBUG(10,("fset_nt_acl_xattr: incoming sd for file %s\n",
- fsp_str_dbg(fsp)));
+ DBG_DEBUG("incoming sd for file %s\n", fsp_str_dbg(fsp));
NDR_PRINT_DEBUG(security_descriptor,
discard_const_p(struct security_descriptor, orig_psd));
}
@@ -1265,12 +1258,12 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp,
}
if (DEBUGLEVEL >= 10) {
- DEBUG(10,("fset_nt_acl_xattr: storing xattr sd for file %s based on system ACL\n",
- fsp_str_dbg(fsp)));
+ DBG_DEBUG("storing xattr sd for file %s based on system ACL\n",
+ fsp_str_dbg(fsp));
NDR_PRINT_DEBUG(security_descriptor,
discard_const_p(struct security_descriptor, psd));
- DEBUG(10,("fset_nt_acl_xattr: storing hash in xattr sd based on system ACL and:\n"));
+ DBG_DEBUG("storing hash in xattr sd based on system ACL and:\n");
NDR_PRINT_DEBUG(security_descriptor,
discard_const_p(struct security_descriptor, pdesc_next));
}
@@ -1282,7 +1275,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp,
status = create_sys_acl_blob(psd, &blob, XATTR_SD_HASH_TYPE_SHA256, hash,
sys_acl_description, sys_acl_hash);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("fset_nt_acl_xattr: create_sys_acl_blob failed\n"));
+ DBG_DEBUG("create_sys_acl_blob failed\n");
TALLOC_FREE(frame);
return status;
}
@@ -1319,9 +1312,8 @@ static int acl_common_remove_object(vfs_handle_struct *handle,
goto out;
}
- DEBUG(10,("acl_common_remove_object: removing %s %s/%s\n",
- is_directory ? "directory" : "file",
- parent_dir, final_component ));
+ DBG_DEBUG("removing %s %s/%s\n", is_directory ? "directory" : "file",
+ parent_dir, final_component);
/* cd into the parent dir to pin it. */
ret = vfs_ChDir(conn, parent_dir);
@@ -1354,10 +1346,9 @@ static int acl_common_remove_object(vfs_handle_struct *handle,
}
if (!fsp) {
- DEBUG(10,("acl_common_remove_object: %s %s/%s "
- "not an open file\n",
- is_directory ? "directory" : "file",
- parent_dir, final_component ));
+ DBG_DEBUG("%s %s/%s not an open file\n",
+ is_directory ? "directory" : "file",
+ parent_dir, final_component);
saved_errno = EACCES;
goto out;
}
@@ -1405,9 +1396,9 @@ static int rmdir_acl_common(struct vfs_handle_struct *handle,
true);
}
- DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n",
- smb_fname->base_name,
- strerror(errno) ));
+ DBG_DEBUG("unlink of %s failed %s\n",
+ smb_fname->base_name,
+ strerror(errno));
return -1;
}
@@ -1434,9 +1425,9 @@ static int unlink_acl_common(struct vfs_handle_struct *handle,
false);
}
- DEBUG(10,("unlink_acl_common: unlink of %s failed %s\n",
- smb_fname->base_name,
- strerror(errno) ));
+ DBG_DEBUG("unlink of %s failed %s\n",
+ smb_fname->base_name,
+ strerror(errno));
return -1;
}
--
2.7.4
More information about the samba
mailing list