Bug 8212 - inconsistent signing behavior with samba 3.5.8
inconsistent signing behavior with samba 3.5.8
Status: RESOLVED FIXED
Product: Samba 3.5
Classification: Unclassified
Component: File services
3.5.8
All All
: P5 normal
: ---
Assigned To: Shirish S. Pargaonkar
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-08 13:52 UTC by Jeff Layton
Modified: 2011-07-29 09:57 UTC (History)
5 users (show)

See Also:


Attachments
xz'ed capture file showing strange signing behavior (1.96 KB, application/x-xz)
2011-06-08 13:52 UTC, Jeff Layton
no flags Details
smbd -d10 log (132.32 KB, text/plain)
2011-06-10 01:47 UTC, Jeff Layton
no flags Details
tarball with captures and logs (50.63 KB, application/octet-stream)
2011-07-06 19:10 UTC, Jeff Layton
no flags Details
patch -- cifs: don't start signing too early (1.66 KB, patch)
2011-07-07 01:42 UTC, Jeff Layton
no flags Details
patch to fix sec=ntlmsspi against Samba server when signing is mandatory (1.12 KB, patch)
2011-07-08 21:11 UTC, Shirish S. Pargaonkar
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Layton 2011-06-08 13:52:24 UTC
Created attachment 6542 [details]
xz'ed capture file showing strange signing behavior

I've been playing with the sec=ntlmssp code in cifs.ko, and noticed I was getting a bunch of these sorts of printk pops:

    CIFS VFS: Unexpected SMB signature

...typically that means that the signing check failed. When I went to look at the captures, I noticed that samba wasn't sending signatures in some packets. The weird thing is that it did send signatures in some of the earlier ones during session setup, but then later it decided to stop sending them and disabled the security signatures bit.

My smb.conf is below:

[global]
	workgroup = POOCHIEREDS
	server string = Samba Server Version %v
	log file = /var/log/samba/log.%m
	max log size = 50
	realm = POOCHIEREDS.NET
	security = user
#	passdb backend = tdbsam
	kerberos method = dedicated keytab
	dedicated keytab file = /etc/samba/samba.keytab
#	password server = salusa.poochiereds.net
	load printers = yes
#	unix extensions = no
	max open files = 50000
	server signing = auto
#	server signing = mandatory
	lanman auth = yes
	min receivefile size = 4096

[scratch]
	path = /scratch
	comment = Scratch Directory
	browsable = yes
	public = yes
	guest ok = yes
	writable = yes
	map archive = no
Comment 1 Jeff Layton 2011-06-08 13:59:45 UTC
For instance, in frame 11, the server sets the signatures bit in flags2 and sends a signature in the SESSION_SETUP reply. Then in frame 13 it doesn't set that bit and the signature field is zeroed out.
Comment 2 Jeremy Allison 2011-06-09 21:21:53 UTC
Can you get me a debug level 10 log from the server when you're reproducing this ?
What happens if you set "server signing = mandatory" ? Does it enforce signatures then ?

I can't see how you're getting this :-).

Jeremy.
Comment 3 Jeremy Allison 2011-06-09 22:23:17 UTC
Ok, I've tried to reproduce this using "server signing = auto" and unsetting the FLAGS2_EXTENDED_SECURITY flag2 on SMBecho requests to see if that stops the server sending signed replies, and it doesn't.

I'm going to need more info on how you reproduce this I'm afraid.

Jeremy.
Comment 4 Jeff Layton 2011-06-10 01:47:20 UTC
Created attachment 6555 [details]
smbd -d10 log

Good call on the -d10 log. That points out the problem. signing = mandatory makes the server shut down the connection immediately. The problem is this:

smb_signing_good: signing negotiated but not required and peer isn't sending correct signatures. Turning off.

...so that seems to be why this is occurring.

I'm reproducing this by just mounting via cifs using sec=ntlmsspi. The thing is though, that when I mount a win2k8 host the same way, it doesn't drop the connection and keeps sending me valid signatures.

