Re: [PATCH][SMB3.1.1] add ability to send signing negotiate context

[Steve French <smfrench@gmail.com>]

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

v3 of patch.  Updated with additional feedback from Ronnie (to make it
more context len and datalength clearer)


On Wed, Jul 7, 2021 at 9:44 PM Steve French <smfrench@gmail.com> wrote:
>
> Support for faster packet signing (using GMAC instead of CMAC) can
> now be negotiated to some newer servers, including Windows.
> See MS-SMB2 section 2.2.3.17.
>
> This patch adds support for sending the new negotiate context
> with the first of three supported signing algorithms (AES-CMAC)
> and decoding the response.  A followon patch will add support
> for sending the other two (including AES-GMAC, which is fastest)
> and changing the signing algorithm used based on what was
> negotiated.
>
> To allow the client to request GMAC signing set module parameter
> "enable_negotiate_signing" to 1.
>
> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/cifs/cifsfs.c   |  4 +++
>  fs/cifs/cifsglob.h |  3 ++
>  fs/cifs/smb2pdu.c  | 83 ++++++++++++++++++++++++++++++++++++++++------
>  fs/cifs/smb2pdu.h  |  5 ++-
>  4 files changed, 84 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 9fb874dd8d24..476b07213fcd 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -65,6 +65,7 @@ bool lookupCacheEnabled = true;
>  bool disable_legacy_dialects; /* false by default */
>  bool enable_gcm_256 = true;
>  bool require_gcm_256; /* false by default */
> +bool enable_negotiate_signing; /* false by default */
>  unsigned int global_secflags = CIFSSEC_DEF;
>  /* unsigned int ntlmv2_support = 0; */
>  unsigned int sign_CIFS_PDUs = 1;
> @@ -104,6 +105,9 @@ MODULE_PARM_DESC(enable_gcm_256, "Enable
> requesting strongest (256 bit) GCM encr
>  module_param(require_gcm_256, bool, 0644);
>  MODULE_PARM_DESC(require_gcm_256, "Require strongest (256 bit) GCM
> encryption. Default: n/N/0");
>
> +module_param(enable_negotiate_signing, bool, 0644);
> +MODULE_PARM_DESC(enable_GMAC_signing, "Enable requesting faster
> (GMAC) packet signing. Default: n/N/0");
> +
>  module_param(disable_legacy_dialects, bool, 0644);
>  MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be "
>     "helpful to restrict the ability to "
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 921680fb7931..3c2e117bb926 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -667,9 +667,11 @@ struct TCP_Server_Info {
>   unsigned int max_write;
>   unsigned int min_offload;
>   __le16 compress_algorithm;
> + __u16 signing_algorithm;
>   __le16 cipher_type;
>   /* save initital negprot hash */
>   __u8 preauth_sha_hash[SMB2_PREAUTH_HASH_SIZE];
> + bool signing_negotiated; /* true if valid signing context rcvd from server */
>   bool posix_ext_supported;
>   struct delayed_work reconnect; /* reconnect workqueue job */
>   struct mutex reconnect_mutex; /* prevent simultaneous reconnects */
> @@ -1869,6 +1871,7 @@ extern unsigned int global_secflags; /* if on,
> session setup sent
>  extern unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
>  extern bool enable_gcm_256; /* allow optional negotiate of strongest
> signing (aes-gcm-256) */
>  extern bool require_gcm_256; /* require use of strongest signing
> (aes-gcm-256) */
> +extern bool enable_negotiate_signing; /* request use of faster (GMAC)
> signing if available */
>  extern bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
>  extern unsigned int CIFSMaxBufSize;  /* max size not including hdr */
>  extern unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 962826dc3316..757f145e70e5 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -433,6 +433,23 @@ build_compression_ctxt(struct
> smb2_compression_capabilities_context *pneg_ctxt)
>   pneg_ctxt->CompressionAlgorithms[2] = SMB3_COMPRESS_LZNT1;
>  }
>
> +static void
> +build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
> +{
> + pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES;
> + /*
> + * Data length must be increased here if more than 3 signing algorithms
> + * sent but currently only 3 are defined. And context Data length must
> + * be rounded to multiple of 8 for some servers.
> + */
> + pneg_ctxt->DataLength =
> + cpu_to_le16(DIV_ROUND_UP(sizeof(struct smb2_signing_capabilities) -
> + sizeof(struct smb2_neg_context), 8) * 8);
> + pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(1);
> + pneg_ctxt->SigningAlgorithms[0] = cpu_to_le16(SIGNING_ALG_AES_CMAC);
> + /* TBD add SIGNING_ALG_AES_GMAC and/or SIGNING_ALG_HMAC_SHA256 */
> +}
> +
>  static void
>  build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
>  {
> @@ -498,7 +515,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>         struct TCP_Server_Info *server, unsigned int *total_len)
>  {
>   char *pneg_ctxt;
> - unsigned int ctxt_len;
> + unsigned int ctxt_len, neg_context_count;
>
>   if (*total_len > 200) {
>   /* In case length corrupted don't want to overrun smb buffer */
> @@ -525,6 +542,17 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>   *total_len += ctxt_len;
>   pneg_ctxt += ctxt_len;
>
> + ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
> + server->hostname);
> + *total_len += ctxt_len;
> + pneg_ctxt += ctxt_len;
> +
> + build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
> + *total_len += sizeof(struct smb2_posix_neg_context);
> + pneg_ctxt += sizeof(struct smb2_posix_neg_context);
> +
> + neg_context_count = 4;
> +
>   if (server->compress_algorithm) {
>   build_compression_ctxt((struct smb2_compression_capabilities_context *)
>   pneg_ctxt);
> @@ -533,17 +561,24 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
>   8) * 8;
>   *total_len += ctxt_len;
>   pneg_ctxt += ctxt_len;
> - req->NegotiateContextCount = cpu_to_le16(5);
> - } else
> - req->NegotiateContextCount = cpu_to_le16(4);
> + neg_context_count++;
> + }
>
> - ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
> - server->hostname);
> - *total_len += ctxt_len;
> - pneg_ctxt += ctxt_len;
> + if (enable_negotiate_signing) {
> + pr_warn_once("requesting GMAC signing is experimental\n");
> + build_signing_ctxt((struct smb2_signing_capabilities *)
> + pneg_ctxt);
> + ctxt_len = DIV_ROUND_UP(
> + sizeof(struct smb2_signing_capabilities),
> + 8) * 8;
> + *total_len += ctxt_len;
> + pneg_ctxt += ctxt_len;
> + neg_context_count++;
> + }
> +
> + /* check for and add transport_capabilities and signing capabilities */
> + req->NegotiateContextCount = cpu_to_le16(neg_context_count);
>
> - build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
> - *total_len += sizeof(struct smb2_posix_neg_context);
>  }
>
>  static void decode_preauth_context(struct smb2_preauth_neg_context *ctxt)
> @@ -632,6 +667,31 @@ static int decode_encrypt_ctx(struct
> TCP_Server_Info *server,
>   return 0;
>  }
>
> +static void decode_signing_ctx(struct TCP_Server_Info *server,
> +        struct smb2_signing_capabilities *pctxt)
> +{
> + unsigned int len = le16_to_cpu(pctxt->DataLength);
> +
> + if ((len < 4) || (len > 16)) {
> + pr_warn_once("server sent bad signing negcontext\n");
> + return;
> + }
> + if (le16_to_cpu(pctxt->SigningAlgorithmCount) != 1) {
> + pr_warn_once("Invalid signing algorithm count\n");
> + return;
> + }
> + if (le16_to_cpu(pctxt->SigningAlgorithms[0]) > 2) {
> + pr_warn_once("unknown signing algorithm\n");
> + return;
> + }
> +
> + server->signing_negotiated = true;
> + server->signing_algorithm = le16_to_cpu(pctxt->SigningAlgorithms[0]);
> + cifs_dbg(FYI, "signing algorithm %d chosen\n",
> +      server->signing_algorithm);
> +}
> +
> +
>  static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,
>        struct TCP_Server_Info *server,
>        unsigned int len_of_smb)
> @@ -675,6 +735,9 @@ static int smb311_decode_neg_context(struct
> smb2_negotiate_rsp *rsp,
>   (struct smb2_compression_capabilities_context *)pctx);
>   else if (pctx->ContextType == SMB2_POSIX_EXTENSIONS_AVAILABLE)
>   server->posix_ext_supported = true;
> + else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES)
> + decode_signing_ctx(server,
> + (struct smb2_signing_capabilities *)pctx);
>   else
>   cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n",
>   le16_to_cpu(pctx->ContextType));
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index ba75e65924ac..4b27cb9105fd 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -329,7 +329,7 @@ struct smb2_neg_context {
>   __le16 ContextType;
>   __le16 DataLength;
>   __le32 Reserved;
> - /* Followed by array of data */
> + /* Followed by array of data. NOTE: some servers require padding to
> 8 byte boundary */
>  } __packed;
>
>  #define SMB311_LINUX_CLIENT_SALT_SIZE 32
> @@ -394,6 +394,7 @@ struct smb2_compression_capabilities_context {
>   __u16 Padding;
>   __u32 Flags;
>   __le16 CompressionAlgorithms[3];
> + /* Check if pad needed */
>  } __packed;
>
>  /*
> @@ -420,6 +421,7 @@ struct smb2_transport_capabilities_context {
>   __le16  DataLength;
>   __u32 Reserved;
>   __le32 Flags;
> + __u32 Pad;
>  } __packed;
>
>  /*
> @@ -458,6 +460,7 @@ struct smb2_signing_capabilities {
>   __u32 Reserved;
>   __le16 SigningAlgorithmCount;
>   __le16 SigningAlgorithms[];
> + /*  Followed by padding to 8 byte boundary (required by some servers) */
>  } __packed;
>
>  #define POSIX_CTXT_DATA_LEN 16
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

