IDMAP_BOTH support in smbd

[simo]

On Wed, 2012-03-21 at 21:27 +1100, Andrew Bartlett wrote: 
> On Wed, 2012-03-21 at 10:47 +0100, Michael Adam wrote:
> > Hi Andrew,
> > 
> > Andrew Bartlett wrote:
> > > I've been working to slowly move to supporting IDMAP_BOTH in smbd, so as
> > > to allow s3fs to function in all circumstances. 
> > 
> > Can you explain under what circumstances s3fs is working and
> > under which it is not? (Maybe we can discuss this later on irc...)
> 
> No, sadly I do not have a failure case.  I'm working on the basis of
> Metze's comments that IDMAP_BOTH needs to be supported, and just trying
> to methodically work over the stack to allow that, without breaking
> existing behaviour.
> 
> > > Attached are my first two (untested at this time) patches, reworking the
> > > in-memory cache and sids_to_unix_ids routine. 
> > 
> > I will look at those patches more closely and test..
> 
> They currently fail some tests, and I'm continuing to try and sort
> things out. 
> 
> > > From here, I plan to:
> > >  - change the passdb pdb_sid_to_id() call to return a struct unixid
> > > (from idmap.idl)
> > >  - change struct wbcUnixId users (including those I've just added) to
> > > struct unixid, avoiding the union in the libwbclient structure
> > >  - consolidate legacy_sid_to_uid and legacy_sid_to_gid into a single
> > > legacy_sid_to_id() call
> > >  - consolidate sid_to_uid() and sid_to_gid() into a sid_to_id(), which
> > > will be a wrapper around sids_to_unix_ids()
> > 
> > sid_to_uid() and sid_to_gid() are already wrappers around the backend
> > sids_to_unixids() calls, so that should be fairly easy!
> > 
> > Please have a look at idmap_backends_sid_to_unixid() which is
> > called by idmap_sid_to_uid() and idmap_sid_to_gid().
> 
> OK, so where I was caught out is on the smbd side of the winbindd pipe,
> they call down to different wbc calls, and I wasn't able to yet prove
> that the type of ID asked for is always disregarded (particularly in
> terms of not staining the cache with a UID or GID, if the underlying
> backend stored a IDMAP_BOTH).  If this is already handled, then great!

No, IIRC we always ask for a uid or gid type. You would need to add a
new type so that winbindd knows you are asking for the 'both' type.

However you will end up with backends that do not support this new type,
so you need also to find out how to handle the case.

> > >  - consolidate the caching layer in such a way that IDMAP_BOTH can be
> > > supported. 
> > > 
> > > To do this, I will need to determine if the winbindd idmap code is
> > > dependent on the type of ID asked for:  That is, if we move from
> > > wbcSidToUid() to wbcSidsToUnixIds(), will the returned values change at
> > > all?
> > 
> > See the comment above.
> > Also see that we are exposing the plural sids_to_unixids to the
> > winbindd api already and using it in some (not all) places.
> > The server implementation is in
> > source3/winbindd/winbindd_dual_srv.c:_wbint_Sids2UnixIDs()
> 
> Indeed, the fact that this is only used for the logon token processing
> is confusing. 

If you think about it you'll understand it is the only place where you
really _need_ it. In other places it can be a nice to have.

> > I am happy to work with you on the idmap topic as time permits.
> 
> Thanks,
> 
> Andrew Bartlett
> 


-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>