[Patch] allow autorid to create a new domain range if the parent already validated the sid (bug #12613)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[Patch] allow autorid to create a new domain range if the parent already validated the sid (bug #12613)

Stefan Metzmacher-2
Hi,

here's a patch for https://bugzilla.samba.org/show_bug.cgi?id=12613

This solved the case where wbinfo --user-sids is called with a sid
of a domain of which no user has authenticated itself, so we don't
have a domain stamp in netsamlogon_cache.tdb yet for the domain.
If autorid.tdb also doesn't have a mapping for that domain yet,
idmap_autorid will refuse to map the sid.

Currently the the parent winbindd process already did a lookup_sid
in order to work out the type (user or group), this hint is then passed
to the idmap backends.

I already checked that wbinfo --user-sids with sid that doesn't exists
(via lookup_sids) will results in ID_TYPE_NOT_SPECIFIED being passed
to the idmap child, if it exists we pass ID_TYPE_UID or ID_TYPE_GID.

In future, when we try to avoid the lookup_sids call completely
because we use a idmap backend with ID_TYPE_BOTH support,
we can pass ID_TYPE_BOTH instead of ID_TYPE_NOT_SPECIFIED
if the callers already knows about the domain sid (in the domain list).

Please review and push:-)

Thanks!
metze

tmp.diff.txt (1K) Download Attachment
signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [Patch] allow autorid to create a new domain range if the parent already validated the sid (bug #12613)

Jeremy Allison
On Tue, Mar 07, 2017 at 12:17:34PM +0100, Stefan Metzmacher wrote:

> Hi,
>
> here's a patch for https://bugzilla.samba.org/show_bug.cgi?id=12613
>
> This solved the case where wbinfo --user-sids is called with a sid
> of a domain of which no user has authenticated itself, so we don't
> have a domain stamp in netsamlogon_cache.tdb yet for the domain.
> If autorid.tdb also doesn't have a mapping for that domain yet,
> idmap_autorid will refuse to map the sid.
>
> Currently the the parent winbindd process already did a lookup_sid
> in order to work out the type (user or group), this hint is then passed
> to the idmap backends.
>
> I already checked that wbinfo --user-sids with sid that doesn't exists
> (via lookup_sids) will results in ID_TYPE_NOT_SPECIFIED being passed
> to the idmap child, if it exists we pass ID_TYPE_UID or ID_TYPE_GID.
>
> In future, when we try to avoid the lookup_sids call completely
> because we use a idmap backend with ID_TYPE_BOTH support,
> we can pass ID_TYPE_BOTH instead of ID_TYPE_NOT_SPECIFIED
> if the callers already knows about the domain sid (in the domain list).
>
> Please review and push:-)

Went through it carefully and think I now understand it :-).

LGTM. Pushed !

> From 8f9c2c00913986f4730a577029c59f96f882be1a Mon Sep 17 00:00:00 2001
> From: Stefan Metzmacher <[hidden email]>
> Date: Mon, 6 Mar 2017 11:53:09 +0000
> Subject: [PATCH] idmap_autorid: allocate new domain range if the callers knows
>  the sid is valid
>
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12613
>
> Signed-off-by: Stefan Metzmacher <[hidden email]>
> ---
>  source3/winbindd/idmap_autorid.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/source3/winbindd/idmap_autorid.c b/source3/winbindd/idmap_autorid.c
> index 786f839..ab89d35 100644
> --- a/source3/winbindd/idmap_autorid.c
> +++ b/source3/winbindd/idmap_autorid.c
> @@ -636,6 +636,19 @@ static NTSTATUS idmap_autorid_sid_to_id(struct idmap_tdb_common_context *common,
>   }
>  
>   /*
> + * If the caller already did a lookup sid and made sure the
> + * domain sid is valid, we can allocate a new range.
> + *
> + * Currently the winbindd parent already does a lookup sids
> + * first, but hopefully changes in future. If the
> + * caller knows the domain sid, ID_TYPE_BOTH should be
> + * passed instead of ID_TYPE_NOT_SPECIFIED.
> + */
> + if (map->xid.type != ID_TYPE_NOT_SPECIFIED) {
> + goto allocate;
> + }
> +
> + /*
>   * Check of last resort: A domain is valid if a user from that
>   * domain has recently logged in. The samlogon_cache these
>   * days also stores the domain sid.
> --
> 1.9.1
>