[-- Attachment #2: 0001-SMB3.1.1-Add-support-for-negotiating-signing-algorit.patch --]
[-- Type: text/x-patch, Size: 9193 bytes --]

From 387c856bf8b8098c2d8750376dcde289f5952ee7 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Mon, 5 Jul 2021 15:05:39 -0500
Subject: [PATCH] SMB3.1.1: Add support for negotiating signing algorithm

Support for faster packet signing (using GMAC instead of CMAC) can
now be negotiated to some newer servers, including Windows.
See MS-SMB2 section 2.2.3.17.

This patch adds support for sending the new negotiate context
with the first of three supported signing algorithms (AES-CMAC)
and decoding the response.  A followon patch will add support
for sending the other two (including AES-GMAC, which is fastest)
and changing the signing algorithm used based on what was
negotiated.

To allow the client to request GMAC signing set module parameter
"enable_negotiate_signing" to 1.

Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifsfs.c   |  4 +++
 fs/cifs/cifsglob.h |  3 ++
 fs/cifs/smb2pdu.c  | 85 ++++++++++++++++++++++++++++++++++++++++------
 fs/cifs/smb2pdu.h  |  5 ++-
 4 files changed, 86 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 9fb874dd8d24..476b07213fcd 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -65,6 +65,7 @@ bool lookupCacheEnabled = true;
 bool disable_legacy_dialects; /* false by default */
 bool enable_gcm_256 = true;
 bool require_gcm_256; /* false by default */
+bool enable_negotiate_signing; /* false by default */
 unsigned int global_secflags = CIFSSEC_DEF;
 /* unsigned int ntlmv2_support = 0; */
 unsigned int sign_CIFS_PDUs = 1;
@@ -104,6 +105,9 @@ MODULE_PARM_DESC(enable_gcm_256, "Enable requesting strongest (256 bit) GCM encr
 module_param(require_gcm_256, bool, 0644);
 MODULE_PARM_DESC(require_gcm_256, "Require strongest (256 bit) GCM encryption. Default: n/N/0");
 
+module_param(enable_negotiate_signing, bool, 0644);
+MODULE_PARM_DESC(enable_GMAC_signing, "Enable requesting faster (GMAC) packet signing. Default: n/N/0");
+
 module_param(disable_legacy_dialects, bool, 0644);
 MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be "
 				  "helpful to restrict the ability to "
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 921680fb7931..3c2e117bb926 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -667,9 +667,11 @@ struct TCP_Server_Info {
 	unsigned int	max_write;
 	unsigned int	min_offload;
 	__le16	compress_algorithm;
+	__u16	signing_algorithm;
 	__le16	cipher_type;
 	 /* save initital negprot hash */
 	__u8	preauth_sha_hash[SMB2_PREAUTH_HASH_SIZE];
+	bool	signing_negotiated; /* true if valid signing context rcvd from server */
 	bool	posix_ext_supported;
 	struct delayed_work reconnect; /* reconnect workqueue job */
 	struct mutex reconnect_mutex; /* prevent simultaneous reconnects */
@@ -1869,6 +1871,7 @@ extern unsigned int global_secflags;	/* if on, session setup sent
 extern unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
 extern bool enable_gcm_256; /* allow optional negotiate of strongest signing (aes-gcm-256) */
 extern bool require_gcm_256; /* require use of strongest signing (aes-gcm-256) */
+extern bool enable_negotiate_signing; /* request use of faster (GMAC) signing if available */
 extern bool linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
 extern unsigned int CIFSMaxBufSize;  /* max size not including hdr */
 extern unsigned int cifs_min_rcv;    /* min size of big ntwrk buf pool */
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 962826dc3316..2258ff413204 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -433,6 +433,28 @@ build_compression_ctxt(struct smb2_compression_capabilities_context *pneg_ctxt)
 	pneg_ctxt->CompressionAlgorithms[2] = SMB3_COMPRESS_LZNT1;
 }
 
+static unsigned int
+build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
+{
+	unsigned int ctxt_len = sizeof(struct smb2_signing_capabilities);
+
+	pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES;
+	/*
+	 * Context Data length must be rounded to multiple of 8 for some servers
+	 */
+	pneg_ctxt->DataLength = cpu_to_le16(DIV_ROUND_UP(
+				sizeof(struct smb2_signing_capabilities) -
+				sizeof(struct smb2_neg_context) +
+				(2 /* sizeof u16 */ * 1 /* SigningAlgorithmCount */), 8) * 8);
+	pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(1);
+	pneg_ctxt->SigningAlgorithms[0] = cpu_to_le16(SIGNING_ALG_AES_CMAC);
+
+	ctxt_len += 2 /* sizeof u16 */ * 1 /* SigningAlgorithmCount */;
+	ctxt_len = DIV_ROUND_UP(ctxt_len, 8) * 8;
+	return ctxt_len;
+	/* TBD add SIGNING_ALG_AES_GMAC and/or SIGNING_ALG_HMAC_SHA256 */
+}
+
 static void
 build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
 {
@@ -498,7 +520,7 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
 		      struct TCP_Server_Info *server, unsigned int *total_len)
 {
 	char *pneg_ctxt;
-	unsigned int ctxt_len;
+	unsigned int ctxt_len, neg_context_count;
 
 	if (*total_len > 200) {
 		/* In case length corrupted don't want to overrun smb buffer */
@@ -525,6 +547,17 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
 	*total_len += ctxt_len;
 	pneg_ctxt += ctxt_len;
 
+	ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
+					server->hostname);
+	*total_len += ctxt_len;
+	pneg_ctxt += ctxt_len;
+
+	build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
+	*total_len += sizeof(struct smb2_posix_neg_context);
+	pneg_ctxt += sizeof(struct smb2_posix_neg_context);
+
+	neg_context_count = 4;
+
 	if (server->compress_algorithm) {
 		build_compression_ctxt((struct smb2_compression_capabilities_context *)
 				pneg_ctxt);
@@ -533,17 +566,21 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
 				8) * 8;
 		*total_len += ctxt_len;
 		pneg_ctxt += ctxt_len;
-		req->NegotiateContextCount = cpu_to_le16(5);
-	} else
-		req->NegotiateContextCount = cpu_to_le16(4);
+		neg_context_count++;
+	}
 
-	ctxt_len = build_netname_ctxt((struct smb2_netname_neg_context *)pneg_ctxt,
-					server->hostname);
-	*total_len += ctxt_len;
-	pneg_ctxt += ctxt_len;
+	if (enable_negotiate_signing) {
+		pr_warn_once("requesting GMAC signing is experimental\n");
+		ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)
+				pneg_ctxt);
+		*total_len += ctxt_len;
+		pneg_ctxt += ctxt_len;
+		neg_context_count++;
+	}
+
+	/* check for and add transport_capabilities and signing capabilities */
+	req->NegotiateContextCount = cpu_to_le16(neg_context_count);
 
