[PATCH] RE: idmap_ad group id mapping.
Giuseppe Ragusa
giuseppe.ragusa at hotmail.com
Tue Sep 11 10:40:42 MDT 2012
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
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
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:59 MDT 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]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba-4.0.0beta8-idmap_ad_pgroup.patch
Type: application/octet-stream
Size: 1998 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120911/b1308fb4/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ATT00001
Type: image/gif
Size: 1338 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120911/b1308fb4/attachment.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ATT00002
Type: image/gif
Size: 1338 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120911/b1308fb4/attachment-0001.gif>
More information about the samba-technical
mailing list