[PATCH] Check idmap config with testparm

Thu Dec 8 09:44:16 UTC 2016

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.

Volker, would be sufficient for your use case
to establish a rule and check that would allow
ranges to be either disjoint or identical?

Do we also need to allow sub-ranges?

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/20161208/504fe357/signature.sig>