-	build_posix_ctxt((struct smb2_posix_neg_context *)pneg_ctxt);
-	*total_len += sizeof(struct smb2_posix_neg_context);
 }
 
 static void decode_preauth_context(struct smb2_preauth_neg_context *ctxt)
@@ -632,6 +669,31 @@ static int decode_encrypt_ctx(struct TCP_Server_Info *server,
 	return 0;
 }
 
+static void decode_signing_ctx(struct TCP_Server_Info *server,
+			       struct smb2_signing_capabilities *pctxt)
+{
+	unsigned int len = le16_to_cpu(pctxt->DataLength);
+
+	if ((len < 4) || (len > 16)) {
+		pr_warn_once("server sent bad signing negcontext\n");
+		return;
+	}
+	if (le16_to_cpu(pctxt->SigningAlgorithmCount) != 1) {
+		pr_warn_once("Invalid signing algorithm count\n");
+		return;
+	}
+	if (le16_to_cpu(pctxt->SigningAlgorithms[0]) > 2) {
+		pr_warn_once("unknown signing algorithm\n");
+		return;
+	}
+
+	server->signing_negotiated = true;
+	server->signing_algorithm = le16_to_cpu(pctxt->SigningAlgorithms[0]);
+	cifs_dbg(FYI, "signing algorithm %d chosen\n",
+		     server->signing_algorithm);
+}
+
+
 static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,
 				     struct TCP_Server_Info *server,
 				     unsigned int len_of_smb)
