From c9a3c1dbfd16c28732355718738c031f52a78cce Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Jan 2012 13:41:55 -0800 Subject: [PATCH 1/2] First part of fix for bug #8673 - NT ACL issue. Simplify the logic in the unlink/rmdir calls - makes it readable (and correct). Add some debug. --- source3/modules/vfs_acl_common.c | 52 ++++++++++++++++++++++++------------- 1 files changed, 34 insertions(+), 18 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index e8c79e6..17e1874 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -626,8 +626,11 @@ static int open_acl_common(vfs_handle_struct *handle, &access_granted); if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("open_acl_xattr: %s open " + "for access 0x%x (0x%x) " "refused with error %s\n", fsp_str_dbg(fsp), + (unsigned int)fsp->access_mask, + (unsigned int)access_granted, nt_errstr(status) )); goto err; } @@ -911,17 +914,23 @@ static int rmdir_acl_common(struct vfs_handle_struct *handle, { int ret; + /* Try the normal rmdir first. */ ret = SMB_VFS_NEXT_RMDIR(handle, path); - if (!(ret == -1 && (errno == EACCES || errno == EPERM))) { - DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n", - path, - strerror(errno) )); - return ret; + if (ret == 0) { + return 0; + } + if (errno == EACCES || errno == EPERM) { + /* Failed due to access denied, + see if we need to root override. */ + return acl_common_remove_object(handle, + path, + true); } - return acl_common_remove_object(handle, - path, - true); + DEBUG(10,("rmdir_acl_common: unlink of %s failed %s\n", + path, + strerror(errno) )); + return -1; } static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle, @@ -1039,21 +1048,28 @@ static int unlink_acl_common(struct vfs_handle_struct *handle, { int ret; + /* Try the normal unlink first. */ ret = SMB_VFS_NEXT_UNLINK(handle, smb_fname); - if (!(ret == -1 && (errno == EACCES || errno == EPERM))) { - DEBUG(10,("unlink_acl_common: unlink of %s failed %s\n", - smb_fname->base_name, - strerror(errno) )); - return ret; - } - /* Don't do anything fancy for streams. */ - if (smb_fname->stream_name) { - return ret; + if (ret == 0) { + return 0; } + if (errno == EACCES || errno == EPERM) { + /* Failed due to access denied, + see if we need to root override. */ - return acl_common_remove_object(handle, + /* Don't do anything fancy for streams. */ + if (smb_fname->stream_name) { + return -1; + } + return acl_common_remove_object(handle, smb_fname->base_name, false); + } + + DEBUG(10,("unlink_acl_common: unlink of %s failed %s\n", + smb_fname->base_name, + strerror(errno) )); + return -1; } static int chmod_acl_module_common(struct vfs_handle_struct *handle, -- 1.7.3.1 From 72e5ea8cdfb6c3dc130c763c155f55dc5c478cea Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Jan 2012 14:43:04 -0800 Subject: [PATCH 2/2] Second part of fix for bug #8673 - NT ACL issue. Ensure we process the entire ACE list instead of returning ACCESS_DENIED and terminating the walk - ensure we only return the exact bits that cause the access to be denied. Some of the S3 fileserver needs to know if we are only denied DELETE access before overriding it by looking at the containing directory ACL. --- source3/lib/util_seaccess.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/lib/util_seaccess.c b/source3/lib/util_seaccess.c index 058bf32..9f8d3fa 100644 --- a/source3/lib/util_seaccess.c +++ b/source3/lib/util_seaccess.c @@ -158,6 +158,7 @@ NTSTATUS se_access_check(const struct security_descriptor *sd, { int i; uint32_t bits_remaining; + uint32_t explicitly_denied_bits = 0; *access_granted = access_desired; bits_remaining = access_desired; @@ -223,15 +224,15 @@ NTSTATUS se_access_check(const struct security_descriptor *sd, break; case SEC_ACE_TYPE_ACCESS_DENIED: case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT: - if (bits_remaining & ace->access_mask) { - return NT_STATUS_ACCESS_DENIED; - } + explicitly_denied_bits |= (bits_remaining & ace->access_mask); break; default: /* Other ACE types not handled/supported */ break; } } + bits_remaining |= explicitly_denied_bits; + done: if (bits_remaining != 0) { *access_granted = bits_remaining; -- 1.7.3.1