FAILED: patch "[PATCH] SMB3: Fix length checking of SMB3.11 negotiate request" failed to apply to 4.16-stable tree

FAILED: patch "[PATCH] SMB3: Fix length checking of SMB3.11 negotiate request" failed to apply to 4.16-stable tree

[gregkh]

The patch below does not apply to the 4.16-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From 136ff1b4b65edf09b6b7173ba94ad53347d3aa83 Mon Sep 17 00:00:00 2001
From: Steve French <smfrench@gmail.com>
Date: Sun, 8 Apr 2018 16:14:31 -0500
Subject: [PATCH] SMB3: Fix length checking of SMB3.11 negotiate request

The length checking for SMB3.11 negotiate request includes
"negotiate contexts" which caused a buffer validation problem
and a confusing warning message on SMB3.11 mount e.g.:

     SMB2 server sent bad RFC1001 len 236 not 170

Fix the length checking for SMB3.11 negotiate to account for
the new negotiate context so that we don't log a warning on
SMB3.11 mount by default but do log warnings if lengths returned
by the server are incorrect.

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 5406e95f5d92..9df9f0b48160 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -93,6 +93,41 @@ static const __le16 smb2_rsp_struct_sizes[NUMBER_OF_SMB2_COMMANDS] = {
 	/* SMB2_OPLOCK_BREAK */ cpu_to_le16(24)
 };
 
+#ifdef CONFIG_CIFS_SMB311
+static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen)
+{
+	__u16 neg_count;
+	__u32 nc_offset, size_of_pad_before_neg_ctxts;
+	struct smb2_negotiate_rsp *pneg_rsp = (struct smb2_negotiate_rsp *)hdr;
+
+	/* Negotiate contexts are only valid for latest dialect SMB3.11 */
+	neg_count = le16_to_cpu(pneg_rsp->NegotiateContextCount);
+	if ((neg_count == 0) ||
+	   (pneg_rsp->DialectRevision != cpu_to_le16(SMB311_PROT_ID)))
+		return 0;
+
+	/* Make sure that negotiate contexts start after gss security blob */
+	nc_offset = le32_to_cpu(pneg_rsp->NegotiateContextOffset);
+	if (nc_offset < non_ctxlen - 4 /* RFC1001 len field */) {
+		printk_once(KERN_WARNING "invalid negotiate context offset\n");
+		return 0;
+	}
+	size_of_pad_before_neg_ctxts = nc_offset - (non_ctxlen - 4);
+
+	/* Verify that at least minimal negotiate contexts fit within frame */
+	if (len < nc_offset + (neg_count * sizeof(struct smb2_neg_context))) {
+		printk_once(KERN_WARNING "negotiate context goes beyond end\n");
+		return 0;
+	}
+
+	cifs_dbg(FYI, "length of negcontexts %d pad %d\n",
+		len - nc_offset, size_of_pad_before_neg_ctxts);
+
+	/* length of negcontexts including pad from end of sec blob to them */
+	return (len - nc_offset) + size_of_pad_before_neg_ctxts;
+}
+#endif /* CIFS_SMB311 */
+
 int
 smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr)
 {
@@ -198,6 +233,10 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr)
 
 	clc_len = smb2_calc_size(hdr);
 
