[PATCH] Check idmap config with testparm

Thu Dec 8 09:24:17 UTC 2016

On 2016-12-08 at 10:47 +0200, Alexander Bokovoy wrote:
> On to, 08 joulu 2016, 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?
> > 
> > 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. They search the web 
> > and what they find there lacks good information explanations and examples.
> > 
> > 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 ;)
> > 
> > So lets improve it :-)
> > 
> What about this patch: add a top level identity management section to
> smb.conf(5) so that we can gather references to other documentation we
> have around the idmap modules?
> 
> The suggestion then would be 'read smb.conf(5), section on identity
> management, and all the references it contains'.
> 
> > 
> > > 
> > > > 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
> > 
> > 
> > ????
> > 
> > 
> > 
> > 	Andreas
> > 
> > 
> > -- 
> > Andreas Schneider                   GPG-ID: CC014E3D
> > Samba Team                             asn at samba.org
> > www.samba.org
> > 
> 
> -- 
> / Alexander Bokovoy

> From d72988a0f4efd967963ddbb960268294a4d74899 Mon Sep 17 00:00:00 2001
> From: Alexander Bokovoy <ab at samba.org>
> Date: Thu, 8 Dec 2016 10:21:53 +0200
> Subject: [PATCH] smb.conf: add identity management section
> 
> Add a generic identity management section that points out to the other
> resources in Samba documentation about idmap modules and their
> configuration.
> 
> This should help users to discover corresponding documentation easily.

This is a very good proposal!
A few detail comments inline below:

> Signed-off-by: Alexander Bokovoy <ab at samba.org>
> ---
>  docs-xml/manpages/smb.conf.5.xml | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/docs-xml/manpages/smb.conf.5.xml b/docs-xml/manpages/smb.conf.5.xml
> index 10c1fb4..03ff609 100644
> --- a/docs-xml/manpages/smb.conf.5.xml
> +++ b/docs-xml/manpages/smb.conf.5.xml
> @@ -754,6 +754,39 @@ chmod 1770 /usr/local/samba/lib/usershares
>  
>  </refsect1>
>  
> +<refsect1 ID="IDMAPCONSIDERATIONS">
> +	<title>IDENTITY MAPPING CONSIDERATIONS</title>
> +
> +	<para>
> +	In SMB protocol users, groups, and machines are represented by their security identifiers (SIDs).

In the SMB protocol, users, ...
   ^^^             ^

> +	On POSIX system Samba processes need to run under corresponding POSIX user identities and
> +	with supplemental POSIX groups to allow access to the files owned by those users and groups.
> +	The process of mapping SIDs to POSIX users and groups is called <emphasis>IDENTITY MAPPING</emphasis>.

... or in short, ID MAPPING.

(I think we should have 'id mapping' in addition to 'identity
mapping', because i guess people will search for that.)

> +	</para>
> +
> +	<para>
> +	Samba supports multiple ways to map SIDs to POSIX users and groups. The configuration is driven by

by the ...
   ^^^

> +	<smbconfoption name="idmap config DOMAIN : OPTION"/> option which
> +	allows to specify identity mapping (idmap) backend options for each domain

allows one to ... (?)
       ^^^

s/backend //

> +	separately.
> +	</para>
> +
> +	<para>
> +	Identity management modules implement different strategies for mapping of SIDs to POSIX user and group

s/management/mapping/

> +	identities. They are applicable to different use cases and scenarios. It is advised to read documentation

read the documentation
     ^^^

> +	of the individual identity management modules before choosing a specific scenario to use. Each identity

s/management/mapping/

> +	management module is documented in a separate manual page. The standard idmap backends are
> +	tdb (<citerefentry><refentrytitle>idmap_tdb</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	tdb2 (<citerefentry><refentrytitle>idmap_tdb2</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	ldap (<citerefentry><refentrytitle>idmap_ldap</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	rid (<citerefentry><refentrytitle>idmap_rid</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	hash (<citerefentry><refentrytitle>idmap_hash</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	autorid (<citerefentry><refentrytitle>idmap_autorid</refentrytitle><manvolnum>8</manvolnum></citerefentry>),
> +	ad (<citerefentry><refentrytitle>idmap_ad</refentrytitle> <manvolnum>8</manvolnum></citerefentry>), and
> +	nss (<citerefentry><refentrytitle>idmap_nss</refentrytitle> <manvolnum>8</manvolnum></citerefentry>).

idmap_rfc2307 is missing.

Apart from these, it looks good to me, thanks!

I want to add a caveat that the overall idmap config should not
lightly be changed after one has started using it, because
it creates the risk of losing access to data, or disclosing data
that should be private, etc. (This is one of the most common
mistakes that we have to deal with in production environments.)

Not 100% sure yet if here or in the idmap config section would be
better...

> +	</para>
> +</refsect1>
> +
>  <refsect1>
>  	<title>EXPLANATION OF EACH PARAMETER</title>
>  	
> -- 
> 2.9.3
> 

-------------- 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/75a04065/signature.sig>