[PATCHES] next MIT Kerberos enablement work patchset
abartlet at samba.org
Thu Apr 26 19:21:37 MDT 2012
On Thu, 2012-04-26 at 19:11 -0400, simo wrote:
> Please take a look at the latest patches for MIt Kerberos support here:
> I plan to push all those not marked WIP if they pass autobuild.
First, thanks for posting these, I do appreciate it. As I mentioned, I
have some concerns, and so would like to see some aspects of these
Explicitly set HAVE_LIBGSSAPI if either gssapi or gssapi_krb5 are found
As I already commented in objection to the patch that introduced
HAVE_LIBGSSAPI, I do not want use to return to having multiple HAVE_
defines for 'is this a build with support for krb5/gssapi'. We do not
support krb5 without gssapi, and so far HAVE_KRB5 has been selected as
that indicator. As such, rather than this patch, I ask that
HAVE_LIBGSSAPI be changed to HAVE_KRB5.
Move kerberos_kinit_keyblock_cc to krb5samba lib
It seems that supporting kinit from a keyblock is causing you some
dramas. I wonder if it would be better not to support this at all?
There is no sensible use case for this - the closest we have is support
for a Samba domain join prepared with a version of Samba so old that it
didn't write the plaintext trust account password into the
To be clear, I don't object to the patch, but I am quite willing to
loose this feature if it helps.
s4-auth-krb: smb_rd_req_return_stuff is used only in gensec_krb5
This code was placed in a separate file specifically:
-/* This file for code taken from the Heimdal code, to preserve licence
You may wish to change the subsystem this file is linked into to be just
the gssapi_krb5 module. This module is not only used for testing, it is
also a key part of support for the kpasswd server. You may choose to
make the compilation of this module dependent on Heimdal, in the same
way that you will presumably be disabling the kdc and kpasswd server.
krb-init: define out heimdal specific stuff in mitkrb build
I have two issues with this patch. I don't like how much inline #ifdef
is being introduced here an in the other patches, as we have gone to
great pains in Samba to have libraries like clikrb5.c that contain the
#ifdefs as much as possible.
However, the major issue I have is the behaviour change:
+ /* The MIT Kerberos build relies on using the system krb5.conf
+ * If you really want to use another file please set KRB5_CONFIG
+ * accordingly. */
I really think that the choice of compiled library should not change our
behaviour in this way. We should, within reason have the same behaviour
if a system Heimdal is used for the components that you are trying to
support with a system MIT krb5.
If a particular deployment situation requires that we not use the local
krb5.conf first, then an option to support this may well be a very good
idea. Or similarly, if this only makes sense in the S4 DC, then an
option (say, in smb.conf, or a server role to cover the AD DC) to enable
it would make sense.
keytab_copy: Fix style, whitespaces
This may be less important with your later patch to change this to
compile with MIT krb5 compat wrappers, but the reason this file was left
in original style was to make it easier to copy a new version in from
Heimdal if required.
s4-auth-krb: Disable code in MIT build
Should these be defines based on the particular missing functions, or be
replaced with null wrappers?
Split normal kinit from s4u2 flavored kinit
This seems like a sensible way to deal with what is clearly an API mess,
with decode_Ticket() calls etc. It's a pity however, as might be a
better way to deal with getting user info in winbindd in the long run.
Finally, thanks again for giving me the chance to look over these. If
you can post a revised branch, I'm sure I'll be in a much better
position to ack the changes.
I wish you the very best with this challenging task, and look forward to
seeing the end result!
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
More information about the samba-technical