[PATCH] documentation fixes and keytab handling regression

[Jeremy Allison]

On Fri, Dec 02, 2016 at 10:08:02PM +0200, Alexander Bokovoy wrote:
> On pe, 02 joulu 2016, Jeremy Allison wrote:
> > On Fri, Dec 02, 2016 at 12:56:03PM +0200, Alexander Bokovoy wrote:
> > > Hi,
> > > 
> > > attached two patches improve documentation for 'logon script' and
> > > 'dedicated keytab file' options.
> > > 
> > > The second patch also fixes a regression introduced by the commit
> > > c2f5c30b which broke specifying storage access format for keytabs.
> > 
> > NAK on the second patch. Sorry, but I don't understand what
> > you're trying to check for w.r.t. relative/absolute paths
> > here.
> > 
> > The original code refuses all paths that don't start with '/'
> > (i.e. don't allow relative paths).
> > 
> > I'm assuming you want to allow paths of the form:
> > 
> > /absol/ute/path
> > FILE:/absol/ute/path
> > WRFILE:/absol/ute/path
> > 
> > but disallow paths of the form:
> > 
> > rela/tive/path
> > FILE:rela/tive/path
> > WRFILE:rela/tive/path
> > 
> > In which case the logic should be:
> > 
> > 	if (keytab_name_req != NULL) {
> > 		if (keytab_name_req[0] != '/') {
> > 			/*
> > 			 * Might still be an absolute path, but
> > 			 * prefixed by FILE:/ or WRFILE:/
> > 			 */
> > 			bool good_file = (strncmp(keytab_name_req, "FILE:/", 6) == 0);
> > 			bool good_wrfile = (strncmp(keytab_name_req, "WRFILE:/", 8) == 0);
> > 
> > 			if (!good_file && !good_wrfile) {
> > 				/* Nope - relative path. Disallow. */
> > 				return KRB5_KT_BADNAME;
> > 			}
> > 		}
> > 	}
> > 
> > Is this correct ?
> The very same logic is in the smb_krb5_kt_open_relative() which
> smb_krb5_kt_open() calls to after this check. However,
> smb_krb5_kt_open_relative() supports also other prefixes, including
> MEMORY: and ANY:. Thus, with your approach you would break the code that
> might want to pull in a keytab created by Samba code somewhere else in
> the process memory using MEMORY: prefix. Actually, it will not be able
> to create such keytab at all, due to this change, unless
> smb_krb5_kt_open_relative() is called directly.

That's the point isn't it ? I thought users who wanted
MEMORY: and ANY: call smb_krb5_kt_open_relative().

There are such in:

source3/libnet/libnet_keytab.c
source4/kdc/ktutil.c
source4/libnet/libnet_export_keytab.c

anyone who doesn't calls smb_krb5_kt_open().

I want to understand what you're trying to
do here before we make code changes.

Jeremy.