http://git.samba.org/?p=vl/samba.git/.git;a=shortlog;h=idmap

[simo]

On Tue, 2008-07-29 at 17:57 +0200, Volker Lendecke wrote:
> Hi!
> 
> Please find a bugfix in $SUBJECT.
> 
> On Mon, Jul 21, 2008 at 03:45:13PM +0000, Simo wrote:
> > Things I don't like or find controversial:
> > - I'd like an explanation of the reasons this patch cut back on some
> > options and functionality and not just the description of a new behavior
> > (so that people can understand why and not just what).
> > 
> > - the interface to query multiple ids at the same time (exp. multiple
> > SIDs to uid/gid) going away (I really do not like this one).
> 
> That code was not used so far. I understand it is
> potentially very useful, but with the idmap cache in
> gencache we can grant r/o-access to it from smbd directly.
> So assuming a long positive idmap cache time (i.e. a week),
> it should be much less of a problem.

I am not too concerned about positive caching, but negative caching.
If you have a substantial number of known un-mapped SIDs it can be quite
expensive.

> And, we can for sure re-add the code again.

Yeah that's true :)

> > - the patch is incompatible with 3.0.25 and 3.2.0 configurations not
> > only at the smb.conf level
> 
> The incompatible change I see is the params argument to the
> module init call and the cleanup of the idmap_domain
> structure. I do think the idmap_domain structure was really
> overloaded, the modules should not have to know if they are
> default, readonly or initialized. If they have to know this,
> they can put their stuff into the private_data pointer.

true enough

> > - the patch allows only the default domain to be written to, as the
> > allocator backend is not allowed a different range from the default
> > backend, and uid2sid checks are bound to the id range anyway.
> 
> Yes, and this is a deliberate choice. Nobody I asked, and I
> did ask quite a few people (except you obviously... ;-))
> understood the interrelationship between the different
> backend types, ranges, and so on. So I think a severe
> simplification is really necessary, potentially sacrificing
> some flexibility.

If we salvage the "default" option we can still have great flexibility
w/o too much pain. Moving to a per range allocator would make things
simpler to understand from a configuration POV, although it would
require some magic from the functions that call allocators (they will
have to specify the domain they need an UID for, not  big deal).

> > Still I didn't want to loose the ability to allocate IDs both in the
> > local domain and in any trusted domain that allowed our server to add id
> > mappings, so domains were identified only by name and SID and uid ranges
> > were only optional and used as filters (mainly for read-only domains).
> > A scenario where this would be useful is a server joined to an AD domain
> > using tdb (or a local ldap) for local mappings and with one of the
> > trusted domains being a Samba domain that shared mappings via LDAP. In
> > this scenario (with a single allocator) you want to allocate mappings on
> > the ldap server so that ids are unique on all servers and yet save
> > mappings both in the local tdb (for local mappings) and on the LDAP
> > server when the mapping is about that trusted domain.
> 
> From my point of view this is a bit too much flexibility.
> Forcing users to assign the same gid for BUILTIN\Users on
> all hosts participating in the LDAP based idmap is something
> I could find arguments for, given that the configuration
> becomes a lot simpler and more predictable this way.

You can always have a different configuration for BUILTIN :)

> > - the 'default' parameter going away if my own domain is not what I want
> > to be the default. If the samba server using idmap_rid is joined to a
> > 'minor' domain in a forest or more complex setup and you want to keep
> > one LDAP server with company-wide mappings (where most other trusted
> > domains are stored), then you may want to have mappings, by default, to
> > go on the LDAP server that is referenced by trusted domains
> > configurations.
> 
> With the code I propose the backend configured with "idmap
> backend" must be writable. You can easily then configure
> "idmap config <mydomain> : backend = rid" where <mydomain>
> is the domain you are joined to. Tested that.

Uhmm, yeah if we make sure we never assume the "default" backend is the
one we use for our main domain I can see this as a good simplification.

I still think we should have per domain allocators (with your patch the
range is mandatory so that should be easy to do), unless the domain is
marked "read only".

It still means it will break post 3.0.25 configurations that started
using the new scheme. I guess we should do the change in 3.3 and not
3.2.x so that there are more chances people will check for such major
changes on a version change.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Senior Software Engineer at Red Hat Inc. <ssorce at redhat.com>