So, looks like sort of a "he said, she said". Is there a "simple" way to tell whether we're sending bad signatures or samba's verification is off?
Comment 5 Jeremy Allison 2011-06-10 17:06:50 UTC
At debug level 10 the signing code prints out valid signatures for sequence number ranging from -5 to +5 around the actual correct sequence number. That should help to see if the sequence number processing is off.

Does the Windows server have mandatory signatures set ?

Jeremy.
Comment 6 Jeff Layton 2011-06-10 18:23:26 UTC
I enabled mandatory signatures on the win2k8 box and mounted from it using "sec=ntlmsspi". It worked just fine.

The log I attached last night was collected with -d10. I'm not seeing those signatures that you allude to in there though. Am I missing something?
Comment 7 Jeff Layton 2011-06-10 18:26:18 UTC
Here's a snippet from the log. Maybe -d10 is broken? I don't see any signature values being printed out for each of the smb_signing_md5 lines...

smb_signing_md5: sequence number 1
smb_signing_sign_pdu: sent SMB signature of
[0000] 98 87 D9 49 5D D5 FD 68                            ...I]..h 
got smb length of 116
smb_signing_md5: sequence number 2
smb_signing_check_pdu: BAD SIG: wanted SMB signature of
[0000] F4 38 CC 5B 3C 07 E2 37                            .8.[<..7 
smb_signing_check_pdu: BAD SIG: got SMB signature of
[0000] 69 42 28 91 D4 D6 E3 C6                            iB(..... 
smb_signing_md5: sequence number 4294967293
smb_signing_md5: sequence number 4294967294
smb_signing_md5: sequence number 4294967295
smb_signing_md5: sequence number 0
smb_signing_md5: sequence number 1
smb_signing_md5: sequence number 2
smb_signing_md5: sequence number 3
smb_signing_md5: sequence number 4
smb_signing_md5: sequence number 5
smb_signing_md5: sequence number 6
smb_signing_good: signing negotiated but not required and peer
isn't sending correct signatures. Turning off.
Comment 8 Jeremy Allison 2011-06-10 19:18:46 UTC
Ah no, the level 10 is fine - you can see it hunting the sequence number space to find the correct sign match (and not finding it).

I'm assuming the sign algorithm calculation is correct, so it must be the key generation that's getting messed up for ntlmssp. Strange.

Can you point at the kernel client code you're using for key generation that's working with Windows and I'll compare with the code in smbd. Can you test with a Windows client against the same server to make sure we're still working with that ?

FYI. I'm off on vacation for a week later this afternoon, won't be back looking at bugs until June 20th. Feel free to re-assign to Volker or Metze or someone else on the Team if you need this looked at before.

Jeremy.
Comment 9 Jeff Layton 2011-06-10 20:07:18 UTC
The kernel client code is a bit hard to follow, especially now that it uses the kernel crypto APIs, but here goes. The crazy naming of the fields also doesn't help (not sure why "response" was chosen for this given that it's actually a challenge from the server -- oh well):

I'm testing this on fairly current mainline code -- if you pull down Steve's or Linus' tree then you should be pretty close to what I'm using.

The session key gets set in cifs_setup_session():

    server->session_key.response = ses->auth_key.response;

...for ntlmssp, which is what I'm using here, the auth_key.response initially gets set in decode_ntlmssp_challenge():

    memcpy(ses->auth_key.response, bcc_ptr + tioffset, tilen);

...which seems to be copying the key straight from the NTLMSSP challege message.

That's later used in cifs_calculate_signature to calculate the signature using the kernel crypto md5 code:

        crypto_shash_update(&server->secmech.sdescmd5->shash,
                server->session_key.response, server->session_key.len);

Clear as mud?
Comment 10 Jeff Layton 2011-06-10 20:07:52 UTC
Shirish wrote most of this code, so if you have more questions he's probably the best person to ask...
Comment 11 Björn Jacke 2011-06-10 20:35:24 UTC
metze and I saw a similar thing in a network sniff between Windows Citrix server and samba 3.5 some weeks ago. No smbd logs unfortunately. With "server signing = mandatory" the "security signatures are supported bit" was set to 0 right in the tree connect response from the samba server. Connection to samba failed accordingly. Resetting "server signing = no" made things work again.
Comment 12 Jeremy Allison 2011-06-10 20:37:58 UTC
Björn that's not the same behavior as this bug. In this bug setting "server signing = auto" seems to be thinking the client supplied signatures are bad and dropping back to unsigned. Setting "server signing = mandatory" works fine.

Jeremy.
Comment 13 Jeff Layton 2011-06-10 20:43:09 UTC
(In reply to comment #12)
> Björn that's not the same behavior as this bug. In this bug setting "server
> signing = auto" seems to be thinking the client supplied signatures are bad and
> dropping back to unsigned. Setting "server signing = mandatory" works fine.
> 

That's not what I'm seeing -- maybe I wasn't clear. When I set "server signing = mandatory", the signature check still fails. It just shuts down the connection which causes the mount to fail instead of just disabling signing and carrying on.
Comment 14 Jeff Layton 2011-06-10 21:01:52 UTC
Björn's problem sounds very similar to what I'm seeing.

Just to verify whether it's a client or server bug, I set up my win2k8 VM to enforce mandatory signing too. Then I went into cifsencrypt.c and changed the code to always set the first byte of the signature to 0xff.

When I did that, the mount to win2k8 also failed as the tree connect always got a STATUS_ACCESS_DENIED error. sec=ntlmsspi seems to work fine between cifs.ko and win2k8. Signing on samba appears to be busted, at least when I use sec=ntlmssp.

I need to go back and see whether it works with "raw" ntlm or ntlmv2 as well...
Comment 15 Jeff Layton 2011-06-13 13:56:04 UTC
I also went back today and tested against samba with sec=ntlmv2i and sec=ntlmi. Both of those seem to work fine, so this problem does seem likely to be an issue with the NTLMSSP key exchange.
Comment 16 adarsh 2011-07-06 08:33:57 UTC
I am seeing the same problem with samba-3.5.9 as well. Please let me know the time line when will this be fixed.
Comment 17 Jeff Layton 2011-07-06 18:52:42 UTC
I just went back and did a bit more testing, this time with a more recent version of samba -- samba-3.6.0-69rc2.fc16.1.x86_64

A win2k8 client can talk to samba with mandatory signing enabled (and I verified that it authenticates using NTLMSSP by sniffing traffic)

cifs.ko can talk to win2k8 with mandatory signing enabled and using sec=ntlmsspi

cifs.ko cannot talk to samba with signing=mandatory and sec=ntlmsspi.

So, this may not be a samba bug after all, but how to determine where the bug actually is?
Comment 18 Shirish S. Pargaonkar 2011-07-06 19:04:29 UTC
(In reply to comment #17)

I will do some testing with the latest Samba server, I need to build one.

But if Win2k8 client can work with Samba with signing enabled/mandatory,
then cifs client should work too.
Perhaps something to do with sequence number used in signature calculation?
If authentication works, which I think it does, and tree connect fails,
then it has something to do with sequence number, that is just a guess though.
Comment 19 Jeff Layton 2011-07-06 19:10:42 UTC
Created attachment 6667 [details]
tarball with captures and logs

Two sets of tcpdump captures and -d10 logs:

win2k8 to samba-3.6.0-69rc2.fc16.1

...and...

cifs to same samba version. Jeremy, any ideas on how to approach this?
Comment 20 Jeremy Allison 2011-07-07 01:02:57 UTC
Are you turning on the client signing too soon in cifs.ko ?

In the Win28k -> Samba pcap we have:

Negprot request   ->                   (sign field 0)
                  <-  Negprot reply    (sign field 0)
SessionSetupX req ->                   (sign field BSRSPYL)
                  <- SessionSetupX reply (sign field 0)
SessionSetupX req ->                   (sign field BSRSPYL)
                  <- SessionSetupX reply (valid sign field)

and then onwards the signature fields in the header are valid.

In the cifs.ko -> Samba pcap we have:

Negprot request   ->                   (sign field 0)
                  <-  Negprot reply    (sign field 0)
SessionSetupX req ->                   (sign field C238D3A5F1036E20) **** ????
                  <- SessionSetupX reply (sign field 0)
SessionSetupX req ->                   (sign field F0ABF7F1E69AB7F6) **** ????
                  <- SessionSetupX reply (valid sign field)

followed by sign fields that are rejected by Samba.

Why are you not sending the BSRSPYL text in the initial sign headers in sessionsetupX ? What generates the sign field values I marked with "**** ????" ?

Jeremy.
Comment 21 Jeff Layton 2011-07-07 01:42:10 UTC
Created attachment 6668 [details]
patch -- cifs: don't start signing too early

This patch makes cifs send the correct BSRSPYL string in the session setup signature fields but it doesn't seem to fix the issue. I'm not certain, but I don't think that we're getting the sequence wrong here.

Once the last session setup response comes in, the cifs code resets the sequence number explicitly to 2, and that's what sequence we should be using to sign the next packet (usually a TREE_CONNECT).
Comment 22 Shirish S. Pargaonkar 2011-07-07 05:55:35 UTC
(In reply to comment #20)
> Why are you not sending the BSRSPYL text in the initial sign headers in
> sessionsetupX ? What generates the sign field values I marked with "**** ????"
> ?
> Jeremy.

If what cifs client sends in signature field is incorrect i.e. not
sending  BSRSPYL, authentication would/should have failed.
So sending a value in signature field instead of BSRSPYL is not a problem.

The signing algorithm that cifs client is following is:

"
A Message Authentication Key is generated by concatenating the
SMB session key and NTLMv2 client response to the server challenge

An authentiacation block is created by concatenating the message 
authentication key, the packet sequence number, and the SMB payload.

The MD5 hash of the authentication block is calculated.

The first 8 bytes of the MD5 hash of the authentication block are
used as the SMB signature and inserted in the SMB header.
"

All of this is done in functions cifs_sign_smb() and cifs_calculate_signature().
The difference between sec=ntlmv2i and sec=ntlmsspi (which uses 
ntlmv2) is in the session key and the size of the session key.

I will have to verify but I think session key for sec=ntlmv2i is the
challenge that server sends during authentication and session key for 
sec=ntlmsspi is the nonce that is encrypted using rc4 and the sizes are different.
Comment 23 Shirish S. Pargaonkar 2011-07-07 21:19:19 UTC
It is possible, either during negotiation or session setup, cifs client
is missing/overlooking something that Samba server sends (negprot response
or type 2 message of ntlmssp exchange) and so
signing fails but Windows client and smbclient honour!
Will keep looking.
Comment 24 Shirish S. Pargaonkar 2011-07-08 21:11:43 UTC
Created attachment 6676 [details]
patch to fix sec=ntlmsspi against Samba server when signing is mandatory

This patch should fix the signing failure against Samba server when smb.conf
has such entries:

        server signing = mandatory
        client signing = mandatory
        client use spnego = yes

It now makes the flags sent in type 1 and type 3 messages of
ntlmssp exchange same. I thought the flags in type 3 message did not matter
i.e. one type 1 and type 2 messages are exchanged, client and server
pretty much know what each other support but for Samba server, it matters
i.e. if Negotiate Key Exchange bit is missing in type 3 message, signing
fails.  So indiacate so.
Comment 25 Shirish S. Pargaonkar 2011-07-10 18:50:43 UTC
Posted the patch on cifs mailing list.
Comment 26 adarsh 2011-07-29 07:54:56 UTC
The bug is not solved in samba 3.5.10 too. Is there any timeline when the fix will be provided for this? Because I want to upgrade to latest samba version and due to signing issue i am not able to upgrade.
Comment 27 Volker Lendecke 2011-07-29 08:32:34 UTC
Is this really a server bug, or rather a client one? I skimmed over the comments but I did not really come to a firm conclusion. Shirish posting something to a cifs client list would indicate to me it's a client bug, but I'm not sure.
Comment 28 Jeff Layton 2011-07-29 09:57:34 UTC
Yes, this appears to have been a cifs.ko bug that Shirish's patch fixed in 3.0 kernels. I think we can close this now...