[PATCH] Check idmap config with testparm

Thu Dec 8 10:10:46 UTC 2016

On Thursday, 8 December 2016 10:44:16 CET Michael Adam wrote:
> On 2016-12-08 at 09:20 +0100, Andreas Schneider wrote:
> > On Thursday, 8 December 2016 08:42:37 CET Michael Adam wrote:
> > > On 2016-12-07 at 18:43 +0100, Andreas Schneider wrote:
> > > > Hello,
> > > > 
> > > > you might know I work for a Distributor and fix winbind issues there
> > > > every
> > > > day.
> > > > I see so many invalid idmap configurations, I think 70% of the configs
> > > > are
> > > > wrong or invalid.
> > > > 
> > > > In addition our documentation for ID mapping really sucks!
> > > 
> > > Hmm, I take this a little bit as a personal affront.
> > > Let me reply with a similar non-diplomatic statement:
> > > 
> > > 
> > > People should learn to read! :-)
> > > 
> > > 
> > > Have you read the section about "idmap config DOMAIN : OPTION" in
> > > "man smb.conf" and the backend specific manpages?
> > > 
> > > Among other things, smb.conf clearly states:
> > >   "The first three of these [idmap_tdb, idmap_tdb2, idmap_ldap]
> > >   create mappings of their own using internal unixid counters and
> > >   store the mappings in a database.  These are suitable for use in
> > >   the default idmap configuration."
> > 
> > I do read those things but our customers don't. So should we abort if
> > something else than these backends are used for the default domain?
> 
> Ideally, the idmap backend code itself should have functions
> to be called upon init and refuse loading... The knowledge
> should be inside the module.
> 
> So there is the thing:
> 
> * some modules can *only safely be used in non-* config:
>   I think these are:
> 
>   idmap_rid, idmap_ad, idmap_rfc2307
> 
> * some modules can safely and reasonably be used in
>   default and per-domain configs:
> 
>   idmap_tdb, idmap_tdb2, idmap_ldap
> 
> * some moules make sense only in default, really:
> 
>   idmap_autorid
> 
> * some modules can be used in default and per-domain
>   config, but default has limited functionality:
> 
>   idmap_nss, idmap_hash, idmap_script
> 
> > Simply do not start winbind ...
> > 
> > > As well as:
> > >   "The configured ranges must be mutually disjoint."
> > > 
> > > Also, for further examples, reading the manpages of idmap_rid,
> > > 
> > > I see:
> > >   "One usually needs to define a writeable default idmap range,
> > >   using a backend like tdb or ldap that can create unix ids."
> > > 
> > > Looking at idmap_ad:
> > >   "the ad backend does not work as the default idmap backend, but
> > >   one has to configure it separately for each domain for which
> > >   one wants to use it, using disjoint ranges."
> > > 
> > > Enough examples. The doc is cetainly not perfect, but
> > > saying it sucks just proves not having read it, imho.
> > 
> > The issue is that often our users do not read manpages.
> 
> A pity. :-(
> 
> > They search the web and what they find there lacks good
> > information explanations and examples.
> 
> At some point we had the manpages on the web.
> 
> > I know how to configure ID mapping, our customers don't and clearly do not
> > read the smb.conf manpage :(
> > 
> > This is not against you. It is also my fault that I didn't improve
> > documentation earlier. But if our customers do not understand it, it sucks
> > ;)
> I was just reacting to the general statement that all
> our idmap documentation sucks: If users don't read it
> that does not imply that all the documentation sucks.
> 
> > So lets improve it :-)
> 
> Sure. I agree to this all:
> 
> A) The manpages (the autoritative info) should be improved, sure.
> B) The web-docs (wiki?) can be improved to contain the same info.
>    This would be very useful.
> C) The code can be changed to protect better from faulty configs.
>    Super useful as well.
> 
> > > > So I had a call with Marc and he started to improve it. See the User
> > > > documentation in the Wiki.
> > > > 
> > > > While trying to chase down a winbindd bug the last days I read all the
> > > > changes last year and stumbled upon Volkers nice
> > > > lp_wi_scan_global_parametrics() function again. So I decided it is
> > > > time
> > > > to check the idmap config in testparm.
> > > 
> > > This is an excellent idea!
> > > (Don't rely on reading capabilities is always the safe bet... ;-)
> > > 
> > > > So here we go ...
> > > > 
> > > > 
> > > > <config>
> > > > 
> > > >         idmap config * : backend = rid
> > > >         idmap config * : range = 1000000-1999999
> > > >         
> > > >         # Winbind domain idmap
> > > >         idmap config EARTH : backend = rid
> > > >         idmap config EARTH : range = 100000000-199999999
> > > >         
> > > >         idmap config MARS : backend = rid
> > > >         idmap config MARS : range = 200000000-299999999
> > > >         
> > > >         idmap config VENUS : backend = rid
> > > >         idmap config VENUS : range = 150000000-399999999
> > > > 
> > > > </config>
> > > > 
> > > > <console>
> > > > bin/testparm smb.conf.ads > /dev/null
> > > > Load smb config files from smb.conf.ads
> > > > 
> > > > ERROR: Do not use the 'rid' backend for the default backend (idmap
> > > > config
> > > > *)!
> > > > 
> > > > ERROR: The idmap range for the domain MARS overlaps with the range of
> > > > VENUS
> > > 
> > > Note that iirc, with Volker's recent work on idmap_ad, it
> > > is not forbidden any more to have overlapping idmap ranges!
> > > 
> > > At least you should be able to have multiple ad backend
> > > configs with the same range...
> > 
> > It still is not clear. Are overlapping ranges allowed
> > 
> > 	idmap config EARTH : backend = ad
> > 	idmap config EARTH : range = 1000-1999
> > 	
> > 	idmap config EARTH : backend = ad
> > 	idmap config EARTH : range = 1500-2500
> > 
> > which I would find very strange. Or use the same range space
> > 
> > 	idmap config EARTH : backend = ad
> > 	idmap config EARTH : range = 1000-1999
> > 	
> > 	idmap config EARTH : backend = ad
> > 	idmap config EARTH : range = 1000-1999
> 
> First of all, both examples use the same domain twice.
> The point is to use overlapping or identical ranges
> for DIFFERENT domains!
> 
> Volker would have the details, but yeah, I think
> technically the first is allowed while the second
> (i.e. identical ranges) is what one would most likely
> expect. The reason that this is allowed for the ad
> module at least is that there may be a forest of
> domains, and the AD admin guarantees that the unix
> IDs are unique across the forest. In this case
> (which is not so uncommon, apparently), it has to
> be possible to use the same range for all idmap_ad
> configs.
> 
> These ranges should still be mutually disjoint
> with configs for *other* backends.

The attched patchset complains if a backend other than 'ad' has overlapping 
ranges with 'ad'. It ignores it if both backend are 'ad'.

	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testparm_idmap.patch
Type: text/x-patch
Size: 8102 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20161208/fe184eef/testparm_idmap.bin>