[PATCH] documentation fixes and keytab handling regression

Andreas Schneider asn at samba.org
Thu Dec 8 08:38:25 UTC 2016


On Wednesday, 7 December 2016 15:39:43 CET Jeremy Allison wrote:
> On Sat, Dec 03, 2016 at 08:37:58AM +0200, Alexander Bokovoy wrote:
> > > I want to understand what you're trying to
> > > do here before we make code changes.
> > 
> > As I said, I'm trying to fix the regression -- Fedora 25 with Samba
> > 4.5.x now broke FreeIPA deployments.
> > 
> > Looking at smb_krb5_kt_open_relative() and smb_krb5_kt_open(), though,
> > I'm not sure what's the purpose of the whole '/' check in
> > smb_krb5_kt_open() -- had it not be there, smb_krb5_kt_open_relative()
> > would equally do the justice and only accept absolute paths to WRFILE:
> > and FILE: prefixed keytabs already.
> > 
> > I'm not really sure why it is named _relative(), though. There is
> > nothing there for relative paths at all. If you passed the keytab name,
> > it gets analyzed whether it is prefixed with WRFILE:/ or FILE:/ and if
> > not, either FILE: or WRFILE: is prepended to the path and then keytab
> > gets open. In the latter case the keytab name is obviously relative.
> > 
> > It would also break for MEMORY: keytabs, as that case is not handled
> > right in the code path for when the keytab name is passed in.
> > 
> > If you don't pass the keytab name, _relative() does try to obtain the
> > name of the default keytab and parse it. Here it expects all kinds of
> > prefixes but there is nothing for the 'relative' paths there either.
> > 
> > It seems to me that smb_krb5_kt_open() refactoring would be to eliminate
> > the distinction between the two as it is not simply useful at all.
> 
> That sounds good to me. git-blame shows Andreas created this
> code (I love that command :-).

Look into 'git log'.

I needed a function for 'samba-tool domain exportkeytab' which creates a 
keytab which doesn't start with a '/'!

So I renamed smb_krb5_kt_open() to smb_krb5_kt_open_relative(), remove the 
check that it starts with a '/' and moved the check for starting with a '/' to 
a new smb_krb5_kt_open().

https://git.samba.org/?
p=samba.git;a=commitdiff;h=c2f5c30bea372182578055a7bd50ee8076946ef3


So I argue that the current smb_krb5_kt_open() is not really different from 
the original function.
For the smb_krb5_kt_open_relative() we should probably move more code to 
smb_krb5_kt_open() which checks for path starting with '/'.


1026 »·······»·······if ((strncmp(keytab_name_req, "WRFILE:/", 8) == 0) ||                                                                                                                                                   
1027 »·······»·······    (strncmp(keytab_name_req, "FILE:/", 6) == 0)) {                                                                                                                                                     
1028 »·······»·······»·······tmp = keytab_name_req;                                                                                                                                                                          
1029 »·······»·······»·······goto resolve;                                                                                                                                                                                   
1030 »·······»·······}


That should probably be moved ...


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org



More information about the samba-technical mailing list