Re: [PATCH 4.13 28/43] SMB3: Validate negotiate request must always be signed

[Steve French <smfrench@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1692 bytes --]

Found a patch which solves the dependency issue.  In my testing (on
4.9, with Windows 2016, and also to Samba) as Pavel suggested this
appears to fix the problem, but I will let Srivatsa confirm that it
also fixes it for him.  The two attached patches for 4.9 should work.

As an aside which may help some in testing stable true problems (as a
point of comparison or alternative), I did a complete backport of all
relevant CIFS/SMB3 patches (ie all patches to cifs.ko that are not
dependent on a VFS changes or global kernel API changes) for kernels
4.9 through 4.15
https://github.com/smfrench/smb3-cifs-linux-stable-backports

The individual patches that were included (and in a distinct directory
all cifs patches that were rejected due to global/VFS dependencies)
are also checked in -
https://github.com/smfrench/smb3-backported-patches.

Given the focus on security, these two git trees may be useful for
those who want a cifs.ko which includes all security and functional
improvements and fixes that more closely matches mainline cifs.ko

Srivatsa,
Let us know if those two patches fix your issue as expected.

On Fri, Mar 16, 2018 at 8:32 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Mar 13, 2018 at 10:21:45AM -0500, Steve French wrote:
>> There will be a fix needed to correct an oops in calc_signature,
>> besides the easy patch (smb3 validate negotiate patch).
>
> Ok, I still have no idea how to parse this for a stable tree submission.
>
> So can someone please just send me a simple "apply these git ids to tree
> X.X.y so we can fix the problem", otherwise I'm not going to do anything
> here as I'm really confused,
>
> greg k-h



-- 
Thanks,

Steve

[-- Attachment #2: 0001-SMB3-Validate-negotiate-request-must-always-be-signe.patch --]
[-- Type: text/x-patch, Size: 1223 bytes --]

From 8ac7b1d15dc973e2092ab2b1b5b698eb92e1d1c3 Mon Sep 17 00:00:00 2001
From: Steve French <smfrench@gmail.com>
Date: Sun, 11 Mar 2018 20:00:27 -0700
Subject: [PATCH 1/2] SMB3: Validate negotiate request must always be signed

According to MS-SMB2 3.2.55 validate_negotiate request must
always be signed. Some Windows can fail the request if you send it unsigned

See kernel bugzilla bug 197311

[Patch fixed up for kernel version 4.9]

CC: Stable <stable@vger.kernel.org>
Acked-by: Ronnie Sahlberg <lsahlber.redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
---
 fs/cifs/smb2pdu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 94c4c1901222..4c2eaf05a6a4 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -1712,6 +1712,9 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid,
 	} else
 		iov[0].iov_len = get_rfc1002_length(req) + 4;
 
+	/* validate negotiate request must be signed - see MS-SMB2 3.2.5.5 */
+	if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO)
+		req->hdr.Flags |= SMB2_FLAGS_SIGNED;
 
 	rc = SendReceive2(xid, ses, iov, num_iovecs, &resp_buftype, 0);
 	rsp = (struct smb2_ioctl_rsp *)iov[0].iov_base;
-- 
2.14.1


[-- Attachment #3: 0002-CIFS-Enable-encryption-during-session-setup-phase.patch --]
[-- Type: text/x-patch, Size: 3310 bytes --]

From c5346223ca952a2868bd69a8888133251e517571 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <pshilov@microsoft.com>
Date: Mon, 7 Nov 2016 18:20:50 -0800
Subject: [PATCH 2/2] CIFS: Enable encryption during session setup phase

In order to allow encryption on SMB connection we need to exchange
a session key and generate encryption and decryption keys.

Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/sess.c    | 22 ++++++++++------------
 fs/cifs/smb2pdu.c | 12 ++----------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 538d9b55699a..c3db2a882aee 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -344,13 +344,12 @@ void build_ntlmssp_negotiate_blob(unsigned char *pbuffer,
 	/* BB is NTLMV2 session security format easier to use here? */
 	flags = NTLMSSP_NEGOTIATE_56 |	NTLMSSP_REQUEST_TARGET |
 		NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
-		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
-	if (ses->server->sign) {
+		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC |
+		NTLMSSP_NEGOTIATE_SEAL;
+	if (ses->server->sign)
 		flags |= NTLMSSP_NEGOTIATE_SIGN;
-		if (!ses->server->session_estab ||
-				ses->ntlmssp->sesskey_per_smbsess)
-			flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
-	}
+	if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess)
+		flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
 
 	sec_blob->NegotiateFlags = cpu_to_le32(flags);
 
@@ -407,13 +406,12 @@ int build_ntlmssp_auth_blob(unsigned char **pbuffer,
 	flags = NTLMSSP_NEGOTIATE_56 |
 		NTLMSSP_REQUEST_TARGET | NTLMSSP_NEGOTIATE_TARGET_INFO |
 		NTLMSSP_NEGOTIATE_128 | NTLMSSP_NEGOTIATE_UNICODE |
-		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC;
-	if (ses->server->sign) {
+		NTLMSSP_NEGOTIATE_NTLM | NTLMSSP_NEGOTIATE_EXTENDED_SEC |
+		NTLMSSP_NEGOTIATE_SEAL;
+	if (ses->server->sign)
 		flags |= NTLMSSP_NEGOTIATE_SIGN;
-		if (!ses->server->session_estab ||
-				ses->ntlmssp->sesskey_per_smbsess)
-			flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
-	}
+	if (!ses->server->session_estab || ses->ntlmssp->sesskey_per_smbsess)
+		flags |= NTLMSSP_NEGOTIATE_KEY_XCH;
 
 	tmp = *pbuffer + sizeof(AUTHENTICATE_MESSAGE);
 	sec_blob->NegotiateFlags = cpu_to_le32(flags);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 4c2eaf05a6a4..7c26286a525d 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -707,15 +707,13 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data)
 	struct cifs_ses *ses = sess_data->ses;
 
 	mutex_lock(&ses->server->srv_mutex);
-	if (ses->server->sign && ses->server->ops->generate_signingkey) {
+	if (ses->server->ops->generate_signingkey) {
 		rc = ses->server->ops->generate_signingkey(ses);
-		kfree(ses->auth_key.response);
-		ses->auth_key.response = NULL;
 		if (rc) {
 			cifs_dbg(FYI,
 				"SMB3 session key generation failed\n");
 			mutex_unlock(&ses->server->srv_mutex);
-			goto keygen_exit;
+			return rc;
 		}
 	}
 	if (!ses->server->session_estab) {
@@ -729,12 +727,6 @@ SMB2_sess_establish_session(struct SMB2_sess_data *sess_data)
 	ses->status = CifsGood;
 	ses->need_reconnect = false;
 	spin_unlock(&GlobalMid_Lock);
-
-keygen_exit:
-	if (!ses->server->sign) {
-		kfree(ses->auth_key.response);
-		ses->auth_key.response = NULL;
-	}
 	return rc;
 }
 
-- 
2.14.1