Second part of the attemt to fix #4308 - Excel save operation corrupts file ACLs.
authorJeremy Allison <jra@samba.org>
Thu, 22 Jan 2009 22:38:57 +0000 (14:38 -0800)
committerJeremy Allison <jra@samba.org>
Thu, 22 Jan 2009 22:38:57 +0000 (14:38 -0800)
If the chown succeeds then the ACL set should also. Ensure this is the case
(refactor some of this code to make it simpler to read also).
Jeremy.

source3/smbd/posix_acls.c

index 951046c56253034882f4121894088a6e632fb1d8..627bfb4634688e6bc94e8dfe05f2584a8511cfa8 100644 (file)
@@ -3426,8 +3426,9 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, const SEC_DESC
        bool acl_perms = False;
        mode_t orig_mode = (mode_t)0;
        NTSTATUS status;
-       uid_t orig_uid;
-       gid_t orig_gid;
+       bool set_acl_as_root = false;
+       bool acl_set_support = false;
+       bool ret = false;
 
        DEBUG(10,("set_nt_acl: called for file %s\n", fsp->fsp_name ));
 
@@ -3448,10 +3449,8 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, const SEC_DESC
                        return map_nt_error_from_unix(errno);
        }
 
-       /* Save the original elements we check against. */
+       /* Save the original element we check against. */
        orig_mode = sbuf.st_mode;
-       orig_uid = sbuf.st_uid;
-       orig_gid = sbuf.st_gid;
 
        /*
         * Unpack the user/group/world id's.
@@ -3468,7 +3467,7 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, const SEC_DESC
         * Noticed by Simo.
         */
 
-       if (((user != (uid_t)-1) && (orig_uid != user)) || (( grp != (gid_t)-1) && (orig_gid != grp))) {
+       if (((user != (uid_t)-1) && (sbuf.st_uid != user)) || (( grp != (gid_t)-1) && (sbuf.st_gid != grp))) {
 
                DEBUG(3,("set_nt_acl: chown %s. uid = %u, gid = %u.\n",
                                fsp->fsp_name, (unsigned int)user, (unsigned int)grp ));
@@ -3493,174 +3492,188 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, const SEC_DESC
                        }
                } else {
 
-                       int ret;
+                       int sret;
 
                        if(fsp->fh->fd == -1)
-                               ret = SMB_VFS_STAT(fsp->conn, fsp->fsp_name, &sbuf);
+                               sret = SMB_VFS_STAT(fsp->conn, fsp->fsp_name, &sbuf);
                        else
-                               ret = SMB_VFS_FSTAT(fsp, &sbuf);
+                               sret = SMB_VFS_FSTAT(fsp, &sbuf);
 
-                       if(ret != 0)
+                       if(sret != 0)
                                return map_nt_error_from_unix(errno);
                }
 
-               /* Save the original elements we check against. */
+               /* Save the original element we check against. */
                orig_mode = sbuf.st_mode;
-               orig_uid = sbuf.st_uid;
-               orig_gid = sbuf.st_gid;
+
+               /* If we successfully chowned, we know we must
+                * be able to set the acl, so do it as root.
+                */
+               set_acl_as_root = true;
        }
 
        create_file_sids(&sbuf, &file_owner_sid, &file_grp_sid);
 
-#if 0
-       /* Disable this - prevents ACL inheritance from the ACL editor. JRA. */
-
-       /* See here: http://www.codeproject.com/KB/winsdk/accessctrl2.aspx
-        * for details and also the log trace in bug #4308. JRA.
-        */
-
-       if ((security_info_sent & DACL_SECURITY_INFORMATION) &&
-               psd->dacl != NULL &&
-               (psd->type & (SE_DESC_DACL_AUTO_INHERITED|
-                             SE_DESC_DACL_AUTO_INHERIT_REQ))==
-                       (SE_DESC_DACL_AUTO_INHERITED|
-                        SE_DESC_DACL_AUTO_INHERIT_REQ) ) {
-               SEC_DESC *new_sd = NULL;
-               status = append_parent_acl(fsp, psd, &new_sd);
-               if (!NT_STATUS_IS_OK(status)) {
-                       return status;
-               }
-               psd = new_sd;
-       }
-#endif
-
        acl_perms = unpack_canon_ace( fsp, &sbuf, &file_owner_sid, &file_grp_sid,
                                        &file_ace_list, &dir_ace_list, security_info_sent, psd);
 
        /* Ignore W2K traverse DACL set. */
