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@xxxxxxxx> wrote:
>
> Hi Steve,
>
> Patch 2:
>
> > From 3897b440fd14dfc7b2ad2b0a922302ea7705b5d9 Mon Sep 17 00:00:00 2001
> > From: Steve French <stfrench@xxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxx>
> > 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@xxxxxxxxxxxxx>
> > ---
> > 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
From ae7d9b6a5a4a2b051a490660f0bffea6651ed385 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
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@xxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
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
From 9160a56cd1849a0a010c17842381fed56828ad4d Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
---
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
