[linux-cifs-client] Re: [PATCH 00/12] cifs: fix string conversions from wire format to local charset (try #3)

[Steve French]

On Thu, Apr 30, 2009 at 8:25 AM, Suresh Jayaraman <sjayaraman at suse.de> wrote:
> I would also want to see patches 9 and 11 merged mainly because the cifs
> code would look lot less confusing without them and they are currently
> unusable. I think they could be part of some testing branch/branch
> linux-next if there are strong reasons for keep them and we forsee some
> future developments/changes.

NTLMSSP is the default mechanism for most real world (smb/cifs etc.)
authentication
so is probably the "safest" one to use.  It also is useful because it
allows you to
detect whether ntlmv2 is supported before we send the password hash
Note that we should never be sending ntlm these days unless the
server is very backlevel, but without ntlmssp it is hard to determine which
mechanism to use.   The addition of Kerberos support to cifs was
helpful to many,
but in practice most target servers are not properly configured to handle
Kerberos authentication, and thus doing NTLMSSP properly is too important
to ignore.

The code which did NTLMSSP was written many, many years ago, and only does
"raw ntlmssp" (not encapsulated in spnego) and does not use the code cleanup
that went into the new session setup implementation in fs/cifs/sess.c
Changes last year to SMBNegotiate may have accidently disabled this code.
Jeff and I spent some time looking through these two old functions which send
the NTLMSSP session setup requests (and which he did not want to convert in
his patch set to the new string functions since he would like them to
be moved/rewritten):

1) CIFSNTLMSSPNegotiateSessSetup
followed by
2) CIFSNTLMSSPAuthSessSetup

These two functions in current form are fairly large (640 lines
total), but with Jeff's new
helper functions and merged into fs/cifs/sess.c should be
significantly smaller, and
port easily. As we went through the code flow in these two functions,
they look fairly
straightforward though.   Basically they are a boring series of assignment
statements to setup the blob, and the format of the NTLMSSP blob
is straightforward (see fs/cifs/ntlmssp.h).

Instead of simply ripping out the code, we discussed moving them into
helper functions
(e.g. to create and initialize the blob, and to setup the unicode or
ascii string areas
in the blob).  Even after porting these two functions would continue
to be disabled by default
at runtime (/proc/fs/cifs/Exerimental) and within
CONFIG_CIFS_EXPERIMENTAL ifdef until
more users have a chance to test this.   A primary objection to the old NTLMSSP
implementation was not the long series of assignment statements which
are inevitable,
but that it was written as two large functions instead of smaller
ones, with cleaner
exit handling, and less nesting.   Jeff and I also discussed that cifs
eventually needs to do NTLMSSP in Spnego, and all three kernel SPNEGO
implementations (cifs, firewall/nat, and nfs/sunrpc) are either bad or
incomplete or both,
but for various reasons we need to address this (not just in cifs), so
it is likely that the
asn support will get better over the next few releases (and with the
blob code separated,
we can reuse it for the spnego encapsulated flavor of ntlmssp).

I asked Jeff to send his patch series, which I will merge without the
two changes
unrelated to string handling, and then I will add NTLMSSP changes into
new helper functions to create the NTLMSSP blob, then delete the old.

The remaining unmerged patch relates to how Windows "junctions" are recognized.
(junctions are implemented as reparse points in Windows, and are
somewhat similar
to symlinks and are a fundamental inode type).  Before we merge that
patch, someone
needs to verify (e.g. using one of the standard windows utilities
fsutil, ln.exe or linkd.exe)
that earlier global changes to symlink code did not break this support
for a common
windows file type.
-- 
Thanks,

Steve