[PATCH] sam backend parameter

[Andrew Bartlett]

Simo Sorce wrote:
> 
> On Thu, 2002-10-03 at 03:01, Andrew Bartlett wrote:
> > Simo Sorce wrote:
> >
> > > Plus I have some questions about the current sam interface:
> > >
> > > - what is all the context thing needed for?
> >
> > I don't like global variables, and this allows us to construct seperate
> > contexts for operations like sam2sam, and testing, without fiddiling
> > with global variables.
> 
> There's no problem in having global variables, provided they are used
> correctly. Insisting to not use them in any case just as a preconcept is
> as wrong as using them too much.
> 
> It is all a question on how and where you use them.

Except that global variables are particularly nasty in C - we don't have
an autoprototyper on them, and we cannot ensure that they are correctly
declared in all modules.  Also, we cannot validate the input into any
such variables, or assume auto-initialisation.  The methods employed in
the pdb and sam provide this.

> sam2sam operation does not need access checking and interface
> virtualization, it is an administrative task only anyway, try to put
> sam2sam above the sam interface instead of making it directly connect to
> the backends is just a bad idea, it add complexity and potential
> problems and gain nothing, imho.
> 
> And sam2 sam is the only case where it could be useful to have 2
> backends loaded at the same time. So it is perfectly possible to make
> it's own init function that load 2 backends into 2 local (to samedit)
> variables, while still loading the sam backend into a global variable
> used by the sam interface inside smbd.

Honestly, it is a matter of taste.  I like the way that pdb (and now the
SAM code) has been constructed, and the distinct lack of 'special
cases'.  If pdbedit becomes a special case, then it is more likely to
break.  If the rest of Samba has to use that code path, then accidents
are less likely to happen.

> > > - what is the handle thing needed for ?
> >
> > Same as the SAM_ACCOUNT in pdb.
> 
> If it's a SAM_ACCOUNT then call it with it's name.
> Also I really to not like the private substructure.
> We are all adult and vaccinated, and must trust each other ability to
> code and use internal structures properly, trying to say: " I do not
> trust other coders " is not a bright move imho, add unneeded complexity,
> SAM_ACCOUNT structure directly will do so regardless if theres a thing
> called private or not. Adding this layer only makes the interface a lot
> more heavier and trickier, make code slower (adds a function call for
> each parameter to be set into a structure) and the sam interface a lot
> bigger.
> 
> Effort should be spent to educate other programmers to use the thing the
> right way by writing documentation, not by setting artificial barriers.

Here we disagree.  I take a different approach, were I prefer to make it
harder to make mistakes.  

The use of 'set' routines has also allowed us to (with minimal changes
to the interface) track 'default' values for many parameters - avoiding
the storage of extra (fixed) values in LDAP.  I intend to expand this to
every attribute for the new SAM.

Furthermore, we use the get/set routines to allow the users of the
passdb interface to get feedback on the ability to get/set *individual*
elements on the SAM handle, rather than the lot.   

I advocate strong interfaces, because I feel it assists in modular
programming.  By using the get/set routines, I can ensure that data is
correctly 'const', for example.  Finally, it allows us to strdup() all
assigned strings, and ensure they are not accidentally assigned stack
pointers, while avoiding memory leaks.

On a project as large as Samba, I prefer all the help I can get in
ensuring code quality.

> > > - what is access desired meant to do ? Authorization is a different
> > > thing then storage, a backend is a storage!
> >
> > The SAM interface layer is the 'choke point'.  If we do not wan't nasty
> > races, then we must reterive things like the security descriptor with
> > the data it applies to.  This implies that the ACL checking code must
> > resise either in the SAM backend, or the SAM interface.  If we export it
> > above this layer, we *will* get places where we don't check it properly.
> 
> It must be in the Interface, putting it on the backend is the wrong move
> for many reasons:
> - code duplication, you have to implement the access checking into evry
> backend.
> - code review, authorization is a critical part of the security in
> samba, if you have to double check every module to be sure it does
> things the right way, you simply castrate the potentiality of a loadble
> module interface, as you put too much responsibility on independent
> module coders.
> - consistency every change you need to make to the access checking code,
> bust be made to any module and will make out of sync any modules not
> under your control, increasing the module versioning nightmare.
> - races may be easily solved supporting locks on the backend.

We cannot assume locks in the backend.  The interface is not designed in
isolation - we have to work with the fact that LDAP *will* be a primary
backend, and does not support this kind of locking.  I prefer to create
a solution that does not impose additional constraints on this.

Your points are very valid, and I have proposed a solution in the my
reply to Jerry's mail.  That is, the backend can do it's own ACL checks
if it likes, but it passes the security descriptor to the interface,
where the 'real' check is performed.

> > > - why do we insist to have a thing called unix accounts? It just does
> > > not make sense to me. We need "real" users/groups mapping instead
> > > (opposed to created on the fly by winbind based accounts).
> >
> > I'm not sure what you mean here - the current code doesn't even know
> > about unix accounts.
> 
> They are present in pdb, and I remember at some point of the discussion
> we had on IRC someone claimed that multiple modules were also a way to
> solve the unix accounts problem like done in with pdb backends.

There is no intention to create a similar backend for the sam.  Instead,
we do intend to solve this problem in exactly the way you indicate
below.  There is no intention to provide a 'mulidb' module either.  The
only way to load more than one module will be to have separate domains.

> What Instead I think is that we should not threat unix accounts in any
> special way, but instead we need to map them to normal users when they
> are present, if a user do not have a local unix user then it will be
> created in winbindd, otherwise the uid->SID mapping will be done so that
> the user is mapped on a unix user.
> 
> All the mapping should consist ONLY of a SID->[u,g]id mapping imho, and
> we should discourage using unix groups provided from /etc/group file and
> use instead winbindd to provide them to the system, but still use them
> from /etc/group if the administrator want to do so.

Agreed.

Andrew Bartlett

-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net