Fix bug 10196 - RW Deny for a specific user is not overriding RW Allow for a group.

When the ID returned is ID_TYPE_BOTH we must *always* add it as both
a user and a group, not just in the owning case. Otherwise DENY
entries are not correctly processed.

Confirmed by the reporter as fixing the problem.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=10196

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: David Disseldorp <ddiss@samba.org>

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index ad1431d76132f1dc503cf976c804ee644287d5da..621457e05109b1361cff652dae7712fdb54829a9 100644 (file)
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1981,47 +1981,50 @@ static bool create_canon_ace_lists(files_struct *fsp,
                        }
 
                        if (unixid.type == ID_TYPE_BOTH) {
-                               /* If it's the owning user, this is a
-                                * user_obj, not a user.  This way, we
-                                * get a valid ACL for groups that own
-                                * files, without putting user ACL
-                                * entries in for groups otherwise */
-                               if (unixid.id == pst->st_ex_uid) {
-                                       current_ace->owner_type = UID_ACE;
-                                       current_ace->unix_ug.type = ID_TYPE_UID;
-                                       current_ace->unix_ug.id = unixid.id;
-                                       current_ace->type = SMB_ACL_USER_OBJ;
-
-                                       /* Add the user object to the posix ACL,
-                                          and proceed to the group mapping
-                                          below. This handles the talloc_free
-                                          of current_ace if not added for some
-                                          reason */
-                                       if (!add_current_ace_to_acl(fsp,
-                                                       psa,
-                                                       &file_ace,
-                                                       &dir_ace,
-                                                       &got_file_allow,
-                                                       &got_dir_allow,
-                                                       &all_aces_are_inherit_only,
-                                                       current_ace)) {
-                                               free_canon_ace_list(file_ace);
-                                               free_canon_ace_list(dir_ace);
-                                               return false;
-                                       }
-
-                                       if ((current_ace = talloc(talloc_tos(),
-                                                       canon_ace)) == NULL) {
-                                               free_canon_ace_list(file_ace);
-                                               free_canon_ace_list(dir_ace);
-                                               DEBUG(0,("create_canon_ace_lists: "
-                                                       "malloc fail.\n"));
-                                               return False;
-                                       }
+                               /*
+                                * We must add both a user and group
+                                * entry POSIX_ACL.
+                                * This is due to the fact that in POSIX
+                                * user entries are more specific than
+                                * groups.
+                                */
+                               current_ace->owner_type = UID_ACE;
+                               current_ace->unix_ug.type = ID_TYPE_UID;
+                               current_ace->unix_ug.id = unixid.id;
+                               current_ace->type =
+                                       (unixid.id == pst->st_ex_uid) ?
+                                               SMB_ACL_USER_OBJ :
+                                               SMB_ACL_USER;
+
+                               /* Add the user object to the posix ACL,
+                                  and proceed to the group mapping
+                                  below. This handles the talloc_free
+                                  of current_ace if not added for some
+                                  reason */
+                               if (!add_current_ace_to_acl(fsp,
+                                               psa,
+                                               &file_ace,
+                                               &dir_ace,
+                                               &got_file_allow,
+                                               &got_dir_allow,
+                                               &all_aces_are_inherit_only,
+                                               current_ace)) {
+                                       free_canon_ace_list(file_ace);
+                                       free_canon_ace_list(dir_ace);
+                                       return false;
+                               }
 
-                                       ZERO_STRUCTP(current_ace);
+                               if ((current_ace = talloc(talloc_tos(),
+                                               canon_ace)) == NULL) {
+                                       free_canon_ace_list(file_ace);
+                                       free_canon_ace_list(dir_ace);
+                                       DEBUG(0,("create_canon_ace_lists: "
+                                               "malloc fail.\n"));
+                                       return False;
                                }
 
+                               ZERO_STRUCTP(current_ace);
+
                                sid_copy(&current_ace->trustee, &psa->trustee);
 
                                current_ace->unix_ug.type = ID_TYPE_GID;