From db1cccf0c0bd06c7d2041704b3dd07440f424de1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 19 Oct 2011 16:56:00 -0700 Subject: [PATCH 01/40] Make mkdir_internal() check the parent ACL for SEC_DIR_ADD_SUBDIR rights. (cherry picked from commit ff8fa5aa2b7665cd38bd589870f52ac58f38c66f) --- source3/smbd/open.c | 35 +++++++++++++++++++++++++++++++++-- 1 files changed, 33 insertions(+), 2 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 09716d9..ac338d2 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2443,9 +2443,12 @@ static NTSTATUS mkdir_internal(connection_struct *conn, NTSTATUS status; bool posix_open = false; bool need_re_stat = false; + struct security_descriptor *parent_sd = NULL; + uint32_t access_mask = SEC_DIR_ADD_SUBDIR; + uint32_t access_granted = 0; - if(!CAN_WRITE(conn)) { - DEBUG(5,("mkdir_internal: failing create on read-only share " + if(access_mask & ~(conn->share_access)) { + DEBUG(5,("mkdir_internal: failing share access " "%s\n", lp_servicename(SNUM(conn)))); return NT_STATUS_ACCESS_DENIED; } @@ -2467,6 +2470,34 @@ static NTSTATUS mkdir_internal(connection_struct *conn, mode = unix_mode(conn, FILE_ATTRIBUTE_DIRECTORY, smb_dname, parent_dir); } + status = SMB_VFS_GET_NT_ACL(conn, + parent_dir, + SECINFO_DACL, + &parent_sd); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(5,("mkdir_internal: SMB_VFS_GET_NT_ACL failed for " + "%s with error %s\n", + parent_dir, + nt_errstr(status))); + return status; + } + + status = smb1_file_se_access_check(conn, + parent_sd, + get_current_nttok(conn), + access_mask, + &access_granted); + if(!NT_STATUS_IS_OK(status)) { + DEBUG(5,("mkdir_internal: access check " + "on directory %s for " + "path %s for mask 0x%x returned %s\n", + parent_dir, + smb_dname->base_name, + access_mask, + nt_errstr(status) )); + return status; + } + if (SMB_VFS_MKDIR(conn, smb_dname->base_name, mode) != 0) { return map_nt_error_from_unix(errno); } -- 1.7.3.1 From a61ae17d78e9abf04b27fe251d1f7f58b4c59757 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Oct 2011 10:01:12 -0700 Subject: [PATCH 02/40] Refactor to create check_parent_access() which can be called for file creation too. Autobuild-User: Jeremy Allison Autobuild-Date: Thu Oct 20 20:29:22 CEST 2011 on sn-devel-104 (cherry picked from commit 30fb5e99698406fd738cbe98f1a8a6cdca170a64) --- source3/smbd/open.c | 87 +++++++++++++++++++++++++++++++++++++------------- 1 files changed, 64 insertions(+), 23 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ac338d2..e78cc07 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -134,6 +134,63 @@ NTSTATUS smbd_check_open_rights(struct connection_struct *conn, return status; } +static NTSTATUS check_parent_access(struct connection_struct *conn, + struct smb_filename *smb_fname, + uint32_t access_mask, + char **pp_parent_dir, + struct security_descriptor **pp_parent_sd) +{ + NTSTATUS status; + char *parent_dir = NULL; + struct security_descriptor *parent_sd = NULL; + uint32_t access_granted = 0; + + if (!parent_dirname(talloc_tos(), + smb_fname->base_name, + &parent_dir, + NULL)) { + return NT_STATUS_NO_MEMORY; + } + + status = SMB_VFS_GET_NT_ACL(conn, + parent_dir, + SECINFO_DACL, + &parent_sd); + + if (!NT_STATUS_IS_OK(status)) { + DEBUG(5,("check_parent_access: SMB_VFS_GET_NT_ACL failed for " + "%s with error %s\n", + parent_dir, + nt_errstr(status))); + return status; + } + + status = smb1_file_se_access_check(conn, + parent_sd, + get_current_nttok(conn), + access_mask, + &access_granted); + if(!NT_STATUS_IS_OK(status)) { + DEBUG(5,("check_parent_access: access check " + "on directory %s for " + "path %s for mask 0x%x returned (0x%x) %s\n", + parent_dir, + smb_fname->base_name, + access_mask, + access_granted, + nt_errstr(status) )); + return status; + } + + if (pp_parent_dir) { + *pp_parent_dir = parent_dir; + } + if (pp_parent_sd) { + *pp_parent_sd = parent_sd; + } + return NT_STATUS_OK; +} + /**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ @@ -2439,13 +2496,11 @@ static NTSTATUS mkdir_internal(connection_struct *conn, uint32 file_attributes) { mode_t mode; - char *parent_dir; + char *parent_dir = NULL; NTSTATUS status; bool posix_open = false; bool need_re_stat = false; - struct security_descriptor *parent_sd = NULL; uint32_t access_mask = SEC_DIR_ADD_SUBDIR; - uint32_t access_granted = 0; if(access_mask & ~(conn->share_access)) { DEBUG(5,("mkdir_internal: failing share access " @@ -2470,30 +2525,16 @@ static NTSTATUS mkdir_internal(connection_struct *conn, mode = unix_mode(conn, FILE_ATTRIBUTE_DIRECTORY, smb_dname, parent_dir); } - status = SMB_VFS_GET_NT_ACL(conn, - parent_dir, - SECINFO_DACL, - &parent_sd); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(5,("mkdir_internal: SMB_VFS_GET_NT_ACL failed for " - "%s with error %s\n", - parent_dir, - nt_errstr(status))); - return status; - } - - status = smb1_file_se_access_check(conn, - parent_sd, - get_current_nttok(conn), + status = check_parent_access(conn, + smb_dname, access_mask, - &access_granted); + &parent_dir, + NULL); if(!NT_STATUS_IS_OK(status)) { - DEBUG(5,("mkdir_internal: access check " - "on directory %s for " - "path %s for mask 0x%x returned %s\n", + DEBUG(5,("mkdir_internal: check_parent_access " + "on directory %s for path %s returned %s\n", parent_dir, smb_dname->base_name, - access_mask, nt_errstr(status) )); return status; } -- 1.7.3.1 From 636a6182e6a2aa5898455aaeabe3e12eddc500fe Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 19 Oct 2011 14:23:38 -0700 Subject: [PATCH 03/40] Make use of the "dir_exists" we already have on directory open. (cherry picked from commit 7b4edc11e3c0eb9b9a8717e28b70cb00e95cf7ec) --- source3/smbd/open.c | 36 ++++++++++++++++++++++++++++-------- 1 files changed, 28 insertions(+), 8 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index e78cc07..0ff73c2 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2705,6 +2705,15 @@ static NTSTATUS open_directory(connection_struct *conn, /* If directory exists error. If directory doesn't * exist create. */ + if (dir_existed) { + status = NT_STATUS_OBJECT_NAME_COLLISION; + DEBUG(2, ("open_directory: unable to create " + "%s. Error was %s\n", + smb_fname_str_dbg(smb_dname), + nt_errstr(status))); + return status; + } + status = mkdir_internal(conn, smb_dname, file_attributes); @@ -2725,18 +2734,29 @@ static NTSTATUS open_directory(connection_struct *conn, * exist create. */ - status = mkdir_internal(conn, smb_dname, + if (dir_existed) { + status = NT_STATUS_OK; + info = FILE_WAS_OPENED; + } else { + status = mkdir_internal(conn, smb_dname, file_attributes); - if (NT_STATUS_IS_OK(status)) { - info = FILE_WAS_CREATED; + if (NT_STATUS_IS_OK(status)) { + info = FILE_WAS_CREATED; + } else { + /* Cope with create race. */ + if (!NT_STATUS_EQUAL(status, + NT_STATUS_OBJECT_NAME_COLLISION)) { + DEBUG(2, ("open_directory: unable to create " + "%s. Error was %s\n", + smb_fname_str_dbg(smb_dname), + nt_errstr(status))); + return status; + } + info = FILE_WAS_OPENED; + } } - if (NT_STATUS_EQUAL(status, - NT_STATUS_OBJECT_NAME_COLLISION)) { - info = FILE_WAS_OPENED; - status = NT_STATUS_OK; - } break; case FILE_SUPERSEDE: -- 1.7.3.1 From 841ed2a1593e6c8c62acf032a4d93cc09cbf5eb4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 19 Oct 2011 14:25:45 -0700 Subject: [PATCH 04/40] Fix error return to be NT_STATUS_NOT_A_DIRECTORY. (cherry picked from commit f64f91f96f71271186b3e171d24f0d0137620cba) --- 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 0ff73c2..761bba1 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2553,9 +2553,9 @@ static NTSTATUS mkdir_internal(connection_struct *conn, } if (!S_ISDIR(smb_dname->st.st_ex_mode)) { - DEBUG(0, ("Directory just '%s' created is not a directory\n", + DEBUG(0, ("Directory '%s' just created is not a directory !\n", smb_fname_str_dbg(smb_dname))); - return NT_STATUS_ACCESS_DENIED; + return NT_STATUS_NOT_A_DIRECTORY; } if (lp_store_dos_attributes(SNUM(conn))) { -- 1.7.3.1 From 981bdf6ea590124a217303c0764b0126f8ee8a54 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 12:08:51 -0700 Subject: [PATCH 05/40] Add early return on stat open without O_CREAT if file doesn't exist. Reduces one level of indentation. (cherry picked from commit 4b9bdee167987affbc2c4dbf381b0c61dfda3364) --- source3/smbd/open.c | 149 ++++++++++++++++++++++++++------------------------- 1 files changed, 76 insertions(+), 73 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 761bba1..d58fb44 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -559,86 +559,89 @@ static NTSTATUS open_file(files_struct *fsp, } } else { + uint32_t access_granted = 0; + fsp->fh->fd = -1; /* What we used to call a stat open. */ - if (file_existed) { - uint32_t access_granted = 0; + if (!file_existed) { + /* File must exist for a stat open. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } - status = smbd_check_open_rights(conn, - smb_fname, - access_mask, - &access_granted); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - /* - * On NT_STATUS_ACCESS_DENIED, access_granted - * contains the denied bits. - */ - - if ((access_mask & FILE_WRITE_ATTRIBUTES) && - (access_granted & FILE_WRITE_ATTRIBUTES) && - (lp_map_readonly(SNUM(conn)) || - lp_map_archive(SNUM(conn)) || - lp_map_hidden(SNUM(conn)) || - lp_map_system(SNUM(conn)))) { - access_granted &= ~FILE_WRITE_ATTRIBUTES; - - DEBUG(10,("open_file: " - "overrode " - "FILE_WRITE_" - "ATTRIBUTES " - "on file %s\n", - smb_fname_str_dbg( - smb_fname))); - } + status = smbd_check_open_rights(conn, + smb_fname, + access_mask, + &access_granted); + if (!NT_STATUS_IS_OK(status)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + /* + * On NT_STATUS_ACCESS_DENIED, access_granted + * contains the denied bits. + */ - if ((access_mask & DELETE_ACCESS) && - (access_granted & DELETE_ACCESS) && - can_delete_file_in_directory(conn, - smb_fname)) { - /* Were we trying to do a stat open - * for delete and didn't get DELETE - * access (only) ? Check if the - * directory allows DELETE_CHILD. - * See here: - * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx - * for details. */ - - access_granted &= ~DELETE_ACCESS; - - DEBUG(10,("open_file: " - "overrode " - "DELETE_ACCESS on " - "file %s\n", - smb_fname_str_dbg( - smb_fname))); - } + if ((access_mask & FILE_WRITE_ATTRIBUTES) && + (access_granted & FILE_WRITE_ATTRIBUTES) && + (lp_map_readonly(SNUM(conn)) || + lp_map_archive(SNUM(conn)) || + lp_map_hidden(SNUM(conn)) || + lp_map_system(SNUM(conn)))) { + access_granted &= ~FILE_WRITE_ATTRIBUTES; - if (access_granted != 0) { - DEBUG(10,("open_file: Access " - "denied on file " - "%s\n", - smb_fname_str_dbg( - smb_fname))); - return status; - } - } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - fsp->posix_open && - S_ISLNK(smb_fname->st.st_ex_mode)) { - /* This is a POSIX stat open for delete - * or rename on a symlink that points - * nowhere. Allow. */ - DEBUG(10,("open_file: allowing POSIX " - "open on bad symlink %s\n", + DEBUG(10,("open_file: " + "overrode " + "FILE_WRITE_" + "ATTRIBUTES " + "on file %s\n", smb_fname_str_dbg( smb_fname))); - } else { + } + + if ((access_mask & DELETE_ACCESS) && + (access_granted & DELETE_ACCESS) && + can_delete_file_in_directory(conn, + smb_fname)) { + /* Were we trying to do a stat open + * for delete and didn't get DELETE + * access (only) ? Check if the + * directory allows DELETE_CHILD. + * See here: + * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx + * for details. */ + + access_granted &= ~DELETE_ACCESS; + DEBUG(10,("open_file: " - "smbd_check_open_rights on file " - "%s returned %s\n", - smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + "overrode " + "DELETE_ACCESS on " + "file %s\n", + smb_fname_str_dbg( + smb_fname))); + } + + if (access_granted != 0) { + DEBUG(10,("open_file: Access " + "denied on file " + "%s\n", + smb_fname_str_dbg( + smb_fname))); return status; } + } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && + fsp->posix_open && + S_ISLNK(smb_fname->st.st_ex_mode)) { + /* This is a POSIX stat open for delete + * or rename on a symlink that points + * nowhere. Allow. */ + DEBUG(10,("open_file: allowing POSIX " + "open on bad symlink %s\n", + smb_fname_str_dbg( + smb_fname))); + } else { + DEBUG(10,("open_file: " + "smbd_check_open_rights on file " + "%s returned %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status) )); + return status; } } } -- 1.7.3.1 From a2d05d1b6ab06b3f8b5838b2b9979e56956665e2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 11:00:11 -0700 Subject: [PATCH 06/40] Remove another level of indentation - deal with !NT_STATUS_OK individually. (cherry picked from commit a62fcc698adaff64a9870ea59d05b8b2f208d873) --- source3/smbd/open.c | 132 +++++++++++++++++++++++++-------------------------- 1 files changed, 65 insertions(+), 67 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index d58fb44..9c853ab 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -571,78 +571,76 @@ static NTSTATUS open_file(files_struct *fsp, smb_fname, access_mask, &access_granted); - if (!NT_STATUS_IS_OK(status)) { - if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - /* - * On NT_STATUS_ACCESS_DENIED, access_granted - * contains the denied bits. - */ - - if ((access_mask & FILE_WRITE_ATTRIBUTES) && - (access_granted & FILE_WRITE_ATTRIBUTES) && - (lp_map_readonly(SNUM(conn)) || - lp_map_archive(SNUM(conn)) || - lp_map_hidden(SNUM(conn)) || - lp_map_system(SNUM(conn)))) { - access_granted &= ~FILE_WRITE_ATTRIBUTES; - - DEBUG(10,("open_file: " - "overrode " - "FILE_WRITE_" - "ATTRIBUTES " - "on file %s\n", - smb_fname_str_dbg( - smb_fname))); - } + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + /* + * On NT_STATUS_ACCESS_DENIED, access_granted + * contains the denied bits. + */ - if ((access_mask & DELETE_ACCESS) && - (access_granted & DELETE_ACCESS) && - can_delete_file_in_directory(conn, - smb_fname)) { - /* Were we trying to do a stat open - * for delete and didn't get DELETE - * access (only) ? Check if the - * directory allows DELETE_CHILD. - * See here: - * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx - * for details. */ - - access_granted &= ~DELETE_ACCESS; - - DEBUG(10,("open_file: " - "overrode " - "DELETE_ACCESS on " - "file %s\n", - smb_fname_str_dbg( - smb_fname))); - } + if ((access_mask & FILE_WRITE_ATTRIBUTES) && + (access_granted & FILE_WRITE_ATTRIBUTES) && + (lp_map_readonly(SNUM(conn)) || + lp_map_archive(SNUM(conn)) || + lp_map_hidden(SNUM(conn)) || + lp_map_system(SNUM(conn)))) { + access_granted &= ~FILE_WRITE_ATTRIBUTES; - if (access_granted != 0) { - DEBUG(10,("open_file: Access " - "denied on file " - "%s\n", - smb_fname_str_dbg( - smb_fname))); - return status; - } - } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - fsp->posix_open && - S_ISLNK(smb_fname->st.st_ex_mode)) { - /* This is a POSIX stat open for delete - * or rename on a symlink that points - * nowhere. Allow. */ - DEBUG(10,("open_file: allowing POSIX " - "open on bad symlink %s\n", + DEBUG(10,("open_file: " + "overrode " + "FILE_WRITE_" + "ATTRIBUTES " + "on file %s\n", smb_fname_str_dbg( smb_fname))); - } else { + } + + if ((access_mask & DELETE_ACCESS) && + (access_granted & DELETE_ACCESS) && + can_delete_file_in_directory(conn, + smb_fname)) { + /* Were we trying to do a stat open + * for delete and didn't get DELETE + * access (only) ? Check if the + * directory allows DELETE_CHILD. + * See here: + * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx + * for details. */ + + access_granted &= ~DELETE_ACCESS; + DEBUG(10,("open_file: " - "smbd_check_open_rights on file " - "%s returned %s\n", - smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + "overrode " + "DELETE_ACCESS on " + "file %s\n", + smb_fname_str_dbg( + smb_fname))); + } + + if (access_granted != 0) { + DEBUG(10,("open_file: Access " + "denied on file " + "%s\n", + smb_fname_str_dbg( + smb_fname))); return status; } + } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && + fsp->posix_open && + S_ISLNK(smb_fname->st.st_ex_mode)) { + /* This is a POSIX stat open for delete + * or rename on a symlink that points + * nowhere. Allow. */ + DEBUG(10,("open_file: allowing POSIX " + "open on bad symlink %s\n", + smb_fname_str_dbg( + smb_fname))); + } else if (!NT_STATUS_IS_OK(status)) { + DEBUG(10,("open_file: " + "smbd_check_open_rights on file " + "%s returned %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status) )); + return status; } } -- 1.7.3.1 From 604d6f49026b35ac55ed4ee1a7b78e03909caab3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 12:41:18 -0700 Subject: [PATCH 07/40] Factor out the code checking if a parent should override DELETE_ACCESS into a function. (cherry picked from commit a9acdc6a4ce351724678842de83201105baba708) --- source3/smbd/open.c | 40 +++++++++++++++++++++++++++++++--------- 1 files changed, 31 insertions(+), 9 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 9c853ab..8c6f689 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -192,6 +192,25 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, } /**************************************************************************** + If the requester wanted DELETE_ACCESS and was only rejected because + the file ACL didn't include DELETE_ACCESS, see if the parent ACL + ovverrides this. +****************************************************************************/ + +static bool parent_override_delete(connection_struct *conn, + struct smb_filename *smb_fname, + uint32_t access_mask, + uint32_t rejected_mask) +{ + if ((access_mask & DELETE_ACCESS) && + (rejected_mask == DELETE_ACCESS) && + can_delete_file_in_directory(conn, smb_fname)) { + return true; + } + return false; +} + +/**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ @@ -594,10 +613,10 @@ static NTSTATUS open_file(files_struct *fsp, smb_fname))); } - if ((access_mask & DELETE_ACCESS) && - (access_granted & DELETE_ACCESS) && - can_delete_file_in_directory(conn, - smb_fname)) { + if (parent_override_delete(conn, + smb_fname, + access_mask, + access_granted)) { /* Were we trying to do a stat open * for delete and didn't get DELETE * access (only) ? Check if the @@ -618,12 +637,14 @@ static NTSTATUS open_file(files_struct *fsp, if (access_granted != 0) { DEBUG(10,("open_file: Access " - "denied on file " + "denied (0x%x) on file " "%s\n", + access_granted, smb_fname_str_dbg( smb_fname))); return status; } + } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && fsp->posix_open && S_ISLNK(smb_fname->st.st_ex_mode)) { @@ -2790,10 +2811,11 @@ static NTSTATUS open_directory(connection_struct *conn, * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx * for details. */ - if ((NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) && - (access_mask & DELETE_ACCESS) && - (access_granted == DELETE_ACCESS) && - can_delete_file_in_directory(conn, smb_dname))) { + if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) && + parent_override_delete(conn, + smb_dname, + access_mask, + access_granted)) { DEBUG(10,("open_directory: overrode ACCESS_DENIED " "on directory %s\n", smb_fname_str_dbg(smb_dname))); -- 1.7.3.1 From 0310b5122746da5c29eb8ee5f64a174e18a91157 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 14:06:41 -0700 Subject: [PATCH 08/40] Make smbd_check_open_rights() static. (cherry picked from commit 999ae2ef6113fd2e23a04e127e07484a1b8a6dd3) --- source3/smbd/globals.h | 4 ---- source3/smbd/open.c | 2 +- 2 files changed, 1 insertions(+), 5 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 663daa4..4ab04bf 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -229,10 +229,6 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, bool file_existed, uint32_t access_mask, uint32_t *access_mask_out); -NTSTATUS smbd_check_open_rights(struct connection_struct *conn, - const struct smb_filename *smb_fname, - uint32_t access_mask, - uint32_t *access_granted); void smbd_notify_cancel_by_smbreq(const struct smb_request *smbreq); diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 8c6f689..54f6813 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -68,7 +68,7 @@ NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, Check if we have open rights. ****************************************************************************/ -NTSTATUS smbd_check_open_rights(struct connection_struct *conn, +static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, const struct smb_filename *smb_fname, uint32_t access_mask, uint32_t *access_granted) -- 1.7.3.1 From 5b6404b3387f5bcb9664ab53b338d80a6d0fcf03 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 14:47:52 -0700 Subject: [PATCH 09/40] Move parent_override_delete() to before I need to use it. (cherry picked from commit 41d611e8c2602296a2c9cf6c33a9e1b96f55ca9d) --- source3/smbd/open.c | 38 +++++++++++++++++++------------------- 1 files changed, 19 insertions(+), 19 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 54f6813..eea7d02 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -65,6 +65,25 @@ NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, } /**************************************************************************** + If the requester wanted DELETE_ACCESS and was only rejected because + the file ACL didn't include DELETE_ACCESS, see if the parent ACL + ovverrides this. +****************************************************************************/ + +static bool parent_override_delete(connection_struct *conn, + struct smb_filename *smb_fname, + uint32_t access_mask, + uint32_t rejected_mask) +{ + if ((access_mask & DELETE_ACCESS) && + (rejected_mask == DELETE_ACCESS) && + can_delete_file_in_directory(conn, smb_fname)) { + return true; + } + return false; +} + +/**************************************************************************** Check if we have open rights. ****************************************************************************/ @@ -192,25 +211,6 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, } /**************************************************************************** - If the requester wanted DELETE_ACCESS and was only rejected because - the file ACL didn't include DELETE_ACCESS, see if the parent ACL - ovverrides this. -****************************************************************************/ - -static bool parent_override_delete(connection_struct *conn, - struct smb_filename *smb_fname, - uint32_t access_mask, - uint32_t rejected_mask) -{ - if ((access_mask & DELETE_ACCESS) && - (rejected_mask == DELETE_ACCESS) && - can_delete_file_in_directory(conn, smb_fname)) { - return true; - } - return false; -} - -/**************************************************************************** fd support routines - attempt to do a dos_open. ****************************************************************************/ -- 1.7.3.1 From 4c28f8e8daad0840990d21dd0cf54a368caf7f89 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 14:58:32 -0700 Subject: [PATCH 10/40] Simplify smbd_check_open_rights() and move all the special casing inside it. (cherry picked from commit 97812fe7eecd9f5f3ab8873c44be706d1acab9e7) --- source3/smbd/open.c | 173 +++++++++++++++++++++------------------------------ 1 files changed, 72 insertions(+), 101 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index eea7d02..8ac3625 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -88,29 +88,31 @@ static bool parent_override_delete(connection_struct *conn, ****************************************************************************/ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, - const struct smb_filename *smb_fname, - uint32_t access_mask, - uint32_t *access_granted) + struct smb_filename *smb_fname, + uint32_t access_mask) { /* Check if we have rights to open. */ NTSTATUS status; struct security_descriptor *sd = NULL; uint32_t rejected_share_access; + uint32_t rejected_mask = 0; rejected_share_access = access_mask & ~(conn->share_access); if (rejected_share_access) { - *access_granted = rejected_share_access; + DEBUG(10, ("smbd_check_open_rights: rejected share access 0x%x " + "on %s (0x%x)\n", + (unsigned int)access_mask, + smb_fname_str_dbg(smb_fname), + (unsigned int)rejected_share_access )); return NT_STATUS_ACCESS_DENIED; } if ((access_mask & DELETE_ACCESS) && !lp_acl_check_permissions(SNUM(conn))) { - *access_granted = access_mask; - DEBUG(10,("smbd_check_open_rights: not checking ACL " "on DELETE_ACCESS on file %s. Granting 0x%x\n", smb_fname_str_dbg(smb_fname), - (unsigned int)*access_granted )); + (unsigned int)access_mask )); return NT_STATUS_OK; } @@ -131,13 +133,13 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, sd, get_current_nttok(conn), access_mask, - access_granted); + &rejected_mask); DEBUG(10,("smbd_check_open_rights: file %s requesting " "0x%x returning 0x%x (%s)\n", smb_fname_str_dbg(smb_fname), (unsigned int)access_mask, - (unsigned int)*access_granted, + (unsigned int)rejected_mask, nt_errstr(status) )); if (!NT_STATUS_IS_OK(status)) { @@ -150,7 +152,53 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, TALLOC_FREE(sd); - return status; + if (NT_STATUS_IS_OK(status) || + !NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + return status; + } + + /* Here we know status == NT_STATUS_ACCESS_DENIED. */ + if ((access_mask & FILE_WRITE_ATTRIBUTES) && + (rejected_mask & FILE_WRITE_ATTRIBUTES) && + (lp_map_readonly(SNUM(conn)) || + lp_map_archive(SNUM(conn)) || + lp_map_hidden(SNUM(conn)) || + lp_map_system(SNUM(conn)))) { + rejected_mask &= ~FILE_WRITE_ATTRIBUTES; + + DEBUG(10,("smbd_check_open_rights: " + "overrode " + "FILE_WRITE_ATTRIBUTES " + "on file %s\n", + smb_fname_str_dbg(smb_fname))); + } + + if (parent_override_delete(conn, + smb_fname, + access_mask, + rejected_mask)) { + /* Were we trying to do an open + * for delete and didn't get DELETE + * access (only) ? Check if the + * directory allows DELETE_CHILD. + * See here: + * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx + * for details. */ + + rejected_mask &= ~DELETE_ACCESS; + + DEBUG(10,("smbd_check_open_rights: " + "overrode " + "DELETE_ACCESS on " + "file %s\n", + smb_fname_str_dbg(smb_fname))); + } + + if (rejected_mask != 0) { + return NT_STATUS_ACCESS_DENIED; + } else { + return NT_STATUS_OK; + } } static NTSTATUS check_parent_access(struct connection_struct *conn, @@ -578,8 +626,6 @@ static NTSTATUS open_file(files_struct *fsp, } } else { - uint32_t access_granted = 0; - fsp->fh->fd = -1; /* What we used to call a stat open. */ if (!file_existed) { /* File must exist for a stat open. */ @@ -588,79 +634,26 @@ static NTSTATUS open_file(files_struct *fsp, status = smbd_check_open_rights(conn, smb_fname, - access_mask, - &access_granted); - if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { - /* - * On NT_STATUS_ACCESS_DENIED, access_granted - * contains the denied bits. - */ - - if ((access_mask & FILE_WRITE_ATTRIBUTES) && - (access_granted & FILE_WRITE_ATTRIBUTES) && - (lp_map_readonly(SNUM(conn)) || - lp_map_archive(SNUM(conn)) || - lp_map_hidden(SNUM(conn)) || - lp_map_system(SNUM(conn)))) { - access_granted &= ~FILE_WRITE_ATTRIBUTES; - - DEBUG(10,("open_file: " - "overrode " - "FILE_WRITE_" - "ATTRIBUTES " - "on file %s\n", - smb_fname_str_dbg( - smb_fname))); - } + access_mask); - if (parent_override_delete(conn, - smb_fname, - access_mask, - access_granted)) { - /* Were we trying to do a stat open - * for delete and didn't get DELETE - * access (only) ? Check if the - * directory allows DELETE_CHILD. - * See here: - * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx - * for details. */ - - access_granted &= ~DELETE_ACCESS; - - DEBUG(10,("open_file: " - "overrode " - "DELETE_ACCESS on " - "file %s\n", - smb_fname_str_dbg( - smb_fname))); - } - - if (access_granted != 0) { - DEBUG(10,("open_file: Access " - "denied (0x%x) on file " - "%s\n", - access_granted, - smb_fname_str_dbg( - smb_fname))); - return status; - } - - } else if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && - fsp->posix_open && - S_ISLNK(smb_fname->st.st_ex_mode)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND) && + fsp->posix_open && + S_ISLNK(smb_fname->st.st_ex_mode)) { /* This is a POSIX stat open for delete * or rename on a symlink that points * nowhere. Allow. */ DEBUG(10,("open_file: allowing POSIX " "open on bad symlink %s\n", - smb_fname_str_dbg( - smb_fname))); - } else if (!NT_STATUS_IS_OK(status)) { + smb_fname_str_dbg(smb_fname))); + status = NT_STATUS_OK; + } + + if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("open_file: " - "smbd_check_open_rights on file " - "%s returned %s\n", - smb_fname_str_dbg(smb_fname), - nt_errstr(status) )); + "smbd_check_open_rights on file " + "%s returned %s\n", + smb_fname_str_dbg(smb_fname), + nt_errstr(status) )); return status; } } @@ -2799,29 +2792,7 @@ static NTSTATUS open_directory(connection_struct *conn, } if (info == FILE_WAS_OPENED) { - uint32_t access_granted = 0; - status = smbd_check_open_rights(conn, smb_dname, access_mask, - &access_granted); - - /* Were we trying to do a directory open - * for delete and didn't get DELETE - * access (only) ? Check if the - * directory allows DELETE_CHILD. - * See here: - * http://blogs.msdn.com/oldnewthing/archive/2004/06/04/148426.aspx - * for details. */ - - if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED) && - parent_override_delete(conn, - smb_dname, - access_mask, - access_granted)) { - DEBUG(10,("open_directory: overrode ACCESS_DENIED " - "on directory %s\n", - smb_fname_str_dbg(smb_dname))); - status = NT_STATUS_OK; - } - + status = smbd_check_open_rights(conn, smb_dname, access_mask); if (!NT_STATUS_IS_OK(status)) { DEBUG(10, ("open_directory: smbd_check_open_rights on " "file %s failed with %s\n", -- 1.7.3.1 From 1562dce4eb0b732667760ba0b421ba2f168f2732 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 15:03:28 -0700 Subject: [PATCH 11/40] Finally do all the open checks inside open_file(). Checks inside vfs_acl_common can now be removed. (cherry picked from commit c8644a370d9febe0de4ad978ab390d1dcaa63878) --- source3/smbd/open.c | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 8ac3625..9d27705 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -612,6 +612,36 @@ static NTSTATUS open_file(files_struct *fsp, return NT_STATUS_OBJECT_NAME_INVALID; } + /* Can we access this file ? */ + if (!fsp->base_fsp) { + /* Only do this check on non-stream open. */ + if (file_existed) { + status = smbd_check_open_rights(conn, + smb_fname, + access_mask); + } else if (local_flags & O_CREAT){ + status = check_parent_access(conn, + smb_fname, + SEC_DIR_ADD_FILE, + NULL, + NULL); + } else { + /* File didn't exist and no O_CREAT. */ + return NT_STATUS_OBJECT_NAME_NOT_FOUND; + } + if (!NT_STATUS_IS_OK(status)) { + DEBUG(10,("open_file: " + "%s on file " + "%s returned %s\n", + file_existed ? + "smbd_check_open_rights" : + "check_parent_access", + smb_fname_str_dbg(smb_fname), + nt_errstr(status) )); + return status; + } + } + /* Actually do the open */ status = fd_open(conn, fsp, local_flags, unx_mode); if (!NT_STATUS_IS_OK(status)) { -- 1.7.3.1 From c2bcb658a845b9697e4b190423682e82c2d5cfaf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 15:30:00 -0700 Subject: [PATCH 12/40] Remove unused "struct security_descriptor" parameter from check_parent_access() (cherry picked from commit fb13e0a0afba4bdb40a9bae1c4d2cf01efbca4e0) --- source3/smbd/open.c | 10 ++-------- 1 files changed, 2 insertions(+), 8 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 9d27705..0b29cef 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -204,8 +204,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, static NTSTATUS check_parent_access(struct connection_struct *conn, struct smb_filename *smb_fname, uint32_t access_mask, - char **pp_parent_dir, - struct security_descriptor **pp_parent_sd) + char **pp_parent_dir) { NTSTATUS status; char *parent_dir = NULL; @@ -252,9 +251,6 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, if (pp_parent_dir) { *pp_parent_dir = parent_dir; } - if (pp_parent_sd) { - *pp_parent_sd = parent_sd; - } return NT_STATUS_OK; } @@ -623,7 +619,6 @@ static NTSTATUS open_file(files_struct *fsp, status = check_parent_access(conn, smb_fname, SEC_DIR_ADD_FILE, - NULL, NULL); } else { /* File didn't exist and no O_CREAT. */ @@ -2573,8 +2568,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn, status = check_parent_access(conn, smb_dname, access_mask, - &parent_dir, - NULL); + &parent_dir); if(!NT_STATUS_IS_OK(status)) { DEBUG(5,("mkdir_internal: check_parent_access " "on directory %s for path %s returned %s\n", -- 1.7.3.1 From fa4bd2b86a1f5ffa2317b935384cca40d07751fe Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 26 Oct 2011 16:16:01 -0700 Subject: [PATCH 13/40] Remove the mkdir and open functions from the ACL modules - main code paths now handle this. --- source3/modules/vfs_acl_common.c | 139 ++------------------------------------ source3/modules/vfs_acl_tdb.c | 2 - source3/modules/vfs_acl_xattr.c | 2 - 3 files changed, 6 insertions(+), 137 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 81c734a..84afc6f 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -587,123 +587,6 @@ static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle, } /********************************************************************* - Check ACL on open. For new files inherit from parent directory. -*********************************************************************/ - -static int open_acl_common(vfs_handle_struct *handle, - struct smb_filename *smb_fname, - files_struct *fsp, - int flags, - mode_t mode) -{ - uint32_t access_granted = 0; - struct security_descriptor *pdesc = NULL; - bool file_existed = true; - char *fname = NULL; - NTSTATUS status; - - if (fsp->base_fsp) { - /* Stream open. Base filename open already did the ACL check. */ - DEBUG(10,("open_acl_common: stream open on %s\n", - fsp_str_dbg(fsp) )); - return SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); - } - - status = get_full_smb_filename(talloc_tos(), smb_fname, - &fname); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - status = get_nt_acl_internal(handle, - NULL, - fname, - (SECINFO_OWNER | - SECINFO_GROUP | - SECINFO_DACL), - &pdesc); - if (NT_STATUS_IS_OK(status)) { - /* See if we can access it. */ - status = smb1_file_se_access_check(handle->conn, - pdesc, - get_current_nttok(handle->conn), - fsp->access_mask, - &access_granted); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(10,("open_acl_xattr: %s open " - "refused with error %s\n", - fsp_str_dbg(fsp), - nt_errstr(status) )); - goto err; - } - } else if (NT_STATUS_EQUAL(status,NT_STATUS_OBJECT_NAME_NOT_FOUND)) { - file_existed = false; - /* - * If O_CREAT is true then we're trying to create a file. - * Check the parent directory ACL will allow this. - */ - if (flags & O_CREAT) { - struct security_descriptor *parent_desc = NULL; - struct security_descriptor **pp_psd = NULL; - - status = check_parent_acl_common(handle, fname, - SEC_DIR_ADD_FILE, &parent_desc); - if (!NT_STATUS_IS_OK(status)) { - goto err; - } - - /* Cache the parent security descriptor for - * later use. */ - - pp_psd = VFS_ADD_FSP_EXTENSION(handle, - fsp, - struct security_descriptor *, - NULL); - if (!pp_psd) { - status = NT_STATUS_NO_MEMORY; - goto err; - } - - *pp_psd = parent_desc; - status = NT_STATUS_OK; - } - } - - DEBUG(10,("open_acl_xattr: get_nt_acl_attr_internal for " - "%s returned %s\n", - fsp_str_dbg(fsp), - nt_errstr(status) )); - - fsp->fh->fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); - return fsp->fh->fd; - - err: - - errno = map_errno_from_nt_status(status); - return -1; -} - -static int mkdir_acl_common(vfs_handle_struct *handle, const char *path, mode_t mode) -{ - int ret; - NTSTATUS status; - SMB_STRUCT_STAT sbuf; - - ret = vfs_stat_smb_fname(handle->conn, path, &sbuf); - if (ret == -1 && errno == ENOENT) { - /* We're creating a new directory. */ - status = check_parent_acl_common(handle, path, - SEC_DIR_ADD_SUBDIR, NULL); - if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); - return -1; - } - } - - return SMB_VFS_NEXT_MKDIR(handle, path, mode); -} - -/********************************************************************* Fetch a security descriptor given an fsp. *********************************************************************/ @@ -951,7 +834,6 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle, files_struct *fsp = NULL; int info; struct security_descriptor *parent_sd = NULL; - struct security_descriptor **pp_parent_sd = NULL; status = SMB_VFS_NEXT_CREATE_FILE(handle, req, @@ -996,18 +878,11 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle, goto out; } - /* See if we have a cached parent sd, if so, use it. */ - pp_parent_sd = (struct security_descriptor **)VFS_FETCH_FSP_EXTENSION(handle, fsp); - if (!pp_parent_sd) { - /* Must be a directory, fetch again (sigh). */ - status = get_parent_acl_common(handle, - fsp->fsp_name->base_name, - &parent_sd); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - } else { - parent_sd = *pp_parent_sd; + status = get_parent_acl_common(handle, + fsp->fsp_name->base_name, + &parent_sd); + if (!NT_STATUS_IS_OK(status)) { + goto out; } if (!parent_sd) { @@ -1026,9 +901,7 @@ static NTSTATUS create_file_acl_common(struct vfs_handle_struct *handle, out: - if (fsp) { - VFS_REMOVE_FSP_EXTENSION(handle, fsp); - } + TALLOC_FREE(parent_sd); if (NT_STATUS_IS_OK(status) && pinfo) { *pinfo = info; diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c index b208b24..265286f 100644 --- a/source3/modules/vfs_acl_tdb.c +++ b/source3/modules/vfs_acl_tdb.c @@ -399,9 +399,7 @@ static struct vfs_fn_pointers vfs_acl_tdb_fns = { .connect_fn = connect_acl_tdb, .disconnect = disconnect_acl_tdb, .opendir = opendir_acl_common, - .mkdir = mkdir_acl_common, .rmdir = rmdir_acl_tdb, - .open_fn = open_acl_common, .create_file = create_file_acl_common, .unlink = unlink_acl_tdb, .chmod = chmod_acl_module_common, diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c index ca23fad..bf1ee3e 100644 --- a/source3/modules/vfs_acl_xattr.c +++ b/source3/modules/vfs_acl_xattr.c @@ -202,9 +202,7 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle, static struct vfs_fn_pointers vfs_acl_xattr_fns = { .connect_fn = connect_acl_xattr, .opendir = opendir_acl_common, - .mkdir = mkdir_acl_common, .rmdir = rmdir_acl_common, - .open_fn = open_acl_common, .create_file = create_file_acl_common, .unlink = unlink_acl_common, .chmod = chmod_acl_module_common, -- 1.7.3.1 From e1c65f1991fe61c5c9ac23527e6a31b7c751870f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 27 Oct 2011 16:48:13 -0700 Subject: [PATCH 14/40] Remove the order dependency in parent_override_delete(), just check for & not ==. (cherry picked from commit 50558bbfe87ca430c2ebe04a3022b8d98fe1b981) --- 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 0b29cef..de1b667 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -65,7 +65,7 @@ NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, } /**************************************************************************** - If the requester wanted DELETE_ACCESS and was only rejected because + If the requester wanted DELETE_ACCESS and was rejected because the file ACL didn't include DELETE_ACCESS, see if the parent ACL ovverrides this. ****************************************************************************/ @@ -76,7 +76,7 @@ static bool parent_override_delete(connection_struct *conn, uint32_t rejected_mask) { if ((access_mask & DELETE_ACCESS) && - (rejected_mask == DELETE_ACCESS) && + (rejected_mask & DELETE_ACCESS) && can_delete_file_in_directory(conn, smb_fname)) { return true; } -- 1.7.3.1 From abfce4fa321e9388e0198bafb651865d2b3525eb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 1 Nov 2011 12:56:06 -0700 Subject: [PATCH 15/40] We don't need check_name() here. All possible paths to dptr_create() have already called check_name. --- source3/smbd/dir.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 9108a80..670b34a 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -430,7 +430,6 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, struct smbd_server_connection *sconn = conn->sconn; struct dptr_struct *dptr = NULL; struct smb_Dir *dir_hnd; - NTSTATUS status; if (fsp && fsp->is_directory && fsp->fh->fd != -1) { path = fsp->fsp_name->base_name; @@ -450,10 +449,6 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, if (fsp) { dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr); } else { - status = check_name(conn,path); - if (!NT_STATUS_IS_OK(status)) { - return status; - } dir_hnd = OpenDir(NULL, conn, path, wcard, attr); } -- 1.7.3.1 From 03c9411920411030f5f346b9a08d302f273660d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 31 Oct 2011 12:37:39 -0700 Subject: [PATCH 16/40] Change function signature of check_parent_access() to take char * instead of struct smb_filename. Expose it so it can be called from directory code. (cherry picked from commit fb4a95321dc9b59d5482378ac99f3b7be31ab8ce) --- source3/smbd/open.c | 12 ++++++------ source3/smbd/proto.h | 4 ++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index de1b667..4c442f0 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -201,8 +201,8 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, } } -static NTSTATUS check_parent_access(struct connection_struct *conn, - struct smb_filename *smb_fname, +NTSTATUS check_parent_access(struct connection_struct *conn, + const char *path, uint32_t access_mask, char **pp_parent_dir) { @@ -212,7 +212,7 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, uint32_t access_granted = 0; if (!parent_dirname(talloc_tos(), - smb_fname->base_name, + path, &parent_dir, NULL)) { return NT_STATUS_NO_MEMORY; @@ -241,7 +241,7 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, "on directory %s for " "path %s for mask 0x%x returned (0x%x) %s\n", parent_dir, - smb_fname->base_name, + path, access_mask, access_granted, nt_errstr(status) )); @@ -617,7 +617,7 @@ static NTSTATUS open_file(files_struct *fsp, access_mask); } else if (local_flags & O_CREAT){ status = check_parent_access(conn, - smb_fname, + smb_fname->base_name, SEC_DIR_ADD_FILE, NULL); } else { @@ -2566,7 +2566,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn, } status = check_parent_access(conn, - smb_dname, + smb_dname->base_name, access_mask, &parent_dir); if(!NT_STATUS_IS_OK(status)) { diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 02b5e40..e527fde 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -579,6 +579,10 @@ NTSTATUS smb1_file_se_access_check(connection_struct *conn, const struct security_token *token, uint32_t access_desired, uint32_t *access_granted); +NTSTATUS check_parent_access(struct connection_struct *conn, + const char *path, + uint32_t access_mask, + char **pp_parent_dir); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, -- 1.7.3.1 From 18953358581f7653ce4b4517f4c875955fcb445a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 31 Oct 2011 12:38:20 -0700 Subject: [PATCH 17/40] Call check_parent_access() on readdir. (cherry picked from commit 1a96425c45ac0555a9215a3803d2fc6d8b2fb816) --- source3/smbd/dir.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 670b34a..3a18128 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -430,6 +430,7 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, struct smbd_server_connection *sconn = conn->sconn; struct dptr_struct *dptr = NULL; struct smb_Dir *dir_hnd; + NTSTATUS status; if (fsp && fsp->is_directory && fsp->fh->fd != -1) { path = fsp->fsp_name->base_name; @@ -446,6 +447,18 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, return NT_STATUS_INVALID_PARAMETER; } + status = check_parent_access(conn, + path, + SEC_DIR_LIST, + NULL); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(5,("dptr_create: parent access check for path " + "%s failed with %s\n", + path, + nt_errstr(status))); + return status; + } + if (fsp) { dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr); } else { -- 1.7.3.1 From 30faf7f259fe768ad69fc2de0420f1fb96df6be5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 31 Oct 2011 12:38:36 -0700 Subject: [PATCH 18/40] Remove opendir() VFS code from ACL modules. (cherry picked from commit d1b2a72483b649bdefd1e762a014b12aa66a70a4) --- source3/modules/vfs_acl_common.c | 48 -------------------------------------- source3/modules/vfs_acl_tdb.c | 1 - source3/modules/vfs_acl_xattr.c | 1 - 3 files changed, 0 insertions(+), 50 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index 84afc6f..38cc4d6 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -551,41 +551,6 @@ static NTSTATUS get_parent_acl_common(vfs_handle_struct *handle, return status; } -static NTSTATUS check_parent_acl_common(vfs_handle_struct *handle, - const char *path, - uint32_t access_mask, - struct security_descriptor **pp_parent_desc) -{ - char *parent_name = NULL; - struct security_descriptor *parent_desc = NULL; - uint32_t access_granted = 0; - NTSTATUS status; - - status = get_parent_acl_common(handle, path, &parent_desc); - if (!NT_STATUS_IS_OK(status)) { - return status; - } - if (pp_parent_desc) { - *pp_parent_desc = parent_desc; - } - status = smb1_file_se_access_check(handle->conn, - parent_desc, - get_current_nttok(handle->conn), - access_mask, - &access_granted); - if(!NT_STATUS_IS_OK(status)) { - DEBUG(10,("check_parent_acl_common: access check " - "on directory %s for " - "path %s for mask 0x%x returned %s\n", - parent_name, - path, - access_mask, - nt_errstr(status) )); - return status; - } - return NT_STATUS_OK; -} - /********************************************************************* Fetch a security descriptor given an fsp. *********************************************************************/ @@ -688,19 +653,6 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, return NT_STATUS_OK; } -static SMB_STRUCT_DIR *opendir_acl_common(vfs_handle_struct *handle, - const char *fname, const char *mask, uint32 attr) -{ - NTSTATUS status = check_parent_acl_common(handle, fname, - SEC_DIR_LIST, NULL); - - if (!NT_STATUS_IS_OK(status)) { - errno = map_errno_from_nt_status(status); - return NULL; - } - return SMB_VFS_NEXT_OPENDIR(handle, fname, mask, attr); -} - static int acl_common_remove_object(vfs_handle_struct *handle, const char *path, bool is_directory) diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c index 265286f..fc7a1ca 100644 --- a/source3/modules/vfs_acl_tdb.c +++ b/source3/modules/vfs_acl_tdb.c @@ -398,7 +398,6 @@ static int sys_acl_set_fd_tdb(vfs_handle_struct *handle, static struct vfs_fn_pointers vfs_acl_tdb_fns = { .connect_fn = connect_acl_tdb, .disconnect = disconnect_acl_tdb, - .opendir = opendir_acl_common, .rmdir = rmdir_acl_tdb, .create_file = create_file_acl_common, .unlink = unlink_acl_tdb, diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c index bf1ee3e..984b851 100644 --- a/source3/modules/vfs_acl_xattr.c +++ b/source3/modules/vfs_acl_xattr.c @@ -201,7 +201,6 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle, static struct vfs_fn_pointers vfs_acl_xattr_fns = { .connect_fn = connect_acl_xattr, - .opendir = opendir_acl_common, .rmdir = rmdir_acl_common, .create_file = create_file_acl_common, .unlink = unlink_acl_common, -- 1.7.3.1 From 409e52553919b5c8dcd70e01130d92ecb255a39e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Nov 2011 11:49:05 -0700 Subject: [PATCH 19/40] Revert "Call check_parent_access() on readdir." This reverts commit a763edaf9c76afe2546c035fc090370301dd347b. Checking the wrong thing.. (cherry picked from commit d433af92b9085e862b6fd3e3f4aea90051d9a2fc) --- source3/smbd/dir.c | 13 ------------- 1 files changed, 0 insertions(+), 13 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 3a18128..670b34a 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -430,7 +430,6 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, struct smbd_server_connection *sconn = conn->sconn; struct dptr_struct *dptr = NULL; struct smb_Dir *dir_hnd; - NTSTATUS status; if (fsp && fsp->is_directory && fsp->fh->fd != -1) { path = fsp->fsp_name->base_name; @@ -447,18 +446,6 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, return NT_STATUS_INVALID_PARAMETER; } - status = check_parent_access(conn, - path, - SEC_DIR_LIST, - NULL); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(5,("dptr_create: parent access check for path " - "%s failed with %s\n", - path, - nt_errstr(status))); - return status; - } - if (fsp) { dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr); } else { -- 1.7.3.1 From fbdea39d37028e970c20e6839869f6257552bf17 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Nov 2011 11:49:22 -0700 Subject: [PATCH 20/40] Revert "Change function signature of check_parent_access() to take char * instead of struct smb_filename." This reverts commit a11c0a41a35aa2b1c14333552045a65e3e50df1e. Not needed. (cherry picked from commit 1fab17de94a38294918e9eab7d6f85b94d6db421) --- source3/smbd/open.c | 12 ++++++------ source3/smbd/proto.h | 4 ---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 4c442f0..de1b667 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -201,8 +201,8 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, } } -NTSTATUS check_parent_access(struct connection_struct *conn, - const char *path, +static NTSTATUS check_parent_access(struct connection_struct *conn, + struct smb_filename *smb_fname, uint32_t access_mask, char **pp_parent_dir) { @@ -212,7 +212,7 @@ NTSTATUS check_parent_access(struct connection_struct *conn, uint32_t access_granted = 0; if (!parent_dirname(talloc_tos(), - path, + smb_fname->base_name, &parent_dir, NULL)) { return NT_STATUS_NO_MEMORY; @@ -241,7 +241,7 @@ NTSTATUS check_parent_access(struct connection_struct *conn, "on directory %s for " "path %s for mask 0x%x returned (0x%x) %s\n", parent_dir, - path, + smb_fname->base_name, access_mask, access_granted, nt_errstr(status) )); @@ -617,7 +617,7 @@ static NTSTATUS open_file(files_struct *fsp, access_mask); } else if (local_flags & O_CREAT){ status = check_parent_access(conn, - smb_fname->base_name, + smb_fname, SEC_DIR_ADD_FILE, NULL); } else { @@ -2566,7 +2566,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn, } status = check_parent_access(conn, - smb_dname->base_name, + smb_dname, access_mask, &parent_dir); if(!NT_STATUS_IS_OK(status)) { diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index e527fde..02b5e40 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -579,10 +579,6 @@ NTSTATUS smb1_file_se_access_check(connection_struct *conn, const struct security_token *token, uint32_t access_desired, uint32_t *access_granted); -NTSTATUS check_parent_access(struct connection_struct *conn, - const char *path, - uint32_t access_mask, - char **pp_parent_dir); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, -- 1.7.3.1 From 7b57e062cbc8d50682d7609a0ee1a4538109231b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 13:11:01 -0700 Subject: [PATCH 21/40] smb1_file_se_access_check() is now static to smbd/open.c (cherry picked from commit 07edf6c65e514064f15ef0b31b5a98250568a505) --- source3/smbd/open.c | 2 +- source3/smbd/proto.h | 5 ----- 2 files changed, 1 insertions(+), 6 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index de1b667..28bdc4f 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -41,7 +41,7 @@ struct deferred_open_record { SMB1 file varient of se_access_check. Never test FILE_READ_ATTRIBUTES. ****************************************************************************/ -NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, +static NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, const struct security_descriptor *sd, const struct security_token *token, uint32_t access_desired, diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 02b5e40..e7e6fee 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -574,11 +574,6 @@ void reply_nttranss(struct smb_request *req); /* The following definitions come from smbd/open.c */ -NTSTATUS smb1_file_se_access_check(connection_struct *conn, - const struct security_descriptor *sd, - const struct security_token *token, - uint32_t access_desired, - uint32_t *access_granted); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, -- 1.7.3.1 From 512f027892ca79ffa04ac810995233810146e672 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 14:07:23 -0700 Subject: [PATCH 22/40] Move root check out of smb1_file_se_access_check() in preparation for deleting this function. (cherry picked from commit 55b9ba79f8c612d6413e8e673b39dd4e0548dc82) --- source3/smbd/open.c | 38 +++++++++++++++++++++++++------------- 1 files changed, 25 insertions(+), 13 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 28bdc4f..e8dc811 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -49,15 +49,6 @@ static NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, { *access_granted = 0; - if (get_current_uid(conn) == (uid_t)0) { - /* I'm sorry sir, I didn't know you were root... */ - *access_granted = access_desired; - if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) { - *access_granted |= FILE_GENERIC_ALL; - } - return NT_STATUS_OK; - } - return se_access_check(sd, token, (access_desired & ~FILE_READ_ATTRIBUTES), @@ -108,6 +99,15 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, return NT_STATUS_ACCESS_DENIED; } + if (get_current_uid(conn) == (uid_t)0) { + /* I'm sorry sir, I didn't know you were root... */ + DEBUG(10,("smbd_check_open_rights: root override " + "on %s. Granting 0x%x\n", + smb_fname_str_dbg(smb_fname), + (unsigned int)access_mask )); + return NT_STATUS_OK; + } + if ((access_mask & DELETE_ACCESS) && !lp_acl_check_permissions(SNUM(conn))) { DEBUG(10,("smbd_check_open_rights: not checking ACL " "on DELETE_ACCESS on file %s. Granting 0x%x\n", @@ -218,6 +218,19 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, return NT_STATUS_NO_MEMORY; } + if (pp_parent_dir) { + *pp_parent_dir = parent_dir; + } + + if (get_current_uid(conn) == (uid_t)0) { + /* I'm sorry sir, I didn't know you were root... */ + DEBUG(10,("check_parent_access: root override " + "on %s. Granting 0x%x\n", + smb_fname_str_dbg(smb_fname), + (unsigned int)access_mask )); + return NT_STATUS_OK; + } + status = SMB_VFS_GET_NT_ACL(conn, parent_dir, SECINFO_DACL, @@ -248,9 +261,6 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, return status; } - if (pp_parent_dir) { - *pp_parent_dir = parent_dir; - } return NT_STATUS_OK; } @@ -1473,7 +1483,9 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, /* Calculate MAXIMUM_ALLOWED_ACCESS if requested. */ if (access_mask & MAXIMUM_ALLOWED_ACCESS) { - if (file_existed) { + if (get_current_uid(conn) == (uid_t)0) { + access_mask |= FILE_GENERIC_ALL; + } else if (file_existed) { struct security_descriptor *sd; uint32_t access_granted = 0; -- 1.7.3.1 From ff46364cd62fa6d29280deef7235b59c496d9b30 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 14:21:35 -0700 Subject: [PATCH 23/40] Replace smb1_file_se_access_check() with just se_access_check(). (cherry picked from commit 0c886eeb89e8a0ba85788ef87002c6853d6798e6) --- source3/smbd/open.c | 51 +++++++++++++++++++++------------------------------ 1 files changed, 21 insertions(+), 30 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index e8dc811..920ae33 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -38,24 +38,6 @@ struct deferred_open_record { }; /**************************************************************************** - SMB1 file varient of se_access_check. Never test FILE_READ_ATTRIBUTES. -****************************************************************************/ - -static NTSTATUS smb1_file_se_access_check(struct connection_struct *conn, - const struct security_descriptor *sd, - const struct security_token *token, - uint32_t access_desired, - uint32_t *access_granted) -{ - *access_granted = 0; - - return se_access_check(sd, - token, - (access_desired & ~FILE_READ_ATTRIBUTES), - access_granted); -} - -/**************************************************************************** If the requester wanted DELETE_ACCESS and was rejected because the file ACL didn't include DELETE_ACCESS, see if the parent ACL ovverrides this. @@ -129,10 +111,13 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, return status; } - status = smb1_file_se_access_check(conn, - sd, + /* + * Never test FILE_READ_ATTRIBUTES. se_access_check() also takes care of + * owner WRITE_DAC and READ_CONTROL. + */ + status = se_access_check(sd, get_current_nttok(conn), - access_mask, + (access_mask & ~FILE_READ_ATTRIBUTES), &rejected_mask); DEBUG(10,("smbd_check_open_rights: file %s requesting " @@ -244,11 +229,14 @@ static NTSTATUS check_parent_access(struct connection_struct *conn, return status; } - status = smb1_file_se_access_check(conn, - parent_sd, - get_current_nttok(conn), - access_mask, - &access_granted); + /* + * Never test FILE_READ_ATTRIBUTES. se_access_check() also takes care of + * owner WRITE_DAC and READ_CONTROL. + */ + status = se_access_check(parent_sd, + get_current_nttok(conn), + (access_mask & ~FILE_READ_ATTRIBUTES), + &access_granted); if(!NT_STATUS_IS_OK(status)) { DEBUG(5,("check_parent_access: access check " "on directory %s for " @@ -1503,10 +1491,13 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, return NT_STATUS_ACCESS_DENIED; } - status = smb1_file_se_access_check(conn, - sd, + /* + * Never test FILE_READ_ATTRIBUTES. se_access_check() + * also takes care of owner WRITE_DAC and READ_CONTROL. + */ + status = se_access_check(sd, get_current_nttok(conn), - access_mask, + (access_mask & ~FILE_READ_ATTRIBUTES), &access_granted); TALLOC_FREE(sd); @@ -1519,7 +1510,7 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, return NT_STATUS_ACCESS_DENIED; } - access_mask = access_granted; + access_mask = (access_granted | FILE_READ_ATTRIBUTES); } else { access_mask = FILE_GENERIC_ALL; } -- 1.7.3.1 From b5f9cb7e06eac291efef0150b9ca8f96d9419561 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 14:28:08 -0700 Subject: [PATCH 24/40] Rename smbd_check_open_rights() to smbd_check_access_rights() as we're going to remove the static from this. (cherry picked from commit 32edc1d047480a0bcf92c9a172ec8161748f3d74) --- source3/smbd/open.c | 30 +++++++++++++++--------------- 1 files changed, 15 insertions(+), 15 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 920ae33..6572763 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -60,7 +60,7 @@ static bool parent_override_delete(connection_struct *conn, Check if we have open rights. ****************************************************************************/ -static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, +static NTSTATUS smbd_check_access_rights(struct connection_struct *conn, struct smb_filename *smb_fname, uint32_t access_mask) { @@ -73,7 +73,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, rejected_share_access = access_mask & ~(conn->share_access); if (rejected_share_access) { - DEBUG(10, ("smbd_check_open_rights: rejected share access 0x%x " + DEBUG(10, ("smbd_check_access_rights: rejected share access 0x%x " "on %s (0x%x)\n", (unsigned int)access_mask, smb_fname_str_dbg(smb_fname), @@ -83,7 +83,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, if (get_current_uid(conn) == (uid_t)0) { /* I'm sorry sir, I didn't know you were root... */ - DEBUG(10,("smbd_check_open_rights: root override " + DEBUG(10,("smbd_check_access_rights: root override " "on %s. Granting 0x%x\n", smb_fname_str_dbg(smb_fname), (unsigned int)access_mask )); @@ -91,7 +91,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, } if ((access_mask & DELETE_ACCESS) && !lp_acl_check_permissions(SNUM(conn))) { - DEBUG(10,("smbd_check_open_rights: not checking ACL " + DEBUG(10,("smbd_check_access_rights: not checking ACL " "on DELETE_ACCESS on file %s. Granting 0x%x\n", smb_fname_str_dbg(smb_fname), (unsigned int)access_mask )); @@ -104,7 +104,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, SECINFO_DACL),&sd); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("smbd_check_open_rights: Could not get acl " + DEBUG(10, ("smbd_check_access_rights: Could not get acl " "on %s: %s\n", smb_fname_str_dbg(smb_fname), nt_errstr(status))); @@ -120,7 +120,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, (access_mask & ~FILE_READ_ATTRIBUTES), &rejected_mask); - DEBUG(10,("smbd_check_open_rights: file %s requesting " + DEBUG(10,("smbd_check_access_rights: file %s requesting " "0x%x returning 0x%x (%s)\n", smb_fname_str_dbg(smb_fname), (unsigned int)access_mask, @@ -129,7 +129,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, if (!NT_STATUS_IS_OK(status)) { if (DEBUGLEVEL >= 10) { - DEBUG(10,("smbd_check_open_rights: acl for %s is:\n", + DEBUG(10,("smbd_check_access_rights: acl for %s is:\n", smb_fname_str_dbg(smb_fname) )); NDR_PRINT_DEBUG(security_descriptor, sd); } @@ -151,7 +151,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, lp_map_system(SNUM(conn)))) { rejected_mask &= ~FILE_WRITE_ATTRIBUTES; - DEBUG(10,("smbd_check_open_rights: " + DEBUG(10,("smbd_check_access_rights: " "overrode " "FILE_WRITE_ATTRIBUTES " "on file %s\n", @@ -172,7 +172,7 @@ static NTSTATUS smbd_check_open_rights(struct connection_struct *conn, rejected_mask &= ~DELETE_ACCESS; - DEBUG(10,("smbd_check_open_rights: " + DEBUG(10,("smbd_check_access_rights: " "overrode " "DELETE_ACCESS on " "file %s\n", @@ -610,7 +610,7 @@ static NTSTATUS open_file(files_struct *fsp, if (!fsp->base_fsp) { /* Only do this check on non-stream open. */ if (file_existed) { - status = smbd_check_open_rights(conn, + status = smbd_check_access_rights(conn, smb_fname, access_mask); } else if (local_flags & O_CREAT){ @@ -627,7 +627,7 @@ static NTSTATUS open_file(files_struct *fsp, "%s on file " "%s returned %s\n", file_existed ? - "smbd_check_open_rights" : + "smbd_check_access_rights" : "check_parent_access", smb_fname_str_dbg(smb_fname), nt_errstr(status) )); @@ -655,7 +655,7 @@ static NTSTATUS open_file(files_struct *fsp, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - status = smbd_check_open_rights(conn, + status = smbd_check_access_rights(conn, smb_fname, access_mask); @@ -673,7 +673,7 @@ static NTSTATUS open_file(files_struct *fsp, if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("open_file: " - "smbd_check_open_rights on file " + "smbd_check_access_rights on file " "%s returned %s\n", smb_fname_str_dbg(smb_fname), nt_errstr(status) )); @@ -2819,9 +2819,9 @@ static NTSTATUS open_directory(connection_struct *conn, } if (info == FILE_WAS_OPENED) { - status = smbd_check_open_rights(conn, smb_dname, access_mask); + status = smbd_check_access_rights(conn, smb_dname, access_mask); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("open_directory: smbd_check_open_rights on " + DEBUG(10, ("open_directory: smbd_check_access_rights on " "file %s failed with %s\n", smb_fname_str_dbg(smb_dname), nt_errstr(status))); -- 1.7.3.1 From b0c9cf19e9671ee67256ec145cee70b8fbfce946 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 14:37:26 -0700 Subject: [PATCH 25/40] Expose smbd_check_access_rights() to other modules. (cherry picked from commit a30f84a21c9d4e702ae0faace9bdf435b9882af7) --- source3/smbd/open.c | 2 +- source3/smbd/proto.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 6572763..ef5f2ac 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -60,7 +60,7 @@ static bool parent_override_delete(connection_struct *conn, Check if we have open rights. ****************************************************************************/ -static NTSTATUS smbd_check_access_rights(struct connection_struct *conn, +NTSTATUS smbd_check_access_rights(struct connection_struct *conn, struct smb_filename *smb_fname, uint32_t access_mask) { diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index e7e6fee..a423e72 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -574,6 +574,9 @@ void reply_nttranss(struct smb_request *req); /* The following definitions come from smbd/open.c */ +NTSTATUS smbd_check_access_rights(struct connection_struct *conn, + struct smb_filename *smb_fname, + uint32_t access_mask); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, const char *inherit_from_dir, -- 1.7.3.1 From d9bd2cd4064520afd622cd1a8ee415e3f569712b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 15:39:55 -0700 Subject: [PATCH 26/40] Add const to the smb_filename argument of smbd_check_access_rights(). (cherry picked from commit 48512193338663df5dc4cd52179bc94337eb7113) --- source3/smbd/file_access.c | 18 +++++------------- source3/smbd/open.c | 4 ++-- source3/smbd/proto.h | 4 ++-- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c index 7485564..c220367 100644 --- a/source3/smbd/file_access.c +++ b/source3/smbd/file_access.c @@ -80,7 +80,7 @@ bool can_access_file_acl(struct connection_struct *conn, ****************************************************************************/ bool can_delete_file_in_directory(connection_struct *conn, - struct smb_filename *smb_fname) + const struct smb_filename *smb_fname) { TALLOC_CTX *ctx = talloc_tos(); char *dname = NULL; @@ -130,18 +130,10 @@ bool can_delete_file_in_directory(connection_struct *conn, /* sticky bit means delete only by owner of file or by root or * by owner of directory. */ if (smb_fname_parent->st.st_ex_mode & S_ISVTX) { - if(SMB_VFS_STAT(conn, smb_fname) != 0) { - if (errno == ENOENT) { - /* If the file doesn't already exist then - * yes we'll be able to delete it. */ - ret = true; - goto out; - } - DEBUG(10,("can_delete_file_in_directory: can't " - "stat file %s (%s)", - smb_fname_str_dbg(smb_fname), - strerror(errno) )); - ret = false; + if (!VALID_STAT(smb_fname->st)) { + /* If the file doesn't already exist then + * yes we'll be able to delete it. */ + ret = true; goto out; } diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ef5f2ac..f7854da 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -44,7 +44,7 @@ struct deferred_open_record { ****************************************************************************/ static bool parent_override_delete(connection_struct *conn, - struct smb_filename *smb_fname, + const struct smb_filename *smb_fname, uint32_t access_mask, uint32_t rejected_mask) { @@ -61,7 +61,7 @@ static bool parent_override_delete(connection_struct *conn, ****************************************************************************/ NTSTATUS smbd_check_access_rights(struct connection_struct *conn, - struct smb_filename *smb_fname, + const struct smb_filename *smb_fname, uint32_t access_mask) { /* Check if we have rights to open. */ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index a423e72..8da5393 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -301,7 +301,7 @@ bool can_access_file_acl(struct connection_struct *conn, const struct smb_filename *smb_fname, uint32_t access_mask); bool can_delete_file_in_directory(connection_struct *conn, - struct smb_filename *smb_fname); + const struct smb_filename *smb_fname); bool can_access_file_data(connection_struct *conn, const struct smb_filename *smb_fname, uint32 access_mask); @@ -575,7 +575,7 @@ void reply_nttranss(struct smb_request *req); /* The following definitions come from smbd/open.c */ NTSTATUS smbd_check_access_rights(struct connection_struct *conn, - struct smb_filename *smb_fname, + const struct smb_filename *smb_fname, uint32_t access_mask); NTSTATUS fd_close(files_struct *fsp); void change_file_owner_to_parent(connection_struct *conn, -- 1.7.3.1 From 97149236f4b24bca104e7bce17c0b5c6e3b96eca Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 15:45:13 -0700 Subject: [PATCH 27/40] Remove can_access_file_data() - make it use the standard smbd_check_access_rights() instead. (cherry picked from commit 60b741415d9357f11ae1db80f93035058fdbe4e8) --- source3/modules/onefs_open.c | 4 +- source3/smbd/file_access.c | 59 ++--------------------------------------- source3/smbd/open.c | 4 +- 3 files changed, 7 insertions(+), 60 deletions(-) diff --git a/source3/modules/onefs_open.c b/source3/modules/onefs_open.c index 101dc5b..d1413de 100644 --- a/source3/modules/onefs_open.c +++ b/source3/modules/onefs_open.c @@ -1042,8 +1042,8 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn, if (((can_access_mask & FILE_WRITE_DATA) && !CAN_WRITE(conn)) || - !can_access_file_data(conn, smb_fname, - can_access_mask)) { + !NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname, can_access_mask))) { can_access = False; } diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c index c220367..81cb7bd 100644 --- a/source3/smbd/file_access.c +++ b/source3/smbd/file_access.c @@ -177,61 +177,6 @@ bool can_delete_file_in_directory(connection_struct *conn, } /**************************************************************************** - Actually emulate the in-kernel access checking for read/write access. We need - this to successfully check for ability to write for dos filetimes. - Note this doesn't take into account share write permissions. -****************************************************************************/ - -bool can_access_file_data(connection_struct *conn, - const struct smb_filename *smb_fname, - uint32 access_mask) -{ - if (!(access_mask & (FILE_READ_DATA|FILE_WRITE_DATA))) { - return False; - } - access_mask &= (FILE_READ_DATA|FILE_WRITE_DATA); - - /* some fast paths first */ - - DEBUG(10,("can_access_file_data: requesting 0x%x on file %s\n", - (unsigned int)access_mask, smb_fname_str_dbg(smb_fname))); - - if (get_current_uid(conn) == (uid_t)0) { - /* I'm sorry sir, I didn't know you were root... */ - return True; - } - - SMB_ASSERT(VALID_STAT(smb_fname->st)); - - /* Check primary owner access. */ - if (get_current_uid(conn) == smb_fname->st.st_ex_uid) { - switch (access_mask) { - case FILE_READ_DATA: - return (smb_fname->st.st_ex_mode & S_IRUSR) ? - True : False; - - case FILE_WRITE_DATA: - return (smb_fname->st.st_ex_mode & S_IWUSR) ? - True : False; - - default: /* FILE_READ_DATA|FILE_WRITE_DATA */ - - if ((smb_fname->st.st_ex_mode & - (S_IWUSR|S_IRUSR)) == - (S_IWUSR|S_IRUSR)) { - return True; - } else { - return False; - } - } - } - - /* now for ACL checks */ - - return can_access_file_acl(conn, smb_fname, access_mask); -} - -/**************************************************************************** Userspace check for write access. Note this doesn't take into account share write permissions. ****************************************************************************/ @@ -239,7 +184,9 @@ bool can_access_file_data(connection_struct *conn, bool can_write_to_file(connection_struct *conn, const struct smb_filename *smb_fname) { - return can_access_file_data(conn, smb_fname, FILE_WRITE_DATA); + return NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname, + FILE_WRITE_DATA)); } /**************************************************************************** diff --git a/source3/smbd/open.c b/source3/smbd/open.c index f7854da..3987696 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -2080,8 +2080,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, if (((can_access_mask & FILE_WRITE_DATA) && !CAN_WRITE(conn)) || - !can_access_file_data(conn, smb_fname, - can_access_mask)) { + !NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname, can_access_mask))) { can_access = False; } -- 1.7.3.1 From 946862ccd9e71f0a38358db6ce799210e6a8657a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 15:55:11 -0700 Subject: [PATCH 28/40] Remove can_access_file_acl(). We no longer need this duplicate code (hurrah!). (cherry picked from commit b988a3233fae309c6f034724ae106d935b1489f7) --- source3/smbd/dir.c | 4 ++- source3/smbd/file_access.c | 51 ++----------------------------------------- source3/smbd/open.c | 4 ++- source3/smbd/proto.h | 6 ----- 4 files changed, 9 insertions(+), 56 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 670b34a..81d23dc 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1178,7 +1178,9 @@ static bool user_can_read_file(connection_struct *conn, return True; } - return can_access_file_acl(conn, smb_fname, FILE_READ_DATA); + return NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname, + FILE_READ_DATA)); } /******************************************************************* diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c index 81cb7bd..ae13a0a 100644 --- a/source3/smbd/file_access.c +++ b/source3/smbd/file_access.c @@ -27,53 +27,6 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_ACLS -/** - * Security descriptor / NT Token level access check function. - */ -bool can_access_file_acl(struct connection_struct *conn, - const struct smb_filename *smb_fname, - uint32_t access_mask) -{ - NTSTATUS status; - uint32_t access_granted; - struct security_descriptor *secdesc = NULL; - bool ret; - - if (get_current_uid(conn) == (uid_t)0) { - /* I'm sorry sir, I didn't know you were root... */ - return true; - } - - status = SMB_VFS_GET_NT_ACL(conn, smb_fname->base_name, - (SECINFO_OWNER | - SECINFO_GROUP | - SECINFO_DACL), - &secdesc); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(5, ("Could not get acl: %s\n", nt_errstr(status))); - ret = false; - goto out; - } - - status = se_access_check(secdesc, get_current_nttok(conn), - access_mask, &access_granted); - ret = NT_STATUS_IS_OK(status); - - if (DEBUGLEVEL >= 10) { - DEBUG(10,("can_access_file_acl for file %s " - "access_mask 0x%x, access_granted 0x%x " - "access %s\n", - smb_fname_str_dbg(smb_fname), - (unsigned int)access_mask, - (unsigned int)access_granted, - ret ? "ALLOWED" : "DENIED" )); - NDR_PRINT_DEBUG(security_descriptor, secdesc); - } - out: - TALLOC_FREE(secdesc); - return ret; -} - /**************************************************************************** Actually emulate the in-kernel access checking for delete access. We need this to successfully return ACCESS_DENIED on a file open for delete access. @@ -169,7 +122,9 @@ bool can_delete_file_in_directory(connection_struct *conn, * check the file DELETE permission separately. */ - ret = can_access_file_acl(conn, smb_fname_parent, FILE_DELETE_CHILD); + ret = NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname_parent, + FILE_DELETE_CHILD)); out: TALLOC_FREE(dname); TALLOC_FREE(smb_fname_parent); diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 3987696..ceabaae 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3268,7 +3268,9 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, if ((create_disposition != FILE_CREATE) && (access_mask & DELETE_ACCESS) && (!(can_delete_file_in_directory(conn, smb_fname) || - can_access_file_acl(conn, smb_fname, DELETE_ACCESS)))) { + NT_STATUS_IS_OK(smbd_check_access_rights(conn, + smb_fname, + DELETE_ACCESS))))) { status = NT_STATUS_ACCESS_DENIED; DEBUG(10,("create_file_unixpath: open file %s " "for delete ACCESS_DENIED\n", diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 8da5393..a594e03 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -297,14 +297,8 @@ void reply_openerror(struct smb_request *req, NTSTATUS status); /* The following definitions come from smbd/file_access.c */ -bool can_access_file_acl(struct connection_struct *conn, - const struct smb_filename *smb_fname, - uint32_t access_mask); bool can_delete_file_in_directory(connection_struct *conn, const struct smb_filename *smb_fname); -bool can_access_file_data(connection_struct *conn, - const struct smb_filename *smb_fname, - uint32 access_mask); bool can_write_to_file(connection_struct *conn, const struct smb_filename *smb_fname); bool directory_has_default_acl(connection_struct *conn, const char *fname); -- 1.7.3.1 From 16abd2919e6237757b8066514f69eddb79b891f7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 15:56:15 -0700 Subject: [PATCH 29/40] No longer do the pre-check on DELETE_ACCESS - we're correctly checking the ACL every time now. (cherry picked from commit bbcb589ef5a9cf1ad98b70fc2ea00d346323c57e) --- source3/smbd/open.c | 22 ---------------------- 1 files changed, 0 insertions(+), 22 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index ceabaae..cf44cc2 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3256,28 +3256,6 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, } } - /* This is the correct thing to do (check every time) but can_delete - * is expensive (it may have to read the parent directory - * permissions). So for now we're not doing it unless we have a strong - * hint the client is really going to delete this file. If the client - * is forcing FILE_CREATE let the filesystem take care of the - * permissions. */ - - /* Setting FILE_SHARE_DELETE is the hint. */ - - if ((create_disposition != FILE_CREATE) - && (access_mask & DELETE_ACCESS) - && (!(can_delete_file_in_directory(conn, smb_fname) || - NT_STATUS_IS_OK(smbd_check_access_rights(conn, - smb_fname, - DELETE_ACCESS))))) { - status = NT_STATUS_ACCESS_DENIED; - DEBUG(10,("create_file_unixpath: open file %s " - "for delete ACCESS_DENIED\n", - smb_fname_str_dbg(smb_fname))); - goto fail; - } - if ((access_mask & SEC_FLAG_SYSTEM_SECURITY) && !security_token_has_privilege(get_current_nttok(conn), SEC_PRIV_SECURITY)) { -- 1.7.3.1 From 664cb094f82423ec62d6925d2516d93c5ceff037 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 16:01:16 -0700 Subject: [PATCH 30/40] can_write_to_file() does now take share permissions into account. Fix comment. (cherry picked from commit 7ff5a5584f7d299be1c46bd881adf3ebc27a2a20) --- source3/smbd/file_access.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c index ae13a0a..4a473d7 100644 --- a/source3/smbd/file_access.c +++ b/source3/smbd/file_access.c @@ -133,7 +133,6 @@ bool can_delete_file_in_directory(connection_struct *conn, /**************************************************************************** Userspace check for write access. - Note this doesn't take into account share write permissions. ****************************************************************************/ bool can_write_to_file(connection_struct *conn, -- 1.7.3.1 From 8ca5a8d680dcdb04ad2e2e77117a9c2affbdaf95 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 10:51:29 -0700 Subject: [PATCH 31/40] Move the SEC_DIR_LIST check into dptr_create for SMB2 and now for SMB1. The pathname check still needs fixing. Autobuild-User: Jeremy Allison Autobuild-Date: Sat Nov 5 01:38:00 CET 2011 on sn-devel-104 (cherry picked from commit 28984858486b9e61142afd3d34ae1e822e0ccef1) --- source3/smbd/dir.c | 6 ++++++ source3/smbd/smb2_find.c | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 81d23dc..db4a48d 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -447,6 +447,12 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, } if (fsp) { + if (!(fsp->access_mask & SEC_DIR_LIST)) { + DEBUG(5,("dptr_create: directory %s " + "not open for LIST access\n", + path)); + return NT_STATUS_ACCESS_DENIED; + } dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr); } else { dir_hnd = OpenDir(NULL, conn, path, wcard, attr); diff --git a/source3/smbd/smb2_find.c b/source3/smbd/smb2_find.c index 3dcc768..ee152a6 100644 --- a/source3/smbd/smb2_find.c +++ b/source3/smbd/smb2_find.c @@ -323,11 +323,6 @@ static struct tevent_req *smbd_smb2_find_send(TALLOC_CTX *mem_ctx, if (fsp->dptr == NULL) { bool wcard_has_wild; - if (!(fsp->access_mask & SEC_DIR_LIST)) { - tevent_req_nterror(req, NT_STATUS_ACCESS_DENIED); - return tevent_req_post(req, ev); - } - wcard_has_wild = ms_has_wild(in_file_name); status = dptr_create(conn, -- 1.7.3.1 From 5f1f2903b3fd2eefd6641aad35785a9b01accf8e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Nov 2011 16:46:47 -0700 Subject: [PATCH 32/40] Add the SEC_DIR_LIST check to dptr_create(). Autobuild-User: Jeremy Allison Autobuild-Date: Mon Nov 7 21:11:03 CET 2011 on sn-devel-104 (cherry picked from commit 60b7dae3fad482c2dabd6c0569e99cd23838d824) --- source3/smbd/dir.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index db4a48d..d3d666d 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -455,6 +455,33 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct *fsp, } dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr); } else { + int ret; + struct smb_filename *smb_dname = NULL; + NTSTATUS status = create_synthetic_smb_fname(talloc_tos(), + path, + NULL, + NULL, + &smb_dname); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + if (lp_posix_pathnames()) { + ret = SMB_VFS_LSTAT(conn, smb_dname); + } else { + ret = SMB_VFS_STAT(conn, smb_dname); + } + if (ret == -1) { + return map_nt_error_from_unix(errno); + } + if (!S_ISDIR(smb_dname->st.st_ex_mode)) { + return NT_STATUS_NOT_A_DIRECTORY; + } + status = smbd_check_access_rights(conn, + smb_dname, + SEC_DIR_LIST); + if (!NT_STATUS_IS_OK(status)) { + return status; + } dir_hnd = OpenDir(NULL, conn, path, wcard, attr); } -- 1.7.3.1 From 5344150ba21b81c04dcde20b57ac069c7feef822 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:14:16 -0800 Subject: [PATCH 33/40] Move handle based access check into handle code path. (cherry picked from commit c27551b1631eff0c91d79fbcadce703eadc3efc1) --- source3/smbd/trans2.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 129ab01..28607b9 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6626,16 +6626,16 @@ static NTSTATUS smb_set_file_allocation_info(connection_struct *conn, allocation_size = smb_roundup(conn, allocation_size); } - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - DEBUG(10,("smb_set_file_allocation_info: file %s : setting new " "allocation size to %.0f\n", smb_fname_str_dbg(smb_fname), (double)allocation_size)); if (fsp && fsp->fh->fd != -1) { /* Open file handle. */ + if (!(fsp->access_mask & FILE_WRITE_DATA)) { + return NT_STATUS_ACCESS_DENIED; + } + /* Only change if needed. */ if (allocation_size != get_file_size_stat(&smb_fname->st)) { if (vfs_allocate_file_space(fsp, allocation_size) == -1) { -- 1.7.3.1 From c5f2a67f893dd13f8e1a8aa93391377748d8feff Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:14:47 -0800 Subject: [PATCH 34/40] Remove unneeded access check. This is done inside smb_set_file_size(). (cherry picked from commit f5cda7160ce63abbde6878ca517680f417ffeb17) --- source3/smbd/trans2.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 28607b9..2dd78a9 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6727,10 +6727,6 @@ static NTSTATUS smb_set_file_end_of_file_info(connection_struct *conn, "file %s to %.0f\n", smb_fname_str_dbg(smb_fname), (double)size)); - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - return smb_set_file_size(conn, req, fsp, smb_fname, -- 1.7.3.1 From 909ffb553b84e47f090bf3a5491e7c3fb7d08b73 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:16:54 -0800 Subject: [PATCH 35/40] Remove unneeded access check. This is done inside smb_set_file_time(). (cherry picked from commit 93000c98ad42a2e089ba6b371d811263f953c23d) --- source3/smbd/trans2.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 2dd78a9..13c19f5 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -6564,10 +6564,6 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; - } - /* create time */ ft.create_time = convert_time_t_to_timespec(srv_make_unix_date2(pdata)); /* access time */ -- 1.7.3.1 From a713cf0d84bb4911de6c7fc5e0ed706d3b6f50a0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:20:44 -0800 Subject: [PATCH 36/40] We've already checked fsp must be non-null here. (cherry picked from commit c6a62f60a23fdadb99da4d4b47694b273342f2a7) --- source3/smbd/trans2.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 13c19f5..395f10f 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -5773,7 +5773,7 @@ static NTSTATUS smb_set_file_full_ea_info(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { + if (!(fsp->access_mask & FILE_WRITE_EA)) { return NT_STATUS_ACCESS_DENIED; } -- 1.7.3.1 From b350a04fef0c78a77defa833c792cd152c3f6dc7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:22:09 -0800 Subject: [PATCH 37/40] Move handle-based access check into handle codepath. (cherry picked from commit edaa7479edd9c6472dacb3642fe6d2a6869e4719) --- source3/smbd/trans2.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 395f10f..14773d7 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -5615,10 +5615,6 @@ static NTSTATUS smb_set_file_size(connection_struct *conn, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - DEBUG(6,("smb_set_file_size: size: %.0f ", (double)size)); if (size == get_file_size_stat(psbuf)) { @@ -5630,6 +5626,10 @@ static NTSTATUS smb_set_file_size(connection_struct *conn, if (fsp && fsp->fh->fd != -1) { /* Handle based call. */ + if (!(fsp->access_mask & FILE_WRITE_DATA)) { + return NT_STATUS_ACCESS_DENIED; + } + if (vfs_set_filelen(fsp, size) == -1) { return map_nt_error_from_unix(errno); } -- 1.7.3.1 From 9044dc3dbdbe76f63165663dc4f7bed98ca9059f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 16:49:42 -0800 Subject: [PATCH 38/40] Always set the attribute first, before the time. (cherry picked from commit 86c16092194836d8478144b97da9ca08aec7fac6) --- source3/smbd/reply.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index ac450bb..f8e629d 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1246,13 +1246,6 @@ void reply_setatr(struct smb_request *req) mode = SVAL(req->vwv+0, 0); mtime = srv_make_unix_date3(req->vwv+1); - ft.mtime = convert_time_t_to_timespec(mtime); - status = smb_set_file_time(conn, NULL, smb_fname, &ft, true); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - if (mode != FILE_ATTRIBUTE_NORMAL) { if (VALID_STAT_OF_DIR(smb_fname->st)) mode |= FILE_ATTRIBUTE_DIRECTORY; @@ -1266,6 +1259,13 @@ void reply_setatr(struct smb_request *req) } } + ft.mtime = convert_time_t_to_timespec(mtime); + status = smb_set_file_time(conn, NULL, smb_fname, &ft, true); + if (!NT_STATUS_IS_OK(status)) { + reply_nterror(req, status); + goto out; + } + reply_outbuf(req, 0, 0); DEBUG(3, ("setatr name=%s mode=%d\n", smb_fname_str_dbg(smb_fname), -- 1.7.3.1 From d421a062f4ad55933c156083fc4eeb54c3aa3cc3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 17:41:48 -0800 Subject: [PATCH 39/40] Remove the check for FILE_WRITE_ATTRIBUTES from smb_set_file_time(). It is called from places like fileio.c that need to update the write time on a file handle only open for write, without neccessarily having FILE_WRITE_ATTRIBUTES permission. Move all checks to before the smb_set_file_time() callers. (cherry picked from commit 865bc0c0ace0a4f8e5eb0277def2315867273071) --- source3/smbd/close.c | 6 ++---- source3/smbd/reply.c | 5 +++++ source3/smbd/trans2.c | 14 ++++++++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/source3/smbd/close.c b/source3/smbd/close.c index 25c8b46..3f1acf8 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -576,11 +576,9 @@ static NTSTATUS update_write_time_on_close(struct files_struct *fsp) } ft.mtime = fsp->close_write_time; - /* We must use NULL for the fsp handle here, as smb_set_file_time() - checks the fsp access_mask, which may not include FILE_WRITE_ATTRIBUTES. - As this is a close based update, we are not directly changing the + /* As this is a close based update, we are not directly changing the file attributes from a client call, but indirectly from a write. */ - status = smb_set_file_time(fsp->conn, NULL, fsp->fsp_name, &ft, false); + status = smb_set_file_time(fsp->conn, fsp, fsp->fsp_name, &ft, false); if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("update_write_time_on_close: smb_set_file_time " "on file %s returned %s\n", diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index f8e629d..4c5bb76 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -7919,6 +7919,11 @@ void reply_setattrE(struct smb_request *req) goto out; } + if (!(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { + reply_nterror(req, NT_STATUS_ACCESS_DENIED); + goto out; + } + status = smb_set_file_time(conn, fsp, fsp->fsp_name, &ft, true); if (!NT_STATUS_IS_OK(status)) { reply_nterror(req, status); diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 14773d7..805266a 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -5440,6 +5440,8 @@ NTSTATUS hardlink_internals(TALLOC_CTX *ctx, /**************************************************************************** Deal with setting the time from any of the setfilepathinfo functions. + NOTE !!!! The check for FILE_WRITE_ATTRIBUTES access must be done *before* + calling this function. ****************************************************************************/ NTSTATUS smb_set_file_time(connection_struct *conn, @@ -5458,10 +5460,6 @@ NTSTATUS smb_set_file_time(connection_struct *conn, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; - } - /* get some defaults (no modifications) if any info is zero or -1. */ if (null_timespec(ft->create_time)) { action &= ~FILE_NOTIFY_CHANGE_CREATION; @@ -6574,6 +6572,10 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, DEBUG(10,("smb_set_info_standard: file %s\n", smb_fname_str_dbg(smb_fname))); + if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { + return NT_STATUS_ACCESS_DENIED; + } + return smb_set_file_time(conn, fsp, smb_fname, @@ -6944,6 +6946,10 @@ static NTSTATUS smb_set_file_unix_basic(connection_struct *conn, } #endif + if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { + return NT_STATUS_ACCESS_DENIED; + } + /* * Deal with the UNIX specific mode set. */ -- 1.7.3.1 From f9f0b177059e8eec1930b55c5d59f1bbe00f74f5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 15 Nov 2011 17:29:59 -0800 Subject: [PATCH 40/40] Final part of patchset to fix bug #8556 - ACL permissions ignored when SMBsetatr is requested. This now plumbs access checks through all setattr calls. Autobuild-User: Jeremy Allison Autobuild-Date: Wed Nov 16 04:20:04 CET 2011 on sn-devel-104 (cherry picked from commit 05e841c82ce7f0f252b5eb565e457f406b3a1b39) --- source3/smbd/proto.h | 4 +++ source3/smbd/reply.c | 7 ++++++ source3/smbd/trans2.c | 56 +++++++++++++++++++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index a594e03..ab3d312 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1048,6 +1048,10 @@ int sys_statvfs(const char *path, vfs_statvfs_struct *statbuf); /* The following definitions come from smbd/trans2.c */ +NTSTATUS check_access(connection_struct *conn, + files_struct *fsp, + const struct smb_filename *smb_fname, + uint32_t access_mask); uint64_t smb_roundup(connection_struct *conn, uint64_t val); uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf); NTSTATUS get_ea_value(TALLOC_CTX *mem_ctx, connection_struct *conn, diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 4c5bb76..dbb40b7 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1252,6 +1252,13 @@ void reply_setatr(struct smb_request *req) else mode &= ~FILE_ATTRIBUTE_DIRECTORY; + status = check_access(conn, NULL, smb_fname, + FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + reply_nterror(req, status); + goto out; + } + if (file_set_dosmode(conn, smb_fname, mode, NULL, false) != 0) { reply_nterror(req, map_nt_error_from_unix(errno)); diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 805266a..fef62d4 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -51,6 +51,30 @@ static char *store_file_unix_basic_info2(connection_struct *conn, const SMB_STRUCT_STAT *psbuf); /******************************************************************** + The canonical "check access" based on object handle or path function. +********************************************************************/ + +NTSTATUS check_access(connection_struct *conn, + files_struct *fsp, + const struct smb_filename *smb_fname, + uint32_t access_mask) +{ + if (fsp) { + if (!(fsp->access_mask & access_mask)) { + return NT_STATUS_ACCESS_DENIED; + } + } else { + NTSTATUS status = smbd_check_access_rights(conn, + smb_fname, + access_mask); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + } + return NT_STATUS_OK; +} + +/******************************************************************** Roundup a value to the nearest allocation roundup size boundary. Only do this for Windows clients. ********************************************************************/ @@ -505,14 +529,16 @@ static void canonicalize_ea_name(connection_struct *conn, files_struct *fsp, con NTSTATUS set_ea(connection_struct *conn, files_struct *fsp, const struct smb_filename *smb_fname, struct ea_list *ea_list) { + NTSTATUS status; char *fname = NULL; if (!lp_ea_support(SNUM(conn))) { return NT_STATUS_EAS_NOT_SUPPORTED; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_EA); + if (!NT_STATUS_IS_OK(status)) { + return status; } /* For now setting EAs on streams isn't supported. */ @@ -5540,6 +5566,8 @@ NTSTATUS smb_set_file_time(connection_struct *conn, /**************************************************************************** Deal with setting the dosmode from any of the setfilepathinfo functions. + NB. The check for FILE_WRITE_ATTRIBUTES access on this path must have been + done before calling this function. ****************************************************************************/ static NTSTATUS smb_set_file_dosmode(connection_struct *conn, @@ -5724,10 +5752,6 @@ static NTSTATUS smb_info_set_ea(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; - } - status = set_ea(conn, fsp, smb_fname, ea_list); return status; @@ -5771,10 +5795,6 @@ static NTSTATUS smb_set_file_full_ea_info(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (!(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; - } - status = set_ea(conn, fsp, fsp->fsp_name, ea_list); DEBUG(10, ("smb_set_file_full_ea_info on file %s returned %s\n", @@ -6514,8 +6534,9 @@ static NTSTATUS smb_set_file_basic_info(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; } /* Set the attributes */ @@ -6554,6 +6575,7 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, files_struct *fsp, const struct smb_filename *smb_fname) { + NTSTATUS status; struct smb_file_time ft; ZERO_STRUCT(ft); @@ -6572,8 +6594,9 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, DEBUG(10,("smb_set_info_standard: file %s\n", smb_fname_str_dbg(smb_fname))); - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; } return smb_set_file_time(conn, @@ -6946,8 +6969,9 @@ static NTSTATUS smb_set_file_unix_basic(connection_struct *conn, } #endif - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; } /* -- 1.7.3.1