Add support for GCM256 encryption

Add support for GCM256 encryption

[Steve French]

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

Patch series attached that adds support for GCM256 encryption.  It
also clarifies errors returned and warnings on mounts where gcm256 is
required but not supported.  To control this two global parms for the
cifs.ko module are introduced
(/sys/module/cifs/parameters/enable_gcm_256 and
/sys/module/cifs/parameters/require_gcm_256  which are both disabled
by default to reduce the risk of any regressions to servers which do
not support gcm256  (a mount option e.g. "seal=gcm256" also can be
introduced after we have had a chance to test against a wider variety
of servers)

When /sys/module/cifs/parameters/enable_gcm_256 is set then we add
gcm256 to the list of ciphers we request during protocol negotiation
(gcm128 preferred, then gcm256, then lowest in the list is ccm128
since it is slower).

When  /sys/module/cifs/parameters/require_gcm_256 is set then we only
request gcm256 and fail if the server does not support it during
protocol negotiation.

One additional change is going to be needed (to set the session key to
the correct size).


-- 
Thanks,

Steve

[-- Attachment #2: 0002-smb3.1.1-add-new-module-load-parm-enable_gcm_256.patch --]
[-- Type: text/x-patch, Size: 4023 bytes --]

From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 14 Oct 2020 20:24:09 -0500
Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256

Add new module load parameter enable_gcm_256. If set, then add
AES-256-GCM (strongest encryption type) to the list of encryption
types requested. Put it in the list as the second choice (since
AES-128-GCM is faster and much more broadly supported by
SMB3 servers).  To make this stronger encryption type, GCM-256,
required (the first and only choice, you would use module parameter
"require_gcm_256."

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

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 462dbbd17c5f..472cb7777e3e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -71,6 +71,7 @@ bool enable_oplocks = true;
 bool linuxExtEnabled = true;
 bool lookupCacheEnabled = true;
 bool disable_legacy_dialects; /* false by default */
+bool enable_gcm_256;  /* false by default, change when more servers support it */
 bool require_gcm_256; /* false by default */
 unsigned int global_secflags = CIFSSEC_DEF;
 /* unsigned int ntlmv2_support = 0; */
@@ -105,6 +106,9 @@ MODULE_PARM_DESC(slow_rsp_threshold, "Amount of time (in seconds) to wait "
 module_param(enable_oplocks, bool, 0644);
 MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks. Default: y/Y/1");
 
+module_param(enable_gcm_256, bool, 0644);
+MODULE_PARM_DESC(enable_gcm_256, "Enable requesting strongest (256 bit) GCM encryption. Default: n/N/0");
+
 module_param(require_gcm_256, bool, 0644);
 MODULE_PARM_DESC(require_gcm_256, "Require strongest (256 bit) GCM encryption. Default: n/N/0");
 
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ec21af833749..a1a1a16acb38 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1956,6 +1956,7 @@ extern bool lookupCacheEnabled;
 extern unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
 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 linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
 extern unsigned int CIFSMaxBufSize;  /* max size not including hdr */
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index fcae1e3dfcc5..8cfc3122ae5c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -453,6 +453,12 @@ build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
 		pneg_ctxt->DataLength = cpu_to_le16(4); /* Cipher Count + 1 cipher */
 		pneg_ctxt->CipherCount = cpu_to_le16(1);
 		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES256_GCM;
+	} else if (enable_gcm_256) {
+		pneg_ctxt->DataLength = cpu_to_le16(8); /* Cipher Count + 3 ciphers */
+		pneg_ctxt->CipherCount = cpu_to_le16(3);
+		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;
+		pneg_ctxt->Ciphers[1] = SMB2_ENCRYPTION_AES256_GCM;
+		pneg_ctxt->Ciphers[2] = SMB2_ENCRYPTION_AES128_CCM;
 	} else {
 		pneg_ctxt->DataLength = cpu_to_le16(6); /* Cipher Count + 2 ciphers */
 		pneg_ctxt->CipherCount = cpu_to_le16(2);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 5932fc0dc62c..cfc1c7e8addf 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
 	__le16	ContextType; /* 2 */
 	__le16	DataLength;
 	__le32	Reserved;
-	__le16	CipherCount; /* AES-128-GCM and AES-128-CCM */
-	__le16	Ciphers[2];
+	/* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
+	__le16	CipherCount; /* AES128-GCM and AES128-CCM by defalt */
+	__le16	Ciphers[3];
 } __packed;
 
 /* See MS-SMB2 2.2.3.1.3 */
-- 
2.25.1


[-- Attachment #3: 0003-smb3.1.1-print-warning-if-server-does-not-support-re.patch --]
[-- Type: text/x-patch, Size: 1842 bytes --]

From 63f2f2136d390656cec783868e6753967b8ba437 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 15 Oct 2020 00:14:47 -0500
Subject: [PATCH 3/5] smb3.1.1: print warning if server does not support
 requested encryption type

If server does not support AES-256-GCM and it was required on mount, print
warning message. Also log and return a different error message (EOPNOTSUPP)
when encryption mechanism is not supported vs the case when an unknown
unrequested encryption mechanism could be returned (EINVAL).

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2pdu.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 8cfc3122ae5c..d504bc296349 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -610,8 +610,19 @@ static int decode_encrypt_ctx(struct TCP_Server_Info *server,
 		return -EINVAL;
 	}
 	cifs_dbg(FYI, "SMB311 cipher type:%d\n", le16_to_cpu(ctxt->Ciphers[0]));
-	if ((ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_CCM) &&
-	    (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_GCM)) {
+	if (require_gcm_256) {
+		if (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES256_GCM) {
+			cifs_dbg(VFS, "Server does not support requested encryption type (AES256 GCM)\n");
+			return -EOPNOTSUPP;
+		}
+	} else if (ctxt->Ciphers[0] == 0) {
+		/* e.g. if server only supported AES256_CCM (very unlikely) */
+		cifs_dbg(VFS, "Server does not support requested encryption types\n");
+		return -EOPNOTSUPP;
+	} else if ((ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_CCM) &&
+		   (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES128_GCM) &&
+		   (ctxt->Ciphers[0] != SMB2_ENCRYPTION_AES256_GCM)) {
+		/* server returned a cipher we didn't ask for */
 		pr_warn_once("Invalid SMB3.11 cipher returned\n");
 		return -EINVAL;
 	}
-- 
2.25.1


[-- Attachment #4: 0004-smb3.1.1-rename-nonces-used-for-GCM-and-CCM-encrypti.patch --]
[-- Type: text/x-patch, Size: 2300 bytes --]

From d971d7d90574dd60cf16452e606365a6b2887121 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 15 Oct 2020 00:25:02 -0500
Subject: [PATCH 4/5] smb3.1.1: rename nonces used for GCM and CCM encryption

Now that 256 bit encryption can be negotiated, update
names of the nonces to match the updated official protocol
documentation (e.g. AES_GCM_NONCE instead of AES_128GCM_NONCE)
since they apply to both 128 bit and 256 bit encryption.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2ops.c | 8 ++++----
 fs/cifs/smb2pdu.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 76d82a60a550..dd1edabec328 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3821,9 +3821,9 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
 	if (cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		get_random_bytes(&tr_hdr->Nonce, SMB3_AES128GCM_NONCE);
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	else
-		get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
 	memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8);
 }
 
@@ -3993,10 +3993,10 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 	}
 
 	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES128GCM_NONCE);
+		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	else {
 		iv[0] = 3;
-		memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
+		memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
 	}
 
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index cfc1c7e8addf..1b1a12f6bb5f 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -128,8 +128,8 @@ struct smb2_sync_pdu {
 	__le16 StructureSize2; /* size of wct area (varies, request specific) */
 } __packed;
 
-#define SMB3_AES128CCM_NONCE 11
-#define SMB3_AES128GCM_NONCE 12
+#define SMB3_AES_CCM_NONCE 11
+#define SMB3_AES_GCM_NONCE 12
 
 /* Transform flags (for 3.0 dialect this flag indicates CCM */
 #define TRANSFORM_FLAG_ENCRYPTED	0x0001
-- 
2.25.1


[-- Attachment #5: 0005-smb3.1.1-set-gcm256-when-requested.patch --]
[-- Type: text/x-patch, Size: 4248 bytes --]

From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Fri, 11 Sep 2020 16:47:09 -0500
Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested

update code to set 32 byte key length and to set gcm256 when requested
on mount.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2glob.h      |  1 +
 fs/cifs/smb2ops.c       | 20 ++++++++++++--------
 fs/cifs/smb2transport.c | 16 ++++++++--------
 3 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
index cf20f0b5d836..99a1951a01ec 100644
--- a/fs/cifs/smb2glob.h
+++ b/fs/cifs/smb2glob.h
@@ -58,6 +58,7 @@
 #define SMB2_HMACSHA256_SIZE (32)
 #define SMB2_CMACAES_SIZE (16)
 #define SMB3_SIGNKEY_SIZE (16)
+#define SMB3_GCM256_CRYPTKEY_SIZE (32)
 
 /* Maximum buffer size value we can send with 1 credit */
 #define SMB2_MAX_BUFFER_SIZE 65536
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index dd1edabec328..d8e74954d101 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3820,10 +3820,10 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM;
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
-	if (cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
-	else
+	if (cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
+	else /* AES 128 and 256 GCM */
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8);
 }
 
@@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 
 	tfm = enc ? server->secmech.ccmaesencrypt :
 						server->secmech.ccmaesdecrypt;
-	rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
+	if (require_gcm_256)
+		rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
+	else
+		rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
 	if (rc) {
 		cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
 		return rc;
@@ -3992,12 +3997,11 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 		goto free_sg;
 	}
 
-	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
-	else {
+	if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM) {
 		iv[0] = 3;
 		memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
-	}
+	} else /* AES128 and 256 GCM */
+		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
 	aead_request_set_ad(req, assoc_data_len);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index c0348e3b1695..2e810ba91f3b 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -849,12 +849,12 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	struct crypto_aead *tfm;
 
 	if (!server->secmech.ccmaesencrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
-		else
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		else /* AES 128 and 256 GCM */
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
-			cifs_server_dbg(VFS, "%s: Failed to alloc encrypt aead\n",
+			cifs_server_dbg(VFS, "%s: Failed alloc encrypt aead\n",
 				 __func__);
 			return PTR_ERR(tfm);
 		}
@@ -862,14 +862,14 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	}
 
 	if (!server->secmech.ccmaesdecrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
-		else
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		else /* AES 128 and 256 GCM */
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
 			crypto_free_aead(server->secmech.ccmaesencrypt);
 			server->secmech.ccmaesencrypt = NULL;
-			cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
+			cifs_server_dbg(VFS, "%s: Failed alloc decrypt aead\n",
 				 __func__);
 			return PTR_ERR(tfm);
 		}
-- 
2.25.1


[-- Attachment #6: 0001-smb3.1.1-add-new-module-load-parm-require_gcm_256.patch --]
[-- Type: text/x-patch, Size: 3282 bytes --]

From 671d6f325f958b6123ee467a3e00fa134cf9195f Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Fri, 11 Sep 2020 16:19:28 -0500
Subject: [PATCH 1/5] smb3.1.1: add new module load parm require_gcm_256

Add new module load parameter require_gcm_256. If set, then only
request AES-256-GCM (strongest encryption type).

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

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 0fb99d25e8a8..462dbbd17c5f 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -71,6 +71,7 @@ bool enable_oplocks = true;
 bool linuxExtEnabled = true;
 bool lookupCacheEnabled = true;
 bool disable_legacy_dialects; /* false by default */
+bool require_gcm_256; /* 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(slow_rsp_threshold, "Amount of time (in seconds) to wait "
 module_param(enable_oplocks, bool, 0644);
 MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks. Default: y/Y/1");
 
+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(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 5a491afafacc..ec21af833749 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1956,6 +1956,7 @@ extern bool lookupCacheEnabled;
 extern unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
 extern unsigned int sign_CIFS_PDUs;  /* enable smb packet signing */
+extern bool require_gcm_256; /* require use of strongest signing (aes-gcm-256) */
 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 96c172d94fba..fcae1e3dfcc5 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -449,10 +449,16 @@ static void
 build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
 {
 	pneg_ctxt->ContextType = SMB2_ENCRYPTION_CAPABILITIES;
-	pneg_ctxt->DataLength = cpu_to_le16(6); /* Cipher Count + two ciphers */
-	pneg_ctxt->CipherCount = cpu_to_le16(2);
-	pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;
-	pneg_ctxt->Ciphers[1] = SMB2_ENCRYPTION_AES128_CCM;
+	if (require_gcm_256) {
+		pneg_ctxt->DataLength = cpu_to_le16(4); /* Cipher Count + 1 cipher */
+		pneg_ctxt->CipherCount = cpu_to_le16(1);
+		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES256_GCM;
+	} else {
+		pneg_ctxt->DataLength = cpu_to_le16(6); /* Cipher Count + 2 ciphers */
+		pneg_ctxt->CipherCount = cpu_to_le16(2);
+		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;
+		pneg_ctxt->Ciphers[1] = SMB2_ENCRYPTION_AES128_CCM;
+	}
 }
 
 static unsigned int
-- 
2.25.1

Re: Add support for GCM256 encryption

[Aurélien Aptel]

Hi Steve,

Patch 2:

> From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> From: Steve French <stfrench@microsoft.com>
> Date: Wed, 14 Oct 2020 20:24:09 -0500
> Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
>  	__le16	ContextType; /* 2 */
>  	__le16	DataLength;
>  	__le32	Reserved;
> -	__le16	CipherCount; /* AES-128-GCM and AES-128-CCM */
> -	__le16	Ciphers[2];
> +	/* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
> +	__le16	CipherCount; /* AES128-GCM and AES128-CCM by defalt */

Typo defalt => default

> +	__le16	Ciphers[3];
>  } __packed;
>  
>  /* See MS-SMB2 2.2.3.1.3 */
> -- 
> 2.25.1
>

Patch 5:

> From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
> From: Steve French <stfrench@microsoft.com>
> Date: Fri, 11 Sep 2020 16:47:09 -0500
> Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
>
> update code to set 32 byte key length and to set gcm256 when requested
> on mount.
>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> ---
>  fs/cifs/smb2glob.h      |  1 +
>  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
>  fs/cifs/smb2transport.c | 16 ++++++++--------
>  3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index dd1edabec328..d8e74954d101 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
>  
>  	tfm = enc ? server->secmech.ccmaesencrypt :
>  						server->secmech.ccmaesdecrypt;
> -	rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> +
> +	if (require_gcm_256)
> +		rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);

Shouldn't the check be on server->cipher_type?

> +	else
> +		rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> +
>  	if (rc) {
>  		cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
>  		return rc;

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)

Re: Add support for GCM256 encryption

[Steve French]

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

Good point.  Updated patches attached.  Also added a one line comment
to smb2pdu.h mentioning why we don't request AES_256_CCM


On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
>
> Hi Steve,
>
> Patch 2:
>
> > From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> > From: Steve French <stfrench@microsoft.com>
> > Date: Wed, 14 Oct 2020 20:24:09 -0500
> > Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
> > --- a/fs/cifs/smb2pdu.h
> > +++ b/fs/cifs/smb2pdu.h
> > @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
> >       __le16  ContextType; /* 2 */
> >       __le16  DataLength;
> >       __le32  Reserved;
> > -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
> > -     __le16  Ciphers[2];
> > +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
> > +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
>
> Typo defalt => default
>
> > +     __le16  Ciphers[3];
> >  } __packed;
> >
> >  /* See MS-SMB2 2.2.3.1.3 */
> > --
> > 2.25.1
> >
>
> Patch 5:
>
> > From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
> > From: Steve French <stfrench@microsoft.com>
> > Date: Fri, 11 Sep 2020 16:47:09 -0500
> > Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
> >
> > update code to set 32 byte key length and to set gcm256 when requested
> > on mount.
> >
> > Signed-off-by: Steve French <stfrench@microsoft.com>
> > ---
> >  fs/cifs/smb2glob.h      |  1 +
> >  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
> >  fs/cifs/smb2transport.c | 16 ++++++++--------
> >  3 files changed, 21 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index dd1edabec328..d8e74954d101 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> >
> >       tfm = enc ? server->secmech.ccmaesencrypt :
> >                                               server->secmech.ccmaesdecrypt;
> > -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > +
> > +     if (require_gcm_256)
> > +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
>
> Shouldn't the check be on server->cipher_type?
>
> > +     else
> > +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > +
> >       if (rc) {
> >               cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
> >               return rc;
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)



-- 
Thanks,

Steve

[-- Attachment #2: 0002-smb3.1.1-add-new-module-load-parm-enable_gcm_256.patch --]
[-- Type: text/x-patch, Size: 4024 bytes --]

From ae7d9b6a5a4a2b051a490660f0bffea6651ed385 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 14 Oct 2020 20:24:09 -0500
Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256

Add new module load parameter enable_gcm_256. If set, then add
AES-256-GCM (strongest encryption type) to the list of encryption
types requested. Put it in the list as the second choice (since
AES-128-GCM is faster and much more broadly supported by
SMB3 servers).  To make this stronger encryption type, GCM-256,
required (the first and only choice, you would use module parameter
"require_gcm_256."

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

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 462dbbd17c5f..472cb7777e3e 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -71,6 +71,7 @@ bool enable_oplocks = true;
 bool linuxExtEnabled = true;
 bool lookupCacheEnabled = true;
 bool disable_legacy_dialects; /* false by default */
+bool enable_gcm_256;  /* false by default, change when more servers support it */
 bool require_gcm_256; /* false by default */
 unsigned int global_secflags = CIFSSEC_DEF;
 /* unsigned int ntlmv2_support = 0; */
@@ -105,6 +106,9 @@ MODULE_PARM_DESC(slow_rsp_threshold, "Amount of time (in seconds) to wait "
 module_param(enable_oplocks, bool, 0644);
 MODULE_PARM_DESC(enable_oplocks, "Enable or disable oplocks. Default: y/Y/1");
 
+module_param(enable_gcm_256, bool, 0644);
+MODULE_PARM_DESC(enable_gcm_256, "Enable requesting strongest (256 bit) GCM encryption. Default: n/N/0");
+
 module_param(require_gcm_256, bool, 0644);
 MODULE_PARM_DESC(require_gcm_256, "Require strongest (256 bit) GCM encryption. Default: n/N/0");
 
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ec21af833749..a1a1a16acb38 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1956,6 +1956,7 @@ extern bool lookupCacheEnabled;
 extern unsigned int global_secflags;	/* if on, session setup sent
 				with more secure ntlmssp2 challenge/resp */
 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 linuxExtEnabled;/*enable Linux/Unix CIFS extensions*/
 extern unsigned int CIFSMaxBufSize;  /* max size not including hdr */
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index fcae1e3dfcc5..8cfc3122ae5c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -453,6 +453,12 @@ build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
 		pneg_ctxt->DataLength = cpu_to_le16(4); /* Cipher Count + 1 cipher */
 		pneg_ctxt->CipherCount = cpu_to_le16(1);
 		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES256_GCM;
+	} else if (enable_gcm_256) {
+		pneg_ctxt->DataLength = cpu_to_le16(8); /* Cipher Count + 3 ciphers */
+		pneg_ctxt->CipherCount = cpu_to_le16(3);
+		pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;
+		pneg_ctxt->Ciphers[1] = SMB2_ENCRYPTION_AES256_GCM;
+		pneg_ctxt->Ciphers[2] = SMB2_ENCRYPTION_AES128_CCM;
 	} else {
 		pneg_ctxt->DataLength = cpu_to_le16(6); /* Cipher Count + 2 ciphers */
 		pneg_ctxt->CipherCount = cpu_to_le16(2);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 5932fc0dc62c..6f65f1cec8ad 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
 	__le16	ContextType; /* 2 */
 	__le16	DataLength;
 	__le32	Reserved;
-	__le16	CipherCount; /* AES-128-GCM and AES-128-CCM */
-	__le16	Ciphers[2];
+	/* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
+	__le16	CipherCount; /* AES128-GCM and AES128-CCM by default */
+	__le16	Ciphers[3];
 } __packed;
 
 /* See MS-SMB2 2.2.3.1.3 */
-- 
2.25.1


[-- Attachment #3: 0005-smb3.1.1-set-gcm256-when-requested.patch --]
[-- Type: text/x-patch, Size: 4854 bytes --]

From 9160a56cd1849a0a010c17842381fed56828ad4d Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Fri, 11 Sep 2020 16:47:09 -0500
Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested

update code to set 32 byte key length and to set gcm256 when requested
on mount.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2glob.h      |  1 +
 fs/cifs/smb2ops.c       | 20 ++++++++++++--------
 fs/cifs/smb2pdu.h       |  1 +
 fs/cifs/smb2transport.c | 16 ++++++++--------
 4 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
index cf20f0b5d836..99a1951a01ec 100644
--- a/fs/cifs/smb2glob.h
+++ b/fs/cifs/smb2glob.h
@@ -58,6 +58,7 @@
 #define SMB2_HMACSHA256_SIZE (32)
 #define SMB2_CMACAES_SIZE (16)
 #define SMB3_SIGNKEY_SIZE (16)
+#define SMB3_GCM256_CRYPTKEY_SIZE (32)
 
 /* Maximum buffer size value we can send with 1 credit */
 #define SMB2_MAX_BUFFER_SIZE 65536
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index dd1edabec328..c6ec161b96e4 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3820,10 +3820,10 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM;
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
-	if (cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
-	else
+	if (cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
+	else /* AES 128 and 256 GCM */
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8);
 }
 
@@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 
 	tfm = enc ? server->secmech.ccmaesencrypt :
 						server->secmech.ccmaesdecrypt;
-	rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
+	if (server->cipher_type == SMB2_ENCRYPTION_AES256_CCM)
+		rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
+	else
+		rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
 	if (rc) {
 		cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
 		return rc;
@@ -3992,12 +3997,11 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 		goto free_sg;
 	}
 
-	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
-	else {
+	if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM) {
 		iv[0] = 3;
 		memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
-	}
+	} else /* AES128 and 256 GCM */
+		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
 	aead_request_set_ad(req, assoc_data_len);
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 05b010e5a061..851c6cd4742a 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -352,6 +352,7 @@ struct smb2_preauth_neg_context {
 /* Encryption Algorithms Ciphers */
 #define SMB2_ENCRYPTION_AES128_CCM	cpu_to_le16(0x0001)
 #define SMB2_ENCRYPTION_AES128_GCM	cpu_to_le16(0x0002)
+/* we currently do not request AES256_CCM since presumably GCM faster */
 #define SMB2_ENCRYPTION_AES256_CCM      cpu_to_le16(0x0003)
 #define SMB2_ENCRYPTION_AES256_GCM      cpu_to_le16(0x0004)
 
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index c0348e3b1695..2e810ba91f3b 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -849,12 +849,12 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	struct crypto_aead *tfm;
 
 	if (!server->secmech.ccmaesencrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
-		else
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		else /* AES 128 and 256 GCM */
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
-			cifs_server_dbg(VFS, "%s: Failed to alloc encrypt aead\n",
+			cifs_server_dbg(VFS, "%s: Failed alloc encrypt aead\n",
 				 __func__);
 			return PTR_ERR(tfm);
 		}
@@ -862,14 +862,14 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	}
 
 	if (!server->secmech.ccmaesdecrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
-			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
-		else
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_CCM)
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		else /* AES 128 and 256 GCM */
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
 			crypto_free_aead(server->secmech.ccmaesencrypt);
 			server->secmech.ccmaesencrypt = NULL;
-			cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
+			cifs_server_dbg(VFS, "%s: Failed alloc decrypt aead\n",
 				 __func__);
 			return PTR_ERR(tfm);
 		}
-- 
2.25.1

Re: Add support for GCM256 encryption

[Steve French]

found another typo in patch 5 ccm instead of gcm - fixing it now

On Thu, Oct 15, 2020 at 11:33 AM Steve French <smfrench@gmail.com> wrote:
>
> Good point.  Updated patches attached.  Also added a one line comment
> to smb2pdu.h mentioning why we don't request AES_256_CCM
>
>
> On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
> >
> > Hi Steve,
> >
> > Patch 2:
> >
> > > From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> > > From: Steve French <stfrench@microsoft.com>
> > > Date: Wed, 14 Oct 2020 20:24:09 -0500
> > > Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
> > > --- a/fs/cifs/smb2pdu.h
> > > +++ b/fs/cifs/smb2pdu.h
> > > @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
> > >       __le16  ContextType; /* 2 */
> > >       __le16  DataLength;
> > >       __le32  Reserved;
> > > -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
> > > -     __le16  Ciphers[2];
> > > +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
> > > +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
> >
> > Typo defalt => default
> >
> > > +     __le16  Ciphers[3];
> > >  } __packed;
> > >
> > >  /* See MS-SMB2 2.2.3.1.3 */
> > > --
> > > 2.25.1
> > >
> >
> > Patch 5:
> >
> > > From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
> > > From: Steve French <stfrench@microsoft.com>
> > > Date: Fri, 11 Sep 2020 16:47:09 -0500
> > > Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
> > >
> > > update code to set 32 byte key length and to set gcm256 when requested
> > > on mount.
> > >
> > > Signed-off-by: Steve French <stfrench@microsoft.com>
> > > ---
> > >  fs/cifs/smb2glob.h      |  1 +
> > >  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
> > >  fs/cifs/smb2transport.c | 16 ++++++++--------
> > >  3 files changed, 21 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > > index dd1edabec328..d8e74954d101 100644
> > > --- a/fs/cifs/smb2ops.c
> > > +++ b/fs/cifs/smb2ops.c
> > > @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> > >
> > >       tfm = enc ? server->secmech.ccmaesencrypt :
> > >                                               server->secmech.ccmaesdecrypt;
> > > -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > > +
> > > +     if (require_gcm_256)
> > > +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
> >
> > Shouldn't the check be on server->cipher_type?
> >
> > > +     else
> > > +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > > +
> > >       if (rc) {
> > >               cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
> > >               return rc;
> >
> > --
> > Aurélien Aptel / SUSE Labs Samba Team
> > GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

Re: Add support for GCM256 encryption

[Steve French]

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

Redid patch 5 (includes Aurelien's suggestion, fixes a typo and fixes
a problem with vers=3.0 mounts) - attached.

On Thu, Oct 15, 2020 at 12:41 PM Steve French <smfrench@gmail.com> wrote:
>
> found another typo in patch 5 ccm instead of gcm - fixing it now
>
> On Thu, Oct 15, 2020 at 11:33 AM Steve French <smfrench@gmail.com> wrote:
> >
> > Good point.  Updated patches attached.  Also added a one line comment
> > to smb2pdu.h mentioning why we don't request AES_256_CCM
> >
> >
> > On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
> > >
> > > Hi Steve,
> > >
> > > Patch 2:
> > >
> > > > From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> > > > From: Steve French <stfrench@microsoft.com>
> > > > Date: Wed, 14 Oct 2020 20:24:09 -0500
> > > > Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
> > > > --- a/fs/cifs/smb2pdu.h
> > > > +++ b/fs/cifs/smb2pdu.h
> > > > @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
> > > >       __le16  ContextType; /* 2 */
> > > >       __le16  DataLength;
> > > >       __le32  Reserved;
> > > > -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
> > > > -     __le16  Ciphers[2];
> > > > +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
> > > > +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
> > >
> > > Typo defalt => default
> > >
> > > > +     __le16  Ciphers[3];
> > > >  } __packed;
> > > >
> > > >  /* See MS-SMB2 2.2.3.1.3 */
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > Patch 5:
> > >
> > > > From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
> > > > From: Steve French <stfrench@microsoft.com>
> > > > Date: Fri, 11 Sep 2020 16:47:09 -0500
> > > > Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
> > > >
> > > > update code to set 32 byte key length and to set gcm256 when requested
> > > > on mount.
> > > >
> > > > Signed-off-by: Steve French <stfrench@microsoft.com>
> > > > ---
> > > >  fs/cifs/smb2glob.h      |  1 +
> > > >  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
> > > >  fs/cifs/smb2transport.c | 16 ++++++++--------
> > > >  3 files changed, 21 insertions(+), 16 deletions(-)
> > > >
> > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > > > index dd1edabec328..d8e74954d101 100644
> > > > --- a/fs/cifs/smb2ops.c
> > > > +++ b/fs/cifs/smb2ops.c
> > > > @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> > > >
> > > >       tfm = enc ? server->secmech.ccmaesencrypt :
> > > >                                               server->secmech.ccmaesdecrypt;
> > > > -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > > > +
> > > > +     if (require_gcm_256)
> > > > +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
> > >
> > > Shouldn't the check be on server->cipher_type?
> > >
> > > > +     else
> > > > +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> > > > +
> > > >       if (rc) {
> > > >               cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
> > > >               return rc;
> > >
> > > --
> > > Aurélien Aptel / SUSE Labs Samba Team
> > > GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
> > > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
> > > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

[-- Attachment #2: 0005-smb3.1.1-set-gcm256-when-requested.patch --]
[-- Type: text/x-patch, Size: 4238 bytes --]

From 33459a620d7b3ee2c707ef9d2f14118bef814b31 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 15 Oct 2020 23:41:40 -0500
Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested

update smb encryption code to set 32 byte key length and to
set gcm256 when requested on mount.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2glob.h      |  1 +
 fs/cifs/smb2ops.c       | 13 ++++++++++---
 fs/cifs/smb2pdu.h       |  1 +
 fs/cifs/smb2transport.c |  8 +++++---
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
index cf20f0b5d836..99a1951a01ec 100644
--- a/fs/cifs/smb2glob.h
+++ b/fs/cifs/smb2glob.h
@@ -58,6 +58,7 @@
 #define SMB2_HMACSHA256_SIZE (32)
 #define SMB2_CMACAES_SIZE (16)
 #define SMB3_SIGNKEY_SIZE (16)
+#define SMB3_GCM256_CRYPTKEY_SIZE (32)
 
 /* Maximum buffer size value we can send with 1 credit */
 #define SMB2_MAX_BUFFER_SIZE 65536
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index dd1edabec328..48657ddbd75e 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3820,7 +3820,8 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM;
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
-	if (cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+	if ((cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
+	    (cipher_type == SMB2_ENCRYPTION_AES256_GCM))
 		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	else
 		get_random_bytes(&tr_hdr->Nonce, SMB3_AES_CCM_NONCE);
@@ -3954,7 +3955,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 
 	tfm = enc ? server->secmech.ccmaesencrypt :
 						server->secmech.ccmaesdecrypt;
-	rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
+	if (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
+		rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
+	else
+		rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
+
 	if (rc) {
 		cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
 		return rc;
@@ -3992,7 +3998,8 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 		goto free_sg;
 	}
 
-	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+	if ((server->cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
+	    (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
 		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES_GCM_NONCE);
 	else {
 		iv[0] = 3;
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 05b010e5a061..851c6cd4742a 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -352,6 +352,7 @@ struct smb2_preauth_neg_context {
 /* Encryption Algorithms Ciphers */
 #define SMB2_ENCRYPTION_AES128_CCM	cpu_to_le16(0x0001)
 #define SMB2_ENCRYPTION_AES128_GCM	cpu_to_le16(0x0002)
+/* we currently do not request AES256_CCM since presumably GCM faster */
 #define SMB2_ENCRYPTION_AES256_CCM      cpu_to_le16(0x0003)
 #define SMB2_ENCRYPTION_AES256_GCM      cpu_to_le16(0x0004)
 
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index c0348e3b1695..ebccd71cc60a 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -849,12 +849,13 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	struct crypto_aead *tfm;
 
 	if (!server->secmech.ccmaesencrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+		if ((server->cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
+		    (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
 			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		else
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
-			cifs_server_dbg(VFS, "%s: Failed to alloc encrypt aead\n",
+			cifs_server_dbg(VFS, "%s: Failed alloc encrypt aead\n",
 				 __func__);
 			return PTR_ERR(tfm);
 		}
@@ -862,7 +863,8 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	}
 
 	if (!server->secmech.ccmaesdecrypt) {
-		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+		if ((server->cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
+		    (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
 			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 		else
 			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
-- 
2.25.1

Re: Add support for GCM256 encryption

[Stefan Metzmacher]

[-- Attachment #1.1: Type: text/plain, Size: 3952 bytes --]

Am 16.10.20 um 06:45 schrieb Steve French:
> Redid patch 5 (includes Aurelien's suggestion, fixes a typo and fixes
> a problem with vers=3.0 mounts) - attached.
> 
> On Thu, Oct 15, 2020 at 12:41 PM Steve French <smfrench@gmail.com> wrote:
>>
>> found another typo in patch 5 ccm instead of gcm - fixing it now
>>
>> On Thu, Oct 15, 2020 at 11:33 AM Steve French <smfrench@gmail.com> wrote:
>>>
>>> Good point.  Updated patches attached.  Also added a one line comment
>>> to smb2pdu.h mentioning why we don't request AES_256_CCM
>>>
>>>
>>> On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
>>>>
>>>> Hi Steve,
>>>>
>>>> Patch 2:
>>>>
>>>>> From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
>>>>> From: Steve French <stfrench@microsoft.com>
>>>>> Date: Wed, 14 Oct 2020 20:24:09 -0500
>>>>> Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
>>>>> --- a/fs/cifs/smb2pdu.h
>>>>> +++ b/fs/cifs/smb2pdu.h
>>>>> @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
>>>>>       __le16  ContextType; /* 2 */
>>>>>       __le16  DataLength;
>>>>>       __le32  Reserved;
>>>>> -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
>>>>> -     __le16  Ciphers[2];
>>>>> +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
>>>>> +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
>>>>
>>>> Typo defalt => default
>>>>
>>>>> +     __le16  Ciphers[3];
>>>>>  } __packed;
>>>>>
>>>>>  /* See MS-SMB2 2.2.3.1.3 */
>>>>> --
>>>>> 2.25.1
>>>>>
>>>>
>>>> Patch 5:
>>>>
>>>>> From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
>>>>> From: Steve French <stfrench@microsoft.com>
>>>>> Date: Fri, 11 Sep 2020 16:47:09 -0500
>>>>> Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
>>>>>
>>>>> update code to set 32 byte key length and to set gcm256 when requested
>>>>> on mount.
>>>>>
>>>>> Signed-off-by: Steve French <stfrench@microsoft.com>
>>>>> ---
>>>>>  fs/cifs/smb2glob.h      |  1 +
>>>>>  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
>>>>>  fs/cifs/smb2transport.c | 16 ++++++++--------
>>>>>  3 files changed, 21 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>>>>> index dd1edabec328..d8e74954d101 100644
>>>>> --- a/fs/cifs/smb2ops.c
>>>>> +++ b/fs/cifs/smb2ops.c
>>>>> @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
>>>>>
>>>>>       tfm = enc ? server->secmech.ccmaesencrypt :
>>>>>                                               server->secmech.ccmaesdecrypt;
>>>>> -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
>>>>> +
>>>>> +     if (require_gcm_256)
>>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
>>>>
>>>> Shouldn't the check be on server->cipher_type?
>>>>
>>>>> +     else
>>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
>>>>> +

You still just use u8 key[SMB3_SIGN_KEY_SIZE];

Shouldn't smb2_get_enc_key() get the buffer size and return the size actually used?

I also don't see where you setup the 32 byte encryption/decryption keys from
the authentication session key?

[MS-SMB2] 3.3.5.5.3 Handling GSS-API Authentication point 11.)
specifies that the full authentication session key should be used as key derivation key
for AES256 (NTLMSSP always returns 16 bytes, kerberos can return 16 or 32 bytes).
3.3.1.8 Per Session says the resulting keys should be 256-bit (32 bytes) for AES-256.

I don't see any of this in your patchset.

Did you actually tested this successful against a Windows Server?

Can you use 'git format-patch --stdout > patches.txt' and attach patches.txt
as inline text attachment? Or use git send-email ...
Individual randomly sorted non text/plain attachments are very hard to comment on
(at least for me).

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Add support for GCM256 encryption

[Steve French]

The patch series is missing the final piece (the small change to
encryption related parms), because I was waiting to test that part
with Pavel.  I did verify the negotiation works fine to the newest
Windows download mentioned earlier but also wanted to try it to Azure
and expect the final change needed to be small.  I had also been
trying to focus on the larger changes needed during the merge window
and finish the final piece off next week of this one.


On Thu, Oct 22, 2020 at 7:59 AM Stefan Metzmacher <metze@samba.org> wrote:
>
> Am 16.10.20 um 06:45 schrieb Steve French:
> > Redid patch 5 (includes Aurelien's suggestion, fixes a typo and fixes
> > a problem with vers=3.0 mounts) - attached.
> >
> > On Thu, Oct 15, 2020 at 12:41 PM Steve French <smfrench@gmail.com> wrote:
> >>
> >> found another typo in patch 5 ccm instead of gcm - fixing it now
> >>
> >> On Thu, Oct 15, 2020 at 11:33 AM Steve French <smfrench@gmail.com> wrote:
> >>>
> >>> Good point.  Updated patches attached.  Also added a one line comment
> >>> to smb2pdu.h mentioning why we don't request AES_256_CCM
> >>>
> >>>
> >>> On Thu, Oct 15, 2020 at 3:49 AM Aurélien Aptel <aaptel@suse.com> wrote:
> >>>>
> >>>> Hi Steve,
> >>>>
> >>>> Patch 2:
> >>>>
> >>>>> From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> >>>>> From: Steve French <stfrench@microsoft.com>
> >>>>> Date: Wed, 14 Oct 2020 20:24:09 -0500
> >>>>> Subject: [PATCH 2/5] smb3.1.1: add new module load parm enable_gcm_256
> >>>>> --- a/fs/cifs/smb2pdu.h
> >>>>> +++ b/fs/cifs/smb2pdu.h
> >>>>> @@ -361,8 +361,9 @@ struct smb2_encryption_neg_context {
> >>>>>       __le16  ContextType; /* 2 */
> >>>>>       __le16  DataLength;
> >>>>>       __le32  Reserved;
> >>>>> -     __le16  CipherCount; /* AES-128-GCM and AES-128-CCM */
> >>>>> -     __le16  Ciphers[2];
> >>>>> +     /* CipherCount usally 2, but can be 3 when AES256-GCM enabled */
> >>>>> +     __le16  CipherCount; /* AES128-GCM and AES128-CCM by defalt */
> >>>>
> >>>> Typo defalt => default
> >>>>
> >>>>> +     __le16  Ciphers[3];
> >>>>>  } __packed;
> >>>>>
> >>>>>  /* See MS-SMB2 2.2.3.1.3 */
> >>>>> --
> >>>>> 2.25.1
> >>>>>
> >>>>
> >>>> Patch 5:
> >>>>
> >>>>> From 314d7476e404c37acb77c3f9ecc142122e7afbfd Mon Sep 17 00:00:00 2001
> >>>>> From: Steve French <stfrench@microsoft.com>
> >>>>> Date: Fri, 11 Sep 2020 16:47:09 -0500
> >>>>> Subject: [PATCH 5/5] smb3.1.1: set gcm256 when requested
> >>>>>
> >>>>> update code to set 32 byte key length and to set gcm256 when requested
> >>>>> on mount.
> >>>>>
> >>>>> Signed-off-by: Steve French <stfrench@microsoft.com>
> >>>>> ---
> >>>>>  fs/cifs/smb2glob.h      |  1 +
> >>>>>  fs/cifs/smb2ops.c       | 20 ++++++++++++--------
> >>>>>  fs/cifs/smb2transport.c | 16 ++++++++--------
> >>>>>  3 files changed, 21 insertions(+), 16 deletions(-)
> >>>>>
> >>>>> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> >>>>> index dd1edabec328..d8e74954d101 100644
> >>>>> --- a/fs/cifs/smb2ops.c
> >>>>> +++ b/fs/cifs/smb2ops.c
> >>>>> @@ -3954,7 +3954,12 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> >>>>>
> >>>>>       tfm = enc ? server->secmech.ccmaesencrypt :
> >>>>>                                               server->secmech.ccmaesdecrypt;
> >>>>> -     rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> >>>>> +
> >>>>> +     if (require_gcm_256)
> >>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_GCM256_CRYPTKEY_SIZE);
> >>>>
> >>>> Shouldn't the check be on server->cipher_type?
> >>>>
> >>>>> +     else
> >>>>> +             rc = crypto_aead_setkey(tfm, key, SMB3_SIGN_KEY_SIZE);
> >>>>> +
>
> You still just use u8 key[SMB3_SIGN_KEY_SIZE];
>
> Shouldn't smb2_get_enc_key() get the buffer size and return the size actually used?
>
> I also don't see where you setup the 32 byte encryption/decryption keys from
> the authentication session key?
>
> [MS-SMB2] 3.3.5.5.3 Handling GSS-API Authentication point 11.)
> specifies that the full authentication session key should be used as key derivation key
> for AES256 (NTLMSSP always returns 16 bytes, kerberos can return 16 or 32 bytes).
> 3.3.1.8 Per Session says the resulting keys should be 256-bit (32 bytes) for AES-256.
>
> I don't see any of this in your patchset.
>
> Did you actually tested this successful against a Windows Server?
>
> Can you use 'git format-patch --stdout > patches.txt' and attach patches.txt
> as inline text attachment? Or use git send-email ...
> Individual randomly sorted non text/plain attachments are very hard to comment on
> (at least for me).
>
> metze
>


--
Thanks,

Steve