+#ifdef CONFIG_CIFS_SMB311
+	if (shdr->Command == SMB2_NEGOTIATE)
+		clc_len += get_neg_ctxt_len(hdr, len, clc_len);
+#endif /* SMB311 */
 	if (srvr->vals->header_preamble_size + len != clc_len) {
 		cifs_dbg(FYI, "Calculated size %u length %zu mismatch mid %llu\n",
 			 clc_len, srvr->vals->header_preamble_size + len, mid);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 253e2c7c952f..0e0a0af89e4d 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -263,6 +263,13 @@ struct smb2_negotiate_req {
 #define SMB2_NT_FIND			0x00100000
 #define SMB2_LARGE_FILES		0x00200000
 
+struct smb2_neg_context {
+	__le16	ContextType;
+	__le16	DataLength;
+	__le32	Reserved;
+	/* Followed by array of data */
+} __packed;
+
 #define SMB311_SALT_SIZE			32
 /* Hash Algorithm Types */
 #define SMB2_PREAUTH_INTEGRITY_SHA512	cpu_to_le16(0x0001)

Re: FAILED: patch "[PATCH] SMB3: Fix length checking of SMB3.11 negotiate request" failed to apply to 4.16-stable tree

[Aurélien Aptel]

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

gregkh@linuxfoundation.org writes:
> The patch below does not apply to the 4.16-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.

Backport of that particular patch was simple: the original patch assumes
ronnie's header_preamble_size are in, with 4s changed to
header_preamble_size. I just kept the 4s to fix the conflict.

But SMB3.11 is broken on this branch as signing is mandatory in 3.11 and
it needs the preauth integrity patches. I get ACCESS_DENIED on
TreeConnect against samba and signing failures against Windows Server
2016.

So until we also backport these, this patch is not very useful.

Here's the backport anyway if you want to apply it:


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3687 bytes --]

>From 021e83eee2029b77acfb479048472aec0e725ccd Mon Sep 17 00:00:00 2001
From: Steve French <smfrench@gmail.com>
Date: Sun, 8 Apr 2018 16:14:31 -0500
Subject: [PATCH] SMB3: Fix length checking of SMB3.11 negotiate request

commit 136ff1b4b65edf09b6b7173ba94ad53347d3aa83 upstream.

The length checking for SMB3.11 negotiate request includes
"negotiate contexts" which caused a buffer validation problem
and a confusing warning message on SMB3.11 mount e.g.:

     SMB2 server sent bad RFC1001 len 236 not 170

Fix the length checking for SMB3.11 negotiate to account for
the new negotiate context so that we don't log a warning on
SMB3.11 mount by default but do log warnings if lengths returned
by the server are incorrect.

CC: Stable <stable@vger.kernel.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/smb2misc.c | 39 +++++++++++++++++++++++++++++++++++++++
 fs/cifs/smb2pdu.h  |  7 +++++++
 2 files changed, 46 insertions(+)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 76d03abaa38c..8ed363dc8279 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -93,6 +93,41 @@ static const __le16 smb2_rsp_struct_sizes[NUMBER_OF_SMB2_COMMANDS] = {
 	/* SMB2_OPLOCK_BREAK */ cpu_to_le16(24)
 };
 
+#ifdef CONFIG_CIFS_SMB311
+static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len, __u32 non_ctxlen)
+{
+	__u16 neg_count;
+	__u32 nc_offset, size_of_pad_before_neg_ctxts;
+	struct smb2_negotiate_rsp *pneg_rsp = (struct smb2_negotiate_rsp *)hdr;
+
+	/* Negotiate contexts are only valid for latest dialect SMB3.11 */
+	neg_count = le16_to_cpu(pneg_rsp->NegotiateContextCount);
+	if ((neg_count == 0) ||
+	   (pneg_rsp->DialectRevision != cpu_to_le16(SMB311_PROT_ID)))
+		return 0;
+
+	/* Make sure that negotiate contexts start after gss security blob */
+	nc_offset = le32_to_cpu(pneg_rsp->NegotiateContextOffset);
+	if (nc_offset < non_ctxlen - 4 /* RFC1001 len field */) {
+		printk_once(KERN_WARNING "invalid negotiate context offset\n");
+		return 0;
+	}
+	size_of_pad_before_neg_ctxts = nc_offset - (non_ctxlen - 4);
+
+	/* Verify that at least minimal negotiate contexts fit within frame */
+	if (len < nc_offset + (neg_count * sizeof(struct smb2_neg_context))) {
+		printk_once(KERN_WARNING "negotiate context goes beyond end\n");
+		return 0;
+	}
+
+	cifs_dbg(FYI, "length of negcontexts %d pad %d\n",
+		len - nc_offset, size_of_pad_before_neg_ctxts);
+
+	/* length of negcontexts including pad from end of sec blob to them */
+	return (len - nc_offset) + size_of_pad_before_neg_ctxts;
+}
+#endif /* CIFS_SMB311 */
+
 int
 smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr)
 {
@@ -197,6 +232,10 @@ smb2_check_message(char *buf, unsigned int length, struct TCP_Server_Info *srvr)
 
 	clc_len = smb2_calc_size(hdr);
 
+#ifdef CONFIG_CIFS_SMB311
+	if (shdr->Command == SMB2_NEGOTIATE)
+		clc_len += get_neg_ctxt_len(hdr, len, clc_len);
+#endif /* SMB311 */
 	if (4 + len != clc_len) {
 		cifs_dbg(FYI, "Calculated size %u length %u mismatch mid %llu\n",
 			 clc_len, 4 + len, mid);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 2a2b34ccaf49..a6e95652ebb3 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -261,6 +261,13 @@ struct smb2_negotiate_req {
 #define SMB2_NT_FIND			0x00100000
 #define SMB2_LARGE_FILES		0x00200000
 
+struct smb2_neg_context {
+	__le16	ContextType;
+	__le16	DataLength;
+	__le32	Reserved;
+	/* Followed by array of data */
+} __packed;
+
 #define SMB311_SALT_SIZE			32
 /* Hash Algorithm Types */
 #define SMB2_PREAUTH_INTEGRITY_SHA512	cpu_to_le16(0x0001)
-- 
2.13.6


[-- Attachment #3: Type: text/plain, Size: 245 bytes --]


-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: FAILED: patch "[PATCH] SMB3: Fix length checking of SMB3.11 negotiate request" failed to apply to 4.16-stable tree

[Greg KH]

On Fri, Apr 20, 2018 at 12:37:01PM +0200, Aur�lien Aptel wrote:
> gregkh@linuxfoundation.org writes:
> > The patch below does not apply to the 4.16-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> 
> Backport of that particular patch was simple: the original patch assumes
> ronnie's header_preamble_size are in, with 4s changed to
> header_preamble_size. I just kept the 4s to fix the conflict.
> 
> But SMB3.11 is broken on this branch as signing is mandatory in 3.11 and
> it needs the preauth integrity patches. I get ACCESS_DENIED on
> TreeConnect against samba and signing failures against Windows Server
> 2016.
> 
> So until we also backport these, this patch is not very useful.
> 
> Here's the backport anyway if you want to apply it:

Sounds like I do not want it.  If someone could send me a proper patch
series of all of these patches that failed to apply, that would be much
more helpful.

thanks,

greg k-h