[PATCH] Check if the idmap_hash range is big enough

Tue Feb 21 01:10:17 UTC 2017

On 2017-02-20 at 11:51 +0100, Andreas Schneider wrote:
> On Friday, 17 February 2017 18:44:34 CET Michael Adam wrote:
> > > -	idmap config * : range = 1000-4000000000
> > > +	idmap config * : range = 100000-4000000000
> > 
> > If you want to catch as much as possible of a domain
> > that gets hashed to 0, then the lower bound needs to
> > be as low as possible, hence the 1000. But then,
> > the first 1000 rids in a domain will be used as well,
> > and hence why not skip this first range entirely
> > and start at 500000 ? ;-)
> 
> Ok, lets start there. I think we should suggest 525000 that is big enough and 
> easy to deal with for our users.
> 
> > 
> > So:
> > 
> > - the idmap hash module, when used for "idmap config *",
> >   ideally should have the full range of
> >   0 - 2147483648 which is not quite possible (at the low
> >   end at least)...
> 
> The best is to start with 500000. 1000 is normally the start for local users.

Now what? 500000 or 525000 ? :-)

> See attached patchset.
> 
> 
> 	Andreas
> 
> 
> -- 
> Andreas Schneider                   GPG-ID: CC014E3D
> Samba Team                             asn at samba.org
> www.samba.org

> From c0f379a680613fdb28a23d0cf2e3ed9ace260fd7 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 15 Feb 2017 08:55:24 +0100
> Subject: [PATCH 1/2] docs: Improve the idmap_hash manpage
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12582
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  docs-xml/manpages/idmap_hash.8.xml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/docs-xml/manpages/idmap_hash.8.xml b/docs-xml/manpages/idmap_hash.8.xml
> index 9f4f1d1933c..a9230498efe 100644
> --- a/docs-xml/manpages/idmap_hash.8.xml
> +++ b/docs-xml/manpages/idmap_hash.8.xml
> @@ -24,6 +24,11 @@
>  	  to support a local name mapping files if enabled via the
>  	  "winbind normalize names" and "winbind nss info"
>  	  parameters in smb.conf.
> +	  The module divides the range into subranges for each domain that is being
> +	  handled by the idmap config.
> +	  Each range has a size of roughly 525000 IDs (20 bit). This means
> +	  that the range for multiple domains needs to be large enough! So a good value
> +	  is normally '100000-4000000' or even bigger.

That's not the main point.
It's not that you need a couple of those ranges of size ~ 525000
in order to accomodate a few domains.  You need *them all*
because each domain has a fixed absolute range associated to it
by the hashing algorithm, and you don't know a priori which domain
will come by...

So no, the above range is not normally a good value,
since those almost 8 ranges out of the more than 4000
ranges that exist, are likely not among those needed
for the domains that enter the system...

(apart from this, the low id of 100000 seems to contradict your
mention of 525000 above...)

>  	</para>
>  </refsynopsisdiv>
>  
> @@ -53,7 +58,7 @@
>  	<programlisting>
>  	[global]
>  	idmap config * : backend = hash
> -	idmap config * : range = 1000-4000000000
> +	idmap config * : range = 100000-4000000000
>  
>  	winbind nss info = hash
>  	winbind normalize names = yes
> -- 
> 2.11.0
> 
> 
> From 7aaeb3f0f5add14106731290e1ad70d84dc7f6a6 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Mon, 20 Feb 2017 11:44:22 +0100
> Subject: [PATCH 2/2] idmap_hash: Make sure the idmap range is big enough
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12582
> 
> Signed-off-by: Andreas Schneider <asn at samba.org>
> ---
>  source3/winbindd/idmap_hash/idmap_hash.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/source3/winbindd/idmap_hash/idmap_hash.c b/source3/winbindd/idmap_hash/idmap_hash.c
> index 743b0ec4ff8..aa3836c0727 100644
> --- a/source3/winbindd/idmap_hash/idmap_hash.c
> +++ b/source3/winbindd/idmap_hash/idmap_hash.c
> @@ -119,6 +119,14 @@ static NTSTATUS idmap_hash_initialize(struct idmap_domain *dom)
>  		return NT_STATUS_INVALID_PARAMETER;
>  	}
>  
> +	if ((dom->high_id - dom->low_id) < 524288) {
> +		DBG_ERR("Error: The idmap_hash range configured for domain "
> +			"'%s' is too small! Please consult the manpage of "
> +			"the idmap_hash module.\n",
> +			dom->name);
> +		return NT_STATUS_INVALID_PARAMETER;
> +	}

Again, this misses the main point, because the hash ranges
are determined absolutely, and not relative to configured
idmap ranges:

Yeah, it's right that a range this small can't even accomodate
a single domain, but even if we are just big enough for one
range, this is likely not an entire range (but starting in the
middle of one range and ending in the middle of the next one),
and even if one full range is included the likelyhood that
it will be this one range that is used by the domain of users
logging on to the samba server is extremely low...

I am really sorry to be coming across so negatively.
I would like to have a better answer, but currently
I only know what is NOT sufficient or completely good... :-/

Michael

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