[PATCH] RE: idmap_ad group id mapping.

Nimrod Sapir NIMRODS at il.ibm.com
Wed Sep 12 10:56:29 MDT 2012


Hi Giuseppe

It seems that you have gone further than me into this code flow. Only 
thing I can mention is that if I understand correctly, doing additional 
query against the AD on the idmap_ad.c is probably wasteful, since if I 
understand correctly, the cache will usually already hold all the required 
information (besides the one we are looking for, of course) at that point. 


Thanks
Nimrod Sapir
IBM - XIV, Israel
NAS Development Team
Office: +972-3-689-7763
Cell:   +972-54-7726-320


Giuseppe Ragusa <giuseppe.ragusa at hotmail.com> wrote on 11/09/2012 
19:40:42:

> From: Giuseppe Ragusa <giuseppe.ragusa at hotmail.com>
> To: Nimrod Sapir/Israel/IBM at IBMIL, 
> Cc: Dan Cohen1/Israel/IBM at IBMIL, Lior Chen/Israel/IBM at IBMIL, "samba-
> technical at lists.samba.org" <samba-technical at lists.samba.org>, 
> "samba-technical-bounces at lists.samba.org" <samba-technical-
> bounces at lists.samba.org>
> Date: 11/09/2012 19:40
> Subject: [PATCH] RE: idmap_ad group id mapping.
> 
> Hi Nimrod,
> 
> 
> it's my time being late now: holidays and then other work pressure 
> got in... sorry
> 
> 
> 
> I've spent other time reading the code and I think I found more 
> interesting places in the code than the query_user* functions:
> 
> 
> *) comparing samba 3.4.17 with 3.5.0 I found one whole file that got
> erased (source3/winbindd/winbindd_user.c) and a whole set of files 
> that got in (source3/winbindd/*pw*.c); in the aforementioned 3.4.17 
> file you can find the function getpwsid_sid2gid_recv with an 
> interesting comment starting at line 356 which seems to describe the
> previous (and now desired again) behaviour: because of the whole 
> source restructuring between 3.4 and 3.5+ a reuse of such code seems
> not trivial
> 
> 
> *) one point where our to-be-patched behaviour seems to be coded (if
> I understand correctly) may be in file (as found in 4.0.0beta8) 
> source3/winbindd/wb_fill_pwent.c in function 
> wb_fill_pwent_sid2uid_done on line 79 and following: here (or in the
> function wb_fill_pwent_sid2gid_done underneath) we would have to 
> check the idmap backend in use and its config options, then 
> conditionally call idmap_ad-backend-specific functions to extract 
> the RFC2307/SFU primary gid directly (but it does really not sound 
> "clean" this way...)
> 
> 
> *) source3/winbindd/idmap_ad.c function idmap_ad_unixids_to_sids 
> (line 450 and following) would be a far nicer place to extract 
> RFC2307/SFU primary gid info but we have no way (that I can see) of 
> passing it up from here, since it gets passed only an id_map 
> structure (source3/winbindd/idmap.c function 
> idmap_backends_sid_to_unixid line 533 and following seems to be its 
> calling point and is in turn called from source3/winbindd/
> idmap_util.c function idmap_sid_to_uid line 187 and following)
> 
> 
> *)  I am also still thinking about my previously sketched way of 
> patching which would involve conditionally forcing the primary_gid 
> and group_sid parts of the struct wbint_userinfo to "match" (by 
> means of an additional gid2sid phase) the RFC2307/SFU derived gid 
> and then hope that everything else follows from here, but I 
> obviously need to better understand their full meaning and use before.
> 
> 
> *) Reading through the code I really think that all the caching gets
> along "naturally", so that if we patch at the right spot it should 
> not be a problem (or something that needs special treatment) at all:
> regarding your previous tests, be always sure to start with an empty
> winbind cache (better to stop the daemons and manually remove the 
> tdb cache/mapping files from /var/lib/samba).
> 
> 
> I'm attaching a first mini-patch simply introducing the needed 
> configuration option as discussed before, just to start "coding": it
> has not been tested since it's a GNDN (goes nowhere does nothing) for 
now...
> 
> 
> Any comments/suggestions/pointers on the above from the original 
> authors of the aforementioned Samba code would be much welcome :)
> 
> 
> Thanks,
> Giuseppe

> To: giuseppe.ragusa at hotmail.com
> CC: DANCOH at il.ibm.com; LIORCH at il.ibm.com; samba-
> technical at lists.samba.org; samba-technical-bounces at lists.samba.org
> Subject: RE: idmap_ad group id mapping.
> From: NIMRODS at il.ibm.com
> Date: Sun, 19 Aug 2012 15:32:44 +0300
> 
> Hi Giuseppe 
> 
> Sorry for taking so long to answer - this message got lost somehow 
> in my inbox... 
> 
> I'm not sure I'm following all your ideas below. But my main 
> conclusion after trying to do a quick-and-dirty patch to the code 
> was that the query_user_list and query_user flows in winbind_ads.c 
> are secondary. In most cases of user query, the flow does not reach 
> those functions at all. This is reasonable, since direct query to 
> the AD for a specific entry is very wasteful. So, fixing the 
> winbind_ads.c is important, but partial. After about 4 hours of 
> investigation I wasn't able to fully map the main flow. I would 
> guess that the solution is somewhere in keeping the SFU entries 
> right at the beginning (if they do exist). A better analysis may 
> lead to other conclusions, but I'm quite certain that this would not
> be a simple fix in a specific location, but a change in multiple 
> data structures. 
> 
> Other questions, such as the abstraction and the method of 
> configuration can probably be answered later (although the 
> configuration syntax looks fine by me) - of course, I am in no 
> position to approve or reject changes in the code, so those 
> questions shouldn't be referred to me. 
> 
> 
> Nimrod Sapir 
> IBM - XIV, Israel 
> NAS Development Team 
> Office: +972-3-689-7763 
> Cell:   +972-54-7726-320 
> [image removed] 
> 
> 
> 
> From:        Giuseppe Ragusa <giuseppe.ragusa at hotmail.com> 
> To:        Nimrod Sapir/Israel/IBM at IBMIL, 
> Cc:        "samba-technical at lists.samba.org" <samba-
> technical at lists.samba.org>, <samba-technical-
> bounces at lists.samba.org>, Lior Chen/Israel/IBM at IBMIL, Dan Cohen1/
> Israel/IBM at IBMIL 
> Date:        08/08/2012 18:55 
> Subject:        RE: idmap_ad group id mapping. 
> 
> 
> 
> Hi Nimrod,
> 
> 
> after reading your reply I took a second look at both winbind_ads.c 
> and idmap_ad.c and it seems more reasonable now (maybe only because 
> I accepted the fact that the logic I was searching for results from 
> the "overall behaviour" and not from an explicit single flow of 
> instructions...).
> 
> 
> 
> I know that it's not my confirmation that you were asking for... ;->
> 
> 
> About the tentative development of a patch: 
> 
> 
> *) did you find any place in the code where the user-info cache is 
> purposely built-up or do you think that it will simply be affected 
> by modifying only the query_user_list and query_user functions in 
> winbind_ads.c?
> 
> 
> *) do you agree on (conditionally) replacing the references (in the 
> two aforementioned winbind_ads.c functions) to the Windows-side 
> primary group with the RFC2307/SFU specified group (through a 
> reverse resolution from UNIX gidNumber to Windows SID etc.)?
> 
> 
> *) do you think that the "primary group implicitly defined as top of
> groups list" logic requires us to (conditionally) modify the 
> functions that build up such a list by injecting the RFC2307/SFU 
> specified group as top of list (hoping that the cache comes along as
> hinted above)? 
> 
> *) do you think that it's reasonable/good-taste adding such 
> (conditional) idmap_ad related logic directly into winbind_ads.c or 
> would it be better to abstract it (and demand the actual code to the
> idmap_*.c plugins)?
> 
> 
> *) do you agree on the syntax of my proposed additional 
> configuration parameter to control such behaviour?
> 
> 
> Maybe if we start coding something, then expert feedback/hints will 
> come along... :-)
> 
> 
> Just to be clear, the logic I would like to code would rely on the 
> AD admins to add a "regular" Windows-side security group complete 
> with an RFC2307/SFU gidNumber matching that which would be in single
> users' RFC2307/SFU attributes; actual Windows-side membership of the
> (member) users into such (UNIX-side primary) group would better be 
> non-mandatory (to alleviate administrative burden), but this is 
> something that will hopefully become clear once actual coding will 
begin.
> 
> 
> Many thanks again,
> Giuseppe
> 
> 
> 
> 
> To: giuseppe.ragusa at hotmail.com
> CC: samba-technical at lists.samba.org; samba-technical-
> bounces at lists.samba.org; LIORCH at il.ibm.com; DANCOH at il.ibm.com
> Subject: Re: idmap_ad group id mapping.
> From: NIMRODS at il.ibm.com
> Date: Wed, 8 Aug 2012 16:15:25 +0300
> 
> Hi Giuseppe 
> 
> I dived into the code myself before writing my previous post. While 
> I found a flow in winbind_ads.c which does a query on the primary 
> group for getting the gid (and can maybe be replaced with querying 
> for the direct gid entry), it seems that this flow is a secondary 
> flow. The main flow, if I understand correctly, revolves around the 
> caching kept in the netsamlogon_cache.tdb. I believe that each time 
> a user connects to the server, Samba gets the entire user 
> information and keeps it in the netsamlogon_cache.tdb file. This is 
> where, in most flows, the primary group info comes from. Actually, 
> Samba does not need to query the primary group directly, since the 
> primary group is always the first group in the list of groups. Since
> this information is distributed across a large number of data 
> structures and flows, the fix becomes non-trivial. Also, the primary
> group and primary group/gid are not in the same format - primary 
> group is a SID, while primary group/gid may contain a GID which is 
> not mapped to any AD group. Of course, the primary group/gid is not 
> always available, so the system needs to verify it's existence (and 
> the ID mapping schema in use) before using this entry rather than 
> the primary group entry. 
> 
> Of course, my understanding is very partial and based on a few hours
> of code review without any previous understanding of the above 
> mechanisms. I would appreciate a feedback on what I just said 
> 
> Thanks. 
> Nimrod Sapir 
> IBM - XIV, Israel 
> NAS Development Team 
> Office: +972-3-689-7763 
> Cell:   +972-54-7726-320 
> [image removed] 
> 
> samba-technical-bounces at lists.samba.org wrote on 08/08/2012 12:25:18:
> 
> > From: Giuseppe Ragusa <giuseppe.ragusa at hotmail.com> 
> > To: "samba-technical at lists.samba.org" 
<samba-technical at lists.samba.org>, 
> > Date: 08/08/2012 12:25 
> > Subject: idmap_ad group id mapping. 
> > Sent by: samba-technical-bounces at lists.samba.org 
> > 
> > 
> > Sorry for replying to myself:
> > I tried (again) to compare latest Samba sources (from beta5) to 3.4.
> > 17 in the source3/winbindd subdir but I was not able to find the 
> > right spot to start working on a patch (btw: was the suggested 
> > parameter/logic sound?).
> > Even looking directly into the winbind_ad.c and idmap_ad.c files (as
> > suggested by Nimrod) I do not seem to find where the current logic 
> > (take uidNumber from user and extract gidNumber from Windows-side 
> > primary group - if not found, "give up" on user) resides.
> > I think I understand the main abstraction layers (the _backends_ 
> > functions and their backend-specific counterparts), but somewhere 
> > around the pull_uint32 and _cached_ function calls I got lost.
> > Could anyone knowing the implementation suggest me where to start 
> > from (or relevant docs to read)?
> > Many thanks again,Giuseppe
> > > Hi all,
> > > 
> > > I think that the behaviour described by Nimrod would be really 
useful,
> > > especially in large organizations with new users assigned to 
> "Domain Users"
> > > by default as primary group (Windows-side), POSIX attributes 
> "granted" only
> > > for a limited number of users/groups in an "experimental way" 
> and no hope of
> > > getting the gidNumber POSIX attribute into such a fundamental 
> Windows group
> > > as "Domain Users" by the powers that be.
> > > 
> > > I think that a similar reasoning was already explained on this 
> > list before and
> > > it is my understanding that the described behaviour was the 
> default in Samba
> > > releases < 3.5.0:
> > > 
> > > https://bugzilla.samba.org/show_bug.cgi?id=8694
> > > 
> > > https://bugzilla.samba.org/show_bug.cgi?id=7582
> > > 
> > > https://bugzilla.samba.org/show_bug.cgi?id=8905
> > > 
> > > I agree that, apart from primary group, membership in other groups
> > > should be governed by the "Windows-side" membership logic (since I
> > recall from
> > > the aforementioned list discussions and other Samba doc sources 
> > that the "pure"
> > > RFC2307bis group membership mechanisms are ill-implemented in AD 
> > and generally
> > > not a viable solution): it seems sensible that groups that are 
> > vital "UNIX-side"
> > > (mainly for filesystem ACEs/ownership or similar authorization 
> > reasons) should
> > > have their gidNumber set in AD.
> > > 
> > > To be clear I'm thinking of an additional parameter like:
> > > 
> > > idmap config DOMAINNAME : primary_group_mode = rfc2307
> > > 
> > > vs.
> > > 
> > > idmap config DOMAINNAME : primary_group_mode = ad
> > > 
> > > with the latter being the default if not specified (for 
> continuity with the
> > > latest versions of Samba). I suppose that "sfu" instead of 
> > "rfc2307" could be
> > > used by those using the corresponding schema_mode.
> > > 
> > > I have a use case exactly like this and would like to contribute to 
the
> > > development of such a patch, even if my previous coding/
> inspecting attempts
> > > (by means of comparing Samba sources for idmap_ad pre and post 
> 3.5.0) failed
> > > miserably; I would dare to suggest that such a patch would be useful 
even
> > > for the still maintained branches of 3.5.x and 3.6.x, once 
> > established in master
> > > (I for one use CentOS 6 and cannot use SSSD nor FreeIPA for the 
> > purpose of AD
> > > integration: submitting the patch to RedHat as an RFE for Samba 
> 3.5.x would
> > > arguably be easier/require the patch accepted upstream).
> > > 
> > > Many thanks,
> > > Giuseppe
> > > 
> > > Nimrod Sapir <NIMRODS at il.ibm.com> wrote on Wed Aug 1 09:59:59MDT 
2012:
> > > 
> > >> Hi Adam
> > >>
> > >> I'm not sure I understand all your comments below. It still seems 
to me 
> > >> that for a customer using SFU for ID mapping, the consistent 
> and intuitive 
> > >> way to determine the GID for a AD account which has UID in the 
schema is 
> > >> the Primary group name/GID entry. When looking at the UNIX 
> attributes tab 
> > >> it is very obvious that this field is meant by Microsoft to be 
> used as the 
> > >> matching GID for the UID. Also, the way it is defined enforces 
> the user to 
> > >> provide GID to every UID, either by choosing a group which has 
> GID, or by 
> > >> adding it manually to the schema. 
> > >>
> > >> Yesterday, I dived into the code trying to understand how this 
patch can 
> > >> be done. The fix doesn't seem to be trivial at all, as the flow
> that leads 
> > >> from the first AD query of the user to the SID->GID resolution in 
AD is 
> > >> quite complicated. I added some hacks to winbind_ads.c and 
> idmap_ad.c, but 
> > >> with partial success. adding the "winbind nss info = rfc2307" did 
change 
> > >> the flow a bit, but eventually the smbd process runs using the 
> gid of the 
> > >> primary group rather than the gid written in the Primary group 
name/GID 
> > >> field. 
> > >>
> > >> Do you think that the change described is of interest to the 
> community? Do 
> > >> you have any suggestion regarding the easiest way to change the 
flow to 
> > >> match this behavior?
> > >>
> > >> Thanks
> > >> Nimrod
> > >>
> > >>
> > >>
> > >> Michael Adam <obnox at samba.org> wrote on 12/07/2012 17:48:42:
> > >>
> > >>> From: Michael Adam <obnox at samba.org>
> > >>> To: Nimrod Sapir/Israel/IBM at IBMIL, 
> > >>> Cc: Dan Cohen1/Israel/IBM at IBMIL, Lior Chen/Israel/IBM at 
> IBMIL, samba-
> > >>> technical <samba-technical at lists.samba.org>
> > >>> Date: 12/07/2012 17:48
> > >>> Subject: Re: idmap_ad group id mapping.
> > >>> 
> > >>> Nimrod Sapir wrote:
> > >>>> Michael Adam <obnox at samba.org> wrote on 12/07/2012 01:20:57:
> > >>>>> 
> > >>>>> Yes, this is actually how it should work:
> > >>>>> Samba takes the windows user token and turns it into
> > >>>>> a unix token. Here the expected thing is to turn the windows
> > >>>>> groups into unix groups (by id mapping) one-to-one.
> > >>>>> 
> > >>>>> I would say that the windows admins should give the
> > >>>>> user a primary (windows) group that also carries a gidnumber
> > >>>>> unix attribute. I can't see why a windows admin would give
> > >>>>> the user a primary windows group (maybe w/o gid number) and
> > >>>>> primary gid number in the unix attributes that refers to a
> > >>>>> different windows group or to no windows group at all.
> > >>>>> 
> > >>>>> But it seems to be a rather frequent request.
> > >>>>> If it is really important, then we could make it configurable
> > >>>>> to let samba choose the primary gid from the windows user
> > >>>>> sfu attributes as the unix primary gid.
> > >>>> 
> > >>>> I would say that the existing behavior is reasonable (as well 
> > as expecting 
> > >>>> the user to enforce the gid value of the primary group) if 
> the "primary 
> > >>>> group name/GID" field was not there, right below the UID 
> field. I, as a 
> > >>>> user, was sure that this field would determine the GID. I 
> > believe this is 
> > >>>> also what Microsoft expect from systems which are using this 
scheme 
> > >>>> (otherwise, why is it there?),
> > >>> 
> > >>> Well, I think the primary use of this is Unix/NFS interaction.
> > >>> Also, from Windows 2003R2 on, the schema extensions are part of
> > >>> the so called "services for NFS"...
> > >>> 
> > >>> http://technet.microsoft.com/en-us/library/cc782783%28v=ws.10%29
> > >>> 
http://technet.microsoft.com/en-us/library/cc753302%28v=ws.10%29.aspx
> > >>> 
> > >>> This is meant for systems that unlike samba/winbindd don't do
> > >>> an id mapping of the windows SIDs themselves.
> > >>> 
> > >>>> and from the perspective of a customer which has large Active
> > >>>> Directory, and want to allocate different GID to different
> > >>>> users, the existing behavior is error-prone while the second
> > >>>> approach ensures consistency.
> > >>> 
> > >>> The point is that the samba server is more on the windows side
> > >>> than on the unix side, from the windows clients' point of view.
> > >>> So we should by default map the windows world to the unix world
> > >>> as 1:1 as possible. We can optionally add in the primary gid
> > >>> from the unix attributes in the idmap_ad scenario. But what
> > >>> relevance would the primary windows group have, if it is also
> > >>> mapped to a GID?
> > >>> 
> > >>> Difficult.
> > >>> 
> > >>> There is by the way already a mechanism for choosing the
> > >>> gid from idmap_ad: you need to configure the nss_info
> > >>> correspondingly. Set
> > >>> 
> > >>> "winbind nss info = sfu"
> > >>> 
> > >>> in addition to your idmap config (or = rfc2307, you have to 
check).
> > >>> Please read the "smb.conf" manpage for more background.
> > >>> 
> > >>> Cheers - Michael
> > >>> 
> > >>> 
> > >>> [attachment "atti45z3.dat" deleted by Nimrod Sapir/Israel/IBM] 
> > 
> > 
> > 
> > 
> [attachment "ATT00001" deleted by Nimrod Sapir/Israel/IBM] 
> [attachment "samba-4.0.0beta8-idmap_ad_pgroup.patch" deleted by 
> Nimrod Sapir/Israel/IBM] 
> [attachment "ATT00001" deleted by Nimrod Sapir/Israel/IBM] 
> [attachment "ATT00002" deleted by Nimrod Sapir/Israel/IBM] 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 1338 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120912/9a354116/attachment.gif>


More information about the samba-technical mailing list