From 801bdc6df1f89e7cc5085fdbaec5e67966fa0080 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 590b80490c00587b5a4035856891e10defb654f6) --- 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 f5af666..85f6c65 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 035d0dd07a9bbce5aca2dfffc89f16332f103591 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 2367eea928593f12f8914f7e7ba613b1b15516de) --- source3/modules/vfs_acl_common.c | 60 ++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 85f6c65..420d909 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; bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), ACL_MODULE_NAME, @@ -506,25 +506,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) { @@ -547,8 +547,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "mismatch (%u) for file %s\n", (unsigned int)hash_type, name)); - TALLOC_FREE(psd); - psd = NULL; + TALLOC_FREE(psd_blob); + psd_blob = NULL; goto out; } @@ -558,8 +558,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "(%u) unexpected for file %s\n", (unsigned int)hash_type, name)); - TALLOC_FREE(psd); - psd = NULL; + TALLOC_FREE(psd_blob); + psd_blob = NULL; goto out; } @@ -642,8 +642,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; } @@ -667,12 +667,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 */ @@ -705,10 +705,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); @@ -761,20 +761,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, status = make_default_filesystem_acl(mem_ctx, 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, name, psbuf, - psd); + psd_blob); if (!NT_STATUS_IS_OK(status)) { TALLOC_FREE(frame); return status; @@ -784,35 +784,35 @@ 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); if (DEBUGLEVEL >= 10) { DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n", - name )); - NDR_PRINT_DEBUG(security_descriptor, psd); + name)); + 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 4f28237cae001b76b90d51e94a601c299bc7ce53 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 9f79084f166208820f586c8e43e1e315d32cd5ce) --- source3/modules/vfs_acl_common.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 420d909..8f89068 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; bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), ACL_MODULE_NAME, "ignore system acls", @@ -615,13 +615,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, name, HASH_SECURITY_INFO, mem_ctx, - &pdesc_next); + &psd_fs); } if (!NT_STATUS_IS_OK(status)) { @@ -633,17 +633,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; } @@ -663,12 +663,12 @@ 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", - name )); - NDR_PRINT_DEBUG(security_descriptor, pdesc_next); + name)); + NDR_PRINT_DEBUG(security_descriptor, psd_fs); } TALLOC_FREE(psd_blob); - psd_blob = pdesc_next; + psd_blob = psd_fs; } out: @@ -681,13 +681,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, name, security_info, mem_ctx, - &pdesc_next); + &psd_fs); } if (!NT_STATUS_IS_OK(status)) { @@ -699,19 +699,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; @@ -757,7 +757,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, name, psbuf, -- 2.7.4 From f46da3fe683b29843bd6026851478f68f2e01669 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit e6f1254a00a6bf85b8d95bfbafef7d3e39ce1dde) --- 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 8f89068..64ee1ec 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -548,7 +548,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, (unsigned int)hash_type, name)); TALLOC_FREE(psd_blob); - psd_blob = NULL; goto out; } @@ -559,7 +558,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, (unsigned int)hash_type, name)); TALLOC_FREE(psd_blob); - psd_blob = NULL; goto out; } -- 2.7.4 From d087459e815fef12de617e1e3f38798d8337ea9a Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 335527c647331148927feea2a7ae2f2c88986bc6) --- source3/modules/vfs_acl_common.c | 135 ++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 72 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 64ee1ec..112874f 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -488,13 +488,16 @@ 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; bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn), 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 && name == NULL) { name = fsp->fsp_name->base_name; @@ -502,11 +505,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); - status = get_acl_blob(frame, handle, fsp, name, &blob); + status = get_acl_blob(mem_ctx, handle, fsp, name, &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, @@ -514,17 +516,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) { @@ -534,10 +531,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; } @@ -566,20 +567,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, name, - frame, + mem_ctx, &sys_acl_blob_description, &sys_acl_blob); } @@ -589,16 +588,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", - name )); + name)); + psd = psd_blob; + psd_blob = NULL; goto out; } } @@ -627,21 +630,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "returned %s\n", 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; } @@ -649,7 +646,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, /* Hash matches, return blob sd. */ DEBUG(10, ("get_nt_acl_internal: blob hash " "matches for file %s\n", - name )); + name)); + psd = psd_blob; + psd_blob = NULL; goto out; } @@ -666,11 +665,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 */ @@ -679,13 +680,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, name, security_info, mem_ctx, - &psd_fs); + &psd); } if (!NT_STATUS_IS_OK(status)) { @@ -693,24 +694,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, "returned %s\n", 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; @@ -722,8 +712,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 { @@ -748,72 +737,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, name, &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, 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, 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", 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 812f1215214c076026319105f7d13c26bdb97811 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 0de5a128cee90694979d074c2590ddbca0071e82) --- source3/modules/vfs_acl_common.c | 210 +++++++++++++++++++++++---------------- 1 file changed, 122 insertions(+), 88 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 112874f..4d357e7 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 char *name, - 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 char *name, + const 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,38 +503,29 @@ 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; + 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 && name == NULL) { - name = fsp->fsp_name->base_name; - } + *ppsd = NULL; + *psd_is_from_fs = false; - DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); + status = parse_acl_blob(blob, + mem_ctx, + &psd_blob, + &hash_type, + &xattr_version, + &hash[0], + &sys_acl_hash[0]); - status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); 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: @@ -531,35 +534,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, - name)); + DBG_DEBUG("ACL blob revision mismatch (%u) for file %s\n", + (unsigned int)hash_type, 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, - name)); + DBG_DEBUG("ACL blob hash type (%u) unexpected for file %s\n", + (unsigned int)hash_type, name); TALLOC_FREE(psd_blob); - goto out; + return NT_STATUS_OK; } /* determine which type of xattr we got */ @@ -597,12 +594,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", - name)); - psd = psd_blob; - psd_blob = NULL; - goto out; + DBG_DEBUG("blob hash matches for file %s\n", + name); + *ppsd = psd_blob; + return NT_STATUS_OK; } } @@ -626,51 +621,96 @@ 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", - name, - nt_errstr(status))); + DBG_DEBUG("get_next_acl for file %s returned %s\n", + 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", - name)); - psd = psd_blob; - psd_blob = NULL; - goto out; + DBG_DEBUG("blob hash matches for file %s\n", 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", - name )); + DBG_DEBUG("blob hash does not match for file %s - returning " + "file system SD mapping.\n", name); if (DEBUGLEVEL >= 10) { - DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n", - name)); + DBG_DEBUG("acl for blob hash for %s is:\n", 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 char *name, + 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; + 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 && name == NULL) { + name = fsp->fsp_name->base_name; + } + + DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); + + status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); + if (NT_STATUS_IS_OK(status)) { + status = validate_nt_acl_blob(mem_ctx, + handle, + fsp, + name, + &blob, + &psd, + &psd_is_from_fs); + TALLOC_FREE(blob.data); + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("ACL validation for [%s] failed\n", + 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 @@ -691,9 +731,7 @@ out: if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s " - "returned %s\n", - name, - nt_errstr(status))); + "returned %s\n", name, nt_errstr(status))); goto fail; } @@ -800,10 +838,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 bcee7dcbd285cf7bae9bace2c260a8fd78632d01 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 61c3d2124fb1a180fae4c8c0b5ab5b32bd56c8ad) --- 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 4d357e7..4227c0f 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, @@ -538,7 +567,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; } @@ -682,11 +711,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, DATA_BLOB blob = data_blob_null; NTSTATUS status; struct security_descriptor *psd = 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 && name == NULL) { name = fsp->fsp_name->base_name; @@ -781,7 +811,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, name, diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c index 62559a2..7dbdde1 100644 --- a/source3/modules/vfs_acl_tdb.c +++ b/source3/modules/vfs_acl_tdb.c @@ -305,6 +305,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; @@ -315,6 +316,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 2338798..d22f89e 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 ed1deee47bfc96adbc6fb8dcefef66109242915c Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 10959698e20de381beec7ab532c8bdc32fa6401c) --- 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 4227c0f..dfe6d23 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -695,6 +695,48 @@ fail: return status; } +static NTSTATUS stat_fsp_or_name(vfs_handle_struct *handle, + files_struct *fsp, + const char *name, + 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, + name, + 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 @@ -777,38 +819,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, - name, - &sbuf); - if (ret == -1) { - status = map_nt_error_from_unix(errno); - goto fail; - } + + status = stat_fsp_or_name(handle, fsp, name, + &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 bca52644833279a2065c493149b923f265afc887 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit f46179ef7310959af095b0ea6234df7523d15457) --- source3/modules/vfs_acl_common.c | 96 ++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index dfe6d23..15002ec 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -787,33 +787,56 @@ 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_name(handle, fsp, name, + &sbuf, &psbuf); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + + status = make_default_filesystem_acl( + mem_ctx, + name, + psbuf, + &psd); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } } else { - status = SMB_VFS_NEXT_GET_NT_ACL(handle, - name, - 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, + name, + 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", name, nt_errstr(status))); - goto fail; - } + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("get_next_acl for file %s " + "returned %s\n", 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 @@ -828,34 +851,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, - name, - psbuf, - &psd); + if (is_directory && !sd_has_inheritable_components(psd, true)) { + status = add_directory_inheritable_components( + handle, + 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, - 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 c67de06639367584be18f18a43a2d61768eba079 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 26a9867ae1a9c69659252ce03c280c7c18a6c58f) --- 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 460c844..1d6587b 100644 --- a/docs-xml/manpages/vfs_acl_tdb.8.xml +++ b/docs-xml/manpages/vfs_acl_tdb.8.xml @@ -63,6 +63,31 @@ + + + acl_tdb:default acl style = [posix|windows] + + + This parameter determines the type of ACL that is synthesized in + case a file or directory lacks an + security.NTACL xattr. + + + When set to posix, an ACL will be + synthesized based on the POSIX mode permissions for user, group + and others, with an additional ACE for NT + Authority\SYSTEM will full rights. + + + When set to windows, an ACL is synthesized + the same way Windows does it, only including permissions for the + owner and NT Authority\SYSTEM. + + + The default for this option is posix. + + + diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml index b6f8c03..ed95632 100644 --- a/docs-xml/manpages/vfs_acl_xattr.8.xml +++ b/docs-xml/manpages/vfs_acl_xattr.8.xml @@ -67,6 +67,31 @@ + + + acl_xattr:default acl style = [posix|windows] + + + This parameter determines the type of ACL that is synthesized in + case a file or directory lacks an + security.NTACL xattr. + + + When set to posix, an ACL will be + synthesized based on the POSIX mode permissions for user, group + and others, with an additional ACE for NT + Authority\SYSTEM will full rights. + + + When set to windows, an ACL is synthesized + the same way Windows does it, only including permissions for the + owner and NT Authority\SYSTEM. + + + The default for this option is posix. + + + diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 15002ec..e8eaa4f 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 * @@ -800,6 +835,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, status = make_default_filesystem_acl( mem_ctx, + config, name, psbuf, &psd); -- 2.7.4 From d741bfd23a3c2c888fd2431429adafd4ecf24f42 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (cherry picked from commit 0730cb7e1ce33dbc5fc48a7363204c1220400c68) --- 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 e8eaa4f..44fef12 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 0d12c8b7871c9b3a43f804ab2b1518a7ae7e33d2 Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 946b93d0e3f6f23fa2325d7aaba4dc6f4cc17cb6) --- selftest/target/Samba3.pm | 9 ++ 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, 328 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 82bd5b8..aef3fd7 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1699,6 +1699,15 @@ 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 7cbd16a..d2b2009 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -313,7 +313,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 @@ -409,6 +409,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 . +*/ + +#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 8966026..d9e704c 100755 --- a/source4/torture/wscript_build +++ b/source4/torture/wscript_build @@ -269,7 +269,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 677a161f4df2a4a98ec3df93a24636bb8d857f7d Mon Sep 17 00:00:00 2001 From: Ralph Boehme 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 Reviewed-by: Jeremy Allison (backported from commit 11dddd59aa01195152199443bc26e3141f162c8f) --- source3/modules/vfs_acl_common.c | 72 +++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 42 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 44fef12..9675fca 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, @@ -875,7 +872,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle, name = fsp->fsp_name->base_name; } - DEBUG(10, ("get_nt_acl_internal: name=%s\n", name)); + DBG_DEBUG("name=%s\n", name); status = get_acl_blob(mem_ctx, handle, fsp, name, &blob); if (NT_STATUS_IS_OK(status)) { @@ -998,8 +995,7 @@ 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", - name)); + DBG_DEBUG("returning acl for %s is:\n", name); NDR_PRINT_DEBUG(security_descriptor, psd); } @@ -1062,9 +1058,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. */ @@ -1087,27 +1082,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; } @@ -1136,8 +1129,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)); } @@ -1255,12 +1247,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)); } @@ -1272,7 +1264,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; } @@ -1309,9 +1301,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); @@ -1344,10 +1335,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; } @@ -1395,9 +1385,7 @@ static int rmdir_acl_common(struct vfs_handle_struct *handle, true); } - DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n", - path, - strerror(errno) )); + DBG_DEBUG("unlink of %s failed %s\n", path, strerror(errno)); return -1; } @@ -1424,9 +1412,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