-       if (file_ace_list || dir_ace_list) {
+       if (!file_ace_list && !dir_ace_list) {
+               return NT_STATUS_OK;
+       }
 
-               if (!acl_perms) {
-                       DEBUG(3,("set_nt_acl: cannot set permissions\n"));
-                       free_canon_ace_list(file_ace_list);
-                       free_canon_ace_list(dir_ace_list); 
-                       return NT_STATUS_ACCESS_DENIED;
-               }
+       if (!acl_perms) {
+               DEBUG(3,("set_nt_acl: cannot set permissions\n"));
+               free_canon_ace_list(file_ace_list);
+               free_canon_ace_list(dir_ace_list);
+               return NT_STATUS_ACCESS_DENIED;
+       }
 
-               /*
-                * Only change security if we got a DACL.
-                */
+       /*
+        * Only change security if we got a DACL.
+        */
 
-               if((security_info_sent & DACL_SECURITY_INFORMATION) && (psd->dacl != NULL)) {
+       if(!(security_info_sent & DACL_SECURITY_INFORMATION) || (psd->dacl == NULL)) {
+               free_canon_ace_list(file_ace_list);
+               free_canon_ace_list(dir_ace_list);
+               return NT_STATUS_OK;
+       }
 
-                       bool acl_set_support = False;
-                       bool ret = False;
+       /*
+        * Try using the POSIX ACL set first. Fall back to chmod if
+        * we have no ACL support on this filesystem.
+        */
+
+       if (acl_perms && file_ace_list) {
+               if (set_acl_as_root) {
+                       become_root();
+               }
+               ret = set_canon_ace_list(fsp, file_ace_list, False, sbuf.st_gid, &acl_set_support);
+               if (set_acl_as_root) {
+                       unbecome_root();
+               }
+               if (acl_set_support && ret == false) {
+                       DEBUG(3,("set_nt_acl: failed to set file acl on file %s (%s).\n", fsp->fsp_name, strerror(errno) ));
+                       free_canon_ace_list(file_ace_list);
+                       free_canon_ace_list(dir_ace_list);
+                       return map_nt_error_from_unix(errno);
+               }
+       }
+
+       if (acl_perms && acl_set_support && fsp->is_directory) {
+               if (dir_ace_list) {
+                       if (set_acl_as_root) {
+                               become_root();
+                       }
+                       ret = set_canon_ace_list(fsp, dir_ace_list, True, sbuf.st_gid, &acl_set_support);
+                       if (set_acl_as_root) {
+                               unbecome_root();
+                       }
+                       if (ret == false) {
+                               DEBUG(3,("set_nt_acl: failed to set default acl on directory %s (%s).\n", fsp->fsp_name, strerror(errno) ));
+                               free_canon_ace_list(file_ace_list);
+                               free_canon_ace_list(dir_ace_list);
+                               return map_nt_error_from_unix(errno);
+                       }
+               } else {
+                       int sret = -1;
 
                        /*
-                        * Try using the POSIX ACL set first. Fall back to chmod if
-                        * we have no ACL support on this filesystem.
+                        * No default ACL - delete one if it exists.
                         */
 
-                       if (acl_perms && file_ace_list) {
-                               ret = set_canon_ace_list(fsp, file_ace_list, False, sbuf.st_gid, &acl_set_support);
-                               if (acl_set_support && ret == False) {
-                                       DEBUG(3,("set_nt_acl: failed to set file acl on file %s (%s).\n", fsp->fsp_name, strerror(errno) ));
+                       if (set_acl_as_root) {
+                               become_root();
+                       }
+                       sret = SMB_VFS_SYS_ACL_DELETE_DEF_FILE(conn, fsp->fsp_name);
+                       if (set_acl_as_root) {
+                               unbecome_root();
+                       }
+                       if (sret == -1) {
+                               if (acl_group_override(conn, sbuf.st_gid, fsp->fsp_name)) {
+                                       DEBUG(5,("set_nt_acl: acl group control on and "
+                                               "current user in file %s primary group. Override delete_def_acl\n",
+                                               fsp->fsp_name ));
+
+                                       become_root();
+                                       sret = SMB_VFS_SYS_ACL_DELETE_DEF_FILE(conn, fsp->fsp_name);
+                                       unbecome_root();
+                               }
+
+                               if (sret == -1) {
+                                       DEBUG(3,("set_nt_acl: sys_acl_delete_def_file failed (%s)\n", strerror(errno)));
                                        free_canon_ace_list(file_ace_list);
-                                       free_canon_ace_list(dir_ace_list); 
+                                       free_canon_ace_list(dir_ace_list);
                                        return map_nt_error_from_unix(errno);
                                }
                        }
+               }
+       }
 
-                       if (acl_perms && acl_set_support && fsp->is_directory) {
-                               if (dir_ace_list) {
-                                       if (!set_canon_ace_list(fsp, dir_ace_list, True, sbuf.st_gid, &acl_set_support)) {
-                                               DEBUG(3,("set_nt_acl: failed to set default acl on directory %s (%s).\n", fsp->fsp_name, strerror(errno) ));
-                                               free_canon_ace_list(file_ace_list);
-                                               free_canon_ace_list(dir_ace_list); 
-                                               return map_nt_error_from_unix(errno);
-                                       }
-                               } else {
+       if (acl_set_support) {
+               if (set_acl_as_root) {
+                       become_root();
+               }
+               store_inheritance_attributes(fsp, file_ace_list, dir_ace_list,
+                               (psd->type & SE_DESC_DACL_PROTECTED) ? True : False);
+               if (set_acl_as_root) {
+                       unbecome_root();
+               }
+       }
 
-                                       /*
-                                        * No default ACL - delete one if it exists.
-                                        */
+       /*
+        * If we cannot set using POSIX ACLs we fall back to checking if we need to chmod.
+        */
 
-                                       if (SMB_VFS_SYS_ACL_DELETE_DEF_FILE(conn, fsp->fsp_name) == -1) {
-                                               int sret = -1;
-
-                                               if (acl_group_override(conn, sbuf.st_gid, fsp->fsp_name)) {
-                                                       DEBUG(5,("set_nt_acl: acl group control on and "
-                                                               "current user in file %s primary group. Override delete_def_acl\n",
-                                                               fsp->fsp_name ));
-
-                                                       become_root();
-                                                       sret = SMB_VFS_SYS_ACL_DELETE_DEF_FILE(conn, fsp->fsp_name);
-                                                       unbecome_root();
-                                               }
-
-                                               if (sret == -1) {
-                                                       DEBUG(3,("set_nt_acl: sys_acl_delete_def_file failed (%s)\n", strerror(errno)));
-                                                       free_canon_ace_list(file_ace_list);
-                                                       free_canon_ace_list(dir_ace_list);
-                                                       return map_nt_error_from_unix(errno);
-                                               }
-                                       }
-                               }
-                       }
+       if(!acl_set_support && acl_perms) {
+               mode_t posix_perms;
 
-                       if (acl_set_support) {
-                               store_inheritance_attributes(fsp, file_ace_list, dir_ace_list,
-                                               (psd->type & SE_DESC_DACL_PROTECTED) ? True : False);
-                       }
+               if (!convert_canon_ace_to_posix_perms( fsp, file_ace_list, &posix_perms)) {
+                       free_canon_ace_list(file_ace_list);
+                       free_canon_ace_list(dir_ace_list);
+                       DEBUG(3,("set_nt_acl: failed to convert file acl to posix permissions for file %s.\n",
+                               fsp->fsp_name ));
+                       return NT_STATUS_ACCESS_DENIED;
+               }
 
-                       /*
-                        * If we cannot set using POSIX ACLs we fall back to checking if we need to chmod.
-                        */
+               if (orig_mode != posix_perms) {
+                       int sret = -1;
 
-                       if(!acl_set_support && acl_perms) {
-                               mode_t posix_perms;
+                       DEBUG(3,("set_nt_acl: chmod %s. perms = 0%o.\n",
+                               fsp->fsp_name, (unsigned int)posix_perms ));
 
-                               if (!convert_canon_ace_to_posix_perms( fsp, file_ace_list, &posix_perms)) {
-                                       free_canon_ace_list(file_ace_list);
-                                       free_canon_ace_list(dir_ace_list);
-                                       DEBUG(3,("set_nt_acl: failed to convert file acl to posix permissions for file %s.\n",
+                       if (set_acl_as_root) {
+                               become_root();
+                       }
+                       sret = SMB_VFS_CHMOD(conn,fsp->fsp_name, posix_perms);
+                       if (set_acl_as_root) {
+                               unbecome_root();
+                       }
+                       if(sret == -1) {
+                               if (acl_group_override(conn, sbuf.st_gid, fsp->fsp_name)) {
+                                       DEBUG(5,("set_nt_acl: acl group control on and "
+                                               "current user in file %s primary group. Override chmod\n",
                                                fsp->fsp_name ));
-                                       return NT_STATUS_ACCESS_DENIED;
+
+                                       become_root();
+                                       sret = SMB_VFS_CHMOD(conn,fsp->fsp_name, posix_perms);
+                                       unbecome_root();
                                }
 
-                               if (orig_mode != posix_perms) {
-
-                                       DEBUG(3,("set_nt_acl: chmod %s. perms = 0%o.\n",
-                                               fsp->fsp_name, (unsigned int)posix_perms ));
-
-                                       if(SMB_VFS_CHMOD(conn,fsp->fsp_name, posix_perms) == -1) {
-                                               int sret = -1;
-                                               if (acl_group_override(conn, sbuf.st_gid, fsp->fsp_name)) {
-                                                       DEBUG(5,("set_nt_acl: acl group control on and "
-                                                               "current user in file %s primary group. Override chmod\n",
-                                                               fsp->fsp_name ));
-
-                                                       become_root();
-                                                       sret = SMB_VFS_CHMOD(conn,fsp->fsp_name, posix_perms);
-                                                       unbecome_root();
-                                               }
-
-                                               if (sret == -1) {
-                                                       DEBUG(3,("set_nt_acl: chmod %s, 0%o failed. Error = %s.\n",
-                                                               fsp->fsp_name, (unsigned int)posix_perms, strerror(errno) ));
-                                                       free_canon_ace_list(file_ace_list);
-                                                       free_canon_ace_list(dir_ace_list);
-                                                       return map_nt_error_from_unix(errno);
-                                               }
-                                       }
+                               if (sret == -1) {
+                                       DEBUG(3,("set_nt_acl: chmod %s, 0%o failed. Error = %s.\n",
+                                               fsp->fsp_name, (unsigned int)posix_perms, strerror(errno) ));
+                                       free_canon_ace_list(file_ace_list);
+                                       free_canon_ace_list(dir_ace_list);
+                                       return map_nt_error_from_unix(errno);
                                }
                        }
                }
-
-               free_canon_ace_list(file_ace_list);
-               free_canon_ace_list(dir_ace_list);
        }
 
+       free_canon_ace_list(file_ace_list);
+       free_canon_ace_list(dir_ace_list);
+
        return NT_STATUS_OK;
 }