[linux-cifs-client] Re: Kerberos5 support in cifs pathset, [PATCH: 2/4] enables extended security in NEG... and SESSION_SETUP...

[Steve French (smfltc)]

> I find all of the CIFSSEC_* flags to be very hard to follow, personally
The main difficulty I have with them is using the security flags and "sec=" for two purposes as NFS does, ie authentication choices and also trying to handle whether we can/must do packet signing vs. not-signing (signing used to be a distinct variable that we used to control this which was easier in a way).

I wonder if we should print out (ie "cat /proc/fs/cifs/SecurityFlags"  the value followed by a newline and then followed by a list of the flags in readable format ie "May use: NTLM, NTLMv2, KRB5, SIGN  Will not use:LANMAN, Plaintext").  Ideally we would handle that by writing a small utility (perhaps two: one gui, one a cli for configuring/displaying cifs settings and debug info, and doing new cifs mounts) as we experimented with a year or two ago.

> Not a comment on your patch, but we seem to have a lot of redundant
> > code in this file. When experimEnabled < 2, we only call
> > CIFS_SessSetup, so the above hunk will only come into effect if
> > someone resets /proc/fs/cifs/Experimental . It's not clear to me
> > whether some of this code is an effort to break up CIFS_SessSetup,
> > or vestigial code that should probably be removed...

All of the session setup code in connect.c is meant to be removed. It
was left in to allow reverting back to working code if sess.c were
broken.  There are two pieces in connect.c that I wanted to make sure
got moved to sess.c though (and once moved can be removed)
	1) the "raw ntlmssp" (ie not encapsulated in SPNEGO) handling code
	which worked and is useful for certain servers (e.g. Windows XP and 
	200x servers when not in a domain)
	2) the beginnings of the ASN parsing code in connect.c although
	not close to complete (needed eventually to handle
	low memory reconnect we may have to be able to do some asn parsing
	so should probably move some of this into #if 0 clause in sess.c)

> Yes, It's not necessary code now and with experimEnabled < 2 it will 
> work just fine. But finely we could end up with separate session
> setup function for kerberos if we will consider implementation of
I don't mind a second session setup function if it makes the code much cleaner
but not sure that we shouldn't simply move the lanman case, ntlmv2 case 
and kerberos case into dsitinct helper functions.
 
>Yes. CIFS is full of really big functions. I'm all for breaking them
>up into smaller ones where reasonable. It would be nice to either get
>that fixed up for good (and remove CIFS_SessSetup)

-------------- next part --------------
HTML attachment scrubbed and removed