Problem working versus CIFS from kernel 5.17

[rshterman]

Hi,

In Linux kernel 5.17 the client started to use Version in the Negotiate message - `commit 52d005337b2c94ab37273d9ad8382d4fb051defd Author: Steve French [email protected] Date: Wed Jan 19 22:00:29 2022 -0600

smb3: send NTLMSSP version information

For improved debugging it can be helpful to send version information
as other clients do during NTLMSSP negotiation. See protocol document
MS-NLMP section 2.2.1.1

Set the major and minor versions based on the kernel version, and the
BuildNumber based on the internal cifs.ko module version number,
and following the recommendation in the protocol documentation
(MS-NLMP section 2.2.10) we set the NTLMRevisionCurrent field to 15.

Reviewed-by: Shyam Prasad N <[email protected]>
Signed-off-by: Steve French <[email protected]>

`

This commit is causing the VERSION flag to be set also in the AUTH message but the client is not sending Version in the auth message so we are doing wrong parsing and failing the login because the first header is written in offset 64 while we are expecting it in offset 72 (because we add the version size). Checking the kernel code I see the logic decides about the offset in the payload only based on the offset mentioned in the first header and not basing it on the struct size, thoughts about changing it to have the same logic?

[simo5]

If the kernel sets a flag but then does not set the corresponding fields it is broken and needs to be fixed. If we are doing that, then we are broken. Can you provide some trace or debug log of what happens?

[simo5]

@smfrench are you aware of this?

[smfrench]

Looks like a straightforward fix. Will do a patch for it now. Any ideas what MacOS and Windows clients do when they use NTLMSSP (is the version info sent on BOTH ntlmssp_negotiate and ntlmssp_auth request or just the ntlmssp_negotiate)? Presumably they would only send it on the ntlmssp_negotiate.

[smfrench]

It looks like Windows client sends version info on BOTH the ntlmssp_negotiate and ntlmssp_auth requests

[simo5]

It doesn't really matter if you send version or not, what matters is that you are consistent with flags sent.

[smfrench]

Our goal in sending the version info was to allow support engineers to be able to identify enough about the client to be able to rule out known bugs in old versions of Linux kernel (that are missing many fixes) when a user complains about a problem running their app on an SMB3.1.1 mount - so sending the version info on the ntlmssp_negotiate is all we need.

[simo5]

You have to be careful that sending the version may change the behavior of the peer, Check the MS-NLMP document, it mention something about Windows making some assumptions when it starts seeing the version field being sent. The content of the version field also matter, you are supposed to send Windows versions not just random numbers.

[smfrench]

Simplest fix seems to be something like this (rather than adding more to the NTLMSSP_AUTH request):

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c index 335c078c42fb..64e202970bbc 100644 --- a/fs/smb/client/sess.c +++ b/fs/smb/client/sess.c @@ -1048,6 +1048,7 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer, flags = ses->ntlmssp->server_flags | NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO | NTLMSSP_NEGOTIATE_WORKSTATION_SUPPLIED;

•   flags = flags & ~NTLMSSP_NEGOTIATE_VERSION;
    tmp = *pbuffer + sizeof(AUTHENTICATE_MESSAGE);
    sec_blob->NegotiateFlags = cpu_to_le32(flags);
  

[rshterman]

@smfrench are you going to push this fix? It looks good to me.

[smfrench]

Added a fix to cifs-2.6.git for-next for this. See https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=19826558210b9102a7d4681c91784d137d60d71b