[PATCH] central range check for sids2xids

[Michael Adam]

On 2016-08-09 at 16:35 -0700, Jeremy Allison wrote:
> On Tue, Aug 09, 2016 at 06:39:36PM +0200, 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?
> 
> Looks correct to me. I looked into:
> 
> source3/winbindd/idmap_ldap.c
> source3/winbindd/idmap_rfc2307.c
> source3/winbindd/idmap_rid.c
> source3/winbindd/idmap_script.c
> source3/winbindd/idmap_tdb2.c
> source3/winbindd/idmap_tdb_common.c
> 
> and they all do this. I also noticed that
> idmap_unix_id_is_in_range() is checked by most
> of these in the unixid_to_sid_fn backends
> too. Should that be done as a follow-up
> patch ?

Well, in the xid2sid case, it is already
done centrally, but one level higher, in
source3/winbindd/wb_xids2sids.c

Usually, for the xids2sids call it is an
entry level check, while for the sids2xids call,
it is an exit level check, verifying the result.

Interestingly, the wb_xids2sids.c code
checks at both entry and exit, because it
disssects the list of ids into domain-specific
ids according to ranges and later copies the
results back into original order.

The entry level check could also be done
on the dual_srv level, in order to make it
more symmetric. In that case it would be put
to idmap_backend_unixids_to_sids(), but since
we can't spare the check in wb_xids2sids, I
thought it is not strictly necessary.

But thinking about it, it might make things
more robust and symmetric, and obvious...

> In the meantime, pushed.

Thanks!

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/20160810/f1c68d96/signature.sig>