Another attempt to fix bug #4308 - Excel save operation corrupts file ACLs.
[sfrench/samba-autobuild/.git] / source3 / smbd / posix_acls.c
index b6cb1d2e5407cbd6dc99c0320acbd5d0fb2c4c42..951046c56253034882f4121894088a6e632fb1d8 100644 (file)
@@ -229,7 +229,7 @@ static void store_inheritance_attributes(files_struct *fsp, canon_ace *file_ace_
        if (!pai_protected && num_inherited_entries(file_ace_list) == 0 && num_inherited_entries(dir_ace_list) == 0) {
                /* Instead just remove the attribute if it exists. */
                if (fsp->fh->fd != -1)
-                       SMB_VFS_FREMOVEXATTR(fsp, fsp->fh->fd, SAMBA_POSIX_INHERITANCE_EA_NAME);
+                       SMB_VFS_FREMOVEXATTR(fsp, SAMBA_POSIX_INHERITANCE_EA_NAME);
                else
                        SMB_VFS_REMOVEXATTR(fsp->conn, fsp->fsp_name, SAMBA_POSIX_INHERITANCE_EA_NAME);
                return;
@@ -238,7 +238,7 @@ static void store_inheritance_attributes(files_struct *fsp, canon_ace *file_ace_
        pai_buf = create_pai_buf(file_ace_list, dir_ace_list, pai_protected, &store_size);
 
        if (fsp->fh->fd != -1)
-               ret = SMB_VFS_FSETXATTR(fsp, fsp->fh->fd, SAMBA_POSIX_INHERITANCE_EA_NAME,
+               ret = SMB_VFS_FSETXATTR(fsp, SAMBA_POSIX_INHERITANCE_EA_NAME,
                                pai_buf, store_size, 0);
        else
                ret = SMB_VFS_SETXATTR(fsp->conn,fsp->fsp_name, SAMBA_POSIX_INHERITANCE_EA_NAME,
@@ -445,7 +445,7 @@ static struct pai_val *fload_inherited_info(files_struct *fsp)
 
        do {
                if (fsp->fh->fd != -1)
-                       ret = SMB_VFS_FGETXATTR(fsp, fsp->fh->fd, SAMBA_POSIX_INHERITANCE_EA_NAME,
+                       ret = SMB_VFS_FGETXATTR(fsp, SAMBA_POSIX_INHERITANCE_EA_NAME,
                                        pai_buf, pai_buf_size);
                else
                        ret = SMB_VFS_GETXATTR(fsp->conn,fsp->fsp_name,SAMBA_POSIX_INHERITANCE_EA_NAME,
@@ -725,7 +725,7 @@ static int map_acl_perms_to_permset(connection_struct *conn, mode_t mode, SMB_AC
  Function to create owner and group SIDs from a SMB_STRUCT_STAT.
 ****************************************************************************/
 
-static void create_file_sids(const SMB_STRUCT_STAT *psbuf, DOM_SID *powner_sid, DOM_SID *pgroup_sid)
+void create_file_sids(const SMB_STRUCT_STAT *psbuf, DOM_SID *powner_sid, DOM_SID *pgroup_sid)
 {
        uid_to_sid( powner_sid, psbuf->st_uid );
        gid_to_sid( pgroup_sid, psbuf->st_gid );
@@ -870,7 +870,7 @@ static void merge_aces( canon_ace **pp_list_head )
  Check if we need to return NT4.x compatible ACL entries.
 ****************************************************************************/
 
-static bool nt4_compatible_acls(void)
+bool nt4_compatible_acls(void)
 {
        int compat = lp_acl_compatibility();
 
@@ -890,13 +890,12 @@ static bool nt4_compatible_acls(void)
  not get. Deny entries are implicit on get with ace->perms = 0.
 ****************************************************************************/
 
-static SEC_ACCESS map_canon_ace_perms(int snum,
+static uint32_t map_canon_ace_perms(int snum,
                                enum security_ace_type *pacl_type,
                                mode_t perms,
                                bool directory_ace)
 {
-       SEC_ACCESS sa;
-       uint32 nt_mask = 0;
+       uint32_t nt_mask = 0;
 
        *pacl_type = SEC_ACE_TYPE_ACCESS_ALLOWED;
 
@@ -904,7 +903,7 @@ static SEC_ACCESS map_canon_ace_perms(int snum,
                if (directory_ace) {
                        nt_mask = UNIX_DIRECTORY_ACCESS_RWX;
                } else {
-                       nt_mask = UNIX_ACCESS_RWX;
+                       nt_mask = (UNIX_ACCESS_RWX & ~DELETE_ACCESS);
                }
        } else if ((perms & ALL_ACE_PERMS) == (mode_t)0) {
                /*
@@ -935,8 +934,7 @@ static SEC_ACCESS map_canon_ace_perms(int snum,
        DEBUG(10,("map_canon_ace_perms: Mapped (UNIX) %x to (NT) %x\n",
                        (unsigned int)perms, (unsigned int)nt_mask ));
 
-       init_sec_access(&sa,nt_mask);
-       return sa;
+       return nt_mask;
 }
 
 /****************************************************************************
@@ -988,7 +986,7 @@ static mode_t map_nt_perms( uint32 *mask, int type)
  Unpack a SEC_DESC into a UNIX owner and group.
 ****************************************************************************/
 
-NTSTATUS unpack_nt_owners(int snum, uid_t *puser, gid_t *pgrp, uint32 security_info_sent, SEC_DESC *psd)
+NTSTATUS unpack_nt_owners(int snum, uid_t *puser, gid_t *pgrp, uint32 security_info_sent, const SEC_DESC *psd)
 {
        DOM_SID owner_sid;
        DOM_SID grp_sid;
@@ -1329,11 +1327,13 @@ static void check_owning_objs(canon_ace *ace, DOM_SID *pfile_owner_sid, DOM_SID
  Unpack a SEC_DESC into two canonical ace lists.
 ****************************************************************************/
 
-static bool create_canon_ace_lists(files_struct *fsp, SMB_STRUCT_STAT *pst,
-                                                       DOM_SID *pfile_owner_sid,
-                                                       DOM_SID *pfile_grp_sid,
-                                                       canon_ace **ppfile_ace, canon_ace **ppdir_ace,
-                                                       SEC_ACL *dacl)
+static bool create_canon_ace_lists(files_struct *fsp,
+                                       SMB_STRUCT_STAT *pst,
+                                       DOM_SID *pfile_owner_sid,
+                                       DOM_SID *pfile_grp_sid,
+                                       canon_ace **ppfile_ace,
+                                       canon_ace **ppdir_ace,
+                                       const SEC_ACL *dacl)
 {
        bool all_aces_are_inherit_only = (fsp->is_directory ? True : False);
        canon_ace *file_ace = NULL;
@@ -1408,12 +1408,12 @@ static bool create_canon_ace_lists(files_struct *fsp, SMB_STRUCT_STAT *pst,
 
                                psa1->flags |= (psa2->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT));
                                psa2->flags &= ~(SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT);
-                               
+
                        } else if (psa2->flags & SEC_ACE_FLAG_INHERIT_ONLY) {
 
                                psa2->flags |= (psa1->flags & (SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT));
                                psa1->flags &= ~(SEC_ACE_FLAG_CONTAINER_INHERIT|SEC_ACE_FLAG_OBJECT_INHERIT);
-                               
+
                        }
                }
        }
@@ -1474,10 +1474,22 @@ static bool create_canon_ace_lists(files_struct *fsp, SMB_STRUCT_STAT *pst,
 
                } else if (sid_to_uid( &current_ace->trustee, &current_ace->unix_ug.uid)) {
                        current_ace->owner_type = UID_ACE;
-                       current_ace->type = SMB_ACL_USER;
+                       /* If it's the owning user, this is a user_obj, not
+                        * a user. */
+                       if (current_ace->unix_ug.uid == pst->st_uid) {
+                               current_ace->type = SMB_ACL_USER_OBJ;
+                       } else {
+                               current_ace->type = SMB_ACL_USER;
+                       }
                } else if (sid_to_gid( &current_ace->trustee, &current_ace->unix_ug.gid)) {
                        current_ace->owner_type = GID_ACE;
-                       current_ace->type = SMB_ACL_GROUP;
+                       /* If it's the primary group, this is a group_obj, not
+                        * a group. */
+                       if (current_ace->unix_ug.gid == pst->st_gid) {
+                               current_ace->type = SMB_ACL_GROUP_OBJ;
+                       } else {
+                               current_ace->type = SMB_ACL_GROUP;
+                       }
                } else {
                        /*
                         * Silently ignore map failures in non-mappable SIDs (NT Authority, BUILTIN etc).
@@ -2004,12 +2016,14 @@ static mode_t create_default_mode(files_struct *fsp, bool interitable_mode)
  succeeding.
 ****************************************************************************/
 
-static bool unpack_canon_ace(files_struct *fsp, 
-                                                       SMB_STRUCT_STAT *pst,
-                                                       DOM_SID *pfile_owner_sid,
-                                                       DOM_SID *pfile_grp_sid,
-                                                       canon_ace **ppfile_ace, canon_ace **ppdir_ace,
-                                                       uint32 security_info_sent, SEC_DESC *psd)
+static bool unpack_canon_ace(files_struct *fsp,
+                               SMB_STRUCT_STAT *pst,
+                               DOM_SID *pfile_owner_sid,
+                               DOM_SID *pfile_grp_sid,
+                               canon_ace **ppfile_ace,
+                               canon_ace **ppdir_ace,
+                               uint32 security_info_sent,
+                               const SEC_DESC *psd)
 {
        canon_ace *file_ace = NULL;
        canon_ace *dir_ace = NULL;
@@ -2195,9 +2209,7 @@ static canon_ace *canonicalise_acl(struct connection_struct *conn,
                posix_id unix_ug;
                enum ace_owner owner_type;
 
-               /* get_next... */
-               if (entry_id == SMB_ACL_FIRST_ENTRY)
-                       entry_id = SMB_ACL_NEXT_ENTRY;
+               entry_id = SMB_ACL_NEXT_ENTRY;
 
                /* Is this a MASK entry ? */
                if (SMB_VFS_SYS_ACL_GET_TAG_TYPE(conn, entry, &tagtype) == -1)
@@ -2352,20 +2364,32 @@ static bool current_user_in_group(gid_t gid)
 }
 
 /****************************************************************************
- Should we override a deny ?  Check deprecated 'acl group control'
- and 'dos filemode'
+ Should we override a deny ? Check 'acl group control' and 'dos filemode'.
 ****************************************************************************/
 
-static bool acl_group_override(connection_struct *conn, gid_t prim_gid)
+static bool acl_group_override(connection_struct *conn,
+                               gid_t prim_gid,
+                               const char *fname)
 {
-       if ( (errno == EACCES || errno == EPERM) 
-               && (lp_acl_group_control(SNUM(conn)) || lp_dos_filemode(SNUM(conn)))
-               && current_user_in_group(prim_gid)) 
-       {
-               return True;
-       } 
+       SMB_STRUCT_STAT sbuf;
 
-       return False;
+       if ((errno != EPERM) && (errno != EACCES)) {
+               return false;
+       }
+
+       /* file primary group == user primary or supplementary group */
+       if (lp_acl_group_control(SNUM(conn)) &&
+                       current_user_in_group(prim_gid)) {
+               return true;
+       }
+
+       /* user has writeable permission */
+       if (lp_dos_filemode(SNUM(conn)) &&
+                       can_write_to_file(conn, fname, &sbuf)) {
+               return true;
+       }
+
+       return false;
 }
 
 /****************************************************************************
@@ -2551,7 +2575,7 @@ static bool set_canon_ace_list(files_struct *fsp, canon_ace *the_ace, bool defau
                                *pacl_set_support = False;
                        }
 
-                       if (acl_group_override(conn, prim_gid)) {
+                       if (acl_group_override(conn, prim_gid, fsp->fsp_name)) {
                                int sret;
 
                                DEBUG(5,("set_canon_ace_list: acl group control on and current user in file %s primary group.\n",
@@ -2582,7 +2606,7 @@ static bool set_canon_ace_list(files_struct *fsp, canon_ace *the_ace, bool defau
                                *pacl_set_support = False;
                        }
 
-                       if (acl_group_override(conn, prim_gid)) {
+                       if (acl_group_override(conn, prim_gid, fsp->fsp_name)) {
                                int sret;
 
                                DEBUG(5,("set_canon_ace_list: acl group control on and current user in file %s primary group.\n",
@@ -2870,7 +2894,6 @@ static NTSTATUS posix_get_nt_acl_common(struct connection_struct *conn,
                {
                        canon_ace *ace;
                        enum security_ace_type nt_acl_type;
-                       int i;
 
                        if (nt4_compatible_acls() && dir_ace) {
                                /*
@@ -2936,12 +2959,8 @@ static NTSTATUS posix_get_nt_acl_common(struct connection_struct *conn,
                         * Create the NT ACE list from the canonical ace lists.
                         */
 
-                       ace = file_ace;
-
-                       for (i = 0; i < num_acls; i++, ace = ace->next) {
-                               SEC_ACCESS acc;
-
-                               acc = map_canon_ace_perms(SNUM(conn),
+                       for (ace = file_ace; ace != NULL; ace = ace->next) {
+                               uint32_t acc = map_canon_ace_perms(SNUM(conn),
                                                &nt_acl_type,
                                                ace->perms,
                                                S_ISDIR(sbuf->st_mode));
@@ -2956,21 +2975,14 @@ static NTSTATUS posix_get_nt_acl_common(struct connection_struct *conn,
                        /* The User must have access to a profile share - even
                         * if we can't map the SID. */
                        if (lp_profile_acls(SNUM(conn))) {
-                               SEC_ACCESS acc;
-
-                               init_sec_access(&acc,FILE_GENERIC_ALL);
                                init_sec_ace(&nt_ace_list[num_aces++],
                                                &global_sid_Builtin_Users,
                                                SEC_ACE_TYPE_ACCESS_ALLOWED,
-                                               acc, 0);
+                                               FILE_GENERIC_ALL, 0);
                        }
 
-                       ace = dir_ace;
-
-                       for (i = 0; i < num_def_acls; i++, ace = ace->next) {
-                               SEC_ACCESS acc;
-
-                               acc = map_canon_ace_perms(SNUM(conn),
+                       for (ace = dir_ace; ace != NULL; ace = ace->next) {
+                               uint32_t acc = map_canon_ace_perms(SNUM(conn),
                                                &nt_acl_type,
                                                ace->perms,
                                                S_ISDIR(sbuf->st_mode));
@@ -2988,10 +3000,7 @@ static NTSTATUS posix_get_nt_acl_common(struct connection_struct *conn,
                        /* The User must have access to a profile share - even
                         * if we can't map the SID. */
                        if (lp_profile_acls(SNUM(conn))) {
-                               SEC_ACCESS acc;
-
-                               init_sec_access(&acc,FILE_GENERIC_ALL);
-                               init_sec_ace(&nt_ace_list[num_aces++], &global_sid_Builtin_Users, SEC_ACE_TYPE_ACCESS_ALLOWED, acc,
+                               init_sec_ace(&nt_ace_list[num_aces++], &global_sid_Builtin_Users, SEC_ACE_TYPE_ACCESS_ALLOWED, FILE_GENERIC_ALL,
                                                SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_CONTAINER_INHERIT|
                                                SEC_ACE_FLAG_INHERIT_ONLY|0);
                        }
@@ -3182,7 +3191,7 @@ int try_chown(connection_struct *conn, const char *fname, uid_t uid, gid_t gid)
                return -1;
        }
 
-       if (!NT_STATUS_IS_OK(open_file_fchmod(conn,fname,&st,&fsp))) {
+       if (!NT_STATUS_IS_OK(open_file_fchmod(NULL, conn, fname, &st, &fsp))) {
                return -1;
        }
 
@@ -3197,111 +3206,63 @@ int try_chown(connection_struct *conn, const char *fname, uid_t uid, gid_t gid)
        ret = SMB_VFS_FCHOWN(fsp, uid, (gid_t)-1);
        unbecome_root();
 
-       close_file_fchmod(fsp);
+       close_file_fchmod(NULL, fsp);
 
        return ret;
 }
 
-static NTSTATUS append_ugw_ace(files_struct *fsp,
-                       SMB_STRUCT_STAT *psbuf,
-                       mode_t unx_mode,
-                       int ugw,
-                       SEC_ACE *se)
-{
-       mode_t perms;
-       SEC_ACCESS acc;
-       enum security_ace_type nt_acl_type;
-       DOM_SID trustee;
-
-       switch (ugw) {
-               case S_IRUSR:
-                       perms = unix_perms_to_acl_perms(unx_mode,
-                                                       S_IRUSR,
-                                                       S_IWUSR,
-                                                       S_IXUSR);
-                       uid_to_sid(&trustee, psbuf->st_uid );
-                       break;
-               case S_IRGRP:
-                       perms = unix_perms_to_acl_perms(unx_mode,
-                                                       S_IRGRP,
-                                                       S_IWGRP,
-                                                       S_IXGRP);
-                       gid_to_sid(&trustee, psbuf->st_gid );
-                       break;
-               case S_IROTH:
-                       perms = unix_perms_to_acl_perms(unx_mode,
-                                                       S_IROTH,
-                                                       S_IWOTH,
-                                                       S_IXOTH);
-                       sid_copy(&trustee, &global_sid_World);
-                       break;
-               default:
-                       return NT_STATUS_INVALID_PARAMETER;
-       }
-       acc = map_canon_ace_perms(SNUM(fsp->conn),
-                               &nt_acl_type,
-                               perms,
-                               fsp->is_directory);
-
-       init_sec_ace(se,
-               &trustee,
-               nt_acl_type,
-               acc,
-               0);
-       return NT_STATUS_OK;
-}
+#if 0
+/* Disable this - prevents ACL inheritance from the ACL editor. JRA. */
 
 /****************************************************************************
- If this is an
+ Take care of parent ACL inheritance.
 ****************************************************************************/
 
-static NTSTATUS append_parent_acl(files_struct *fsp,
-                               SMB_STRUCT_STAT *psbuf,
-                               SEC_DESC *psd,
+NTSTATUS append_parent_acl(files_struct *fsp,
+                               const SEC_DESC *pcsd,
                                SEC_DESC **pp_new_sd)
 {
        SEC_DESC *parent_sd = NULL;
        files_struct *parent_fsp = NULL;
-       TALLOC_CTX *mem_ctx = talloc_parent(psd);
+       TALLOC_CTX *mem_ctx = talloc_tos();
        char *parent_name = NULL;
        SEC_ACE *new_ace = NULL;
-       unsigned int num_aces = psd->dacl->num_aces;
+       unsigned int num_aces = pcsd->dacl->num_aces;
        SMB_STRUCT_STAT sbuf;
        NTSTATUS status;
        int info;
        unsigned int i, j;
-       mode_t unx_mode;
+       SEC_DESC *psd = dup_sec_desc(talloc_tos(), pcsd);
+       bool is_dacl_protected = (pcsd->type & SE_DESC_DACL_PROTECTED);
 
        ZERO_STRUCT(sbuf);
 
-       if (mem_ctx == NULL) {
+       if (psd == NULL) {
                return NT_STATUS_NO_MEMORY;
        }
 
-       if (!parent_dirname_talloc(mem_ctx,
-                               fsp->fsp_name,
-                               &parent_name,
-                               NULL)) {
+       if (!parent_dirname(mem_ctx, fsp->fsp_name, &parent_name, NULL)) {
                return NT_STATUS_NO_MEMORY;
        }
 
-       /* Create a default mode for u/g/w. */
-       unx_mode = unix_mode(fsp->conn,
-                       aARCH | (fsp->is_directory ? aDIR : 0),
-                       fsp->fsp_name,
-                       parent_name);
-
-       status = open_directory(fsp->conn,
-                               NULL,
-                               parent_name,
-                               &sbuf,
-                               FILE_READ_ATTRIBUTES, /* Just a stat open */
-                               FILE_SHARE_NONE, /* Ignored for stat opens */
-                               FILE_OPEN,
-                               0,
-                               INTERNAL_OPEN_ONLY,
-                               &info,
-                               &parent_fsp);
+       status = SMB_VFS_CREATE_FILE(
+               fsp->conn,                              /* conn */
+               NULL,                                   /* req */
+               0,                                      /* root_dir_fid */
+               parent_name,                            /* fname */
+               0,                                      /* create_file_flags */
+               FILE_READ_ATTRIBUTES,                   /* access_mask */
+               FILE_SHARE_NONE,                        /* share_access */
+               FILE_OPEN,                              /* create_disposition*/
+               FILE_DIRECTORY_FILE,                    /* create_options */
+               0,                                      /* file_attributes */
+               INTERNAL_OPEN_ONLY,                     /* oplock_request */
+               0,                                      /* allocation_size */
+               NULL,                                   /* sd */
+               NULL,                                   /* ea_list */
+               &parent_fsp,                            /* result */
+               &info,                                  /* pinfo */
+               &sbuf);                                 /* psbuf */
 
        if (!NT_STATUS_IS_OK(status)) {
                return status;
@@ -3310,7 +3271,7 @@ static NTSTATUS append_parent_acl(files_struct *fsp,
        status = SMB_VFS_GET_NT_ACL(parent_fsp->conn, parent_fsp->fsp_name,
                                    DACL_SECURITY_INFORMATION, &parent_sd );
 
-       close_file(parent_fsp, NORMAL_CLOSE);
+       close_file(NULL, parent_fsp, NORMAL_CLOSE);
 
        if (!NT_STATUS_IS_OK(status)) {
                return status;
@@ -3318,20 +3279,23 @@ static NTSTATUS append_parent_acl(files_struct *fsp,
 
        /*
         * Make room for potentially all the ACLs from
-        * the parent, plus the user/group/other triple.
+        * the parent. We used to add the ugw triple here,
+        * as we knew we were dealing with POSIX ACLs.
+        * We no longer need to do so as we can guarentee
+        * that a default ACL from the parent directory will
+        * be well formed for POSIX ACLs if it came from a
+        * POSIX ACL source, and if we're not writing to a
+        * POSIX ACL sink then we don't care if it's not well
+        * formed. JRA.
         */
 
-       num_aces += parent_sd->dacl->num_aces + 3;
+       num_aces += parent_sd->dacl->num_aces;
 
        if((new_ace = TALLOC_ZERO_ARRAY(mem_ctx, SEC_ACE,
                                        num_aces)) == NULL) {
                return NT_STATUS_NO_MEMORY;
        }
 
-       DEBUG(10,("append_parent_acl: parent ACL has %u entries. New "
-               "ACL has %u entries\n",
-               parent_sd->dacl->num_aces, num_aces ));
-
        /* Start by copying in all the given ACE entries. */
        for (i = 0; i < psd->dacl->num_aces; i++) {
                sec_ace_copy(&new_ace[i], &psd->dacl->aces[i]);
@@ -3342,57 +3306,106 @@ static NTSTATUS append_parent_acl(files_struct *fsp,
         * as that really only applies to newly created files. JRA.
         */
 
-        /*
-         * Append u/g/w.
-         */
-
-       status = append_ugw_ace(fsp, psbuf, unx_mode, S_IRUSR, &new_ace[i++]);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-       status = append_ugw_ace(fsp, psbuf, unx_mode, S_IRGRP, &new_ace[i++]);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-       status = append_ugw_ace(fsp, psbuf, unx_mode, S_IROTH, &new_ace[i++]);
-       if (!NT_STATUS_IS_OK(status)) {
-               return status;
-       }
-
        /* Finally append any inherited ACEs. */
        for (j = 0; j < parent_sd->dacl->num_aces; j++) {
                SEC_ACE *se = &parent_sd->dacl->aces[j];
-               uint32 i_flags = se->flags & (SEC_ACE_FLAG_OBJECT_INHERIT|
-                                       SEC_ACE_FLAG_CONTAINER_INHERIT|
-                                       SEC_ACE_FLAG_INHERIT_ONLY);
 
                if (fsp->is_directory) {
-                       if (i_flags == SEC_ACE_FLAG_OBJECT_INHERIT) {
-                               /* Should only apply to a file - ignore. */
+                       if (!(se->flags & SEC_ACE_FLAG_CONTAINER_INHERIT)) {
+                               /* Doesn't apply to a directory - ignore. */
+                               DEBUG(10,("append_parent_acl: directory %s "
+                                       "ignoring non container "
+                                       "inherit flags %u on ACE with sid %s "
+                                       "from parent %s\n",
+                                       fsp->fsp_name,
+                                       (unsigned int)se->flags,
+                                       sid_string_dbg(&se->trustee),
+                                       parent_name));
                                continue;
                        }
                } else {
-                       if ((i_flags & (SEC_ACE_FLAG_OBJECT_INHERIT|
-                                       SEC_ACE_FLAG_INHERIT_ONLY)) !=
-                                       SEC_ACE_FLAG_OBJECT_INHERIT) {
-                               /* Should not apply to a file - ignore. */
+                       if (!(se->flags & SEC_ACE_FLAG_OBJECT_INHERIT)) {
+                               /* Doesn't apply to a file - ignore. */
+                               DEBUG(10,("append_parent_acl: file %s "
+                                       "ignoring non object "
+                                       "inherit flags %u on ACE with sid %s "
+                                       "from parent %s\n",
+                                       fsp->fsp_name,
+                                       (unsigned int)se->flags,
+                                       sid_string_dbg(&se->trustee),
+                                       parent_name));
                                continue;
                        }
                }
+
+               if (is_dacl_protected) {
+                       /* If the DACL is protected it means we must
+                        * not overwrite an existing ACE entry with the
+                        * same SID. This is order N^2. Ouch :-(. JRA. */
+                       unsigned int k;
+                       for (k = 0; k < psd->dacl->num_aces; k++) {
+                               if (sid_equal(&psd->dacl->aces[k].trustee,
+                                               &se->trustee)) {
+                                       break;
+                               }
+                       }
+                       if (k < psd->dacl->num_aces) {
+                               /* SID matched. Ignore. */
+                               DEBUG(10,("append_parent_acl: path %s "
+                                       "ignoring ACE with protected sid %s "
+                                       "from parent %s\n",
+                                       fsp->fsp_name,
+                                       sid_string_dbg(&se->trustee),
+                                       parent_name));
+                               continue;
+                       }
+               }
+
                sec_ace_copy(&new_ace[i], se);
                if (se->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) {
                        new_ace[i].flags &= ~(SEC_ACE_FLAG_VALID_INHERIT);
                }
                new_ace[i].flags |= SEC_ACE_FLAG_INHERITED_ACE;
+
+               if (fsp->is_directory) {
+                       /*
+                        * Strip off any inherit only. It's applied.
+                        */
+                       new_ace[i].flags &= ~(SEC_ACE_FLAG_INHERIT_ONLY);
+                       if (se->flags & SEC_ACE_FLAG_NO_PROPAGATE_INHERIT) {
+                               /* No further inheritance. */
+                               new_ace[i].flags &=
+                                       ~(SEC_ACE_FLAG_CONTAINER_INHERIT|
+                                       SEC_ACE_FLAG_OBJECT_INHERIT);
+                       }
+               } else {
+                       /*
+                        * Strip off any container or inherit
+                        * flags, they can't apply to objects.
+                        */
+                       new_ace[i].flags &= ~(SEC_ACE_FLAG_CONTAINER_INHERIT|
+                                               SEC_ACE_FLAG_INHERIT_ONLY|
+                                               SEC_ACE_FLAG_NO_PROPAGATE_INHERIT);
+               }
                i++;
+
+               DEBUG(10,("append_parent_acl: path %s "
+                       "inheriting ACE with sid %s "
+                       "from parent %s\n",
+                       fsp->fsp_name,
+                       sid_string_dbg(&se->trustee),
+                       parent_name));
        }
 
-       parent_sd->dacl->aces = new_ace;
-       parent_sd->dacl->num_aces = i;
+       psd->dacl->aces = new_ace;
+       psd->dacl->num_aces = i;
+       psd->type &= ~(SE_DESC_DACL_AUTO_INHERITED|
+                         SE_DESC_DACL_AUTO_INHERIT_REQ);
 
-       *pp_new_sd = parent_sd;
+       *pp_new_sd = psd;
        return status;
 }
+#endif
 
 /****************************************************************************
  Reply to set a security descriptor on an fsp. security_info_sent is the
@@ -3400,7 +3413,7 @@ static NTSTATUS append_parent_acl(files_struct *fsp,
  This should be the only external function needed for the UNIX style set ACL.
 ****************************************************************************/
 
-NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
+NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, const SEC_DESC *psd)
 {
        connection_struct *conn = fsp->conn;
        uid_t user = (uid_t)-1;
@@ -3413,6 +3426,8 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
        bool acl_perms = False;
        mode_t orig_mode = (mode_t)0;
        NTSTATUS status;
+       uid_t orig_uid;
+       gid_t orig_gid;
 
        DEBUG(10,("set_nt_acl: called for file %s\n", fsp->fsp_name ));
 
@@ -3435,6 +3450,8 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 
        /* Save the original elements 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.
@@ -3446,10 +3463,12 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
        }
 
        /*
-        * Do we need to chown ?
+        * Do we need to chown ? If so this must be done first as the incoming
+        * CREATOR_OWNER acl will be relative to the *new* owner, not the old.
+        * Noticed by Simo.
         */
 
-       if (((user != (uid_t)-1) && (sbuf.st_uid != user)) || (( grp != (gid_t)-1) && (sbuf.st_gid != grp))) {
+       if (((user != (uid_t)-1) && (orig_uid != user)) || (( grp != (gid_t)-1) && (orig_gid != grp))) {
 
                DEBUG(3,("set_nt_acl: chown %s. uid = %u, gid = %u.\n",
                                fsp->fsp_name, (unsigned int)user, (unsigned int)grp ));
@@ -3487,21 +3506,33 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 
                /* Save the original elements we check against. */
                orig_mode = sbuf.st_mode;
+               orig_uid = sbuf.st_uid;
+               orig_gid = sbuf.st_gid;
        }
 
        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) ) {
-               status = append_parent_acl(fsp, &sbuf, psd, &psd);
+               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);
@@ -3557,7 +3588,7 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
                                        if (SMB_VFS_SYS_ACL_DELETE_DEF_FILE(conn, fsp->fsp_name) == -1) {
                                                int sret = -1;
 
-                                               if (acl_group_override(conn, sbuf.st_gid)) {
+                                               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 ));
@@ -3604,7 +3635,7 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 
                                        if(SMB_VFS_CHMOD(conn,fsp->fsp_name, posix_perms) == -1) {
                                                int sret = -1;
-                                               if (acl_group_override(conn, sbuf.st_gid)) {
+                                               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 ));
@@ -3627,7 +3658,7 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
                }
 
                free_canon_ace_list(file_ace_list);
-               free_canon_ace_list(dir_ace_list); 
+               free_canon_ace_list(dir_ace_list);
        }
 
        return NT_STATUS_OK;
@@ -3654,9 +3685,7 @@ int get_acl_group_bits( connection_struct *conn, const char *fname, mode_t *mode
                SMB_ACL_TAG_T tagtype;
                SMB_ACL_PERMSET_T permset;
 
-               /* get_next... */
-               if (entry_id == SMB_ACL_FIRST_ENTRY)
-                       entry_id = SMB_ACL_NEXT_ENTRY;
+               entry_id = SMB_ACL_NEXT_ENTRY;
 
                if (SMB_VFS_SYS_ACL_GET_TAG_TYPE(conn, entry, &tagtype) ==-1)
                        break;
@@ -3694,9 +3723,7 @@ static int chmod_acl_internals( connection_struct *conn, SMB_ACL_T posix_acl, mo
                SMB_ACL_PERMSET_T permset;
                mode_t perms;
 
-               /* get_next... */
-               if (entry_id == SMB_ACL_FIRST_ENTRY)
-                       entry_id = SMB_ACL_NEXT_ENTRY;
+               entry_id = SMB_ACL_NEXT_ENTRY;
 
                if (SMB_VFS_SYS_ACL_GET_TAG_TYPE(conn, entry, &tagtype) == -1)
                        return -1;
@@ -3753,7 +3780,7 @@ static int chmod_acl_internals( connection_struct *conn, SMB_ACL_T posix_acl, mo
  resulting ACL on TO.  Note that name is in UNIX character set.
 ****************************************************************************/
 
-static int copy_access_acl(connection_struct *conn, const char *from, const char *to, mode_t mode)
+static int copy_access_posix_acl(connection_struct *conn, const char *from, const char *to, mode_t mode)
 {
        SMB_ACL_T posix_acl = NULL;
        int ret = -1;
@@ -3780,7 +3807,27 @@ static int copy_access_acl(connection_struct *conn, const char *from, const char
 
 int chmod_acl(connection_struct *conn, const char *name, mode_t mode)
 {
-       return copy_access_acl(conn, name, name, mode);
+       return copy_access_posix_acl(conn, name, name, mode);
+}
+
+/****************************************************************************
+ Check for an existing default POSIX ACL on a directory.
+****************************************************************************/
+
+static bool directory_has_default_posix_acl(connection_struct *conn, const char *fname)
+{
+       SMB_ACL_T def_acl = SMB_VFS_SYS_ACL_GET_FILE( conn, fname, SMB_ACL_TYPE_DEFAULT);
+       bool has_acl = False;
+       SMB_ACL_ENTRY_T entry;
+
+       if (def_acl != NULL && (SMB_VFS_SYS_ACL_GET_ENTRY(conn, def_acl, SMB_ACL_FIRST_ENTRY, &entry) == 1)) {
+               has_acl = True;
+       }
+
+       if (def_acl) {
+               SMB_VFS_SYS_ACL_FREE_ACL(conn, def_acl);
+       }
+        return has_acl;
 }
 
 /****************************************************************************
@@ -3788,13 +3835,13 @@ int chmod_acl(connection_struct *conn, const char *name, mode_t mode)
  inherit this Access ACL to file name.
 ****************************************************************************/
 
-int inherit_access_acl(connection_struct *conn, const char *inherit_from_dir,
+int inherit_access_posix_acl(connection_struct *conn, const char *inherit_from_dir,
                       const char *name, mode_t mode)
 {
-       if (directory_has_default_acl(conn, inherit_from_dir))
+       if (directory_has_default_posix_acl(conn, inherit_from_dir))
                return 0;
 
-       return copy_access_acl(conn, inherit_from_dir, name, mode);
+       return copy_access_posix_acl(conn, inherit_from_dir, name, mode);
 }
 
 /****************************************************************************
@@ -3822,26 +3869,6 @@ int fchmod_acl(files_struct *fsp, mode_t mode)
        return ret;
 }
 
-/****************************************************************************
- Check for an existing default POSIX ACL on a directory.
-****************************************************************************/
-
-bool directory_has_default_acl(connection_struct *conn, const char *fname)
-{
-       SMB_ACL_T def_acl = SMB_VFS_SYS_ACL_GET_FILE( conn, fname, SMB_ACL_TYPE_DEFAULT);
-       bool has_acl = False;
-       SMB_ACL_ENTRY_T entry;
-
-       if (def_acl != NULL && (SMB_VFS_SYS_ACL_GET_ENTRY(conn, def_acl, SMB_ACL_FIRST_ENTRY, &entry) == 1)) {
-               has_acl = True;
-       }
-
-       if (def_acl) {
-               SMB_VFS_SYS_ACL_FREE_ACL(conn, def_acl);
-       }
-        return has_acl;
-}
-
 /****************************************************************************
  Map from wire type to permset.
 ****************************************************************************/
@@ -4116,9 +4143,7 @@ static bool remove_posix_acl(connection_struct *conn, files_struct *fsp, const c
                SMB_ACL_TAG_T tagtype;
                SMB_ACL_PERMSET_T permset;
 
-               /* get_next... */
-               if (entry_id == SMB_ACL_FIRST_ENTRY)
-                       entry_id = SMB_ACL_NEXT_ENTRY;
+               entry_id = SMB_ACL_NEXT_ENTRY;
 
                if (SMB_VFS_SYS_ACL_GET_TAG_TYPE(conn, entry, &tagtype) == -1) {
                        DEBUG(5,("remove_posix_acl: failed to get tagtype from ACL on file %s (%s).\n",
@@ -4232,30 +4257,29 @@ bool set_unix_posix_acl(connection_struct *conn, files_struct *fsp, const char *
 SEC_DESC *get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname)
 {
        SEC_DESC *psd, *ret_sd;
-       connection_struct conn;
+       connection_struct *conn;
        files_struct finfo;
        struct fd_handle fh;
 
-       ZERO_STRUCT( conn );
-
-       if ( !(conn.mem_ctx = talloc_init( "novfs_get_nt_acl" )) ) {
-               DEBUG(0,("get_nt_acl_no_snum: talloc() failed!\n"));
+       conn = TALLOC_ZERO_P(ctx, connection_struct);
+       if (conn == NULL) {
+               DEBUG(0, ("talloc failed\n"));
                return NULL;
        }
 
-       if (!(conn.params = TALLOC_P(conn.mem_ctx, struct share_params))) {
+       if (!(conn->params = TALLOC_P(conn, struct share_params))) {
                DEBUG(0,("get_nt_acl_no_snum: talloc() failed!\n"));
-               TALLOC_FREE(conn.mem_ctx);
+               TALLOC_FREE(conn);
                return NULL;
        }
 
-       conn.params->service = -1;
+       conn->params->service = -1;
 
-       set_conn_connectpath(&conn, "/");
+       set_conn_connectpath(conn, "/");
 
-       if (!smbd_vfs_init(&conn)) {
+       if (!smbd_vfs_init(conn)) {
                DEBUG(0,("get_nt_acl_no_snum: Unable to create a fake connection struct!\n"));
-               conn_free_internal( &conn );
+               conn_free_internal( conn );
                return NULL;
         }
 
@@ -4263,20 +4287,20 @@ SEC_DESC *get_nt_acl_no_snum( TALLOC_CTX *ctx, const char *fname)
        ZERO_STRUCT( fh );
 
        finfo.fnum = -1;
-       finfo.conn = &conn;
+       finfo.conn = conn;
        finfo.fh = &fh;
        finfo.fh->fd = -1;
        finfo.fsp_name = CONST_DISCARD(char *,fname);
 
-       if (!NT_STATUS_IS_OK(posix_fget_nt_acl( &finfo, DACL_SECURITY_INFORMATION, &psd))) {
+       if (!NT_STATUS_IS_OK(SMB_VFS_FGET_NT_ACL( &finfo, DACL_SECURITY_INFORMATION, &psd))) {
                DEBUG(0,("get_nt_acl_no_snum: get_nt_acl returned zero.\n"));
-               conn_free_internal( &conn );
+               conn_free_internal( conn );
                return NULL;
        }
 
        ret_sd = dup_sec_desc( ctx, psd );
 
-       conn_free_internal( &conn );
+       conn_free_internal( conn );
 
        return ret_sd;
 }