[PATCH] central range check for sids2xids

[Michael Adam]

Pushed to autobuild after having received
review+ from Volker and Andreas directly.

Michael

On 2016-08-16 at 13:49 +0200, Michael Adam wrote:
> On 2016-08-10 at 09:25 +0200, Andreas Schneider wrote:
> > On Tuesday, 9 August 2016 18:39:36 CEST Michael Adam wrote:
> > > Hi all,
> > > 
> > > The attached patch introduces a central range check
> > > for the unix ids produced by the id mapping backends
> > > (sids2xids).
> > > 
> > > I noticed that some backends (at least ad and hash),
> > > have no range check any more. This is dangerous
> > > because it can lead to ids leaking out of id-mapping
> > > that are from ranges that this backend is not
> > > responsible for the backward mapping xids2sids
> > > would then lead to a different sid than the one
> > > started with.
> > > 
> > > Instead of adding this to all backends, here is
> > > a patch that adds the check to the central
> > > winbind code.
> > > 
> > > Opinions?
> > 
> > I missed that mail yesterday. Normally a bug should be opened before we create 
> > the master patch so the bug URL is already present. We need this backported!
> > 
> > 
> > Please open a bug for that. Thanks!
> 
> Bug is here: https://bugzilla.samba.org/show_bug.cgi?id=12155
> 
> The minimal patch that fixes this in the central place
> is attached. The problem with the original patch was that
> the idmap_unix_id_is_in_range() function unconditionally
> excluded xid == 0. But in the case of a passdb idmap backend
> (with implicit range 0-0 / unset), this uid must actually
> currently be allowed (AD setup). Hence adapting the
> in_range function first.
> 
> So this patch implements a centra patch but still
> puts it into the idmap child, not in the parent as
> Volker had suggested.
> 
> If we need a minimal bugfix patch now, I'd say this
> is it. I'm currently working on follow-up patches
> that will clean up an refactor.
> 
> First, with the central check, the range checks
> in all the backends can be removed.
> 
> Then some refactoring of the idmap code in winbindd_dual_srv.c .
> Also some correction of treatment of status SOME_UNMAPPED.
> 
> And last, i'm thinking about how to best move the
> checks (and some idmap logic) into the parent.
> But that will be the most extensive reconstructions.
> 
> Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160816/132c16bb/signature.sig>