Quoting Marco Gaiarin (g...@sv.lnf.it): > Package: samba > Version: 2:3.2.5-4lenny6 > Severity: grave > > > I'm using a plain lenny, fresh updated, standard kernel on amd64 (but i > think that does not care). > > After upgrading from etch to lenny i've found many complain about my > user that file was not more accessible, particulary on complex shares > where more that one group have to read and write file.
Attached is upstream patch, backported to 3.2.5. Is there a chance that you can test by youself reproducing this bug, with the patch applied...or should I build a test package for you? (hint: I prefer the first solution..:-))
Index: source/smbd/posix_acls.c
===================================================================
--- source/smbd/posix_acls.c (révision 3236)
+++ source/smbd/posix_acls.c (copie de travail)
@@ -732,33 +732,12 @@
}
/****************************************************************************
- Is the identity in two ACEs equal ? Check both SID and uid/gid.
-****************************************************************************/
-
-static bool identity_in_ace_equal(canon_ace *ace1, canon_ace *ace2)
-{
- if (sid_equal(&ace1->trustee, &ace2->trustee)) {
- return True;
- }
- if (ace1->owner_type == ace2->owner_type) {
- if (ace1->owner_type == UID_ACE &&
- ace1->unix_ug.uid == ace2->unix_ug.uid) {
- return True;
- } else if (ace1->owner_type == GID_ACE &&
- ace1->unix_ug.gid == ace2->unix_ug.gid) {
- return True;
- }
- }
- return False;
-}
-
-/****************************************************************************
Merge aces with a common sid - if both are allow or deny, OR the permissions together and
delete the second one. If the first is deny, mask the permissions off and delete the allow
if the permissions become zero, delete the deny if the permissions are non zero.
****************************************************************************/
-static void merge_aces( canon_ace **pp_list_head )
+static void merge_aces( canon_ace **pp_list_head, bool dir_acl)
{
canon_ace *list_head = *pp_list_head;
canon_ace *curr_ace_outer;
@@ -776,12 +755,24 @@
curr_ace_outer_next = curr_ace_outer->next; /* Save the link in case we delete. */
for (curr_ace = curr_ace_outer->next; curr_ace; curr_ace = curr_ace_next) {
+ bool can_merge = false;
curr_ace_next = curr_ace->next; /* Save the link in case of delete. */
- if (identity_in_ace_equal(curr_ace, curr_ace_outer) &&
- (curr_ace->attr == curr_ace_outer->attr)) {
+ /* For file ACLs we can merge if the SIDs and ALLOW/DENY
+ * types are the same. For directory acls we must also
+ * ensure the POSIX ACL types are the same. */
+ if (!dir_acl) {
+ can_merge = (sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
+ (curr_ace->attr == curr_ace_outer->attr));
+ } else {
+ can_merge = (sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
+ (curr_ace->type == curr_ace_outer->type) &&
+ (curr_ace->attr == curr_ace_outer->attr));
+ }
+
+ if (can_merge) {
if( DEBUGLVL( 10 )) {
dbgtext("merge_aces: Merging ACE's\n");
print_canon_ace( curr_ace_outer, 0);
@@ -819,7 +810,7 @@
* we've put on the ACL, we know the deny must be the first one.
*/
- if (identity_in_ace_equal(curr_ace, curr_ace_outer) &&
+ if (sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
(curr_ace_outer->attr == DENY_ACE) && (curr_ace->attr == ALLOW_ACE)) {
if( DEBUGLVL( 10 )) {
@@ -1326,6 +1317,50 @@
}
/****************************************************************************
+ If an ACE entry is SMB_ACL_USER_OBJ and not CREATOR_OWNER, map to SMB_ACL_USER.
+ If an ACE entry is SMB_ACL_GROUP_OBJ and not CREATOR_GROUP, map to SMB_ACL_GROUP
+****************************************************************************/
+
+static bool dup_owning_ace(canon_ace *dir_ace, canon_ace *ace)
+{
+ /* dir ace must be followings.
+ SMB_ACL_USER_OBJ : trustee(CREATOR_OWNER) -> Posix ACL d:u::perm
+ SMB_ACL_USER : not trustee -> Posix ACL u:user:perm
+ SMB_ACL_USER_OBJ : trustee -> convert to SMB_ACL_USER : trustee
+ Posix ACL u:trustee:perm
+
+ SMB_ACL_GROUP_OBJ: trustee(CREATOR_GROUP) -> Posix ACL d:g::perm
+ SMB_ACL_GROUP : not trustee -> Posix ACL g:group:perm
+ SMB_ACL_GROUP_OBJ: trustee -> convert to SMB_ACL_GROUP : trustee
+ Posix ACL g:trustee:perm
+ */
+
+ if (ace->type == SMB_ACL_USER_OBJ &&
+ !(sid_equal(&ace->trustee, &global_sid_Creator_Owner))) {
+ canon_ace *dup_ace = dup_canon_ace(ace);
+
+ if (dup_ace == NULL) {
+ return false;
+ }
+ dup_ace->type = SMB_ACL_USER;
+ DLIST_ADD_END(dir_ace, dup_ace, canon_ace *);
+ }
+
+ if (ace->type == SMB_ACL_GROUP_OBJ &&
+ !(sid_equal(&ace->trustee, &global_sid_Creator_Group))) {
+ canon_ace *dup_ace = dup_canon_ace(ace);
+
+ if (dup_ace == NULL) {
+ return false;
+ }
+ dup_ace->type = SMB_ACL_GROUP;
+ DLIST_ADD_END(dir_ace, dup_ace, canon_ace *);
+ }
+
+ return true;
+}
+
+/****************************************************************************
Unpack a SEC_DESC into two canonical ace lists.
****************************************************************************/
@@ -1454,11 +1489,11 @@
/*
* The Creator Owner entry only specifies inheritable permissions,
* never access permissions. WinNT doesn't always set the ACE to
- *INHERIT_ONLY, though.
+ * INHERIT_ONLY, though.
*/
- if (nt4_compatible_acls())
- psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
+ psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
+
} else if (sid_equal(¤t_ace->trustee, &global_sid_Creator_Group)) {
current_ace->owner_type = GID_ACE;
current_ace->unix_ug.gid = pst->st_gid;
@@ -1467,10 +1502,9 @@
/*
* The Creator Group entry only specifies inheritable permissions,
* never access permissions. WinNT doesn't always set the ACE to
- *INHERIT_ONLY, though.
+ * INHERIT_ONLY, though.
*/
- if (nt4_compatible_acls())
- psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
+ psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
} else if (sid_to_uid( ¤t_ace->trustee, ¤t_ace->unix_ug.uid)) {
current_ace->owner_type = UID_ACE;
@@ -1561,6 +1595,34 @@
}
/*
+ * We have a lossy mapping: directory ACE entries
+ * CREATOR_OWNER ------\
+ * (map to) +---> SMB_ACL_USER_OBJ
+ * owning sid ------/
+ *
+ * CREATOR_GROUP ------\
+ * (map to) +---> SMB_ACL_GROUP_OBJ
+ * primary group sid --/
+ *
+ * on set. And on read of a directory ACL
+ *
+ * SMB_ACL_USER_OBJ ----> CREATOR_OWNER
+ * SMB_ACL_GROUP_OBJ ---> CREATOR_GROUP.
+ *
+ * Deal with this on set by duplicating
+ * owning sid and primary group sid ACE
+ * entries into the directory ACL.
+ * Fix from Tsukasa Hamano <ham...@osstech.co.jp>.
+ */
+
+ if (!dup_owning_ace(dir_ace, current_ace)) {
+ DEBUG(0,("create_canon_ace_lists: malloc fail !\n"));
+ free_canon_ace_list(file_ace);
+ free_canon_ace_list(dir_ace);
+ return false;
+ }
+
+ /*
* If this is not an inherit only ACE we need to add a duplicate
* to the file acl.
*/
@@ -1580,6 +1642,13 @@
* pointer is now owned by the dir_ace list.
*/
current_ace = dup_ace;
+ /* We've essentially split this ace into two,
+ * and added the ace with inheritance request
+ * bits to the directory ACL. Drop those bits for
+ * the ACE we're adding to the file list. */
+ current_ace->ace_flags &= ~(SEC_ACE_FLAG_OBJECT_INHERIT|
+ SEC_ACE_FLAG_CONTAINER_INHERIT|
+ SEC_ACE_FLAG_INHERIT_ONLY);
} else {
/*
* We must not free current_ace here as its
@@ -2062,10 +2131,10 @@
*/
print_canon_ace_list( "file ace - before merge", file_ace);
- merge_aces( &file_ace );
+ merge_aces( &file_ace, false);
print_canon_ace_list( "dir ace - before merge", dir_ace);
- merge_aces( &dir_ace );
+ merge_aces( &dir_ace, true);
/*
* NT ACLs are order dependent. Go through the acl lists and
signature.asc
Description: Digital signature