@@ -675,6 +737,9 @@ static int smb311_decode_neg_context(struct smb2_negotiate_rsp *rsp,
 				(struct smb2_compression_capabilities_context *)pctx);
 		else if (pctx->ContextType == SMB2_POSIX_EXTENSIONS_AVAILABLE)
 			server->posix_ext_supported = true;
+		else if (pctx->ContextType == SMB2_SIGNING_CAPABILITIES)
+			decode_signing_ctx(server,
+				(struct smb2_signing_capabilities *)pctx);
 		else
 			cifs_server_dbg(VFS, "unknown negcontext of type %d ignored\n",
 				le16_to_cpu(pctx->ContextType));
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index ba75e65924ac..4b27cb9105fd 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -329,7 +329,7 @@ struct smb2_neg_context {
 	__le16	ContextType;
 	__le16	DataLength;
 	__le32	Reserved;
-	/* Followed by array of data */
+	/* Followed by array of data. NOTE: some servers require padding to 8 byte boundary */
 } __packed;
 
 #define SMB311_LINUX_CLIENT_SALT_SIZE			32
@@ -394,6 +394,7 @@ struct smb2_compression_capabilities_context {
 	__u16	Padding;
 	__u32	Flags;
 	__le16	CompressionAlgorithms[3];
+	/* Check if pad needed */
 } __packed;
 
 /*
@@ -420,6 +421,7 @@ struct smb2_transport_capabilities_context {
 	__le16  DataLength;
 	__u32	Reserved;
 	__le32	Flags;
+	__u32	Pad;
 } __packed;
 
 /*
@@ -458,6 +460,7 @@ struct smb2_signing_capabilities {
 	__u32	Reserved;
 	__le16	SigningAlgorithmCount;
 	__le16	SigningAlgorithms[];
+	/*  Followed by padding to 8 byte boundary (required by some servers) */
 } __packed;
 
 #define POSIX_CTXT_DATA_LEN	16
-- 
2.30.2