[PATCH] central range check for sids2xids

[Jeremy Allison]

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 ?

In the meantime, pushed.

> From 1e16a960e0c33a27dc5f224538ec5e67f1975471 Mon Sep 17 00:00:00 2001
> From: Michael Adam <obnox at samba.org>
> Date: Tue, 9 Aug 2016 18:25:12 +0200
> Subject: [PATCH] idmap: centrally check that unix IDs returned by the idmap
>  backends are in range
> 
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
>  source3/winbindd/winbindd_dual_srv.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c
> index fb65e9d..0484e19 100644
> --- a/source3/winbindd/winbindd_dual_srv.c
> +++ b/source3/winbindd/winbindd_dual_srv.c
> @@ -189,6 +189,10 @@ NTSTATUS _wbint_Sids2UnixIDs(struct pipes_struct *p,
>  	for (i=0; i<num_ids; i++) {
>  		struct id_map *m = id_map_ptrs[i];
>  
> +		if (!idmap_unix_id_is_in_range(m->xid.id, dom)) {
> +			m->status = ID_UNMAPPED;
> +		}
> +
>  		if (m->status == ID_MAPPED) {
>  			ids[i].xid = m->xid;
>  		} else {
> -- 
> 2.5.5
>