[Patch] Add an idmap implementation to winbind

simo idra at samba.org
Thu Feb 14 16:12:23 GMT 2008


On Thu, 2008-02-14 at 12:45 +0100, Kai Blin wrote:
> Hi folks,
> 
> the attached patches allow S4 winbind to do sid<->uid/gid mappings, storing 
> the mappings in a database. I have also switched internal callers of the old 
> dsdb/common/sidmap.c code over to the winbind/idmap.c code.
> 
> This allows us to get rid of the unixName: attributes in the samdb.
> 
> In the long run, the callers should be converted to using the winbind SID2UID 
> etc calls. We will also want to supply a way to import and export mappings in 
> a way compatible to S3, maybe via net idmap dump/restore as well.
> 
> These patches add one of the pieces missing from a full featured member mode 
> for Samba4.
> 
> Comments?
> 
> Kai
> 
> PS: If you'd rather read code than patches,[1] also has these.
> 
> [1]http://gitweb.samba.org/?p=kai/samba/kai-work-in-progress.git;a=shortlog;h=idmap

Reading the first patch I see you re-introduced idmap uid and idmap gid
ranges in smb.conf, please don't do that, as this is a new
implementation anyway, please keep the ranges in the database itself.
Also I'd suggest to only use one range for both uid and gid, the added
flexibility is not worth the confusion imo.

Also I would not introduce the idmap database, why can't we put this
stuff into sam.ldb?
Eventually as a separate partition?
We have an LDAP server which we can use to share mappings easily between
different servers now, why not using it by default ?


Reading the fourth patch it appears like you are using your functions in
a set of composite functions, this means that you are introducing
blocking synchronous calls (gendb_search) in an a supposedly async set
of calls, not good.


Reading the fifth patch I see no call to validate a SID before consuming
a uid/gid to make a mapping. This means someone can simply query for N
non existing SIDs and deplete the given range (DoS).

Also the high watermark is simply replaced, not deleted and added, this
means in theory 2 concurrent process can allocate the samba uid/gid to 2
different SIDs and never notice, as the high watermark update is not
atomic. Transactions are not used either so there is no way to detect it
later and rollback.


Simo.

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



More information about the samba-technical mailing list