From ce3ced23b9a4e1d5c38268cf6f80d302dd934cde Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Jan 2012 13:41:55 -0800 Subject: [PATCH 1/3] 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 f82f031..4554dc8 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -631,8 +631,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; } @@ -917,17 +920,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, @@ -1047,21 +1056,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 90f11b18a5d7677255f7a1d7043ff92fc096a256 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Jan 2012 13:48:18 -0800 Subject: [PATCH 2/3] 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. --- libcli/security/access_check.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c index 6bb64ae..1b02a86 100644 --- a/libcli/security/access_check.c +++ b/libcli/security/access_check.c @@ -158,6 +158,7 @@ NTSTATUS se_access_check(const struct security_descriptor *sd, { uint32_t i; uint32_t bits_remaining; + uint32_t explicitly_denied_bits = 0; *access_granted = access_desired; bits_remaining = access_desired; @@ -232,15 +233,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 From 9dfc27dcc5b0ece47fa409566e911c80e16839b6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Jan 2012 13:49:03 -0800 Subject: [PATCH 3/3] Third part of fix for bug #8673 - NT ACL issue. (Not needed in master as this code has changed). Ensure we set a temp access mask before calling open(O_RDONLY|O_DIRECTORY) on the directory. --- source3/smbd/open.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ce86b4c..202643f 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2740,10 +2740,6 @@ static NTSTATUS open_directory(connection_struct *conn, fsp->share_access = share_access; fsp->fh->private_options = 0; - /* - * According to Samba4, SEC_FILE_READ_ATTRIBUTE is always granted, - */ - fsp->access_mask = access_mask | FILE_READ_ATTRIBUTES; fsp->print_file = NULL; fsp->modified = False; fsp->oplock_type = NO_OPLOCK; @@ -2758,6 +2754,8 @@ static NTSTATUS open_directory(connection_struct *conn, mtimespec = smb_dname->st.st_ex_mtime; + /* Temporary access mask used to open the directory fd. */ + fsp->access_mask = FILE_READ_DATA | FILE_READ_ATTRIBUTES; #ifdef O_DIRECTORY status = fd_open(conn, fsp, O_RDONLY|O_DIRECTORY, 0); #else @@ -2773,6 +2771,12 @@ static NTSTATUS open_directory(connection_struct *conn, return status; } + /* + * According to Samba4, SEC_FILE_READ_ATTRIBUTE is always granted, + * Set the real access mask for later access (possibly delete). + */ + fsp->access_mask = access_mask | FILE_READ_ATTRIBUTES; + status = vfs_stat_fsp(fsp); if (!NT_STATUS_IS_OK(status)) { fd_close(fsp); -- 1.7.3.1