removing wrappers and the krb5 refactor

[simo]

On Fri, 2012-04-13 at 08:00 +1000, Andrew Bartlett wrote: 
> Simo and Andreas,
> 
> I'm sorry I wasn't given a chance to comment on this major re-factor, as
> I do have a passionate interest in our kerberos code, and this is the
> first I've heard of patches to restructure this area.
> 
> As I support these efforts, and I would appreciate it if you could
> include me in your review of further changes in this area.  
> 
> On the specific changes mentioned above, I'm wondering what the major
> issue was with the various krb5 wrappers you removed, so I can better
> understand your patches?  

We are working on making it possible to build as much as possible of the
samba4 client (and non-DC related server) stuff with system Kerberos
libraries.

As part of this goal we are going to make sure that dependency hell that
now force almost all the code to link against internal libkdc is
resolved.

A first step is to clean up dependencies around krb5 wrappers so that we
end up with clean wrappers that depend exclusively on krb5 libraries and
(for now) loadparm, and nothing else. The patches I pushed in this case
remove an obnoxious ldb dependency in core krb wrappers (which I also
intend to move i a more common location at some point), and clean up
other similar dependencies.

There is still a lot of code with wrong layers of abstraction or even
reversed dependencies that affect gensec, we will soon start attacking
that interface too. I will make sure to post patchsets in advance for
your information when we get to that.

The goal is to avoid dragging in the whole server dependencies in client
libraries, as the current code has even pure external dependencies like
openchange libs end up depending and trying to access things like the
secrets database or the ldb database. All these dependencies need to be
broken and appropriate callbacks be built so that they are reversed and
called only in the right cases, so that we can build client libraries
excluding Heimdal KDC bits and have them working against a system
Kerberos library.

> In particular, the system to enrol the principal and keytab in talloc
> wrappers was specifically designed to avoid leaking memory in error
> cases, and as such I would have preferred it was retained, and used more
> extensively.  It also wraps the result in a structure that can be
> predeclared, rather than a typedef. 
> 
> On the smb_krb5_context, this is less important, but again serves as a
> simple wrapper to avoid the krb5_context typedef (it is also used to
> potentially specify certain additional options).

I carefully considered a few key elements before breaking them, with
special care to dependency issues, readability of the code to someone
familiar with gssapi and krb5 libraries, and actual effectiveness of the
wrappers.
If you look carefully though you will see that these wrappers were
avoided in closely controlled code and their "non"-use is limited mostly
to static functions within a single file.

> Perhaps if you could lay out your general approach and proposed outcome,
> some of this would be clearer.

See above.

Simo.

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