Bug 1139 - ACL create_canon_ace_lists fails after NT user migration
Summary: ACL create_canon_ace_lists fails after NT user migration
Status: CLOSED FIXED
Alias: None
Product: Samba 3.0
Classification: Unclassified
Component: User/Group Accounts (show other bugs)
Version: 3.0.2
Hardware: All All
: P3 normal
Target Milestone: none
Assignee: Gerald (Jerry) Carter (dead mail address)
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-02-29 01:25 UTC by Sebastian Hetze
Modified: 2005-08-24 10:18 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sebastian Hetze 2004-02-29 01:25:55 UTC
How to reproduce that bug:

After migrating users from NT4 to samba you get lots of RIDs that
do not match the rid algorithm. As one such user, prefereably one
with an odd RID, create a new file on some samba share with Linux
ACL enabled. Now open the Properties->Security->??? dialog
(Eigenschaften->Sicherheit->Berechtigungen in German)
and change anything. Add write permission to everyone, for example.
Now take a look at that file in the Linux filesystem, specially
the ACL on that file. The owner has lost write permission and
some group has got full access instead.
The GID of this (possible not even existing) group is exactly
the result of the RID algorithm calculation.

What is happening?:

My brief investigations indicate that the function
create_canon_ace_lists() from posix_acls.c calls both sid_to_gid()
and sid_to_uid() in turn with the same SID just to try if it matches
in one case or the other. Unfortunately, sid_to_gid() falls back to
algorithmic mapping and in the case shown above it succeeds to
calculate a gid out of the migrated users RID.
Comment 1 Ed Ravin 2004-03-26 21:36:25 UTC
we encountered this at my shop - it happens with odd-numbered SIDs.  My 
colleague jdev@panix.com says it's due to a typo in posix_acls.c, this should 
fix it (patch against 3.0.2a):

--- source/smbd/posix_acls.c.orig       Fri Mar 26 18:40:20 2004
+++ source/smbd/posix_acls.c    Fri Mar 26 18:40:48 2004
@@ -1365,12 +1365,12 @@
                        if (nt4_compatible_acls())
                                psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
 
-               } else if (NT_STATUS_IS_OK(sid_to_gid( &current_ace->trustee, 
&current_ace->unix_ug.gid))) {
-                       current_ace->owner_type = GID_ACE;
-                       current_ace->type = SMB_ACL_GROUP;
                } else if (NT_STATUS_IS_OK(sid_to_uid( &current_ace->trustee, 
&current_ace->unix_ug.uid))) {
                        current_ace->owner_type = UID_ACE;
                        current_ace->type = SMB_ACL_USER;
+               } else if (NT_STATUS_IS_OK(sid_to_gid( &current_ace->trustee, 
&current_ace->unix_ug.gid))) {
+                       current_ace->owner_type = GID_ACE;
+                       current_ace->type = SMB_ACL_GROUP;
                } else {
                        fstring str;
Comment 2 Andrew Bartlett 2004-03-27 07:13:36 UTC
No, that just means that it will affect even-numbered RIDs, which are groups.

The issue is that we need to consult all mapping tables before we try algoritmic
mappings.

Andrew Bartlett
Comment 3 Jed Davis 2004-03-30 13:52:02 UTC
[Copied from my post to the internals mailing list]

I don't see anything in sid_to_uid or local_sid_to_uid that will do
any kind of fallback if a local SID isn't in the passdb.  If the call
to pdb_getsampwuid fails, local_sid_to_uid returns False to
sid_to_uid, which then returns NT_STATUS_UNSUCCESSFUL.

The _to_gid equivalents, however, do fall back on algorithmic mapping
for any local SID with an odd RID, which I assume is because groups
don't need any special registration with Samba the way users do, and
thus it makes sense to implicitly map them back and forth.

So, unless I've missed something, swapping the calls in
create_canon_ace_lists shouldn't create any new bugs while fixing thus
one.

To test this, I created a group with `net groupmap` having RID 2094,
which corresponds algorithmically to uid 547, which belongs to an
account that was imported from Windows, and thus isn't using that RID.
The associated Unix group is 77 (pcap), which I picked for no
particular reason from /etc/group.  I then added an ACL entry for that
group to a file from a Windows client (with the permissions GUI),
while running the modified smbd with -d 10.  The result:

[2004/03/29 14:57:50, 10] passdb/lookup_sid.c:sid_to_uid(396)
  sid_to_uid: my domain (S-1-5-21-4061480941-3245480631-1304912463-2094) -
trying local.
{...}
[2004/03/29 14:57:50, 5] passdb/pdb_tdb.c:tdbsam_getsampwrid(327)
  pdb_getsampwrid (TDB): error looking up RID 2094 by key RID_0000082e.
   Error: Record does not exist
{...}
[2004/03/29 14:57:50, 8] passdb/passdb.c:local_sid_to_uid(1164)
  local_sid_to_uid: Could not find SID
S-1-5-21-4061480941-3245480631-1304912463-2094 in passdb
[2004/03/29 14:57:50, 10] passdb/lookup_sid.c:sid_to_uid(401)
  sid_to_uid: local lookup failed
[2004/03/29 14:57:50, 3] passdb/lookup_sid.c:fetch_gid_from_cache(256)
  fetch uid from cache 77 -> S-1-5-21-4061480941-3245480631-1304912463-2094
{Is this^^^ a typo?}
[2004/03/29 14:57:50, 10] smbd/posix_acls.c:create_canon_ace_lists(1488)
  create_canon_ace_lists: adding file ACL:
  canon_ace index 0. Type = allow SID =
S-1-5-21-4061480941-3245480631-1304912463-2094 gid 77 (pcap) SMB_ACL_GR\
OUP perms r-x

The user lookup fails and no fallback is done; group lookup is done
instead, and returns the correct group.
Comment 4 Jeremy Allison 2004-04-05 07:37:47 UTC
Ok, this looks workable. Of course the long term fix is to tidy up the way
algorithmic mapping is done, but that's a task for another day....
Jeremy.
Comment 5 Gerald (Jerry) Carter (dead mail address) 2005-08-24 10:18:45 UTC
sorry for the same, cleaning up the database to prevent unecessary reopens of bugs.