Bug 11103 - Samba does not set the required flags in the SMB2/SMB3 Negotiate Protocol Response when signing required by client
Summary: Samba does not set the required flags in the SMB2/SMB3 Negotiate Protocol Res...
Status: RESOLVED INVALID
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.2.0rc4
Hardware: All All
: P5 major (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-19 00:35 UTC by Steve French
Modified: 2015-02-20 22:41 UTC (History)
3 users (show)

See Also:


Attachments
Possible patch for master/4.2.0 (1.14 KB, patch)
2015-02-19 00:38 UTC, Jeremy Allison
no flags Details
trace showing incorrect flags in negotiate response (1.51 KB, application/x-pcapng)
2015-02-19 00:51 UTC, Steve French
no flags Details
git-am fix for master and 4.2.0 (1.77 KB, patch)
2015-02-19 05:29 UTC, Jeremy Allison
sfrench: review+
Details
git-am fix for 4.2.0 (1.88 KB, patch)
2015-02-19 22:31 UTC, Jeremy Allison
no flags Details
git-am back-port for 4.1.next, 4.0.next. (1.88 KB, patch)
2015-02-19 22:38 UTC, Jeremy Allison
jra: review? (sfrench)
Details
capture with smbclient -mSMB2 --signing=required against Windows 2012 standalone (11.27 KB, application/vnd.tcpdump.pcap)
2015-02-20 19:08 UTC, Stefan Metzmacher
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Steve French 2015-02-19 00:35:10 UTC
Testing with the Microsoft SMB2/SMB3 Functional Test Cases against Samba I found that SigningTestCaseS0 fails since Samba 4.2 doesn't set signing required to true in the negotiate response when the client sends it in the Negotiate Request (dialect is SMB3)

See MS-SMB2 section 3.3.5.4

 If RequireMessageSigning is TRUE, the server MUST also set SMB2_NEGOTIATE_SIGNING_REQUIRED in the SecurityMode field.

5. Session.SigningRequired MUST be set to TRUE under the following conditions:  If the SMB2_NEGOTIATE_SIGNING_REQUIRED bit is set in the SecurityMode field of the client request.  If the SMB2_SESSION_FLAG_IS_GUEST bit is not set in the SessionFlags field and Session.IsAnonymous is FALSE and
Comment 1 Jeremy Allison 2015-02-19 00:38:49 UTC
Created attachment 10744 [details]
Possible patch for master/4.2.0

Steve - please test ! Thanks,

Jeremy.
Comment 2 Steve French 2015-02-19 00:51:17 UTC
Created attachment 10745 [details]
trace showing incorrect flags in negotiate response

See frame 6
Comment 3 Jeremy Allison 2015-02-19 01:01:54 UTC
(In reply to Steve French from comment #2)

Is this with my patch or without ?
Comment 4 Steve French 2015-02-19 01:17:02 UTC
(In reply to Jeremy Allison from comment #3)
Your patch fixes the problem.  Signing test S0 now passes.  This trace was without your patch.  With your patch it gets past this, then does a guest session setup which works - resulting in the test passing.
Comment 5 Jeremy Allison 2015-02-19 05:29:55 UTC
Created attachment 10746 [details]
git-am fix for master and 4.2.0
Comment 6 Steve French 2015-02-19 06:07:36 UTC
Reviewed fix and approve
Comment 7 Jeremy Allison 2015-02-19 06:09:52 UTC
Comment on attachment 10746 [details]
git-am fix for master and 4.2.0

You need to change the '?' in the details dialog to '+' to formally approve the fix for 4.2.0.

Also, please send a reply to the samba-technical mailing list in order to approve the patch for master (you can just hit reply-all on the [PATCH] mail I sent).
Comment 8 Steve French 2015-02-19 06:11:57 UTC
Comment on attachment 10746 [details]
git-am fix for master and 4.2.0

Code Reviewed and ptch tested. Looks fine.
Comment 9 Jeremy Allison 2015-02-19 22:31:45 UTC
Created attachment 10755 [details]
git-am fix for 4.2.0

Cherry-picked from the fix that went into master (reviewed by Steve).
Comment 10 Jeremy Allison 2015-02-19 22:38:57 UTC
Created attachment 10756 [details]
git-am back-port for 4.1.next, 4.0.next.

Backport for 4.1.next, 4.0.next.

Steve please review.
Comment 11 Jeremy Allison 2015-02-19 22:39:28 UTC
Re-assigning to Karolin to get the 4.2.0 patch in, 4.1.x, 4.0.x patches need Stevef review.
Comment 12 Stefan Metzmacher 2015-02-20 19:06:53 UTC
These patches seems to be wrong.

A standalone Windows 2012 doesn't behave like this...
Comment 13 Stefan Metzmacher 2015-02-20 19:08:21 UTC
Created attachment 10759 [details]
capture with smbclient -mSMB2 --signing=required against Windows 2012 standalone

See frames 7 and 8!
Comment 14 Jeremy Allison 2015-02-20 19:27:48 UTC
(In reply to Stefan (metze) Metzmacher from comment #13)

Hmmm. So Windows isn't behaving the way the SMB2 spec requires. What a surprise :-).

Can you try against Windows 8 ? I'm guessing the Windows test authors were only running against their latest server build when they froze the 'correct' responses.
Comment 15 Steve French 2015-02-20 19:45:58 UTC
Comment on attachment 10759 [details]
capture with smbclient -mSMB2 --signing=required against Windows 2012 standalone

You may be right that we should not include this patch - I tested against Windows 10 and did not see this behavior either.  The Microsoft tests have some confusing settings relating to whether encryption is mandated or not (not just signing) and I am rerunning with different settings in the test configuration.   Currently I have at least all but six of the tests in the encryption subcategories passing without JRA's negotiation change (test 2038 for example fails on the negotiate in a similar way to what is described in the bug report but I am working on figuring out if we can configure around it).  I reran all tests (about 200 total) in the signing, encryption and negotiation categories and the only failing ones are these six encryption ones:

2038, 3378, 3707, 3930, 4360 and 4494



-
Comment 16 Jeremy Allison 2015-02-20 19:50:34 UTC
OK, let me know if you want me to revert from master. We should at the very least raise a dochelp request to get them to document in a Windows behavior note that Windows servers themselves don't follow the written spec :-).
Comment 17 Stefan Metzmacher 2015-02-20 20:30:08 UTC
(In reply to Jeremy Allison from comment #16)

Yes, please revert it in master.

A Windows 10 preview release behaves like windows 2012.
Comment 18 Jeremy Allison 2015-02-20 22:41:39 UTC
Looks like this is an SMB2 spec bug. Reverting in master.