From d24b7e26e5bdb7bb383cb2c5ea31b625c9229c81 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 12 Nov 2012 16:17:19 -0800 Subject: [PATCH 1/5] First part of #9374 - Allow smb2.acls torture test to pass against smbd with a POSIX ACLs backend. Use the requested access mask before making the fd_open request in open_directory() rather than faking up an access mask of FILE_READ_DATA | FILE_READ_ATTRIBUTES. The underlying ACL may not permit FILE_READ_DATA. --- source3/smbd/open.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 6b94a6d..10c2180 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2752,8 +2752,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; + fsp->access_mask = access_mask; + #ifdef O_DIRECTORY status = fd_open(conn, fsp, O_RDONLY|O_DIRECTORY, 0); #else -- 1.7.7.3 From 69cb61a086e808a4f42f7f1177af29dff1bb5530 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 12 Nov 2012 16:21:15 -0800 Subject: [PATCH 2/5] Add comment explaining exactly *why* we don't check FILE_READ_ATTRIBUTES when evaluating file/directory ACE's. If we can access the path to this file, by default we have FILE_READ_ATTRIBUTES from the containing directory. See the section. "Algorithm to Check Access to an Existing File" in MS-FSA.pdf. --- source3/smbd/open.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 10c2180..f97a3ec 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -58,6 +58,13 @@ NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, return NT_STATUS_OK; } + /* + * If we can access the path to this file, by + * default we have FILE_READ_ATTRIBUTES from the + * containing directory. See the section: + * "Algorithm to Check Access to an Existing File" + * in MS-FSA.pdf. + */ return se_access_check(sd, token, (access_desired & ~FILE_READ_ATTRIBUTES), -- 1.7.7.3 From 4fc1f68baf5dbb7dc94686929ac36d3249e7ffb9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 12 Nov 2012 16:22:52 -0800 Subject: [PATCH 3/5] Ensure when calculating the access mask for MAXIMUM_ALLOWED_ACCESS that we add in FILE_READ_ATTRIBUTES, even if this doesn't come from the file/directory ACL. If we can access the path to this file, by default we have FILE_READ_ATTRIBUTES from the containing directory. See the section. "Algorithm to Check Access to an Existing File" in MS-FSA.pdf. --- source3/smbd/open.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index f97a3ec..d10b697 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1423,7 +1423,14 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, } } - access_mask = access_granted; + /* + * If we can access the path to this file, by + * default we have FILE_READ_ATTRIBUTES from the + * containing directory. See the section. + * "Algorithm to Check Access to an Existing File" + * in MS-FSA.pdf. + */ + access_mask = access_granted | FILE_READ_ATTRIBUTES; } else { access_mask = FILE_GENERIC_ALL; } -- 1.7.7.3 From c5f43ff286cbae1227de1878402414bf5cc922dc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 12 Nov 2012 16:26:25 -0800 Subject: [PATCH 4/5] More for #9374 - Allow smb2.acls torture test to pass against smbd with a POSIX ACLs backend. Change can_delete_directory() to can_delete_directory_fsp(), as we only ever call this from an open directory file handle. This allows us to use OpenDir_fsp() instead of OpenDir(). OpenDir() re-checks the ACL on the directory, which may refuse DIR_LIST permissions. OpenDir_fsp() does not. As this is a file-server internal check to see if the directory actually contains any files before setting delete on close, we can ignore the ACL here (Windows does). --- source3/include/proto.h | 3 +-- source3/lib/dummysmbd.c | 3 +-- source3/locking/locking.c | 3 +-- source3/smbd/dir.c | 15 +++++++++------ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index 720f431..189b286 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -1970,8 +1970,7 @@ void cancel_pending_lock_requests_by_fid(files_struct *fsp, enum file_close_type close_type); void send_stat_cache_delete_message(struct messaging_context *msg_ctx, const char *name); -NTSTATUS can_delete_directory(struct connection_struct *conn, - const char *dirname); +NTSTATUS can_delete_directory_fsp(files_struct *fsp); bool change_to_root_user(void); struct event_context *smbd_event_context(void); void contend_level2_oplocks_begin(files_struct *fsp, diff --git a/source3/lib/dummysmbd.c b/source3/lib/dummysmbd.c index 2465e65..0ff0f2e 100644 --- a/source3/lib/dummysmbd.c +++ b/source3/lib/dummysmbd.c @@ -44,8 +44,7 @@ void send_stat_cache_delete_message(struct messaging_context *msg_ctx, { } -NTSTATUS can_delete_directory(struct connection_struct *conn, - const char *dirname) +NTSTATUS can_delete_directory_fsp(files_struct *fsp) { return NT_STATUS_OK; } diff --git a/source3/locking/locking.c b/source3/locking/locking.c index 4379847..e60c4a8 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -1476,8 +1476,7 @@ NTSTATUS can_set_delete_on_close(files_struct *fsp, uint32 dosmode) return NT_STATUS_ACCESS_DENIED; } - return can_delete_directory(fsp->conn, - fsp->fsp_name->base_name); + return can_delete_directory_fsp(fsp); } return NT_STATUS_OK; diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 9108a80..92be816 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1643,16 +1643,19 @@ bool SearchDir(struct smb_Dir *dirp, const char *name, long *poffset) Is this directory empty ? *****************************************************************/ -NTSTATUS can_delete_directory(struct connection_struct *conn, - const char *dirname) +NTSTATUS can_delete_directory_fsp(files_struct *fsp) { NTSTATUS status = NT_STATUS_OK; long dirpos = 0; const char *dname = NULL; char *talloced = NULL; SMB_STRUCT_STAT st; - struct smb_Dir *dir_hnd = OpenDir(talloc_tos(), conn, - dirname, NULL, 0); + struct connection_struct *conn = fsp->conn; + struct smb_Dir *dir_hnd = OpenDir_fsp(talloc_tos(), + conn, + fsp, + NULL, + 0); if (!dir_hnd) { return map_nt_error_from_unix(errno); @@ -1667,12 +1670,12 @@ NTSTATUS can_delete_directory(struct connection_struct *conn, } } - if (!is_visible_file(conn, dirname, dname, &st, True)) { + if (!is_visible_file(conn, fsp->fsp_name->base_name, dname, &st, True)) { TALLOC_FREE(talloced); continue; } - DEBUG(10,("can_delete_directory: got name %s - can't delete\n", + DEBUG(10,("can_delete_directory_fsp: got name %s - can't delete\n", dname )); status = NT_STATUS_DIRECTORY_NOT_EMPTY; break; -- 1.7.7.3 From f73c977237f5f9f3773cdef7cb9de26d7f7ee824 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 12 Nov 2012 16:30:32 -0800 Subject: [PATCH 5/5] Final part of #9374 - Allow smb2.acls torture test to pass against smbd with a POSIX ACLs backend. We need to do the same check for overriding ACCESS_DENIED on DELETE_ACCESS as we do in smbd/open.c, as the ACL check is duplicated here. This has been fixed in 4.0.0 and later code. --- source3/modules/vfs_acl_common.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index a537011..11c34e0 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -646,7 +646,22 @@ static int open_acl_common(vfs_handle_struct *handle, get_current_nttok(handle->conn), fsp->access_mask, &access_granted); - if (!NT_STATUS_IS_OK(status)) { + /* + * Check if we need to override ACCESS_DENIED for DELETE_ACCESS. + * Do this if we only failed open on DELETE_ACCESS, and + * we have permission to delete from the parent directory. + */ + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) && + (fsp->access_mask & DELETE_ACCESS) && + (access_granted == DELETE_ACCESS) && + can_delete_file_in_directory(handle->conn, smb_fname)) { + DEBUG(10,("open_acl_xattr: " + "overrode " + "DELETE_ACCESS on " + "file %s\n", + smb_fname_str_dbg(smb_fname))); + status = NT_STATUS_OK; + } else if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("open_acl_xattr: %s open " "for access 0x%x (0x%x) " "refused with error %s\n", -- 1.7.7.3