* [PATCH v3 1/8] smb3: rename encryption/decryption TFMs
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 5:18 ` Steve French
2022-09-29 1:56 ` [PATCH v3 2/8] cifs: secmech: use shash_desc directly, remove sdesc Enzo Matsumiya
` (6 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
Detach the TFM name from a specific algorithm (AES-CCM) as
AES-GCM is also supported, making the name misleading.
s/ccmaesencrypt/enc/
s/ccmaesdecrypt/dec/
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsencrypt.c | 12 ++++++------
fs/cifs/cifsglob.h | 4 ++--
fs/cifs/smb2ops.c | 3 +--
fs/cifs/smb2transport.c | 12 ++++++------
4 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 46f5718754f9..f622d2ba6bd0 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -743,14 +743,14 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
server->secmech.hmacmd5 = NULL;
}
- if (server->secmech.ccmaesencrypt) {
- crypto_free_aead(server->secmech.ccmaesencrypt);
- server->secmech.ccmaesencrypt = NULL;
+ if (server->secmech.enc) {
+ crypto_free_aead(server->secmech.enc);
+ server->secmech.enc = NULL;
}
- if (server->secmech.ccmaesdecrypt) {
- crypto_free_aead(server->secmech.ccmaesdecrypt);
- server->secmech.ccmaesdecrypt = NULL;
+ if (server->secmech.dec) {
+ crypto_free_aead(server->secmech.dec);
+ server->secmech.dec = NULL;
}
kfree(server->secmech.sdesccmacaes);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ae7f571a7dba..cbb108b15412 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -171,8 +171,8 @@ struct cifs_secmech {
struct sdesc *sdeschmacsha256; /* ctxt to generate smb2 signature */
struct sdesc *sdesccmacaes; /* ctxt to generate smb3 signature */
struct sdesc *sdescsha512; /* ctxt to generate smb3.11 signing key */
- struct crypto_aead *ccmaesencrypt; /* smb3 encryption aead */
- struct crypto_aead *ccmaesdecrypt; /* smb3 decryption aead */
+ struct crypto_aead *enc; /* smb3 AEAD encryption TFM (AES-CCM and AES-GCM) */
+ struct crypto_aead *dec; /* smb3 AEAD decryption TFM (AES-CCM and AES-GCM) */
};
/* per smb session structure/fields */
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 421be43af425..d1528755f330 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4344,8 +4344,7 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
return rc;
}
- tfm = enc ? server->secmech.ccmaesencrypt :
- server->secmech.ccmaesdecrypt;
+ tfm = enc ? server->secmech.enc : server->secmech.dec;
if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
(server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 4640fc4a8b13..d4e1a5d74dcd 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -904,7 +904,7 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
{
struct crypto_aead *tfm;
- if (!server->secmech.ccmaesencrypt) {
+ if (!server->secmech.enc) {
if ((server->cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
(server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
@@ -915,23 +915,23 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
__func__);
return PTR_ERR(tfm);
}
- server->secmech.ccmaesencrypt = tfm;
+ server->secmech.enc = tfm;
}
- if (!server->secmech.ccmaesdecrypt) {
+ if (!server->secmech.dec) {
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)) {
- crypto_free_aead(server->secmech.ccmaesencrypt);
- server->secmech.ccmaesencrypt = NULL;
+ crypto_free_aead(server->secmech.enc);
+ server->secmech.enc = NULL;
cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
__func__);
return PTR_ERR(tfm);
}
- server->secmech.ccmaesdecrypt = tfm;
+ server->secmech.dec = tfm;
}
return 0;
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 1/8] smb3: rename encryption/decryption TFMs
2022-09-29 1:56 ` [PATCH v3 1/8] smb3: rename encryption/decryption TFMs Enzo Matsumiya
@ 2022-09-29 5:18 ` Steve French
0 siblings, 0 replies; 18+ messages in thread
From: Steve French @ 2022-09-29 5:18 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: linux-cifs, pc, ronniesahlberg, nspmangalore, tom, metze
looks fine - merged into cifs-2.6.git for-next
still reviewing/testing others in the series. Feedback on those would
be appreciated.
On Wed, Sep 28, 2022 at 8:57 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Detach the TFM name from a specific algorithm (AES-CCM) as
> AES-GCM is also supported, making the name misleading.
>
> s/ccmaesencrypt/enc/
> s/ccmaesdecrypt/dec/
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/cifsencrypt.c | 12 ++++++------
> fs/cifs/cifsglob.h | 4 ++--
> fs/cifs/smb2ops.c | 3 +--
> fs/cifs/smb2transport.c | 12 ++++++------
> 4 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 46f5718754f9..f622d2ba6bd0 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -743,14 +743,14 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
> server->secmech.hmacmd5 = NULL;
> }
>
> - if (server->secmech.ccmaesencrypt) {
> - crypto_free_aead(server->secmech.ccmaesencrypt);
> - server->secmech.ccmaesencrypt = NULL;
> + if (server->secmech.enc) {
> + crypto_free_aead(server->secmech.enc);
> + server->secmech.enc = NULL;
> }
>
> - if (server->secmech.ccmaesdecrypt) {
> - crypto_free_aead(server->secmech.ccmaesdecrypt);
> - server->secmech.ccmaesdecrypt = NULL;
> + if (server->secmech.dec) {
> + crypto_free_aead(server->secmech.dec);
> + server->secmech.dec = NULL;
> }
>
> kfree(server->secmech.sdesccmacaes);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index ae7f571a7dba..cbb108b15412 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -171,8 +171,8 @@ struct cifs_secmech {
> struct sdesc *sdeschmacsha256; /* ctxt to generate smb2 signature */
> struct sdesc *sdesccmacaes; /* ctxt to generate smb3 signature */
> struct sdesc *sdescsha512; /* ctxt to generate smb3.11 signing key */
> - struct crypto_aead *ccmaesencrypt; /* smb3 encryption aead */
> - struct crypto_aead *ccmaesdecrypt; /* smb3 decryption aead */
> + struct crypto_aead *enc; /* smb3 AEAD encryption TFM (AES-CCM and AES-GCM) */
> + struct crypto_aead *dec; /* smb3 AEAD decryption TFM (AES-CCM and AES-GCM) */
> };
>
> /* per smb session structure/fields */
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 421be43af425..d1528755f330 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -4344,8 +4344,7 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> return rc;
> }
>
> - tfm = enc ? server->secmech.ccmaesencrypt :
> - server->secmech.ccmaesdecrypt;
> + tfm = enc ? server->secmech.enc : server->secmech.dec;
>
> if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
> (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index 4640fc4a8b13..d4e1a5d74dcd 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -904,7 +904,7 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
> {
> struct crypto_aead *tfm;
>
> - if (!server->secmech.ccmaesencrypt) {
> + if (!server->secmech.enc) {
> if ((server->cipher_type == SMB2_ENCRYPTION_AES128_GCM) ||
> (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM))
> tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
> @@ -915,23 +915,23 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
> __func__);
> return PTR_ERR(tfm);
> }
> - server->secmech.ccmaesencrypt = tfm;
> + server->secmech.enc = tfm;
> }
>
> - if (!server->secmech.ccmaesdecrypt) {
> + if (!server->secmech.dec) {
> 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)) {
> - crypto_free_aead(server->secmech.ccmaesencrypt);
> - server->secmech.ccmaesencrypt = NULL;
> + crypto_free_aead(server->secmech.enc);
> + server->secmech.enc = NULL;
> cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
> __func__);
> return PTR_ERR(tfm);
> }
> - server->secmech.ccmaesdecrypt = tfm;
> + server->secmech.dec = tfm;
> }
>
> return 0;
> --
> 2.35.3
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 2/8] cifs: secmech: use shash_desc directly, remove sdesc
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 1/8] smb3: rename encryption/decryption TFMs Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 3/8] cifs: allocate ephemeral secmechs only on demand Enzo Matsumiya
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
The struct sdesc is just a wrapper around shash_desc, with exact same
memory layout. Replace the hashing TFMs with shash_desc as it's what's
passed to the crypto API anyway.
Also remove the crypto_shash pointers as they can be accessed via
shash_desc->tfm (and are actually only used in the setkey calls).
Adapt cifs_{alloc,free}_hash functions to this change.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsencrypt.c | 86 +++++++++++++----------------------------
fs/cifs/cifsglob.h | 26 ++++---------
fs/cifs/cifsproto.h | 5 +--
fs/cifs/link.c | 13 +++----
fs/cifs/misc.c | 49 ++++++++++++-----------
fs/cifs/smb2misc.c | 13 +++----
fs/cifs/smb2transport.c | 72 +++++++++++++---------------------
7 files changed, 98 insertions(+), 166 deletions(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index f622d2ba6bd0..30ece0c58c71 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -103,26 +103,24 @@ static int cifs_calc_signature(struct smb_rqst *rqst,
if (!rqst->rq_iov || !signature || !server)
return -EINVAL;
- rc = cifs_alloc_hash("md5", &server->secmech.md5,
- &server->secmech.sdescmd5);
+ rc = cifs_alloc_hash("md5", &server->secmech.md5);
if (rc)
return -1;
- rc = crypto_shash_init(&server->secmech.sdescmd5->shash);
+ rc = crypto_shash_init(server->secmech.md5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init md5\n", __func__);
return rc;
}
- rc = crypto_shash_update(&server->secmech.sdescmd5->shash,
+ rc = crypto_shash_update(server->secmech.md5,
server->session_key.response, server->session_key.len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
return rc;
}
- return __cifs_calc_signature(rqst, server, signature,
- &server->secmech.sdescmd5->shash);
+ return __cifs_calc_signature(rqst, server, signature, server->secmech.md5);
}
/* must be called with server->srv_mutex held */
@@ -412,7 +410,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
wchar_t *domain;
wchar_t *server;
- if (!ses->server->secmech.sdeschmacmd5) {
+ if (!ses->server->secmech.hmacmd5) {
cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
return -1;
}
@@ -420,14 +418,14 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
/* calculate md4 hash of password */
E_md4hash(ses->password, nt_hash, nls_cp);
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5, nt_hash,
+ rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm, nt_hash,
CIFS_NTHASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NT Hash as a key\n", __func__);
return rc;
}
- rc = crypto_shash_init(&ses->server->secmech.sdeschmacmd5->shash);
+ rc = crypto_shash_init(ses->server->secmech.hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
return rc;
@@ -448,7 +446,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
memset(user, '\0', 2);
}
- rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_update(ses->server->secmech.hmacmd5,
(char *)user, 2 * len);
kfree(user);
if (rc) {
@@ -468,7 +466,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
len = cifs_strtoUTF16((__le16 *)domain, ses->domainName, len,
nls_cp);
rc =
- crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
+ crypto_shash_update(ses->server->secmech.hmacmd5,
(char *)domain, 2 * len);
kfree(domain);
if (rc) {
@@ -488,7 +486,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
len = cifs_strtoUTF16((__le16 *)server, ses->ip_addr, len,
nls_cp);
rc =
- crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
+ crypto_shash_update(ses->server->secmech.hmacmd5,
(char *)server, 2 * len);
kfree(server);
if (rc) {
@@ -498,7 +496,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
}
- rc = crypto_shash_final(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_final(ses->server->secmech.hmacmd5,
ntlmv2_hash);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
@@ -518,12 +516,12 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
hash_len = ses->auth_key.len - (CIFS_SESS_KEY_SIZE +
offsetof(struct ntlmv2_resp, challenge.key[0]));
- if (!ses->server->secmech.sdeschmacmd5) {
+ if (!ses->server->secmech.hmacmd5) {
cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
return -1;
}
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5,
+ rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
@@ -531,7 +529,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
return rc;
}
- rc = crypto_shash_init(&ses->server->secmech.sdeschmacmd5->shash);
+ rc = crypto_shash_init(ses->server->secmech.hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
return rc;
@@ -543,7 +541,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
else
memcpy(ntlmv2->challenge.key,
ses->server->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
- rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_update(ses->server->secmech.hmacmd5,
ntlmv2->challenge.key, hash_len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
@@ -551,7 +549,7 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
}
/* Note that the MD5 digest over writes anon.challenge_key.key */
- rc = crypto_shash_final(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_final(ses->server->secmech.hmacmd5,
ntlmv2->ntlmv2_hash);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
@@ -627,9 +625,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
cifs_server_lock(ses->server);
- rc = cifs_alloc_hash("hmac(md5)",
- &ses->server->secmech.hmacmd5,
- &ses->server->secmech.sdeschmacmd5);
+ rc = cifs_alloc_hash("hmac(md5)", &ses->server->secmech.hmacmd5);
if (rc) {
goto unlock;
}
@@ -649,7 +645,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
}
/* now calculate the session key for NTLMv2 */
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5,
+ rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
@@ -657,13 +653,13 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
goto unlock;
}
- rc = crypto_shash_init(&ses->server->secmech.sdeschmacmd5->shash);
+ rc = crypto_shash_init(ses->server->secmech.hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
goto unlock;
}
- rc = crypto_shash_update(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_update(ses->server->secmech.hmacmd5,
ntlmv2->ntlmv2_hash,
CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
@@ -671,7 +667,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
goto unlock;
}
- rc = crypto_shash_final(&ses->server->secmech.sdeschmacmd5->shash,
+ rc = crypto_shash_final(ses->server->secmech.hmacmd5,
ses->auth_key.response);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
@@ -718,30 +714,11 @@ calc_seckey(struct cifs_ses *ses)
void
cifs_crypto_secmech_release(struct TCP_Server_Info *server)
{
- if (server->secmech.cmacaes) {
- crypto_free_shash(server->secmech.cmacaes);
- server->secmech.cmacaes = NULL;
- }
-
- if (server->secmech.hmacsha256) {
- crypto_free_shash(server->secmech.hmacsha256);
- server->secmech.hmacsha256 = NULL;
- }
-
- if (server->secmech.md5) {
- crypto_free_shash(server->secmech.md5);
- server->secmech.md5 = NULL;
- }
-
- if (server->secmech.sha512) {
- crypto_free_shash(server->secmech.sha512);
- server->secmech.sha512 = NULL;
- }
-
- if (server->secmech.hmacmd5) {
- crypto_free_shash(server->secmech.hmacmd5);
- server->secmech.hmacmd5 = NULL;
- }
+ cifs_free_hash(&server->secmech.aes_cmac);
+ cifs_free_hash(&server->secmech.hmacsha256);
+ cifs_free_hash(&server->secmech.md5);
+ cifs_free_hash(&server->secmech.sha512);
+ cifs_free_hash(&server->secmech.hmacmd5);
if (server->secmech.enc) {
crypto_free_aead(server->secmech.enc);
@@ -752,15 +729,4 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
crypto_free_aead(server->secmech.dec);
server->secmech.dec = NULL;
}
-
- kfree(server->secmech.sdesccmacaes);
- server->secmech.sdesccmacaes = NULL;
- kfree(server->secmech.sdeschmacsha256);
- server->secmech.sdeschmacsha256 = NULL;
- kfree(server->secmech.sdeschmacmd5);
- server->secmech.sdeschmacmd5 = NULL;
- kfree(server->secmech.sdescmd5);
- server->secmech.sdescmd5 = NULL;
- kfree(server->secmech.sdescsha512);
- server->secmech.sdescsha512 = NULL;
}
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index cbb108b15412..ea76f4d7ef62 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -153,26 +153,16 @@ struct session_key {
char *response;
};
-/* crypto security descriptor definition */
-struct sdesc {
- struct shash_desc shash;
- char ctx[];
-};
-
/* crypto hashing related structure/fields, not specific to a sec mech */
struct cifs_secmech {
- struct crypto_shash *hmacmd5; /* hmac-md5 hash function */
- struct crypto_shash *md5; /* md5 hash function */
- struct crypto_shash *hmacsha256; /* hmac-sha256 hash function */
- struct crypto_shash *cmacaes; /* block-cipher based MAC function */
- struct crypto_shash *sha512; /* sha512 hash function */
- struct sdesc *sdeschmacmd5; /* ctxt to generate ntlmv2 hash, CR1 */
- struct sdesc *sdescmd5; /* ctxt to generate cifs/smb signature */
- struct sdesc *sdeschmacsha256; /* ctxt to generate smb2 signature */
- struct sdesc *sdesccmacaes; /* ctxt to generate smb3 signature */
- struct sdesc *sdescsha512; /* ctxt to generate smb3.11 signing key */
- struct crypto_aead *enc; /* smb3 AEAD encryption TFM (AES-CCM and AES-GCM) */
- struct crypto_aead *dec; /* smb3 AEAD decryption TFM (AES-CCM and AES-GCM) */
+ struct shash_desc *hmacmd5; /* hmacmd5 hash function, for NTLMv2/CR1 hashes */
+ struct shash_desc *md5; /* md5 hash function, for CIFS/SMB1 signatures */
+ struct shash_desc *hmacsha256; /* hmac-sha256 hash function, for SMB2 signatures */
+ struct shash_desc *sha512; /* sha512 hash function, for SMB3.1.1 preauth hash */
+ struct shash_desc *aes_cmac; /* block-cipher based MAC function, for SMB3 signatures */
+
+ struct crypto_aead *enc; /* smb3 encryption AEAD TFM (AES-CCM and AES-GCM) */
+ struct crypto_aead *dec; /* smb3 decryption AEAD TFM (AES-CCM and AES-GCM) */
};
/* per smb session structure/fields */
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 3bc94bcc7177..f5adcb8ea04d 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -598,9 +598,8 @@ struct cifs_aio_ctx *cifs_aio_ctx_alloc(void);
void cifs_aio_ctx_release(struct kref *refcount);
int setup_aio_ctx_iter(struct cifs_aio_ctx *ctx, struct iov_iter *iter, int rw);
-int cifs_alloc_hash(const char *name, struct crypto_shash **shash,
- struct sdesc **sdesc);
-void cifs_free_hash(struct crypto_shash **shash, struct sdesc **sdesc);
+int cifs_alloc_hash(const char *name, struct shash_desc **sdesc);
+void cifs_free_hash(struct shash_desc **sdesc);
extern void rqst_page_get_length(struct smb_rqst *rqst, unsigned int page,
unsigned int *len, unsigned int *offset);
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 6803cb27eecc..cd29c296cec6 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -38,29 +38,28 @@ static int
symlink_hash(unsigned int link_len, const char *link_str, u8 *md5_hash)
{
int rc;
- struct crypto_shash *md5 = NULL;
- struct sdesc *sdescmd5 = NULL;
+ struct shash_desc *md5 = NULL;
- rc = cifs_alloc_hash("md5", &md5, &sdescmd5);
+ rc = cifs_alloc_hash("md5", &md5);
if (rc)
goto symlink_hash_err;
- rc = crypto_shash_init(&sdescmd5->shash);
+ rc = crypto_shash_init(md5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init md5 shash\n", __func__);
goto symlink_hash_err;
}
- rc = crypto_shash_update(&sdescmd5->shash, link_str, link_len);
+ rc = crypto_shash_update(md5, link_str, link_len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with link_str\n", __func__);
goto symlink_hash_err;
}
- rc = crypto_shash_final(&sdescmd5->shash, md5_hash);
+ rc = crypto_shash_final(md5, md5_hash);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
symlink_hash_err:
- cifs_free_hash(&md5, &sdescmd5);
+ cifs_free_hash(&md5);
return rc;
}
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index c6679398fff9..535dbe6ff994 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -1071,59 +1071,58 @@ setup_aio_ctx_iter(struct cifs_aio_ctx *ctx, struct iov_iter *iter, int rw)
/**
* cifs_alloc_hash - allocate hash and hash context together
* @name: The name of the crypto hash algo
- * @shash: Where to put the pointer to the hash algo
- * @sdesc: Where to put the pointer to the hash descriptor
+ * @sdesc: SHASH descriptor where to put the pointer to the hash TFM
*
* The caller has to make sure @sdesc is initialized to either NULL or
- * a valid context. Both can be freed via cifs_free_hash().
+ * a valid context. It can be freed via cifs_free_hash().
*/
int
-cifs_alloc_hash(const char *name,
- struct crypto_shash **shash, struct sdesc **sdesc)
+cifs_alloc_hash(const char *name, struct shash_desc **sdesc)
{
int rc = 0;
- size_t size;
+ struct crypto_shash *alg = NULL;
- if (*sdesc != NULL)
+ if (*sdesc)
return 0;
- *shash = crypto_alloc_shash(name, 0, 0);
- if (IS_ERR(*shash)) {
- cifs_dbg(VFS, "Could not allocate crypto %s\n", name);
- rc = PTR_ERR(*shash);
- *shash = NULL;
+ alg = crypto_alloc_shash(name, 0, 0);
+ if (IS_ERR(alg)) {
+ cifs_dbg(VFS, "Could not allocate shash TFM '%s'\n", name);
+ rc = PTR_ERR(alg);
*sdesc = NULL;
return rc;
}
- size = sizeof(struct shash_desc) + crypto_shash_descsize(*shash);
- *sdesc = kmalloc(size, GFP_KERNEL);
+ *sdesc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(alg), GFP_KERNEL);
if (*sdesc == NULL) {
- cifs_dbg(VFS, "no memory left to allocate crypto %s\n", name);
- crypto_free_shash(*shash);
- *shash = NULL;
+ cifs_dbg(VFS, "no memory left to allocate shash TFM '%s'\n", name);
+ crypto_free_shash(alg);
return -ENOMEM;
}
- (*sdesc)->shash.tfm = *shash;
+ (*sdesc)->tfm = alg;
return 0;
}
/**
* cifs_free_hash - free hash and hash context together
- * @shash: Where to find the pointer to the hash algo
- * @sdesc: Where to find the pointer to the hash descriptor
+ * @sdesc: Where to find the pointer to the hash TFM
*
- * Freeing a NULL hash or context is safe.
+ * Freeing a NULL descriptor is safe.
*/
void
-cifs_free_hash(struct crypto_shash **shash, struct sdesc **sdesc)
+cifs_free_hash(struct shash_desc **sdesc)
{
+ if (unlikely(!sdesc) || !*sdesc)
+ return;
+
+ if ((*sdesc)->tfm) {
+ crypto_free_shash((*sdesc)->tfm);
+ (*sdesc)->tfm = NULL;
+ }
+
kfree(*sdesc);
*sdesc = NULL;
- if (*shash)
- crypto_free_shash(*shash);
- *shash = NULL;
}
/**
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index d73e5672aac4..7db5c09ecceb 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -870,8 +870,8 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
struct kvec *iov, int nvec)
{
int i, rc;
- struct sdesc *d;
struct smb2_hdr *hdr;
+ struct shash_desc *sha512 = NULL;
hdr = (struct smb2_hdr *)iov[0].iov_base;
/* neg prot are always taken */
@@ -901,14 +901,14 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
if (rc)
return rc;
- d = server->secmech.sdescsha512;
- rc = crypto_shash_init(&d->shash);
+ sha512 = server->secmech.sha512;
+ rc = crypto_shash_init(sha512);
if (rc) {
cifs_dbg(VFS, "%s: Could not init sha512 shash\n", __func__);
return rc;
}
- rc = crypto_shash_update(&d->shash, ses->preauth_sha_hash,
+ rc = crypto_shash_update(sha512, ses->preauth_sha_hash,
SMB2_PREAUTH_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not update sha512 shash\n", __func__);
@@ -916,8 +916,7 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
}
for (i = 0; i < nvec; i++) {
- rc = crypto_shash_update(&d->shash,
- iov[i].iov_base, iov[i].iov_len);
+ rc = crypto_shash_update(sha512, iov[i].iov_base, iov[i].iov_len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update sha512 shash\n",
__func__);
@@ -925,7 +924,7 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
}
}
- rc = crypto_shash_final(&d->shash, ses->preauth_sha_hash);
+ rc = crypto_shash_final(sha512, ses->preauth_sha_hash);
if (rc) {
cifs_dbg(VFS, "%s: Could not finalize sha512 shash\n",
__func__);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index d4e1a5d74dcd..dfcbcc0b86e4 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -32,19 +32,17 @@ smb3_crypto_shash_allocate(struct TCP_Server_Info *server)
struct cifs_secmech *p = &server->secmech;
int rc;
- rc = cifs_alloc_hash("hmac(sha256)",
- &p->hmacsha256,
- &p->sdeschmacsha256);
+ rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
if (rc)
goto err;
- rc = cifs_alloc_hash("cmac(aes)", &p->cmacaes, &p->sdesccmacaes);
+ rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
if (rc)
goto err;
return 0;
err:
- cifs_free_hash(&p->hmacsha256, &p->sdeschmacsha256);
+ cifs_free_hash(&p->hmacsha256);
return rc;
}
@@ -54,25 +52,23 @@ smb311_crypto_shash_allocate(struct TCP_Server_Info *server)
struct cifs_secmech *p = &server->secmech;
int rc = 0;
- rc = cifs_alloc_hash("hmac(sha256)",
- &p->hmacsha256,
- &p->sdeschmacsha256);
+ rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
if (rc)
return rc;
- rc = cifs_alloc_hash("cmac(aes)", &p->cmacaes, &p->sdesccmacaes);
+ rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
if (rc)
goto err;
- rc = cifs_alloc_hash("sha512", &p->sha512, &p->sdescsha512);
+ rc = cifs_alloc_hash("sha512", &p->sha512);
if (rc)
goto err;
return 0;
err:
- cifs_free_hash(&p->cmacaes, &p->sdesccmacaes);
- cifs_free_hash(&p->hmacsha256, &p->sdeschmacsha256);
+ cifs_free_hash(&p->aes_cmac);
+ cifs_free_hash(&p->hmacsha256);
return rc;
}
@@ -220,8 +216,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
struct cifs_ses *ses;
struct shash_desc *shash;
- struct crypto_shash *hash;
- struct sdesc *sdesc = NULL;
struct smb_rqst drqst;
ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
@@ -234,19 +228,17 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
if (allocate_crypto) {
- rc = cifs_alloc_hash("hmac(sha256)", &hash, &sdesc);
+ rc = cifs_alloc_hash("hmac(sha256)", &shash);
if (rc) {
cifs_server_dbg(VFS,
"%s: sha256 alloc failed\n", __func__);
goto out;
}
- shash = &sdesc->shash;
} else {
- hash = server->secmech.hmacsha256;
- shash = &server->secmech.sdeschmacsha256->shash;
+ shash = server->secmech.hmacsha256;
}
- rc = crypto_shash_setkey(hash, ses->auth_key.response,
+ rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response,
SMB2_NTLMV2_SESSKEY_SIZE);
if (rc) {
cifs_server_dbg(VFS,
@@ -288,7 +280,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
out:
if (allocate_crypto)
- cifs_free_hash(&hash, &sdesc);
+ cifs_free_hash(&shash);
if (ses)
cifs_put_smb_ses(ses);
return rc;
@@ -315,42 +307,38 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
goto smb3signkey_ret;
}
- rc = crypto_shash_setkey(server->secmech.hmacsha256,
+ rc = crypto_shash_setkey(server->secmech.hmacsha256->tfm,
ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not set with session key\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_init(&server->secmech.sdeschmacsha256->shash);
+ rc = crypto_shash_init(server->secmech.hmacsha256);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not init sign hmac\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- i, 4);
+ rc = crypto_shash_update(server->secmech.hmacsha256, i, 4);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with n\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- label.iov_base, label.iov_len);
+ rc = crypto_shash_update(server->secmech.hmacsha256, label.iov_base, label.iov_len);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with label\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- &zero, 1);
+ rc = crypto_shash_update(server->secmech.hmacsha256, &zero, 1);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with zero\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- context.iov_base, context.iov_len);
+ rc = crypto_shash_update(server->secmech.hmacsha256, context.iov_base, context.iov_len);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with context\n", __func__);
goto smb3signkey_ret;
@@ -358,19 +346,16 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
(server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)) {
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- L256, 4);
+ rc = crypto_shash_update(server->secmech.hmacsha256, L256, 4);
} else {
- rc = crypto_shash_update(&server->secmech.sdeschmacsha256->shash,
- L128, 4);
+ rc = crypto_shash_update(server->secmech.hmacsha256, L128, 4);
}
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with L\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_final(&server->secmech.sdeschmacsha256->shash,
- hashptr);
+ rc = crypto_shash_final(server->secmech.hmacsha256, hashptr);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not generate sha256 hash\n", __func__);
goto smb3signkey_ret;
@@ -551,8 +536,6 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
struct kvec *iov = rqst->rq_iov;
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
struct shash_desc *shash;
- struct crypto_shash *hash;
- struct sdesc *sdesc = NULL;
struct smb_rqst drqst;
u8 key[SMB3_SIGN_KEY_SIZE];
@@ -563,27 +546,24 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
}
if (allocate_crypto) {
- rc = cifs_alloc_hash("cmac(aes)", &hash, &sdesc);
+ rc = cifs_alloc_hash("cmac(aes)", &shash);
if (rc)
return rc;
-
- shash = &sdesc->shash;
} else {
- hash = server->secmech.cmacaes;
- shash = &server->secmech.sdesccmacaes->shash;
+ shash = server->secmech.aes_cmac;
}
memset(smb3_signature, 0x0, SMB2_CMACAES_SIZE);
memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
- rc = crypto_shash_setkey(hash, key, SMB2_CMACAES_SIZE);
+ rc = crypto_shash_setkey(shash->tfm, key, SMB2_CMACAES_SIZE);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not set key for cmac aes\n", __func__);
goto out;
}
/*
- * we already allocate sdesccmacaes when we init smb3 signing key,
+ * we already allocate aes_cmac when we init smb3 signing key,
* so unlike smb2 case we do not have to check here if secmech are
* initialized
*/
@@ -619,7 +599,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
out:
if (allocate_crypto)
- cifs_free_hash(&hash, &sdesc);
+ cifs_free_hash(&shash);
return rc;
}
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 3/8] cifs: allocate ephemeral secmechs only on demand
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 1/8] smb3: rename encryption/decryption TFMs Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 2/8] cifs: secmech: use shash_desc directly, remove sdesc Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 5:23 ` Steve French
2022-09-29 1:56 ` [PATCH v3 4/8] cifs: create sign/verify secmechs, don't leave keys in memory Enzo Matsumiya
` (4 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
Reduce memory usage a bit by removing some secmechs as they are
only briefly used on session setup, and not needed anymore
throughout a server's object lifetime. Allocate and free them on
demand now.
HMAC-SHA256 is an exception because it's used both for SMB2 signatures
as for generating SMB3+ signatures, so allocate/free a dedicated one in
generate_key() too to keep things separated.
smb3*_crypto_shash_allocate functions are removed since we're now
calling cifs_alloc_hash() directly and especifically.
Also move smb3_crypto_aead_allocate() call to generate_key(), right
after when crypto keys are generated.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsencrypt.c | 73 +++++++++++++++---------------------
fs/cifs/cifsglob.h | 2 -
fs/cifs/misc.c | 2 +-
fs/cifs/sess.c | 12 ------
fs/cifs/smb2misc.c | 18 ++++-----
fs/cifs/smb2ops.c | 8 ++--
fs/cifs/smb2pdu.c | 19 ++++++++++
fs/cifs/smb2proto.h | 1 -
fs/cifs/smb2transport.c | 83 ++++++++++++-----------------------------
9 files changed, 86 insertions(+), 132 deletions(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 30ece0c58c71..ed25ac811f05 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -401,7 +401,8 @@ find_timestamp(struct cifs_ses *ses)
}
static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
- const struct nls_table *nls_cp)
+ const struct nls_table *nls_cp,
+ struct shash_desc *hmacmd5)
{
int rc = 0;
int len;
@@ -410,7 +411,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
wchar_t *domain;
wchar_t *server;
- if (!ses->server->secmech.hmacmd5) {
+ if (!hmacmd5) {
cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
return -1;
}
@@ -418,14 +419,13 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
/* calculate md4 hash of password */
E_md4hash(ses->password, nt_hash, nls_cp);
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm, nt_hash,
- CIFS_NTHASH_SIZE);
+ rc = crypto_shash_setkey(hmacmd5->tfm, nt_hash, CIFS_NTHASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NT Hash as a key\n", __func__);
return rc;
}
- rc = crypto_shash_init(ses->server->secmech.hmacmd5);
+ rc = crypto_shash_init(hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
return rc;
@@ -446,8 +446,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
memset(user, '\0', 2);
}
- rc = crypto_shash_update(ses->server->secmech.hmacmd5,
- (char *)user, 2 * len);
+ rc = crypto_shash_update(hmacmd5, (char *)user, 2 * len);
kfree(user);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with user\n", __func__);
@@ -465,9 +464,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
len = cifs_strtoUTF16((__le16 *)domain, ses->domainName, len,
nls_cp);
- rc =
- crypto_shash_update(ses->server->secmech.hmacmd5,
- (char *)domain, 2 * len);
+ rc = crypto_shash_update(hmacmd5, (char *)domain, 2 * len);
kfree(domain);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with domain\n",
@@ -485,9 +482,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
len = cifs_strtoUTF16((__le16 *)server, ses->ip_addr, len,
nls_cp);
- rc =
- crypto_shash_update(ses->server->secmech.hmacmd5,
- (char *)server, 2 * len);
+ rc = crypto_shash_update(hmacmd5, (char *)server, 2 * len);
kfree(server);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with server\n",
@@ -496,8 +491,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
}
- rc = crypto_shash_final(ses->server->secmech.hmacmd5,
- ntlmv2_hash);
+ rc = crypto_shash_final(hmacmd5, ntlmv2_hash);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
@@ -505,7 +499,8 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
}
static int
-CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
+CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash,
+ struct shash_desc *hmacmd5)
{
int rc;
struct ntlmv2_resp *ntlmv2 = (struct ntlmv2_resp *)
@@ -516,20 +511,19 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
hash_len = ses->auth_key.len - (CIFS_SESS_KEY_SIZE +
offsetof(struct ntlmv2_resp, challenge.key[0]));
- if (!ses->server->secmech.hmacmd5) {
+ if (!hmacmd5) {
cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
return -1;
}
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
- ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
+ rc = crypto_shash_setkey(hmacmd5->tfm, ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
__func__);
return rc;
}
- rc = crypto_shash_init(ses->server->secmech.hmacmd5);
+ rc = crypto_shash_init(hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
return rc;
@@ -541,16 +535,14 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
else
memcpy(ntlmv2->challenge.key,
ses->server->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
- rc = crypto_shash_update(ses->server->secmech.hmacmd5,
- ntlmv2->challenge.key, hash_len);
+ rc = crypto_shash_update(hmacmd5, ntlmv2->challenge.key, hash_len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
return rc;
}
/* Note that the MD5 digest over writes anon.challenge_key.key */
- rc = crypto_shash_final(ses->server->secmech.hmacmd5,
- ntlmv2->ntlmv2_hash);
+ rc = crypto_shash_final(hmacmd5, ntlmv2->ntlmv2_hash);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
@@ -567,6 +559,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
char ntlmv2_hash[16];
unsigned char *tiblob = NULL; /* target info blob */
__le64 rsp_timestamp;
+ struct shash_desc *hmacmd5 = NULL;
if (nls_cp == NULL) {
cifs_dbg(VFS, "%s called with nls_cp==NULL\n", __func__);
@@ -625,53 +618,51 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
cifs_server_lock(ses->server);
- rc = cifs_alloc_hash("hmac(md5)", &ses->server->secmech.hmacmd5);
+ rc = cifs_alloc_hash("hmac(md5)", &hmacmd5);
if (rc) {
goto unlock;
}
/* calculate ntlmv2_hash */
- rc = calc_ntlmv2_hash(ses, ntlmv2_hash, nls_cp);
+ rc = calc_ntlmv2_hash(ses, ntlmv2_hash, nls_cp, hmacmd5);
if (rc) {
cifs_dbg(VFS, "Could not get v2 hash rc %d\n", rc);
- goto unlock;
+ goto out_free_hash;
}
/* calculate first part of the client response (CR1) */
- rc = CalcNTLMv2_response(ses, ntlmv2_hash);
+ rc = CalcNTLMv2_response(ses, ntlmv2_hash, hmacmd5);
if (rc) {
cifs_dbg(VFS, "Could not calculate CR1 rc: %d\n", rc);
- goto unlock;
+ goto out_free_hash;
}
/* now calculate the session key for NTLMv2 */
- rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
- ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
+ rc = crypto_shash_setkey(hmacmd5->tfm, ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
__func__);
- goto unlock;
+ goto out_free_hash;
}
- rc = crypto_shash_init(ses->server->secmech.hmacmd5);
+ rc = crypto_shash_init(hmacmd5);
if (rc) {
cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
- goto unlock;
+ goto out_free_hash;
}
- rc = crypto_shash_update(ses->server->secmech.hmacmd5,
- ntlmv2->ntlmv2_hash,
- CIFS_HMAC_MD5_HASH_SIZE);
+ rc = crypto_shash_update(hmacmd5, ntlmv2->ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
- goto unlock;
+ goto out_free_hash;
}
- rc = crypto_shash_final(ses->server->secmech.hmacmd5,
- ses->auth_key.response);
+ rc = crypto_shash_final(hmacmd5, ses->auth_key.response);
if (rc)
cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
+out_free_hash:
+ cifs_free_hash(&hmacmd5);
unlock:
cifs_server_unlock(ses->server);
setup_ntlmv2_rsp_ret:
@@ -717,8 +708,6 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
cifs_free_hash(&server->secmech.aes_cmac);
cifs_free_hash(&server->secmech.hmacsha256);
cifs_free_hash(&server->secmech.md5);
- cifs_free_hash(&server->secmech.sha512);
- cifs_free_hash(&server->secmech.hmacmd5);
if (server->secmech.enc) {
crypto_free_aead(server->secmech.enc);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ea76f4d7ef62..5da71d946012 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -155,10 +155,8 @@ struct session_key {
/* crypto hashing related structure/fields, not specific to a sec mech */
struct cifs_secmech {
- struct shash_desc *hmacmd5; /* hmacmd5 hash function, for NTLMv2/CR1 hashes */
struct shash_desc *md5; /* md5 hash function, for CIFS/SMB1 signatures */
struct shash_desc *hmacsha256; /* hmac-sha256 hash function, for SMB2 signatures */
- struct shash_desc *sha512; /* sha512 hash function, for SMB3.1.1 preauth hash */
struct shash_desc *aes_cmac; /* block-cipher based MAC function, for SMB3 signatures */
struct crypto_aead *enc; /* smb3 encryption AEAD TFM (AES-CCM and AES-GCM) */
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index 535dbe6ff994..c7eade06e2de 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -1093,7 +1093,7 @@ cifs_alloc_hash(const char *name, struct shash_desc **sdesc)
return rc;
}
- *sdesc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(alg), GFP_KERNEL);
+ *sdesc = kzalloc(sizeof(struct shash_desc) + crypto_shash_descsize(alg), GFP_KERNEL);
if (*sdesc == NULL) {
cifs_dbg(VFS, "no memory left to allocate shash TFM '%s'\n", name);
crypto_free_shash(alg);
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 3af3b05b6c74..d59dec7a2a55 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -454,18 +454,6 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
spin_unlock(&ses->chan_lock);
mutex_lock(&ses->session_mutex);
- /*
- * We need to allocate the server crypto now as we will need
- * to sign packets before we generate the channel signing key
- * (we sign with the session key)
- */
- rc = smb311_crypto_shash_allocate(chan->server);
- if (rc) {
- cifs_dbg(VFS, "%s: crypto alloc failed\n", __func__);
- mutex_unlock(&ses->session_mutex);
- goto out;
- }
-
rc = cifs_negotiate_protocol(xid, ses, chan->server);
if (!rc)
rc = cifs_setup_session(xid, ses, chan->server, cifs_sb->local_nls);
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 7db5c09ecceb..39a9fc60eb9e 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -897,22 +897,21 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
return 0;
ok:
- rc = smb311_crypto_shash_allocate(server);
+ rc = cifs_alloc_hash("sha512", &sha512);
if (rc)
return rc;
- sha512 = server->secmech.sha512;
rc = crypto_shash_init(sha512);
if (rc) {
cifs_dbg(VFS, "%s: Could not init sha512 shash\n", __func__);
- return rc;
+ goto out_free_hash;
}
rc = crypto_shash_update(sha512, ses->preauth_sha_hash,
SMB2_PREAUTH_HASH_SIZE);
if (rc) {
cifs_dbg(VFS, "%s: Could not update sha512 shash\n", __func__);
- return rc;
+ goto out_free_hash;
}
for (i = 0; i < nvec; i++) {
@@ -920,16 +919,15 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
if (rc) {
cifs_dbg(VFS, "%s: Could not update sha512 shash\n",
__func__);
- return rc;
+ goto out_free_hash;
}
}
rc = crypto_shash_final(sha512, ses->preauth_sha_hash);
- if (rc) {
+ if (rc)
cifs_dbg(VFS, "%s: Could not finalize sha512 shash\n",
__func__);
- return rc;
- }
-
- return 0;
+out_free_hash:
+ cifs_free_hash(&sha512);
+ return rc;
}
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index d1528755f330..34dea8aa854b 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4338,10 +4338,10 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
return rc;
}
- rc = smb3_crypto_aead_allocate(server);
- if (rc) {
- cifs_server_dbg(VFS, "%s: crypto alloc failed\n", __func__);
- return rc;
+ /* sanity check -- TFMs were allocated after negotiate protocol */
+ if (unlikely(!server->secmech.enc || !server->secmech.dec)) {
+ cifs_server_dbg(VFS, "%s: crypto TFMs are NULL\n", __func__);
+ return -EIO;
}
tfm = enc ? server->secmech.enc : server->secmech.dec;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6352ab32c7e7..48b25054354c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -927,6 +927,16 @@ SMB2_negotiate(const unsigned int xid,
else
req->SecurityMode = 0;
+ if (req->SecurityMode) {
+ /*
+ * Allocate HMAC-SHA256 regardless of dialect requested, change to AES-CMAC later,
+ * if SMB3+ is negotiated
+ */
+ rc = cifs_alloc_hash("hmac(sha256)",&server->secmech.hmacsha256);
+ if (rc)
+ goto neg_exit;
+ }
+
req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
if (ses->chan_max > 1)
req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
@@ -1071,6 +1081,15 @@ SMB2_negotiate(const unsigned int xid,
rc = cifs_enable_signing(server, ses->sign);
if (rc)
goto neg_exit;
+
+ if (server->sign && server->dialect >= SMB30_PROT_ID) {
+ /* free HMAC-SHA256 allocated earlier for negprot */
+ cifs_free_hash(&server->secmech.hmacsha256);
+ rc = cifs_alloc_hash("cmac(aes)", &server->secmech.aes_cmac);
+ if (rc)
+ goto neg_exit;
+ }
+
if (blob_length) {
rc = decode_negTokenInit(security_blob, blob_length, server);
if (rc == 1)
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 3f740f24b96a..a975144c63bf 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -267,7 +267,6 @@ extern int smb2_validate_and_copy_iov(unsigned int offset,
extern void smb2_copy_fs_info_to_kstatfs(
struct smb2_fs_full_size_info *pfs_inf,
struct kstatfs *kst);
-extern int smb311_crypto_shash_allocate(struct TCP_Server_Info *server);
extern int smb311_update_preauth_hash(struct cifs_ses *ses,
struct TCP_Server_Info *server,
struct kvec *iov, int nvec);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index dfcbcc0b86e4..2dca2c255239 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -26,53 +26,6 @@
#include "smb2status.h"
#include "smb2glob.h"
-static int
-smb3_crypto_shash_allocate(struct TCP_Server_Info *server)
-{
- struct cifs_secmech *p = &server->secmech;
- int rc;
-
- rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
- if (rc)
- goto err;
-
- rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
- if (rc)
- goto err;
-
- return 0;
-err:
- cifs_free_hash(&p->hmacsha256);
- return rc;
-}
-
-int
-smb311_crypto_shash_allocate(struct TCP_Server_Info *server)
-{
- struct cifs_secmech *p = &server->secmech;
- int rc = 0;
-
- rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
- if (rc)
- return rc;
-
- rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
- if (rc)
- goto err;
-
- rc = cifs_alloc_hash("sha512", &p->sha512);
- if (rc)
- goto err;
-
- return 0;
-
-err:
- cifs_free_hash(&p->aes_cmac);
- cifs_free_hash(&p->hmacsha256);
- return rc;
-}
-
-
static
int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
{
@@ -215,7 +168,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
struct kvec *iov = rqst->rq_iov;
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
struct cifs_ses *ses;
- struct shash_desc *shash;
+ struct shash_desc *shash = NULL;
struct smb_rqst drqst;
ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
@@ -297,48 +250,50 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
unsigned char prfhash[SMB2_HMACSHA256_SIZE];
unsigned char *hashptr = prfhash;
struct TCP_Server_Info *server = ses->server;
+ struct shash_desc *hmac_sha256 = NULL;
memset(prfhash, 0x0, SMB2_HMACSHA256_SIZE);
memset(key, 0x0, key_size);
- rc = smb3_crypto_shash_allocate(server);
+ /* do not reuse the server's secmech TFM */
+ rc = cifs_alloc_hash("hmac(sha256)", &hmac_sha256);
if (rc) {
cifs_server_dbg(VFS, "%s: crypto alloc failed\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_setkey(server->secmech.hmacsha256->tfm,
- ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
+ rc = crypto_shash_setkey(hmac_sha256->tfm, ses->auth_key.response,
+ SMB2_NTLMV2_SESSKEY_SIZE);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not set with session key\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_init(server->secmech.hmacsha256);
+ rc = crypto_shash_init(hmac_sha256);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not init sign hmac\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(server->secmech.hmacsha256, i, 4);
+ rc = crypto_shash_update(hmac_sha256, i, 4);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with n\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(server->secmech.hmacsha256, label.iov_base, label.iov_len);
+ rc = crypto_shash_update(hmac_sha256, label.iov_base, label.iov_len);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with label\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(server->secmech.hmacsha256, &zero, 1);
+ rc = crypto_shash_update(hmac_sha256, &zero, 1);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with zero\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_update(server->secmech.hmacsha256, context.iov_base, context.iov_len);
+ rc = crypto_shash_update(hmac_sha256, context.iov_base, context.iov_len);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with context\n", __func__);
goto smb3signkey_ret;
@@ -346,16 +301,16 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
(server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)) {
- rc = crypto_shash_update(server->secmech.hmacsha256, L256, 4);
+ rc = crypto_shash_update(hmac_sha256, L256, 4);
} else {
- rc = crypto_shash_update(server->secmech.hmacsha256, L128, 4);
+ rc = crypto_shash_update(hmac_sha256, L128, 4);
}
if (rc) {
cifs_server_dbg(VFS, "%s: Could not update with L\n", __func__);
goto smb3signkey_ret;
}
- rc = crypto_shash_final(server->secmech.hmacsha256, hashptr);
+ rc = crypto_shash_final(hmac_sha256, hashptr);
if (rc) {
cifs_server_dbg(VFS, "%s: Could not generate sha256 hash\n", __func__);
goto smb3signkey_ret;
@@ -364,6 +319,7 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
memcpy(key, hashptr, key_size);
smb3signkey_ret:
+ cifs_free_hash(&hmac_sha256);
return rc;
}
@@ -428,12 +384,19 @@ generate_smb3signingkey(struct cifs_ses *ses,
ptriplet->encryption.context,
ses->smb3encryptionkey,
SMB3_ENC_DEC_KEY_SIZE);
+ if (rc)
+ return rc;
+
rc = generate_key(ses, ptriplet->decryption.label,
ptriplet->decryption.context,
ses->smb3decryptionkey,
SMB3_ENC_DEC_KEY_SIZE);
if (rc)
return rc;
+
+ rc = smb3_crypto_aead_allocate(server);
+ if (rc)
+ return rc;
}
if (rc)
@@ -535,7 +498,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
unsigned char *sigptr = smb3_signature;
struct kvec *iov = rqst->rq_iov;
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
- struct shash_desc *shash;
+ struct shash_desc *shash = NULL;
struct smb_rqst drqst;
u8 key[SMB3_SIGN_KEY_SIZE];
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 3/8] cifs: allocate ephemeral secmechs only on demand
2022-09-29 1:56 ` [PATCH v3 3/8] cifs: allocate ephemeral secmechs only on demand Enzo Matsumiya
@ 2022-09-29 5:23 ` Steve French
0 siblings, 0 replies; 18+ messages in thread
From: Steve French @ 2022-09-29 5:23 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: linux-cifs, pc, ronniesahlberg, nspmangalore, tom, metze
could you fix the trivial checkpatch nit?
0003-cifs-allocate-ephemeral-secmechs-only-on-demand.patch
----------------------------------------------------------
ERROR: space required after that ',' (ctx:VxO)
#390: FILE: fs/cifs/smb2pdu.c:935:
+ rc = cifs_alloc_hash("hmac(sha256)",&server->secmech.hmacsha256);
^
ERROR: space required before that '&' (ctx:OxV)
#390: FILE: fs/cifs/smb2pdu.c:935:
+ rc = cifs_alloc_hash("hmac(sha256)",&server->secmech.hmacsha256);
On Wed, Sep 28, 2022 at 8:57 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Reduce memory usage a bit by removing some secmechs as they are
> only briefly used on session setup, and not needed anymore
> throughout a server's object lifetime. Allocate and free them on
> demand now.
>
> HMAC-SHA256 is an exception because it's used both for SMB2 signatures
> as for generating SMB3+ signatures, so allocate/free a dedicated one in
> generate_key() too to keep things separated.
>
> smb3*_crypto_shash_allocate functions are removed since we're now
> calling cifs_alloc_hash() directly and especifically.
>
> Also move smb3_crypto_aead_allocate() call to generate_key(), right
> after when crypto keys are generated.
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/cifsencrypt.c | 73 +++++++++++++++---------------------
> fs/cifs/cifsglob.h | 2 -
> fs/cifs/misc.c | 2 +-
> fs/cifs/sess.c | 12 ------
> fs/cifs/smb2misc.c | 18 ++++-----
> fs/cifs/smb2ops.c | 8 ++--
> fs/cifs/smb2pdu.c | 19 ++++++++++
> fs/cifs/smb2proto.h | 1 -
> fs/cifs/smb2transport.c | 83 ++++++++++++-----------------------------
> 9 files changed, 86 insertions(+), 132 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 30ece0c58c71..ed25ac811f05 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -401,7 +401,8 @@ find_timestamp(struct cifs_ses *ses)
> }
>
> static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> - const struct nls_table *nls_cp)
> + const struct nls_table *nls_cp,
> + struct shash_desc *hmacmd5)
> {
> int rc = 0;
> int len;
> @@ -410,7 +411,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> wchar_t *domain;
> wchar_t *server;
>
> - if (!ses->server->secmech.hmacmd5) {
> + if (!hmacmd5) {
> cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
> return -1;
> }
> @@ -418,14 +419,13 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> /* calculate md4 hash of password */
> E_md4hash(ses->password, nt_hash, nls_cp);
>
> - rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm, nt_hash,
> - CIFS_NTHASH_SIZE);
> + rc = crypto_shash_setkey(hmacmd5->tfm, nt_hash, CIFS_NTHASH_SIZE);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not set NT Hash as a key\n", __func__);
> return rc;
> }
>
> - rc = crypto_shash_init(ses->server->secmech.hmacmd5);
> + rc = crypto_shash_init(hmacmd5);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
> return rc;
> @@ -446,8 +446,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> memset(user, '\0', 2);
> }
>
> - rc = crypto_shash_update(ses->server->secmech.hmacmd5,
> - (char *)user, 2 * len);
> + rc = crypto_shash_update(hmacmd5, (char *)user, 2 * len);
> kfree(user);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update with user\n", __func__);
> @@ -465,9 +464,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> }
> len = cifs_strtoUTF16((__le16 *)domain, ses->domainName, len,
> nls_cp);
> - rc =
> - crypto_shash_update(ses->server->secmech.hmacmd5,
> - (char *)domain, 2 * len);
> + rc = crypto_shash_update(hmacmd5, (char *)domain, 2 * len);
> kfree(domain);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update with domain\n",
> @@ -485,9 +482,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> }
> len = cifs_strtoUTF16((__le16 *)server, ses->ip_addr, len,
> nls_cp);
> - rc =
> - crypto_shash_update(ses->server->secmech.hmacmd5,
> - (char *)server, 2 * len);
> + rc = crypto_shash_update(hmacmd5, (char *)server, 2 * len);
> kfree(server);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update with server\n",
> @@ -496,8 +491,7 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> }
> }
>
> - rc = crypto_shash_final(ses->server->secmech.hmacmd5,
> - ntlmv2_hash);
> + rc = crypto_shash_final(hmacmd5, ntlmv2_hash);
> if (rc)
> cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
>
> @@ -505,7 +499,8 @@ static int calc_ntlmv2_hash(struct cifs_ses *ses, char *ntlmv2_hash,
> }
>
> static int
> -CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
> +CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash,
> + struct shash_desc *hmacmd5)
> {
> int rc;
> struct ntlmv2_resp *ntlmv2 = (struct ntlmv2_resp *)
> @@ -516,20 +511,19 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
> hash_len = ses->auth_key.len - (CIFS_SESS_KEY_SIZE +
> offsetof(struct ntlmv2_resp, challenge.key[0]));
>
> - if (!ses->server->secmech.hmacmd5) {
> + if (!hmacmd5) {
> cifs_dbg(VFS, "%s: can't generate ntlmv2 hash\n", __func__);
> return -1;
> }
>
> - rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
> - ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> + rc = crypto_shash_setkey(hmacmd5->tfm, ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
> __func__);
> return rc;
> }
>
> - rc = crypto_shash_init(ses->server->secmech.hmacmd5);
> + rc = crypto_shash_init(hmacmd5);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
> return rc;
> @@ -541,16 +535,14 @@ CalcNTLMv2_response(const struct cifs_ses *ses, char *ntlmv2_hash)
> else
> memcpy(ntlmv2->challenge.key,
> ses->server->cryptkey, CIFS_SERVER_CHALLENGE_SIZE);
> - rc = crypto_shash_update(ses->server->secmech.hmacmd5,
> - ntlmv2->challenge.key, hash_len);
> + rc = crypto_shash_update(hmacmd5, ntlmv2->challenge.key, hash_len);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
> return rc;
> }
>
> /* Note that the MD5 digest over writes anon.challenge_key.key */
> - rc = crypto_shash_final(ses->server->secmech.hmacmd5,
> - ntlmv2->ntlmv2_hash);
> + rc = crypto_shash_final(hmacmd5, ntlmv2->ntlmv2_hash);
> if (rc)
> cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
>
> @@ -567,6 +559,7 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
> char ntlmv2_hash[16];
> unsigned char *tiblob = NULL; /* target info blob */
> __le64 rsp_timestamp;
> + struct shash_desc *hmacmd5 = NULL;
>
> if (nls_cp == NULL) {
> cifs_dbg(VFS, "%s called with nls_cp==NULL\n", __func__);
> @@ -625,53 +618,51 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp)
>
> cifs_server_lock(ses->server);
>
> - rc = cifs_alloc_hash("hmac(md5)", &ses->server->secmech.hmacmd5);
> + rc = cifs_alloc_hash("hmac(md5)", &hmacmd5);
> if (rc) {
> goto unlock;
> }
>
> /* calculate ntlmv2_hash */
> - rc = calc_ntlmv2_hash(ses, ntlmv2_hash, nls_cp);
> + rc = calc_ntlmv2_hash(ses, ntlmv2_hash, nls_cp, hmacmd5);
> if (rc) {
> cifs_dbg(VFS, "Could not get v2 hash rc %d\n", rc);
> - goto unlock;
> + goto out_free_hash;
> }
>
> /* calculate first part of the client response (CR1) */
> - rc = CalcNTLMv2_response(ses, ntlmv2_hash);
> + rc = CalcNTLMv2_response(ses, ntlmv2_hash, hmacmd5);
> if (rc) {
> cifs_dbg(VFS, "Could not calculate CR1 rc: %d\n", rc);
> - goto unlock;
> + goto out_free_hash;
> }
>
> /* now calculate the session key for NTLMv2 */
> - rc = crypto_shash_setkey(ses->server->secmech.hmacmd5->tfm,
> - ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> + rc = crypto_shash_setkey(hmacmd5->tfm, ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not set NTLMV2 Hash as a key\n",
> __func__);
> - goto unlock;
> + goto out_free_hash;
> }
>
> - rc = crypto_shash_init(ses->server->secmech.hmacmd5);
> + rc = crypto_shash_init(hmacmd5);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not init hmacmd5\n", __func__);
> - goto unlock;
> + goto out_free_hash;
> }
>
> - rc = crypto_shash_update(ses->server->secmech.hmacmd5,
> - ntlmv2->ntlmv2_hash,
> - CIFS_HMAC_MD5_HASH_SIZE);
> + rc = crypto_shash_update(hmacmd5, ntlmv2->ntlmv2_hash, CIFS_HMAC_MD5_HASH_SIZE);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
> - goto unlock;
> + goto out_free_hash;
> }
>
> - rc = crypto_shash_final(ses->server->secmech.hmacmd5,
> - ses->auth_key.response);
> + rc = crypto_shash_final(hmacmd5, ses->auth_key.response);
> if (rc)
> cifs_dbg(VFS, "%s: Could not generate md5 hash\n", __func__);
>
> +out_free_hash:
> + cifs_free_hash(&hmacmd5);
> unlock:
> cifs_server_unlock(ses->server);
> setup_ntlmv2_rsp_ret:
> @@ -717,8 +708,6 @@ cifs_crypto_secmech_release(struct TCP_Server_Info *server)
> cifs_free_hash(&server->secmech.aes_cmac);
> cifs_free_hash(&server->secmech.hmacsha256);
> cifs_free_hash(&server->secmech.md5);
> - cifs_free_hash(&server->secmech.sha512);
> - cifs_free_hash(&server->secmech.hmacmd5);
>
> if (server->secmech.enc) {
> crypto_free_aead(server->secmech.enc);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index ea76f4d7ef62..5da71d946012 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -155,10 +155,8 @@ struct session_key {
>
> /* crypto hashing related structure/fields, not specific to a sec mech */
> struct cifs_secmech {
> - struct shash_desc *hmacmd5; /* hmacmd5 hash function, for NTLMv2/CR1 hashes */
> struct shash_desc *md5; /* md5 hash function, for CIFS/SMB1 signatures */
> struct shash_desc *hmacsha256; /* hmac-sha256 hash function, for SMB2 signatures */
> - struct shash_desc *sha512; /* sha512 hash function, for SMB3.1.1 preauth hash */
> struct shash_desc *aes_cmac; /* block-cipher based MAC function, for SMB3 signatures */
>
> struct crypto_aead *enc; /* smb3 encryption AEAD TFM (AES-CCM and AES-GCM) */
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index 535dbe6ff994..c7eade06e2de 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -1093,7 +1093,7 @@ cifs_alloc_hash(const char *name, struct shash_desc **sdesc)
> return rc;
> }
>
> - *sdesc = kmalloc(sizeof(struct shash_desc) + crypto_shash_descsize(alg), GFP_KERNEL);
> + *sdesc = kzalloc(sizeof(struct shash_desc) + crypto_shash_descsize(alg), GFP_KERNEL);
> if (*sdesc == NULL) {
> cifs_dbg(VFS, "no memory left to allocate shash TFM '%s'\n", name);
> crypto_free_shash(alg);
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 3af3b05b6c74..d59dec7a2a55 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -454,18 +454,6 @@ cifs_ses_add_channel(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses,
> spin_unlock(&ses->chan_lock);
>
> mutex_lock(&ses->session_mutex);
> - /*
> - * We need to allocate the server crypto now as we will need
> - * to sign packets before we generate the channel signing key
> - * (we sign with the session key)
> - */
> - rc = smb311_crypto_shash_allocate(chan->server);
> - if (rc) {
> - cifs_dbg(VFS, "%s: crypto alloc failed\n", __func__);
> - mutex_unlock(&ses->session_mutex);
> - goto out;
> - }
> -
> rc = cifs_negotiate_protocol(xid, ses, chan->server);
> if (!rc)
> rc = cifs_setup_session(xid, ses, chan->server, cifs_sb->local_nls);
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 7db5c09ecceb..39a9fc60eb9e 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -897,22 +897,21 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
> return 0;
>
> ok:
> - rc = smb311_crypto_shash_allocate(server);
> + rc = cifs_alloc_hash("sha512", &sha512);
> if (rc)
> return rc;
>
> - sha512 = server->secmech.sha512;
> rc = crypto_shash_init(sha512);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not init sha512 shash\n", __func__);
> - return rc;
> + goto out_free_hash;
> }
>
> rc = crypto_shash_update(sha512, ses->preauth_sha_hash,
> SMB2_PREAUTH_HASH_SIZE);
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update sha512 shash\n", __func__);
> - return rc;
> + goto out_free_hash;
> }
>
> for (i = 0; i < nvec; i++) {
> @@ -920,16 +919,15 @@ smb311_update_preauth_hash(struct cifs_ses *ses, struct TCP_Server_Info *server,
> if (rc) {
> cifs_dbg(VFS, "%s: Could not update sha512 shash\n",
> __func__);
> - return rc;
> + goto out_free_hash;
> }
> }
>
> rc = crypto_shash_final(sha512, ses->preauth_sha_hash);
> - if (rc) {
> + if (rc)
> cifs_dbg(VFS, "%s: Could not finalize sha512 shash\n",
> __func__);
> - return rc;
> - }
> -
> - return 0;
> +out_free_hash:
> + cifs_free_hash(&sha512);
> + return rc;
> }
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index d1528755f330..34dea8aa854b 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -4338,10 +4338,10 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> return rc;
> }
>
> - rc = smb3_crypto_aead_allocate(server);
> - if (rc) {
> - cifs_server_dbg(VFS, "%s: crypto alloc failed\n", __func__);
> - return rc;
> + /* sanity check -- TFMs were allocated after negotiate protocol */
> + if (unlikely(!server->secmech.enc || !server->secmech.dec)) {
> + cifs_server_dbg(VFS, "%s: crypto TFMs are NULL\n", __func__);
> + return -EIO;
> }
>
> tfm = enc ? server->secmech.enc : server->secmech.dec;
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 6352ab32c7e7..48b25054354c 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -927,6 +927,16 @@ SMB2_negotiate(const unsigned int xid,
> else
> req->SecurityMode = 0;
>
> + if (req->SecurityMode) {
> + /*
> + * Allocate HMAC-SHA256 regardless of dialect requested, change to AES-CMAC later,
> + * if SMB3+ is negotiated
> + */
> + rc = cifs_alloc_hash("hmac(sha256)",&server->secmech.hmacsha256);
> + if (rc)
> + goto neg_exit;
> + }
> +
> req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
> if (ses->chan_max > 1)
> req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
> @@ -1071,6 +1081,15 @@ SMB2_negotiate(const unsigned int xid,
> rc = cifs_enable_signing(server, ses->sign);
> if (rc)
> goto neg_exit;
> +
> + if (server->sign && server->dialect >= SMB30_PROT_ID) {
> + /* free HMAC-SHA256 allocated earlier for negprot */
> + cifs_free_hash(&server->secmech.hmacsha256);
> + rc = cifs_alloc_hash("cmac(aes)", &server->secmech.aes_cmac);
> + if (rc)
> + goto neg_exit;
> + }
> +
> if (blob_length) {
> rc = decode_negTokenInit(security_blob, blob_length, server);
> if (rc == 1)
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 3f740f24b96a..a975144c63bf 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -267,7 +267,6 @@ extern int smb2_validate_and_copy_iov(unsigned int offset,
> extern void smb2_copy_fs_info_to_kstatfs(
> struct smb2_fs_full_size_info *pfs_inf,
> struct kstatfs *kst);
> -extern int smb311_crypto_shash_allocate(struct TCP_Server_Info *server);
> extern int smb311_update_preauth_hash(struct cifs_ses *ses,
> struct TCP_Server_Info *server,
> struct kvec *iov, int nvec);
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index dfcbcc0b86e4..2dca2c255239 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -26,53 +26,6 @@
> #include "smb2status.h"
> #include "smb2glob.h"
>
> -static int
> -smb3_crypto_shash_allocate(struct TCP_Server_Info *server)
> -{
> - struct cifs_secmech *p = &server->secmech;
> - int rc;
> -
> - rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
> - if (rc)
> - goto err;
> -
> - rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
> - if (rc)
> - goto err;
> -
> - return 0;
> -err:
> - cifs_free_hash(&p->hmacsha256);
> - return rc;
> -}
> -
> -int
> -smb311_crypto_shash_allocate(struct TCP_Server_Info *server)
> -{
> - struct cifs_secmech *p = &server->secmech;
> - int rc = 0;
> -
> - rc = cifs_alloc_hash("hmac(sha256)", &p->hmacsha256);
> - if (rc)
> - return rc;
> -
> - rc = cifs_alloc_hash("cmac(aes)", &p->aes_cmac);
> - if (rc)
> - goto err;
> -
> - rc = cifs_alloc_hash("sha512", &p->sha512);
> - if (rc)
> - goto err;
> -
> - return 0;
> -
> -err:
> - cifs_free_hash(&p->aes_cmac);
> - cifs_free_hash(&p->hmacsha256);
> - return rc;
> -}
> -
> -
> static
> int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
> {
> @@ -215,7 +168,7 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> struct kvec *iov = rqst->rq_iov;
> struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
> struct cifs_ses *ses;
> - struct shash_desc *shash;
> + struct shash_desc *shash = NULL;
> struct smb_rqst drqst;
>
> ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
> @@ -297,48 +250,50 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
> unsigned char prfhash[SMB2_HMACSHA256_SIZE];
> unsigned char *hashptr = prfhash;
> struct TCP_Server_Info *server = ses->server;
> + struct shash_desc *hmac_sha256 = NULL;
>
> memset(prfhash, 0x0, SMB2_HMACSHA256_SIZE);
> memset(key, 0x0, key_size);
>
> - rc = smb3_crypto_shash_allocate(server);
> + /* do not reuse the server's secmech TFM */
> + rc = cifs_alloc_hash("hmac(sha256)", &hmac_sha256);
> if (rc) {
> cifs_server_dbg(VFS, "%s: crypto alloc failed\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_setkey(server->secmech.hmacsha256->tfm,
> - ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
> + rc = crypto_shash_setkey(hmac_sha256->tfm, ses->auth_key.response,
> + SMB2_NTLMV2_SESSKEY_SIZE);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not set with session key\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_init(server->secmech.hmacsha256);
> + rc = crypto_shash_init(hmac_sha256);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not init sign hmac\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_update(server->secmech.hmacsha256, i, 4);
> + rc = crypto_shash_update(hmac_sha256, i, 4);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not update with n\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_update(server->secmech.hmacsha256, label.iov_base, label.iov_len);
> + rc = crypto_shash_update(hmac_sha256, label.iov_base, label.iov_len);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not update with label\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_update(server->secmech.hmacsha256, &zero, 1);
> + rc = crypto_shash_update(hmac_sha256, &zero, 1);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not update with zero\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_update(server->secmech.hmacsha256, context.iov_base, context.iov_len);
> + rc = crypto_shash_update(hmac_sha256, context.iov_base, context.iov_len);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not update with context\n", __func__);
> goto smb3signkey_ret;
> @@ -346,16 +301,16 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
>
> if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
> (server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)) {
> - rc = crypto_shash_update(server->secmech.hmacsha256, L256, 4);
> + rc = crypto_shash_update(hmac_sha256, L256, 4);
> } else {
> - rc = crypto_shash_update(server->secmech.hmacsha256, L128, 4);
> + rc = crypto_shash_update(hmac_sha256, L128, 4);
> }
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not update with L\n", __func__);
> goto smb3signkey_ret;
> }
>
> - rc = crypto_shash_final(server->secmech.hmacsha256, hashptr);
> + rc = crypto_shash_final(hmac_sha256, hashptr);
> if (rc) {
> cifs_server_dbg(VFS, "%s: Could not generate sha256 hash\n", __func__);
> goto smb3signkey_ret;
> @@ -364,6 +319,7 @@ static int generate_key(struct cifs_ses *ses, struct kvec label,
> memcpy(key, hashptr, key_size);
>
> smb3signkey_ret:
> + cifs_free_hash(&hmac_sha256);
> return rc;
> }
>
> @@ -428,12 +384,19 @@ generate_smb3signingkey(struct cifs_ses *ses,
> ptriplet->encryption.context,
> ses->smb3encryptionkey,
> SMB3_ENC_DEC_KEY_SIZE);
> + if (rc)
> + return rc;
> +
> rc = generate_key(ses, ptriplet->decryption.label,
> ptriplet->decryption.context,
> ses->smb3decryptionkey,
> SMB3_ENC_DEC_KEY_SIZE);
> if (rc)
> return rc;
> +
> + rc = smb3_crypto_aead_allocate(server);
> + if (rc)
> + return rc;
> }
>
> if (rc)
> @@ -535,7 +498,7 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
> unsigned char *sigptr = smb3_signature;
> struct kvec *iov = rqst->rq_iov;
> struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
> - struct shash_desc *shash;
> + struct shash_desc *shash = NULL;
> struct smb_rqst drqst;
> u8 key[SMB3_SIGN_KEY_SIZE];
>
> --
> 2.35.3
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 4/8] cifs: create sign/verify secmechs, don't leave keys in memory
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
` (2 preceding siblings ...)
2022-09-29 1:56 ` [PATCH v3 3/8] cifs: allocate ephemeral secmechs only on demand Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1 Enzo Matsumiya
` (3 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
This patch creates separate TFMs for signing and verifying requests.
This detaches the secmechs fields' names from any particular algorithm.
Since the TFMs are now required to know/have the key set beforehand,
we have the added benefit of not needing private keys laying around
in memory anymore. This patch sets signing and encryption keys right
after they are generated, and zero the generated ones upon success.
If CONFIG_CIFS_DUMP_KEYS option is enabled, the keys are copied over to
their previous locations for debugging.
These changes also makes the smb2_calc_signature and smb3_calc_signature
functions exactly the same, so converge them into one
(smb2_calc_signature). The only difference now is the output buffer
size (32 bytes for HMAC-SHA256 vs 16 bytes for AES-CMAC) where the
generated hash is stored, but only the first 16 bytes are actually
copied to the header signature, so the same 32-byte buffer can be used
for both without any problems.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsencrypt.c | 35 +++--
fs/cifs/cifsglob.h | 25 +++-
fs/cifs/smb1ops.c | 6 +
fs/cifs/smb2ops.c | 53 +-------
fs/cifs/smb2pdu.c | 43 +++++-
fs/cifs/smb2proto.h | 5 +-
fs/cifs/smb2transport.c | 283 +++++++++++++---------------------------
7 files changed, 176 insertions(+), 274 deletions(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index ed25ac811f05..4ae58ab29458 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -95,32 +95,33 @@ int __cifs_calc_signature(struct smb_rqst *rqst,
* the sequence number before this function is called. Also, this function
* should be called with the server->srv_mutex held.
*/
-static int cifs_calc_signature(struct smb_rqst *rqst,
- struct TCP_Server_Info *server, char *signature)
+static int cifs_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
+ char *signature, bool verify)
{
+ struct shash_desc *shash = NULL;
int rc;
if (!rqst->rq_iov || !signature || !server)
return -EINVAL;
- rc = cifs_alloc_hash("md5", &server->secmech.md5);
- if (rc)
- return -1;
+ if (verify)
+ shash = server->secmech.verify;
+ else
+ shash = server->secmech.sign;
- rc = crypto_shash_init(server->secmech.md5);
+ rc = crypto_shash_init(shash);
if (rc) {
cifs_dbg(VFS, "%s: Could not init md5\n", __func__);
return rc;
}
- rc = crypto_shash_update(server->secmech.md5,
- server->session_key.response, server->session_key.len);
+ rc = crypto_shash_update(shash, server->session_key.response, server->session_key.len);
if (rc) {
cifs_dbg(VFS, "%s: Could not update with response\n", __func__);
return rc;
}
- return __cifs_calc_signature(rqst, server, signature, server->secmech.md5);
+ return __cifs_calc_signature(rqst, server, signature, shash);
}
/* must be called with server->srv_mutex held */
@@ -158,7 +159,7 @@ int cifs_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server,
*pexpected_response_sequence_number = ++server->sequence_number;
++server->sequence_number;
- rc = cifs_calc_signature(rqst, server, smb_signature);
+ rc = cifs_calc_signature(rqst, server, smb_signature, false);
if (rc)
memset(cifs_pdu->Signature.SecuritySignature, 0, 8);
else
@@ -197,7 +198,7 @@ int cifs_verify_signature(struct smb_rqst *rqst,
{
unsigned int rc;
char server_response_sig[8];
- char what_we_think_sig_should_be[20];
+ char sig[20];
struct smb_hdr *cifs_pdu = (struct smb_hdr *)rqst->rq_iov[0].iov_base;
if (rqst->rq_iov[0].iov_len != 4 ||
@@ -234,16 +235,13 @@ int cifs_verify_signature(struct smb_rqst *rqst,
cifs_pdu->Signature.Sequence.Reserved = 0;
cifs_server_lock(server);
- rc = cifs_calc_signature(rqst, server, what_we_think_sig_should_be);
+ rc = cifs_calc_signature(rqst, server, sig, true);
cifs_server_unlock(server);
if (rc)
return rc;
-/* cifs_dump_mem("what we think it should be: ",
- what_we_think_sig_should_be, 16); */
-
- if (memcmp(server_response_sig, what_we_think_sig_should_be, 8))
+ if (memcmp(server_response_sig, sig, 8))
return -EACCES;
else
return 0;
@@ -705,9 +703,8 @@ calc_seckey(struct cifs_ses *ses)
void
cifs_crypto_secmech_release(struct TCP_Server_Info *server)
{
- cifs_free_hash(&server->secmech.aes_cmac);
- cifs_free_hash(&server->secmech.hmacsha256);
- cifs_free_hash(&server->secmech.md5);
+ cifs_free_hash(&server->secmech.sign);
+ cifs_free_hash(&server->secmech.verify);
if (server->secmech.enc) {
crypto_free_aead(server->secmech.enc);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 5da71d946012..30b3fadb4b06 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -153,14 +153,27 @@ struct session_key {
char *response;
};
-/* crypto hashing related structure/fields, not specific to a sec mech */
+/**
+ * cifs_secmech - Crypto hashing related structure/fields, not specific to one mechanism
+ * @sign: SHASH descriptor to allocate a hashing TFM for signing requests
+ * @verify: SHASH descriptor to allocate a hashing TFM for verifying requests' signatures
+ * @enc: AEAD TFM for SMB3+ encryption
+ * @dec: AEAD TFM for SMB3+ decryption
+ *
+ * @sign and @verify are allocated per-server, and the negotiated connection dialect will dictate
+ * which algorithm to use:
+ * - MD5 for SMB1
+ * - HMAC-SHA256 for SMB2
+ * - AES-CMAC for SMB3
+ *
+ * @enc and @dec holds the encryption/decryption TFMs, where it'll be either AES-CCM or AES-GCM.
+ */
struct cifs_secmech {
- struct shash_desc *md5; /* md5 hash function, for CIFS/SMB1 signatures */
- struct shash_desc *hmacsha256; /* hmac-sha256 hash function, for SMB2 signatures */
- struct shash_desc *aes_cmac; /* block-cipher based MAC function, for SMB3 signatures */
+ struct shash_desc *sign;
+ struct shash_desc *verify;
- struct crypto_aead *enc; /* smb3 encryption AEAD TFM (AES-CCM and AES-GCM) */
- struct crypto_aead *dec; /* smb3 decryption AEAD TFM (AES-CCM and AES-GCM) */
+ struct crypto_aead *enc;
+ struct crypto_aead *dec;
};
/* per smb session structure/fields */
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index f36b2d2d40ca..28e0ff0bb35d 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -433,6 +433,12 @@ cifs_negotiate(const unsigned int xid,
if (rc == -EAGAIN)
rc = -EHOSTDOWN;
}
+
+ if (!rc) {
+ rc = cifs_alloc_hash("md5", &server->secmech.sign.shash);
+ if (!rc)
+ rc = cifs_alloc_hash("md5", &server->secmech.verify.shash);
+ }
return rc;
}
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 34dea8aa854b..0aaad18e1ec8 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4282,30 +4282,6 @@ init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
return sg;
}
-static int
-smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
-{
- struct cifs_ses *ses;
- u8 *ses_enc_key;
-
- spin_lock(&cifs_tcp_ses_lock);
- list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
- list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
- if (ses->Suid == ses_id) {
- spin_lock(&ses->ses_lock);
- ses_enc_key = enc ? ses->smb3encryptionkey :
- ses->smb3decryptionkey;
- memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);
- spin_unlock(&ses->ses_lock);
- spin_unlock(&cifs_tcp_ses_lock);
- return 0;
- }
- }
- }
- spin_unlock(&cifs_tcp_ses_lock);
-
- return -EAGAIN;
-}
/*
* Encrypt or decrypt @rqst message. @rqst[0] has the following format:
* iov[0] - transform header (associate data),
@@ -4323,7 +4299,6 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
int rc = 0;
struct scatterlist *sg;
u8 sign[SMB2_SIGNATURE_SIZE] = {};
- u8 key[SMB3_ENC_DEC_KEY_SIZE];
struct aead_request *req;
char *iv;
unsigned int iv_len;
@@ -4331,13 +4306,6 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
struct crypto_aead *tfm;
unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
- rc = smb2_get_enc_key(server, le64_to_cpu(tr_hdr->SessionId), enc, key);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
- enc ? "en" : "de");
- return rc;
- }
-
/* sanity check -- TFMs were allocated after negotiate protocol */
if (unlikely(!server->secmech.enc || !server->secmech.dec)) {
cifs_server_dbg(VFS, "%s: crypto TFMs are NULL\n", __func__);
@@ -4346,23 +4314,6 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
tfm = enc ? server->secmech.enc : server->secmech.dec;
- if ((server->cipher_type == SMB2_ENCRYPTION_AES256_CCM) ||
- (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_GCM128_CRYPTKEY_SIZE);
-
- if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set aead key %d\n", __func__, rc);
- return rc;
- }
-
- rc = crypto_aead_setauthsize(tfm, SMB2_SIGNATURE_SIZE);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set authsize %d\n", __func__, rc);
- return rc;
- }
-
req = aead_request_alloc(tfm, GFP_KERNEL);
if (!req) {
cifs_server_dbg(VFS, "%s: Failed to alloc aead request\n", __func__);
@@ -5470,7 +5421,7 @@ struct smb_version_operations smb30_operations = {
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
.generate_signingkey = generate_smb30signingkey,
- .calc_signature = smb3_calc_signature,
+ .calc_signature = smb2_calc_signature,
.set_integrity = smb3_set_integrity,
.is_read_op = smb21_is_read_op,
.set_oplock_level = smb3_set_oplock_level,
@@ -5584,7 +5535,7 @@ struct smb_version_operations smb311_operations = {
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
.generate_signingkey = generate_smb311signingkey,
- .calc_signature = smb3_calc_signature,
+ .calc_signature = smb2_calc_signature,
.set_integrity = smb3_set_integrity,
.is_read_op = smb21_is_read_op,
.set_oplock_level = smb3_set_oplock_level,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 48b25054354c..e5939c374c35 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -932,9 +932,15 @@ SMB2_negotiate(const unsigned int xid,
* Allocate HMAC-SHA256 regardless of dialect requested, change to AES-CMAC later,
* if SMB3+ is negotiated
*/
- rc = cifs_alloc_hash("hmac(sha256)",&server->secmech.hmacsha256);
+ rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.sign);
if (rc)
goto neg_exit;
+
+ rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.verify);
+ if (rc) {
+ cifs_free_hash(&server->secmech.sign);
+ goto neg_exit;
+ }
}
req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
@@ -1084,10 +1090,17 @@ SMB2_negotiate(const unsigned int xid,
if (server->sign && server->dialect >= SMB30_PROT_ID) {
/* free HMAC-SHA256 allocated earlier for negprot */
- cifs_free_hash(&server->secmech.hmacsha256);
- rc = cifs_alloc_hash("cmac(aes)", &server->secmech.aes_cmac);
+ cifs_free_hash(&server->secmech.sign);
+ cifs_free_hash(&server->secmech.verify);
+ rc = cifs_alloc_hash("cmac(aes)", &server->secmech.sign);
if (rc)
goto neg_exit;
+
+ rc = cifs_alloc_hash("cmac(aes)", &server->secmech.verify);
+ if (rc) {
+ cifs_free_hash(&server->secmech.sign);
+ goto neg_exit;
+ }
}
if (blob_length) {
@@ -1660,8 +1673,26 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
}
rc = SMB2_sess_establish_session(sess_data);
-#ifdef CONFIG_CIFS_DEBUG_DUMP_KEYS
+ if (rc)
+ goto out;
+
if (ses->server->dialect < SMB30_PROT_ID) {
+ rc = crypto_shash_setkey(server->secmech.sign->tfm,
+ ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
+ if (rc) {
+ cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 signing key, rc=%d\n",
+ __func__, rc);
+ goto out;
+ }
+
+ rc = crypto_shash_setkey(server->secmech.verify->tfm,
+ ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
+ if (rc) {
+ cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 verify key, rc=%d\n",
+ __func__, rc);
+ goto out;
+ }
+#ifdef CONFIG_CIFS_DEBUG_DUMP_KEYS
cifs_dbg(VFS, "%s: dumping generated SMB2 session keys\n", __func__);
/*
* The session id is opaque in terms of endianness, so we can't
@@ -1673,8 +1704,10 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
SMB2_NTLMV2_SESSKEY_SIZE, ses->auth_key.response);
cifs_dbg(VFS, "Signing Key %*ph\n",
SMB3_SIGN_KEY_SIZE, ses->auth_key.response);
+#else /* CONFIG_CIFS_DEBUG_DUMP_KEYS */
+ memzero_explicit(ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
+#endif /* !CONFIG_CIFS_DEBUG_DUMP_KEYS */
}
-#endif
out:
kfree(ntlmssp_blob);
SMB2_sess_free_buffer(sess_data);
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index a975144c63bf..33af35b6e586 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -43,10 +43,7 @@ extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
__u64 ses_id, __u32 tid);
extern int smb2_calc_signature(struct smb_rqst *rqst,
struct TCP_Server_Info *server,
- bool allocate_crypto);
-extern int smb3_calc_signature(struct smb_rqst *rqst,
- struct TCP_Server_Info *server,
- bool allocate_crypto);
+ bool verify);
extern void smb2_echo_request(struct work_struct *work);
extern __le32 smb2_get_lease_state(struct cifsInodeInfo *cinode);
extern bool smb2_is_valid_oplock_break(char *buffer,
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 2dca2c255239..cf319fc25161 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -26,63 +26,61 @@
#include "smb2status.h"
#include "smb2glob.h"
-static
-int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
+static int
+smb3_setup_keys(struct TCP_Server_Info *server, u8 *sign_key, u8 *enc_key, u8 *dec_key)
{
- struct cifs_chan *chan;
- struct cifs_ses *ses = NULL;
- struct TCP_Server_Info *it = NULL;
- int i;
+ unsigned int crypt_keylen = 0;
int rc = 0;
- spin_lock(&cifs_tcp_ses_lock);
+ if (!(server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION) || !enc_key || !dec_key)
+ goto setup_sign;
- list_for_each_entry(it, &cifs_tcp_ses_list, tcp_ses_list) {
- list_for_each_entry(ses, &it->smb_ses_list, smb_ses_list) {
- if (ses->Suid == ses_id)
- goto found;
- }
+ if (server->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
+ server->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
+ crypt_keylen = SMB3_GCM256_CRYPTKEY_SIZE;
+ else
+ crypt_keylen = SMB3_GCM128_CRYPTKEY_SIZE;
+
+ rc = crypto_aead_setkey(server->secmech.enc, enc_key, crypt_keylen);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption key, rc=%d\n",
+ __func__, rc);
+ return rc;
}
- cifs_server_dbg(VFS, "%s: Could not find session 0x%llx\n",
- __func__, ses_id);
- rc = -ENOENT;
- goto out;
-found:
- spin_lock(&ses->chan_lock);
- if (cifs_chan_needs_reconnect(ses, server) &&
- !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
- /*
- * If we are in the process of binding a new channel
- * to an existing session, use the master connection
- * session key
- */
- memcpy(key, ses->smb3signingkey, SMB3_SIGN_KEY_SIZE);
- spin_unlock(&ses->chan_lock);
- goto out;
+ rc = crypto_aead_setauthsize(server->secmech.enc, SMB2_SIGNATURE_SIZE);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption authsize, rc=%d\n",
+ __func__, rc);
+ return rc;
}
- /*
- * Otherwise, use the channel key.
- */
+ rc = crypto_aead_setkey(server->secmech.dec, dec_key, crypt_keylen);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption key, rc=%d\n",
+ __func__, rc);
+ return rc;
+ }
- for (i = 0; i < ses->chan_count; i++) {
- chan = ses->chans + i;
- if (chan->server == server) {
- memcpy(key, chan->signkey, SMB3_SIGN_KEY_SIZE);
- spin_unlock(&ses->chan_lock);
- goto out;
- }
+ rc = crypto_aead_setauthsize(server->secmech.dec, SMB2_SIGNATURE_SIZE);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption authsize, rc=%d\n",
+ __func__, rc);
+ return rc;
+ }
+setup_sign:
+ rc = crypto_shash_setkey(server->secmech.sign->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC signing key, rc=%d\n",
+ __func__, rc);
+ return rc;
}
- spin_unlock(&ses->chan_lock);
- cifs_dbg(VFS,
- "%s: Could not find channel signing key for session 0x%llx\n",
- __func__, ses_id);
- rc = -ENOENT;
+ rc = crypto_shash_setkey(server->secmech.verify->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
+ if (rc)
+ cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC verify key, rc=%d\n",
+ __func__, rc);
-out:
- spin_unlock(&cifs_tcp_ses_lock);
return rc;
}
@@ -159,51 +157,29 @@ smb2_find_smb_tcon(struct TCP_Server_Info *server, __u64 ses_id, __u32 tid)
}
int
-smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
- bool allocate_crypto)
+smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify)
{
int rc;
- unsigned char smb2_signature[SMB2_HMACSHA256_SIZE];
- unsigned char *sigptr = smb2_signature;
+ unsigned char sig[SMB2_HMACSHA256_SIZE]; /* big enough for HMAC-SHA256 and AES-CMAC */
+ unsigned char *sigptr = sig;
struct kvec *iov = rqst->rq_iov;
struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
- struct cifs_ses *ses;
struct shash_desc *shash = NULL;
struct smb_rqst drqst;
- ses = smb2_find_smb_ses(server, le64_to_cpu(shdr->SessionId));
- if (unlikely(!ses)) {
- cifs_server_dbg(VFS, "%s: Could not find session\n", __func__);
- return -ENOENT;
- }
-
- memset(smb2_signature, 0x0, SMB2_HMACSHA256_SIZE);
+ memset(sig, 0x0, SMB2_HMACSHA256_SIZE);
memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
- if (allocate_crypto) {
- rc = cifs_alloc_hash("hmac(sha256)", &shash);
- if (rc) {
- cifs_server_dbg(VFS,
- "%s: sha256 alloc failed\n", __func__);
- goto out;
- }
- } else {
- shash = server->secmech.hmacsha256;
- }
-
- rc = crypto_shash_setkey(shash->tfm, ses->auth_key.response,
- SMB2_NTLMV2_SESSKEY_SIZE);
- if (rc) {
- cifs_server_dbg(VFS,
- "%s: Could not update with response\n",
- __func__);
- goto out;
- }
+ if (verify)
+ shash = server->secmech.verify;
+ else
+ shash = server->secmech.sign;
rc = crypto_shash_init(shash);
if (rc) {
- cifs_server_dbg(VFS, "%s: Could not init sha256", __func__);
- goto out;
+ cifs_server_dbg(VFS, "%s: Could not init %s\n", __func__,
+ crypto_shash_alg_name(shash->tfm));
+ return rc;
}
/*
@@ -215,13 +191,11 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
*/
drqst = *rqst;
if (drqst.rq_nvec >= 2 && iov[0].iov_len == 4) {
- rc = crypto_shash_update(shash, iov[0].iov_base,
- iov[0].iov_len);
+ rc = crypto_shash_update(shash, iov[0].iov_base, iov[0].iov_len);
if (rc) {
- cifs_server_dbg(VFS,
- "%s: Could not update with payload\n",
- __func__);
- goto out;
+ cifs_server_dbg(VFS, "%s: Could not update with payload, rc=%d\n",
+ __func__, rc);
+ return rc;
}
drqst.rq_iov++;
drqst.rq_nvec--;
@@ -231,11 +205,6 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
if (!rc)
memcpy(shdr->Signature, sigptr, SMB2_SIGNATURE_SIZE);
-out:
- if (allocate_crypto)
- cifs_free_hash(&shash);
- if (ses)
- cifs_put_smb_ses(ses);
return rc;
}
@@ -342,6 +311,9 @@ generate_smb3signingkey(struct cifs_ses *ses,
int rc;
bool is_binding = false;
int chan_index = 0;
+ u8 sign_key[SMB3_SIGN_KEY_SIZE] = { 0 };
+ u8 enc_key[SMB3_ENC_DEC_KEY_SIZE] = { 0 };
+ u8 dec_key[SMB3_ENC_DEC_KEY_SIZE] = { 0 };
spin_lock(&ses->chan_lock);
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
@@ -362,47 +334,54 @@ generate_smb3signingkey(struct cifs_ses *ses,
if (is_binding) {
rc = generate_key(ses, ptriplet->signing.label,
ptriplet->signing.context,
- ses->chans[chan_index].signkey,
- SMB3_SIGN_KEY_SIZE);
+ sign_key, SMB3_SIGN_KEY_SIZE);
if (rc)
- return rc;
+ goto out_zero_keys;
+
+ rc = smb3_setup_keys(ses->chans[chan_index].server, sign_key, NULL, NULL);
+ if (rc)
+ goto out_zero_keys;
} else {
rc = generate_key(ses, ptriplet->signing.label,
ptriplet->signing.context,
- ses->smb3signingkey,
- SMB3_SIGN_KEY_SIZE);
+ sign_key, SMB3_SIGN_KEY_SIZE);
if (rc)
- return rc;
-
- /* safe to access primary channel, since it will never go away */
- spin_lock(&ses->chan_lock);
- memcpy(ses->chans[0].signkey, ses->smb3signingkey,
- SMB3_SIGN_KEY_SIZE);
- spin_unlock(&ses->chan_lock);
+ goto out_zero_keys;
rc = generate_key(ses, ptriplet->encryption.label,
ptriplet->encryption.context,
- ses->smb3encryptionkey,
- SMB3_ENC_DEC_KEY_SIZE);
+ enc_key, SMB3_ENC_DEC_KEY_SIZE);
if (rc)
- return rc;
+ goto out_zero_keys;
rc = generate_key(ses, ptriplet->decryption.label,
ptriplet->decryption.context,
- ses->smb3decryptionkey,
- SMB3_ENC_DEC_KEY_SIZE);
+ dec_key, SMB3_ENC_DEC_KEY_SIZE);
if (rc)
- return rc;
+ goto out_zero_keys;
rc = smb3_crypto_aead_allocate(server);
if (rc)
- return rc;
- }
-
- if (rc)
- return rc;
+ goto out_zero_keys;
+ rc = smb3_setup_keys(ses->server, sign_key, enc_key, dec_key);
+ if (rc)
+ goto out_zero_keys;
+ }
+out_zero_keys:
#ifdef CONFIG_CIFS_DEBUG_DUMP_KEYS
+ /* only leave keys in memory if debugging */
+ memcpy(ses->smb3encryptionkey, enc_key, SMB3_ENC_DEC_KEY_SIZE);
+ memcpy(ses->smb3decryptionkey, dec_key, SMB3_ENC_DEC_KEY_SIZE);
+ spin_lock(&ses->chan_lock);
+ if (is_binding)
+ memcpy(ses->chans[chan_index].signkey, sign_key, SMB3_SIGN_KEY_SIZE);
+ else
+ /* safe to access primary channel, since it will never go away */
+ memcpy(ses->chans[0].signkey, sign_key, SMB3_SIGN_KEY_SIZE);
+ spin_unlock(&ses->chan_lock);
+ memcpy(ses->smb3signingkey, sign_key, SMB3_SIGN_KEY_SIZE);
+
cifs_dbg(VFS, "%s: dumping generated AES session keys\n", __func__);
/*
* The session id is opaque in terms of endianness, so we can't
@@ -428,6 +407,9 @@ generate_smb3signingkey(struct cifs_ses *ses,
SMB3_GCM128_CRYPTKEY_SIZE, ses->smb3decryptionkey);
}
#endif
+ memzero_explicit(sign_key, SMB3_SIGN_KEY_SIZE);
+ memzero_explicit(enc_key, SMB3_ENC_DEC_KEY_SIZE);
+ memzero_explicit(dec_key, SMB3_ENC_DEC_KEY_SIZE);
return rc;
}
@@ -489,83 +471,6 @@ generate_smb311signingkey(struct cifs_ses *ses,
return generate_smb3signingkey(ses, server, &triplet);
}
-int
-smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server,
- bool allocate_crypto)
-{
- int rc;
- unsigned char smb3_signature[SMB2_CMACAES_SIZE];
- unsigned char *sigptr = smb3_signature;
- struct kvec *iov = rqst->rq_iov;
- struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
- struct shash_desc *shash = NULL;
- struct smb_rqst drqst;
- u8 key[SMB3_SIGN_KEY_SIZE];
-
- rc = smb2_get_sign_key(le64_to_cpu(shdr->SessionId), server, key);
- if (unlikely(rc)) {
- cifs_server_dbg(VFS, "%s: Could not get signing key\n", __func__);
- return rc;
- }
-
- if (allocate_crypto) {
- rc = cifs_alloc_hash("cmac(aes)", &shash);
- if (rc)
- return rc;
- } else {
- shash = server->secmech.aes_cmac;
- }
-
- memset(smb3_signature, 0x0, SMB2_CMACAES_SIZE);
- memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
-
- rc = crypto_shash_setkey(shash->tfm, key, SMB2_CMACAES_SIZE);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Could not set key for cmac aes\n", __func__);
- goto out;
- }
-
- /*
- * we already allocate aes_cmac when we init smb3 signing key,
- * so unlike smb2 case we do not have to check here if secmech are
- * initialized
- */
- rc = crypto_shash_init(shash);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Could not init cmac aes\n", __func__);
- goto out;
- }
-
- /*
- * For SMB2+, __cifs_calc_signature() expects to sign only the actual
- * data, that is, iov[0] should not contain a rfc1002 length.
- *
- * Sign the rfc1002 length prior to passing the data (iov[1-N]) down to
- * __cifs_calc_signature().
- */
- drqst = *rqst;
- if (drqst.rq_nvec >= 2 && iov[0].iov_len == 4) {
- rc = crypto_shash_update(shash, iov[0].iov_base,
- iov[0].iov_len);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Could not update with payload\n",
- __func__);
- goto out;
- }
- drqst.rq_iov++;
- drqst.rq_nvec--;
- }
-
- rc = __cifs_calc_signature(&drqst, server, sigptr, shash);
- if (!rc)
- memcpy(shdr->Signature, sigptr, SMB2_SIGNATURE_SIZE);
-
-out:
- if (allocate_crypto)
- cifs_free_hash(&shash);
- return rc;
-}
-
/* must be called with server->srv_mutex held */
static int
smb2_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server)
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
` (3 preceding siblings ...)
2022-09-29 1:56 ` [PATCH v3 4/8] cifs: create sign/verify secmechs, don't leave keys in memory Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 5:14 ` Stefan Metzmacher
2022-09-29 5:22 ` Steve French
2022-09-29 1:56 ` [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param Enzo Matsumiya
` (2 subsequent siblings)
7 siblings, 2 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
Implement support for AES-GMAC message signing, as specified in
MS-SMB2 3.1.4.1 "Signing An Outgoing Message".
The core function is smb311_calc_signature(), which will be used as the
->calc_signature op when SIGNING_ALG_AES_GMAC has been negotiated with
the server.
If "enable_negotiate_signing" is false (default) or if
SIGNING_ALG_AES_GMAC was not negotiated, use AES-CMAC.
Changes:
- convert cifs_secmech sign/verify to unions, where .aead is for AES-GMAC
TFM and .shash for all the others signing algorithms
- init_sg(): rename to smb3_init_sg(), make it non-static, remove skip
variable. This makes it fit for both crypt_message() and
smb311_calc_signature()
- crypt_message(): advance the 20 bytes of rqst[0].rq_iov[0] that are
not part of the encrypted blob before calling smb3_init_sg(). Rewind
it back right after the call. (this was done in init_sg() via the
skip variable)
Other smaller modifications:
- smb2_setup_request(): check if the request has a transform header as we
must not sign an encrypted message (MS-SMB2 3.2.4.1.1)
- smb2_verify_signature():
* check if MessageId is 0xFFFFFFFFFFFFFFFF or if status is STATUS_PENDING
(MS-SMB2 3.2.5.1.3)
* remove extra call to zero the header signature as this is already
done by all ->calc_signature implementations
* remove useless call to check for "BSRSPYL" signature (SMB1 only),
and from smb2_sign_rqst() as well
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsencrypt.c | 23 ++-
fs/cifs/cifsglob.h | 40 +++--
fs/cifs/smb2ops.c | 36 ++---
fs/cifs/smb2pdu.c | 82 +++++-----
fs/cifs/smb2proto.h | 9 +-
fs/cifs/smb2transport.c | 331 ++++++++++++++++++++++++++++++----------
6 files changed, 354 insertions(+), 167 deletions(-)
diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
index 4ae58ab29458..3a78efc45a23 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -23,6 +23,7 @@
#include <linux/fips.h>
#include "../smbfs_common/arc4.h"
#include <crypto/aead.h>
+#include "smb2proto.h"
int __cifs_calc_signature(struct smb_rqst *rqst,
struct TCP_Server_Info *server, char *signature,
@@ -105,9 +106,9 @@ static int cifs_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *se
return -EINVAL;
if (verify)
- shash = server->secmech.verify;
+ shash = server->secmech.verify.shash;
else
- shash = server->secmech.sign;
+ shash = server->secmech.sign.shash;
rc = crypto_shash_init(shash);
if (rc) {
@@ -703,16 +704,14 @@ calc_seckey(struct cifs_ses *ses)
void
cifs_crypto_secmech_release(struct TCP_Server_Info *server)
{
- cifs_free_hash(&server->secmech.sign);
- cifs_free_hash(&server->secmech.verify);
-
- if (server->secmech.enc) {
- crypto_free_aead(server->secmech.enc);
- server->secmech.enc = NULL;
+ if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
+ smb3_crypto_aead_free(&server->secmech.sign.aead);
+ smb3_crypto_aead_free(&server->secmech.verify.aead);
+ } else {
+ cifs_free_hash(&server->secmech.sign.shash);
+ cifs_free_hash(&server->secmech.verify.shash);
}
- if (server->secmech.dec) {
- crypto_free_aead(server->secmech.dec);
- server->secmech.dec = NULL;
- }
+ smb3_crypto_aead_free(&server->secmech.enc);
+ smb3_crypto_aead_free(&server->secmech.dec);
}
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 30b3fadb4b06..81a8eff06467 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -153,24 +153,48 @@ struct session_key {
char *response;
};
+struct smb_rqst;
+struct TCP_Server_Info;
/**
* cifs_secmech - Crypto hashing related structure/fields, not specific to one mechanism
- * @sign: SHASH descriptor to allocate a hashing TFM for signing requests
- * @verify: SHASH descriptor to allocate a hashing TFM for verifying requests' signatures
+ * @sign.shash: SHASH descriptor for signing TFM
+ * @sign.aead: AEAD TFM for signing
+ * @sign_wait: Completion struct for signing operations
+ * @verify.shash: SHASH descriptor for verifying TFM
+ * @verify.aead: AEAD TFM for verifying
+ * @verify_wait: Completion struct for verifying operations
+ * @calc_signature: Signature calculation function to be used.
* @enc: AEAD TFM for SMB3+ encryption
* @dec: AEAD TFM for SMB3+ decryption
*
- * @sign and @verify are allocated per-server, and the negotiated connection dialect will dictate
- * which algorithm to use:
+ * @sign and @verify TFMs are allocated per-server, and the negotiated dialect will dictate which
+ * algorithm to use:
* - MD5 for SMB1
* - HMAC-SHA256 for SMB2
* - AES-CMAC for SMB3
+ * - AES-GMAC for SMB3.1.1
+ *
+ * The completion structs @sign_wait and @verify_wait are required so that we can serialize access
+ * to the AEAD TFMs, since they're asynchronous by design. Using a stack-allocated structure could
+ * cause concurrent access to the TFMs to overwrite the completion status of a previous operation.
*
- * @enc and @dec holds the encryption/decryption TFMs, where it'll be either AES-CCM or AES-GCM.
+ * @enc and @dec holds the encryption/decryption TFMs, also allocated per server, where each will
+ * be either AES-CCM or AES-GCM.
*/
struct cifs_secmech {
- struct shash_desc *sign;
- struct shash_desc *verify;
+ union {
+ struct shash_desc *shash;
+ struct crypto_aead *aead;
+ } sign;
+ struct crypto_wait sign_wait;
+
+ union {
+ struct shash_desc *shash;
+ struct crypto_aead *aead;
+ } verify;
+ struct crypto_wait verify_wait;
+
+ int (*calc_signature)(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify);
struct crypto_aead *enc;
struct crypto_aead *dec;
@@ -445,8 +469,6 @@ struct smb_version_operations {
void (*new_lease_key)(struct cifs_fid *);
int (*generate_signingkey)(struct cifs_ses *ses,
struct TCP_Server_Info *server);
- int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *,
- bool allocate_crypto);
int (*set_integrity)(const unsigned int, struct cifs_tcon *tcon,
struct cifsFileInfo *src_file);
int (*enum_snapshots)(const unsigned int xid, struct cifs_tcon *tcon,
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 0aaad18e1ec8..22b40d181bba 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4234,21 +4234,14 @@ static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
sg_set_page(sg, addr, buflen, offset_in_page(buf));
}
-/* Assumes the first rqst has a transform header as the first iov.
- * I.e.
- * rqst[0].rq_iov[0] is transform header
- * rqst[0].rq_iov[1+] data to be encrypted/decrypted
- * rqst[1+].rq_iov[0+] data to be encrypted/decrypted
- */
-static struct scatterlist *
-init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
+struct scatterlist *
+smb3_init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
{
unsigned int sg_len;
struct scatterlist *sg;
unsigned int i;
unsigned int j;
unsigned int idx = 0;
- int skip;
sg_len = 1;
for (i = 0; i < num_rqst; i++)
@@ -4261,15 +4254,10 @@ init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
sg_init_table(sg, sg_len);
for (i = 0; i < num_rqst; i++) {
for (j = 0; j < rqst[i].rq_nvec; j++) {
- /*
- * The first rqst has a transform header where the
- * first 20 bytes are not part of the encrypted blob
- */
- skip = (i == 0) && (j == 0) ? 20 : 0;
smb2_sg_set_buf(&sg[idx++],
- rqst[i].rq_iov[j].iov_base + skip,
- rqst[i].rq_iov[j].iov_len - skip);
- }
+ rqst[i].rq_iov[j].iov_base,
+ rqst[i].rq_iov[j].iov_len);
+ }
for (j = 0; j < rqst[i].rq_npages; j++) {
unsigned int len, offset;
@@ -4325,7 +4313,15 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
crypt_len += SMB2_SIGNATURE_SIZE;
}
- sg = init_sg(num_rqst, rqst, sign);
+ /* Skip the first 20 bytes of the first iov of the first request as they're not part of the
+ * encrypted blob */
+ rqst[0].rq_iov[0].iov_base += 20;
+ rqst[0].rq_iov[0].iov_len -= 20;
+ sg = smb3_init_sg(num_rqst, rqst, sign);
+ /* Rewind those 20 bytes before going any further */
+ rqst[0].rq_iov[0].iov_base -= 20;
+ rqst[0].rq_iov[0].iov_len += 20;
+
if (!sg) {
cifs_server_dbg(VFS, "%s: Failed to init sg\n", __func__);
rc = -ENOMEM;
@@ -5213,7 +5209,6 @@ struct smb_version_operations smb20_operations = {
.get_lease_key = smb2_get_lease_key,
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
- .calc_signature = smb2_calc_signature,
.is_read_op = smb2_is_read_op,
.set_oplock_level = smb2_set_oplock_level,
.create_lease_buf = smb2_create_lease_buf,
@@ -5314,7 +5309,6 @@ struct smb_version_operations smb21_operations = {
.get_lease_key = smb2_get_lease_key,
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
- .calc_signature = smb2_calc_signature,
.is_read_op = smb21_is_read_op,
.set_oplock_level = smb21_set_oplock_level,
.create_lease_buf = smb2_create_lease_buf,
@@ -5421,7 +5415,6 @@ struct smb_version_operations smb30_operations = {
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
.generate_signingkey = generate_smb30signingkey,
- .calc_signature = smb2_calc_signature,
.set_integrity = smb3_set_integrity,
.is_read_op = smb21_is_read_op,
.set_oplock_level = smb3_set_oplock_level,
@@ -5535,7 +5528,6 @@ struct smb_version_operations smb311_operations = {
.set_lease_key = smb2_set_lease_key,
.new_lease_key = smb2_new_lease_key,
.generate_signingkey = generate_smb311signingkey,
- .calc_signature = smb2_calc_signature,
.set_integrity = smb3_set_integrity,
.is_read_op = smb21_is_read_op,
.set_oplock_level = smb3_set_oplock_level,
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index e5939c374c35..2c2bf28382bc 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -460,7 +460,7 @@ static unsigned int
build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
{
unsigned int ctxt_len = sizeof(struct smb2_signing_capabilities);
- unsigned short num_algs = 1; /* number of signing algorithms sent */
+ unsigned short num_algs = 2; /* number of signing algorithms sent */
pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES;
/*
@@ -471,12 +471,18 @@ build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
sizeof(struct smb2_neg_context) +
(num_algs * 2 /* sizeof u16 */), 8) * 8);
pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(num_algs);
- pneg_ctxt->SigningAlgorithms[0] = cpu_to_le16(SIGNING_ALG_AES_CMAC);
+
+ /*
+ * Set AES-GMAC as preferred, but will fall back to AES-CMAC if server doesn't support it.
+ * MS-SMB2 2.2.3.1.7
+ */
+ pneg_ctxt->SigningAlgorithms[0] = SIGNING_ALG_AES_GMAC_LE;
+ pneg_ctxt->SigningAlgorithms[1] = SIGNING_ALG_AES_CMAC_LE;
+ /* SMB 3.1.1 doesn't accept HMAC-SHA256, so no need to send it */
ctxt_len += 2 /* sizeof le16 */ * num_algs;
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
@@ -927,22 +933,6 @@ SMB2_negotiate(const unsigned int xid,
else
req->SecurityMode = 0;
- if (req->SecurityMode) {
- /*
- * Allocate HMAC-SHA256 regardless of dialect requested, change to AES-CMAC later,
- * if SMB3+ is negotiated
- */
- rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.sign);
- if (rc)
- goto neg_exit;
-
- rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.verify);
- if (rc) {
- cifs_free_hash(&server->secmech.sign);
- goto neg_exit;
- }
- }
-
req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
if (ses->chan_max > 1)
req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
@@ -1087,22 +1077,6 @@ SMB2_negotiate(const unsigned int xid,
rc = cifs_enable_signing(server, ses->sign);
if (rc)
goto neg_exit;
-
- if (server->sign && server->dialect >= SMB30_PROT_ID) {
- /* free HMAC-SHA256 allocated earlier for negprot */
- cifs_free_hash(&server->secmech.sign);
- cifs_free_hash(&server->secmech.verify);
- rc = cifs_alloc_hash("cmac(aes)", &server->secmech.sign);
- if (rc)
- goto neg_exit;
-
- rc = cifs_alloc_hash("cmac(aes)", &server->secmech.verify);
- if (rc) {
- cifs_free_hash(&server->secmech.sign);
- goto neg_exit;
- }
- }
-
if (blob_length) {
rc = decode_negTokenInit(security_blob, blob_length, server);
if (rc == 1)
@@ -1112,12 +1086,35 @@ SMB2_negotiate(const unsigned int xid,
}
if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
+ server->signing_algorithm = SIGNING_ALG_AES_CMAC;
+ server->signing_negotiated = false;
+
if (rsp->NegotiateContextCount)
rc = smb311_decode_neg_context(rsp, server,
rsp_iov.iov_len);
else
cifs_server_dbg(VFS, "Missing expected negotiate contexts\n");
+
+ /*
+ * Some servers will not send a SMB2_SIGNING_CAPABILITIES context response (*),
+ * so use AES-CMAC signing algorithm as it is expected to be accepted.
+ * See MS-SMB2 note <125> Section 3.2.4.2.2.2
+ */
+ if (!server->signing_negotiated)
+ cifs_dbg(VFS, "signing capabilities were not negotiated, using "
+ "AES-CMAC for message signing\n");
+ } else if (server->dialect >= SMB30_PROT_ID) {
+ server->signing_algorithm = SIGNING_ALG_AES_CMAC;
+ } else if (server->dialect >= SMB20_PROT_ID) {
+ server->signing_algorithm = SIGNING_ALG_HMAC_SHA256;
}
+
+ rc = smb2_init_secmechs(server);
+ if (rc) {
+ cifs_dbg(VFS, "Failed to initialize secmechs, rc=%d\n", rc);
+ goto neg_exit;
+ }
+
neg_exit:
free_rsp_buf(resp_buftype, rsp);
return rc;
@@ -1677,18 +1674,13 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
goto out;
if (ses->server->dialect < SMB30_PROT_ID) {
- rc = crypto_shash_setkey(server->secmech.sign->tfm,
- ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
- if (rc) {
- cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 signing key, rc=%d\n",
- __func__, rc);
- goto out;
- }
-
- rc = crypto_shash_setkey(server->secmech.verify->tfm,
+ rc = crypto_shash_setkey(server->secmech.sign.shash->tfm,
ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
+ if (!rc)
+ rc = crypto_shash_setkey(server->secmech.verify.shash->tfm,
+ ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
if (rc) {
- cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 verify key, rc=%d\n",
+ cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 signing keys, rc=%d\n",
__func__, rc);
goto out;
}
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 33af35b6e586..e4db59e6cee6 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -14,6 +14,7 @@
struct statfs;
struct smb_rqst;
+struct crypto_aead;
/*
*****************************************************************
@@ -37,6 +38,7 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses,
struct smb_rqst *rqst);
extern struct mid_q_entry *smb2_setup_async_request(
struct TCP_Server_Info *server, struct smb_rqst *rqst);
+extern int smb2_init_secmechs(struct TCP_Server_Info *server);
extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server,
__u64 ses_id);
extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
@@ -44,6 +46,10 @@ extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
extern int smb2_calc_signature(struct smb_rqst *rqst,
struct TCP_Server_Info *server,
bool verify);
+extern int smb311_calc_signature(struct smb_rqst *rqst,
+ struct TCP_Server_Info *server,
+ bool verify);
+extern struct scatterlist *smb3_init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign);
extern void smb2_echo_request(struct work_struct *work);
extern __le32 smb2_get_lease_state(struct cifsInodeInfo *cinode);
extern bool smb2_is_valid_oplock_break(char *buffer,
@@ -99,7 +105,8 @@ extern int smb2_unlock_range(struct cifsFileInfo *cfile,
struct file_lock *flock, const unsigned int xid);
extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
extern void smb2_reconnect_server(struct work_struct *work);
-extern int smb3_crypto_aead_allocate(struct TCP_Server_Info *server);
+extern int smb3_crypto_aead_allocate(const char *name, struct crypto_aead **tfm);
+extern void smb3_crypto_aead_free(struct crypto_aead **tfm);
extern unsigned long smb_rqst_len(struct TCP_Server_Info *server,
struct smb_rqst *rqst);
extern void smb2_set_next_command(struct cifs_tcon *tcon,
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index cf319fc25161..ded7144d1578 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -26,6 +26,63 @@
#include "smb2status.h"
#include "smb2glob.h"
+int
+smb2_init_secmechs(struct TCP_Server_Info *server)
+{
+ int rc = 0;
+
+ if (server->dialect >= SMB30_PROT_ID &&
+ (server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)) {
+ if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM ||
+ server->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
+ rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.enc);
+ if (!rc)
+ rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.dec);
+ } else {
+ rc = smb3_crypto_aead_allocate("ccm(aes)", &server->secmech.enc);
+ if (!rc)
+ rc = smb3_crypto_aead_allocate("ccm(aes)", &server->secmech.dec);
+ }
+
+ if (rc)
+ return rc;
+ }
+
+ if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
+ cifs_free_hash(&server->secmech.sign.shash);
+ cifs_free_hash(&server->secmech.verify.shash);
+
+ rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.sign.aead);
+ if (!rc)
+ rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.verify.aead);
+ if (rc) {
+ smb3_crypto_aead_free(&server->secmech.sign.aead);
+ return rc;
+ }
+
+ server->secmech.calc_signature = smb311_calc_signature;
+ } else {
+ char *shash_alg;
+
+ if (server->dialect >= SMB30_PROT_ID)
+ shash_alg = "cmac(aes)";
+ else
+ shash_alg = "hmac(sha256)";
+
+ rc = cifs_alloc_hash(shash_alg, &server->secmech.sign.shash);
+ if (!rc)
+ rc = cifs_alloc_hash(shash_alg, &server->secmech.verify.shash);
+ if (rc) {
+ cifs_free_hash(&server->secmech.sign.shash);
+ return rc;
+ }
+
+ server->secmech.calc_signature = smb2_calc_signature;
+ }
+
+ return rc;
+}
+
static int
smb3_setup_keys(struct TCP_Server_Info *server, u8 *sign_key, u8 *enc_key, u8 *dec_key)
{
@@ -42,44 +99,51 @@ smb3_setup_keys(struct TCP_Server_Info *server, u8 *sign_key, u8 *enc_key, u8 *d
crypt_keylen = SMB3_GCM128_CRYPTKEY_SIZE;
rc = crypto_aead_setkey(server->secmech.enc, enc_key, crypt_keylen);
+ if (!rc)
+ rc = crypto_aead_setkey(server->secmech.dec, dec_key, crypt_keylen);
if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption key, rc=%d\n",
+ cifs_server_dbg(VFS, "%s: Failed to set encryption/decryption key, rc=%d\n",
__func__, rc);
return rc;
}
rc = crypto_aead_setauthsize(server->secmech.enc, SMB2_SIGNATURE_SIZE);
+ if (!rc)
+ rc = crypto_aead_setauthsize(server->secmech.dec, SMB2_SIGNATURE_SIZE);
if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption authsize, rc=%d\n",
- __func__, rc);
- return rc;
- }
-
- rc = crypto_aead_setkey(server->secmech.dec, dec_key, crypt_keylen);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption key, rc=%d\n",
- __func__, rc);
- return rc;
- }
-
- rc = crypto_aead_setauthsize(server->secmech.dec, SMB2_SIGNATURE_SIZE);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption authsize, rc=%d\n",
+ cifs_server_dbg(VFS, "%s: Failed to set encryption/decryption authsize, rc=%d\n",
__func__, rc);
return rc;
}
setup_sign:
- rc = crypto_shash_setkey(server->secmech.sign->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
- if (rc) {
- cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC signing key, rc=%d\n",
- __func__, rc);
- return rc;
- }
+ if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
+ rc = crypto_aead_setkey(server->secmech.sign.aead, sign_key, SMB3_SIGN_KEY_SIZE);
+ if (!rc)
+ rc = crypto_aead_setkey(server->secmech.verify.aead, sign_key,
+ SMB3_SIGN_KEY_SIZE);
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to set AES-GMAC key, rc=%d\n",
+ __func__, rc);
+ return rc;
+ }
- rc = crypto_shash_setkey(server->secmech.verify->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
- if (rc)
- cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC verify key, rc=%d\n",
- __func__, rc);
+ rc = crypto_aead_setauthsize(server->secmech.sign.aead, SMB2_SIGNATURE_SIZE);
+ if (!rc)
+ rc = crypto_aead_setauthsize(server->secmech.verify.aead,
+ SMB2_SIGNATURE_SIZE);
+ if (rc)
+ cifs_server_dbg(VFS, "%s: Failed to set AES-GMAC authsize, rc=%d\n",
+ __func__, rc);
+ } else {
+ rc = crypto_shash_setkey(server->secmech.sign.shash->tfm, sign_key,
+ SMB3_SIGN_KEY_SIZE);
+ if (!rc)
+ rc = crypto_shash_setkey(server->secmech.verify.shash->tfm, sign_key,
+ SMB3_SIGN_KEY_SIZE);
+ if (rc)
+ cifs_server_dbg(VFS, "%s: Failed to set %s signing key, rc=%d\n", __func__,
+ crypto_shash_alg_name(server->secmech.sign.shash->tfm), rc);
+ }
return rc;
}
@@ -171,9 +235,9 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool
memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
if (verify)
- shash = server->secmech.verify;
+ shash = server->secmech.verify.shash;
else
- shash = server->secmech.sign;
+ shash = server->secmech.sign.shash;
rc = crypto_shash_init(shash);
if (rc) {
@@ -360,10 +424,6 @@ generate_smb3signingkey(struct cifs_ses *ses,
if (rc)
goto out_zero_keys;
- rc = smb3_crypto_aead_allocate(server);
- if (rc)
- goto out_zero_keys;
-
rc = smb3_setup_keys(ses->server, sign_key, enc_key, dec_key);
if (rc)
goto out_zero_keys;
@@ -471,6 +531,125 @@ generate_smb311signingkey(struct cifs_ses *ses,
return generate_smb3signingkey(ses, server, &triplet);
}
+/*
+ * This function implements AES-GMAC signing for SMB2 messages as described in MS-SMB2
+ * specification. This algorithm is only supported on SMB 3.1.1.
+ *
+ * Note: even though Microsoft mentions RFC4543 in MS-SMB2, the mechanism used _must_ be the "raw"
+ * AES-128-GCM ("gcm(aes)"); RFC4543 is designed for IPsec and trying to use "rfc4543(gcm(aes)))"
+ * will fail the signature computation.
+ *
+ * MS-SMB2 3.1.4.1
+ */
+int
+smb311_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify)
+{
+ union {
+ struct {
+ /* for MessageId (8 bytes) */
+ __le64 mid;
+ /* for role (client or server) and if SMB2 CANCEL (4 bytes) */
+ __le32 role;
+ };
+ u8 buffer[12];
+ } __packed nonce;
+ u8 sig[SMB2_SIGNATURE_SIZE] = { 0 };
+ struct aead_request *aead_req = NULL;
+ struct crypto_aead *tfm = NULL;
+ struct scatterlist *sg = NULL;
+ unsigned long assoclen;
+ struct smb2_hdr *shdr = NULL;
+ struct crypto_wait *wait;
+ unsigned int save_npages = 0;
+ int rc = 0;
+
+ if (verify) {
+ wait = &server->secmech.verify_wait;
+ tfm = server->secmech.verify.aead;
+ } else {
+ wait = &server->secmech.sign_wait;
+ tfm = server->secmech.sign.aead;
+ }
+
+ if (completion_done(&wait->completion))
+ reinit_completion(&wait->completion);
+
+ shdr = (struct smb2_hdr *)rqst->rq_iov[0].iov_base;
+
+ memset(shdr->Signature, 0, SMB2_SIGNATURE_SIZE);
+ memset(&nonce, 0, SMB3_AES_GCM_NONCE);
+
+ /* note that nonce must always be little endian */
+ nonce.mid = shdr->MessageId;
+ /* request is coming from the server, set LSB */
+ nonce.role |= shdr->Flags & SMB2_FLAGS_SERVER_TO_REDIR;
+ /* set penultimate LSB if SMB2_CANCEL command */
+ if (shdr->Command == SMB2_CANCEL)
+ nonce.role |= cpu_to_le32(1UL << 1);
+
+ aead_req = aead_request_alloc(tfm, GFP_KERNEL);
+ if (!aead_req) {
+ cifs_dbg(VFS, "%s: Failed to alloc AEAD request\n", __func__);
+ return -ENOMEM;
+ }
+
+ /* skip page data if non-success error status, as it will compute an invalid signature */
+ if (shdr->Status != 0 && rqst->rq_npages > 0) {
+ save_npages = rqst->rq_npages;
+ rqst->rq_npages = 0;
+ }
+
+ assoclen = smb_rqst_len(server, rqst);
+
+ sg = smb3_init_sg(1, rqst, sig);
+ if (!sg) {
+ cifs_dbg(VFS, "%s: Failed to init SG\n", __func__);
+ goto out_free_req;
+ }
+
+ /* cryptlen == 0 because we're not encrypting anything */
+ aead_request_set_crypt(aead_req, sg, sg, 0, nonce.buffer);
+ aead_request_set_ad(aead_req, assoclen);
+ aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG, crypto_req_done, wait);
+
+ /*
+ * Reminder: we must always use the encrypt function, as AES-GCM decrypt will internally
+ * try to match the authentication codes, where we pass a zeroed buffer, and the operation
+ * will fail with -EBADMSG (expectedly).
+ *
+ * Also note we can't use crypto_wait_req() here since it's not interruptible.
+ */
+ rc = crypto_aead_encrypt(aead_req);
+ if (!rc)
+ goto out;
+
+ if (rc == -EINPROGRESS || rc == -EBUSY) {
+ rc = wait_for_completion_interruptible(&wait->completion);
+ if (!rc)
+ /* wait->err is set by crypto_req_done callback above */
+ rc = wait->err;
+ }
+
+ if (rc) {
+ cifs_server_dbg(VFS, "%s: Failed to compute AES-GMAC signature, rc=%d\n",
+ __func__, rc);
+ goto out_free_sg;
+ }
+
+out:
+ memcpy(&shdr->Signature, sig, SMB2_SIGNATURE_SIZE);
+out_free_sg:
+ kfree(sg);
+out_free_req:
+ kfree(aead_req);
+
+ /* restore rq_npages for further processing */
+ if (shdr->Status != 0 && save_npages > 0)
+ rqst->rq_npages = save_npages;
+
+ return rc;
+}
+
/* must be called with server->srv_mutex held */
static int
smb2_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server)
@@ -497,12 +676,10 @@ smb2_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server)
return 0;
}
spin_unlock(&server->srv_lock);
- if (!is_binding && !server->session_estab) {
- strncpy(shdr->Signature, "BSRSPYL", 8);
+ if (!is_binding && !server->session_estab)
return 0;
- }
- rc = server->ops->calc_signature(rqst, server, false);
+ rc = server->secmech.calc_signature(rqst, server, false);
return rc;
}
@@ -518,6 +695,8 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
if ((shdr->Command == SMB2_NEGOTIATE) ||
(shdr->Command == SMB2_SESSION_SETUP) ||
(shdr->Command == SMB2_OPLOCK_BREAK) ||
+ (shdr->MessageId == cpu_to_le64(0xFFFFFFFFFFFFFFFF)) ||
+ (shdr->Status == STATUS_PENDING) ||
server->ignore_signature ||
(!server->session_estab))
return 0;
@@ -527,20 +706,13 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
* server does not send one? BB
*/
- /* Do not need to verify session setups with signature "BSRSPYL " */
- if (memcmp(shdr->Signature, "BSRSPYL ", 8) == 0)
- cifs_dbg(FYI, "dummy signature received for smb command 0x%x\n",
- shdr->Command);
-
/*
* Save off the origiginal signature so we can modify the smb and check
* our calculated signature against what the server sent.
*/
memcpy(server_response_sig, shdr->Signature, SMB2_SIGNATURE_SIZE);
- memset(shdr->Signature, 0, SMB2_SIGNATURE_SIZE);
-
- rc = server->ops->calc_signature(rqst, server, true);
+ rc = server->secmech.calc_signature(rqst, server, true);
if (rc)
return rc;
@@ -693,6 +865,8 @@ smb2_setup_request(struct cifs_ses *ses, struct TCP_Server_Info *server,
int rc;
struct smb2_hdr *shdr =
(struct smb2_hdr *)rqst->rq_iov[0].iov_base;
+ struct smb2_transform_hdr *trhdr =
+ (struct smb2_transform_hdr *)rqst->rq_iov[0].iov_base;
struct mid_q_entry *mid;
smb2_seq_num_into_buf(server, shdr);
@@ -703,11 +877,22 @@ smb2_setup_request(struct cifs_ses *ses, struct TCP_Server_Info *server,
return ERR_PTR(rc);
}
- rc = smb2_sign_rqst(rqst, server);
- if (rc) {
- revert_current_mid_from_hdr(server, shdr);
- delete_mid(mid);
- return ERR_PTR(rc);
+ /*
+ * Client must not sign the request if it's encrypted.
+ *
+ * Note: we can't rely on SMB2_SESSION_FLAG_ENCRYPT_DATA or SMB2_GLOBAL_CAP_ENCRYPTION
+ * here because they might be set, but not being actively used (e.g. not mounted with
+ * "seal"), so just check if header is a transform header.
+ *
+ * MS-SMB2 3.2.4.1.1
+ */
+ if (trhdr->ProtocolId != SMB2_TRANSFORM_PROTO_NUM) {
+ rc = smb2_sign_rqst(rqst, server);
+ if (rc) {
+ revert_current_mid_from_hdr(server, shdr);
+ delete_mid(mid);
+ return ERR_PTR(rc);
+ }
}
return mid;
@@ -748,39 +933,29 @@ smb2_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
}
int
-smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
+smb3_crypto_aead_allocate(const char *name, struct crypto_aead **tfm)
{
- struct crypto_aead *tfm;
+ if (unlikely(!tfm))
+ return -EIO;
- if (!server->secmech.enc) {
- 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 alloc encrypt aead\n",
- __func__);
- return PTR_ERR(tfm);
- }
- server->secmech.enc = tfm;
- }
+ if (*tfm)
+ return 0;
- if (!server->secmech.dec) {
- 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)) {
- crypto_free_aead(server->secmech.enc);
- server->secmech.enc = NULL;
- cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
- __func__);
- return PTR_ERR(tfm);
- }
- server->secmech.dec = tfm;
+ *tfm = crypto_alloc_aead(name, CRYPTO_ALG_TYPE_AEAD, 0);
+ if (IS_ERR(*tfm)) {
+ cifs_dbg(VFS, "%s: Failed to alloc %s crypto TFM, rc=%ld\n",
+ __func__, name, PTR_ERR(*tfm));
+ return PTR_ERR(*tfm);
}
return 0;
}
+
+void smb3_crypto_aead_free(struct crypto_aead **tfm)
+{
+ if (!tfm || !*tfm)
+ return;
+
+ crypto_free_aead(*tfm);
+ *tfm = NULL;
+}
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1
2022-09-29 1:56 ` [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1 Enzo Matsumiya
@ 2022-09-29 5:14 ` Stefan Metzmacher
2022-09-29 14:16 ` Enzo Matsumiya
2022-09-29 5:22 ` Steve French
1 sibling, 1 reply; 18+ messages in thread
From: Stefan Metzmacher @ 2022-09-29 5:14 UTC (permalink / raw)
To: Enzo Matsumiya, linux-cifs
Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom
Hi Enzo,
> +/*
> + * This function implements AES-GMAC signing for SMB2 messages as described in MS-SMB2
> + * specification. This algorithm is only supported on SMB 3.1.1.
> + *
> + * Note: even though Microsoft mentions RFC4543 in MS-SMB2, the mechanism used_must_ be the "raw"
> + * AES-128-GCM ("gcm(aes)"); RFC4543 is designed for IPsec and trying to use "rfc4543(gcm(aes)))"
> + * will fail the signature computation.
> + *
> + * MS-SMB2 3.1.4.1
> + */
> +int
> +smb311_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify)
> +{
Can you please add aes_gmac to the function name?
> + union {
> + struct {
> + /* for MessageId (8 bytes) */
> + __le64 mid;
> + /* for role (client or server) and if SMB2 CANCEL (4 bytes) */
> + __le32 role;
> + };
> + u8 buffer[12];
> + } __packed nonce;
Can you use SMB3_AES_GCM_NONCE instead of '12'?
metze
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1
2022-09-29 5:14 ` Stefan Metzmacher
@ 2022-09-29 14:16 ` Enzo Matsumiya
0 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 14:16 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: linux-cifs, smfrench, pc, ronniesahlberg, nspmangalore, tom
Hi metze,
On 09/29, Stefan Metzmacher wrote:
>
>Hi Enzo,
>
>>+/*
>>+ * This function implements AES-GMAC signing for SMB2 messages as described in MS-SMB2
>>+ * specification. This algorithm is only supported on SMB 3.1.1.
>>+ *
>>+ * Note: even though Microsoft mentions RFC4543 in MS-SMB2, the mechanism used_must_ be the "raw"
>>+ * AES-128-GCM ("gcm(aes)"); RFC4543 is designed for IPsec and trying to use "rfc4543(gcm(aes)))"
>>+ * will fail the signature computation.
>>+ *
>>+ * MS-SMB2 3.1.4.1
>>+ */
>>+int
>>+smb311_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify)
>>+{
>
>Can you please add aes_gmac to the function name?
Sure. Should I also change smb2_calc_signature to smb2_calc_shash or
something similar, since it fits now for SMB[2.x,3.0.x]?
>>+ union {
>>+ struct {
>>+ /* for MessageId (8 bytes) */
>>+ __le64 mid;
>>+ /* for role (client or server) and if SMB2 CANCEL (4 bytes) */
>>+ __le32 role;
>>+ };
>>+ u8 buffer[12];
>>+ } __packed nonce;
>
>Can you use SMB3_AES_GCM_NONCE instead of '12'?
I was going to submit a follow up series replacing the defines we use
with the crypto ones to clarify meanings, e.g. SMB3_AES_GCM_NONCE made
me wonder at first sight if it was different from GCM_AES_IV_SIZE.
But sure I can change it for the time being.
>metze
Cheers,
Enzo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1
2022-09-29 1:56 ` [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1 Enzo Matsumiya
2022-09-29 5:14 ` Stefan Metzmacher
@ 2022-09-29 5:22 ` Steve French
1 sibling, 0 replies; 18+ messages in thread
From: Steve French @ 2022-09-29 5:22 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: linux-cifs, pc, ronniesahlberg, nspmangalore, tom, metze
could you fix the minor checkpatch nit?
WARNING: Block comments use a trailing */ on a separate line
#220: FILE: fs/cifs/smb2ops.c:4317:
+ * encrypted blob */
On Wed, Sep 28, 2022 at 8:57 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Implement support for AES-GMAC message signing, as specified in
> MS-SMB2 3.1.4.1 "Signing An Outgoing Message".
>
> The core function is smb311_calc_signature(), which will be used as the
> ->calc_signature op when SIGNING_ALG_AES_GMAC has been negotiated with
> the server.
>
> If "enable_negotiate_signing" is false (default) or if
> SIGNING_ALG_AES_GMAC was not negotiated, use AES-CMAC.
>
> Changes:
> - convert cifs_secmech sign/verify to unions, where .aead is for AES-GMAC
> TFM and .shash for all the others signing algorithms
> - init_sg(): rename to smb3_init_sg(), make it non-static, remove skip
> variable. This makes it fit for both crypt_message() and
> smb311_calc_signature()
> - crypt_message(): advance the 20 bytes of rqst[0].rq_iov[0] that are
> not part of the encrypted blob before calling smb3_init_sg(). Rewind
> it back right after the call. (this was done in init_sg() via the
> skip variable)
>
> Other smaller modifications:
> - smb2_setup_request(): check if the request has a transform header as we
> must not sign an encrypted message (MS-SMB2 3.2.4.1.1)
> - smb2_verify_signature():
> * check if MessageId is 0xFFFFFFFFFFFFFFFF or if status is STATUS_PENDING
> (MS-SMB2 3.2.5.1.3)
> * remove extra call to zero the header signature as this is already
> done by all ->calc_signature implementations
> * remove useless call to check for "BSRSPYL" signature (SMB1 only),
> and from smb2_sign_rqst() as well
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/cifsencrypt.c | 23 ++-
> fs/cifs/cifsglob.h | 40 +++--
> fs/cifs/smb2ops.c | 36 ++---
> fs/cifs/smb2pdu.c | 82 +++++-----
> fs/cifs/smb2proto.h | 9 +-
> fs/cifs/smb2transport.c | 331 ++++++++++++++++++++++++++++++----------
> 6 files changed, 354 insertions(+), 167 deletions(-)
>
> diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c
> index 4ae58ab29458..3a78efc45a23 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -23,6 +23,7 @@
> #include <linux/fips.h>
> #include "../smbfs_common/arc4.h"
> #include <crypto/aead.h>
> +#include "smb2proto.h"
>
> int __cifs_calc_signature(struct smb_rqst *rqst,
> struct TCP_Server_Info *server, char *signature,
> @@ -105,9 +106,9 @@ static int cifs_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *se
> return -EINVAL;
>
> if (verify)
> - shash = server->secmech.verify;
> + shash = server->secmech.verify.shash;
> else
> - shash = server->secmech.sign;
> + shash = server->secmech.sign.shash;
>
> rc = crypto_shash_init(shash);
> if (rc) {
> @@ -703,16 +704,14 @@ calc_seckey(struct cifs_ses *ses)
> void
> cifs_crypto_secmech_release(struct TCP_Server_Info *server)
> {
> - cifs_free_hash(&server->secmech.sign);
> - cifs_free_hash(&server->secmech.verify);
> -
> - if (server->secmech.enc) {
> - crypto_free_aead(server->secmech.enc);
> - server->secmech.enc = NULL;
> + if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
> + smb3_crypto_aead_free(&server->secmech.sign.aead);
> + smb3_crypto_aead_free(&server->secmech.verify.aead);
> + } else {
> + cifs_free_hash(&server->secmech.sign.shash);
> + cifs_free_hash(&server->secmech.verify.shash);
> }
>
> - if (server->secmech.dec) {
> - crypto_free_aead(server->secmech.dec);
> - server->secmech.dec = NULL;
> - }
> + smb3_crypto_aead_free(&server->secmech.enc);
> + smb3_crypto_aead_free(&server->secmech.dec);
> }
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 30b3fadb4b06..81a8eff06467 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -153,24 +153,48 @@ struct session_key {
> char *response;
> };
>
> +struct smb_rqst;
> +struct TCP_Server_Info;
> /**
> * cifs_secmech - Crypto hashing related structure/fields, not specific to one mechanism
> - * @sign: SHASH descriptor to allocate a hashing TFM for signing requests
> - * @verify: SHASH descriptor to allocate a hashing TFM for verifying requests' signatures
> + * @sign.shash: SHASH descriptor for signing TFM
> + * @sign.aead: AEAD TFM for signing
> + * @sign_wait: Completion struct for signing operations
> + * @verify.shash: SHASH descriptor for verifying TFM
> + * @verify.aead: AEAD TFM for verifying
> + * @verify_wait: Completion struct for verifying operations
> + * @calc_signature: Signature calculation function to be used.
> * @enc: AEAD TFM for SMB3+ encryption
> * @dec: AEAD TFM for SMB3+ decryption
> *
> - * @sign and @verify are allocated per-server, and the negotiated connection dialect will dictate
> - * which algorithm to use:
> + * @sign and @verify TFMs are allocated per-server, and the negotiated dialect will dictate which
> + * algorithm to use:
> * - MD5 for SMB1
> * - HMAC-SHA256 for SMB2
> * - AES-CMAC for SMB3
> + * - AES-GMAC for SMB3.1.1
> + *
> + * The completion structs @sign_wait and @verify_wait are required so that we can serialize access
> + * to the AEAD TFMs, since they're asynchronous by design. Using a stack-allocated structure could
> + * cause concurrent access to the TFMs to overwrite the completion status of a previous operation.
> *
> - * @enc and @dec holds the encryption/decryption TFMs, where it'll be either AES-CCM or AES-GCM.
> + * @enc and @dec holds the encryption/decryption TFMs, also allocated per server, where each will
> + * be either AES-CCM or AES-GCM.
> */
> struct cifs_secmech {
> - struct shash_desc *sign;
> - struct shash_desc *verify;
> + union {
> + struct shash_desc *shash;
> + struct crypto_aead *aead;
> + } sign;
> + struct crypto_wait sign_wait;
> +
> + union {
> + struct shash_desc *shash;
> + struct crypto_aead *aead;
> + } verify;
> + struct crypto_wait verify_wait;
> +
> + int (*calc_signature)(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify);
>
> struct crypto_aead *enc;
> struct crypto_aead *dec;
> @@ -445,8 +469,6 @@ struct smb_version_operations {
> void (*new_lease_key)(struct cifs_fid *);
> int (*generate_signingkey)(struct cifs_ses *ses,
> struct TCP_Server_Info *server);
> - int (*calc_signature)(struct smb_rqst *, struct TCP_Server_Info *,
> - bool allocate_crypto);
> int (*set_integrity)(const unsigned int, struct cifs_tcon *tcon,
> struct cifsFileInfo *src_file);
> int (*enum_snapshots)(const unsigned int xid, struct cifs_tcon *tcon,
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 0aaad18e1ec8..22b40d181bba 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -4234,21 +4234,14 @@ static inline void smb2_sg_set_buf(struct scatterlist *sg, const void *buf,
> sg_set_page(sg, addr, buflen, offset_in_page(buf));
> }
>
> -/* Assumes the first rqst has a transform header as the first iov.
> - * I.e.
> - * rqst[0].rq_iov[0] is transform header
> - * rqst[0].rq_iov[1+] data to be encrypted/decrypted
> - * rqst[1+].rq_iov[0+] data to be encrypted/decrypted
> - */
> -static struct scatterlist *
> -init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
> +struct scatterlist *
> +smb3_init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
> {
> unsigned int sg_len;
> struct scatterlist *sg;
> unsigned int i;
> unsigned int j;
> unsigned int idx = 0;
> - int skip;
>
> sg_len = 1;
> for (i = 0; i < num_rqst; i++)
> @@ -4261,15 +4254,10 @@ init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign)
> sg_init_table(sg, sg_len);
> for (i = 0; i < num_rqst; i++) {
> for (j = 0; j < rqst[i].rq_nvec; j++) {
> - /*
> - * The first rqst has a transform header where the
> - * first 20 bytes are not part of the encrypted blob
> - */
> - skip = (i == 0) && (j == 0) ? 20 : 0;
> smb2_sg_set_buf(&sg[idx++],
> - rqst[i].rq_iov[j].iov_base + skip,
> - rqst[i].rq_iov[j].iov_len - skip);
> - }
> + rqst[i].rq_iov[j].iov_base,
> + rqst[i].rq_iov[j].iov_len);
> + }
>
> for (j = 0; j < rqst[i].rq_npages; j++) {
> unsigned int len, offset;
> @@ -4325,7 +4313,15 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
> crypt_len += SMB2_SIGNATURE_SIZE;
> }
>
> - sg = init_sg(num_rqst, rqst, sign);
> + /* Skip the first 20 bytes of the first iov of the first request as they're not part of the
> + * encrypted blob */
> + rqst[0].rq_iov[0].iov_base += 20;
> + rqst[0].rq_iov[0].iov_len -= 20;
> + sg = smb3_init_sg(num_rqst, rqst, sign);
> + /* Rewind those 20 bytes before going any further */
> + rqst[0].rq_iov[0].iov_base -= 20;
> + rqst[0].rq_iov[0].iov_len += 20;
> +
> if (!sg) {
> cifs_server_dbg(VFS, "%s: Failed to init sg\n", __func__);
> rc = -ENOMEM;
> @@ -5213,7 +5209,6 @@ struct smb_version_operations smb20_operations = {
> .get_lease_key = smb2_get_lease_key,
> .set_lease_key = smb2_set_lease_key,
> .new_lease_key = smb2_new_lease_key,
> - .calc_signature = smb2_calc_signature,
> .is_read_op = smb2_is_read_op,
> .set_oplock_level = smb2_set_oplock_level,
> .create_lease_buf = smb2_create_lease_buf,
> @@ -5314,7 +5309,6 @@ struct smb_version_operations smb21_operations = {
> .get_lease_key = smb2_get_lease_key,
> .set_lease_key = smb2_set_lease_key,
> .new_lease_key = smb2_new_lease_key,
> - .calc_signature = smb2_calc_signature,
> .is_read_op = smb21_is_read_op,
> .set_oplock_level = smb21_set_oplock_level,
> .create_lease_buf = smb2_create_lease_buf,
> @@ -5421,7 +5415,6 @@ struct smb_version_operations smb30_operations = {
> .set_lease_key = smb2_set_lease_key,
> .new_lease_key = smb2_new_lease_key,
> .generate_signingkey = generate_smb30signingkey,
> - .calc_signature = smb2_calc_signature,
> .set_integrity = smb3_set_integrity,
> .is_read_op = smb21_is_read_op,
> .set_oplock_level = smb3_set_oplock_level,
> @@ -5535,7 +5528,6 @@ struct smb_version_operations smb311_operations = {
> .set_lease_key = smb2_set_lease_key,
> .new_lease_key = smb2_new_lease_key,
> .generate_signingkey = generate_smb311signingkey,
> - .calc_signature = smb2_calc_signature,
> .set_integrity = smb3_set_integrity,
> .is_read_op = smb21_is_read_op,
> .set_oplock_level = smb3_set_oplock_level,
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index e5939c374c35..2c2bf28382bc 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -460,7 +460,7 @@ static unsigned int
> build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
> {
> unsigned int ctxt_len = sizeof(struct smb2_signing_capabilities);
> - unsigned short num_algs = 1; /* number of signing algorithms sent */
> + unsigned short num_algs = 2; /* number of signing algorithms sent */
>
> pneg_ctxt->ContextType = SMB2_SIGNING_CAPABILITIES;
> /*
> @@ -471,12 +471,18 @@ build_signing_ctxt(struct smb2_signing_capabilities *pneg_ctxt)
> sizeof(struct smb2_neg_context) +
> (num_algs * 2 /* sizeof u16 */), 8) * 8);
> pneg_ctxt->SigningAlgorithmCount = cpu_to_le16(num_algs);
> - pneg_ctxt->SigningAlgorithms[0] = cpu_to_le16(SIGNING_ALG_AES_CMAC);
> +
> + /*
> + * Set AES-GMAC as preferred, but will fall back to AES-CMAC if server doesn't support it.
> + * MS-SMB2 2.2.3.1.7
> + */
> + pneg_ctxt->SigningAlgorithms[0] = SIGNING_ALG_AES_GMAC_LE;
> + pneg_ctxt->SigningAlgorithms[1] = SIGNING_ALG_AES_CMAC_LE;
> + /* SMB 3.1.1 doesn't accept HMAC-SHA256, so no need to send it */
>
> ctxt_len += 2 /* sizeof le16 */ * num_algs;
> 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
> @@ -927,22 +933,6 @@ SMB2_negotiate(const unsigned int xid,
> else
> req->SecurityMode = 0;
>
> - if (req->SecurityMode) {
> - /*
> - * Allocate HMAC-SHA256 regardless of dialect requested, change to AES-CMAC later,
> - * if SMB3+ is negotiated
> - */
> - rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.sign);
> - if (rc)
> - goto neg_exit;
> -
> - rc = cifs_alloc_hash("hmac(sha256)", &server->secmech.verify);
> - if (rc) {
> - cifs_free_hash(&server->secmech.sign);
> - goto neg_exit;
> - }
> - }
> -
> req->Capabilities = cpu_to_le32(server->vals->req_capabilities);
> if (ses->chan_max > 1)
> req->Capabilities |= cpu_to_le32(SMB2_GLOBAL_CAP_MULTI_CHANNEL);
> @@ -1087,22 +1077,6 @@ SMB2_negotiate(const unsigned int xid,
> rc = cifs_enable_signing(server, ses->sign);
> if (rc)
> goto neg_exit;
> -
> - if (server->sign && server->dialect >= SMB30_PROT_ID) {
> - /* free HMAC-SHA256 allocated earlier for negprot */
> - cifs_free_hash(&server->secmech.sign);
> - cifs_free_hash(&server->secmech.verify);
> - rc = cifs_alloc_hash("cmac(aes)", &server->secmech.sign);
> - if (rc)
> - goto neg_exit;
> -
> - rc = cifs_alloc_hash("cmac(aes)", &server->secmech.verify);
> - if (rc) {
> - cifs_free_hash(&server->secmech.sign);
> - goto neg_exit;
> - }
> - }
> -
> if (blob_length) {
> rc = decode_negTokenInit(security_blob, blob_length, server);
> if (rc == 1)
> @@ -1112,12 +1086,35 @@ SMB2_negotiate(const unsigned int xid,
> }
>
> if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
> + server->signing_algorithm = SIGNING_ALG_AES_CMAC;
> + server->signing_negotiated = false;
> +
> if (rsp->NegotiateContextCount)
> rc = smb311_decode_neg_context(rsp, server,
> rsp_iov.iov_len);
> else
> cifs_server_dbg(VFS, "Missing expected negotiate contexts\n");
> +
> + /*
> + * Some servers will not send a SMB2_SIGNING_CAPABILITIES context response (*),
> + * so use AES-CMAC signing algorithm as it is expected to be accepted.
> + * See MS-SMB2 note <125> Section 3.2.4.2.2.2
> + */
> + if (!server->signing_negotiated)
> + cifs_dbg(VFS, "signing capabilities were not negotiated, using "
> + "AES-CMAC for message signing\n");
> + } else if (server->dialect >= SMB30_PROT_ID) {
> + server->signing_algorithm = SIGNING_ALG_AES_CMAC;
> + } else if (server->dialect >= SMB20_PROT_ID) {
> + server->signing_algorithm = SIGNING_ALG_HMAC_SHA256;
> }
> +
> + rc = smb2_init_secmechs(server);
> + if (rc) {
> + cifs_dbg(VFS, "Failed to initialize secmechs, rc=%d\n", rc);
> + goto neg_exit;
> + }
> +
> neg_exit:
> free_rsp_buf(resp_buftype, rsp);
> return rc;
> @@ -1677,18 +1674,13 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
> goto out;
>
> if (ses->server->dialect < SMB30_PROT_ID) {
> - rc = crypto_shash_setkey(server->secmech.sign->tfm,
> - ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
> - if (rc) {
> - cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 signing key, rc=%d\n",
> - __func__, rc);
> - goto out;
> - }
> -
> - rc = crypto_shash_setkey(server->secmech.verify->tfm,
> + rc = crypto_shash_setkey(server->secmech.sign.shash->tfm,
> ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
> + if (!rc)
> + rc = crypto_shash_setkey(server->secmech.verify.shash->tfm,
> + ses->auth_key.response, SMB2_NTLMV2_SESSKEY_SIZE);
> if (rc) {
> - cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 verify key, rc=%d\n",
> + cifs_dbg(VFS, "%s: Failed to set HMAC-SHA256 signing keys, rc=%d\n",
> __func__, rc);
> goto out;
> }
> diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
> index 33af35b6e586..e4db59e6cee6 100644
> --- a/fs/cifs/smb2proto.h
> +++ b/fs/cifs/smb2proto.h
> @@ -14,6 +14,7 @@
>
> struct statfs;
> struct smb_rqst;
> +struct crypto_aead;
>
> /*
> *****************************************************************
> @@ -37,6 +38,7 @@ extern struct mid_q_entry *smb2_setup_request(struct cifs_ses *ses,
> struct smb_rqst *rqst);
> extern struct mid_q_entry *smb2_setup_async_request(
> struct TCP_Server_Info *server, struct smb_rqst *rqst);
> +extern int smb2_init_secmechs(struct TCP_Server_Info *server);
> extern struct cifs_ses *smb2_find_smb_ses(struct TCP_Server_Info *server,
> __u64 ses_id);
> extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
> @@ -44,6 +46,10 @@ extern struct cifs_tcon *smb2_find_smb_tcon(struct TCP_Server_Info *server,
> extern int smb2_calc_signature(struct smb_rqst *rqst,
> struct TCP_Server_Info *server,
> bool verify);
> +extern int smb311_calc_signature(struct smb_rqst *rqst,
> + struct TCP_Server_Info *server,
> + bool verify);
> +extern struct scatterlist *smb3_init_sg(int num_rqst, struct smb_rqst *rqst, u8 *sign);
> extern void smb2_echo_request(struct work_struct *work);
> extern __le32 smb2_get_lease_state(struct cifsInodeInfo *cinode);
> extern bool smb2_is_valid_oplock_break(char *buffer,
> @@ -99,7 +105,8 @@ extern int smb2_unlock_range(struct cifsFileInfo *cfile,
> struct file_lock *flock, const unsigned int xid);
> extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
> extern void smb2_reconnect_server(struct work_struct *work);
> -extern int smb3_crypto_aead_allocate(struct TCP_Server_Info *server);
> +extern int smb3_crypto_aead_allocate(const char *name, struct crypto_aead **tfm);
> +extern void smb3_crypto_aead_free(struct crypto_aead **tfm);
> extern unsigned long smb_rqst_len(struct TCP_Server_Info *server,
> struct smb_rqst *rqst);
> extern void smb2_set_next_command(struct cifs_tcon *tcon,
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index cf319fc25161..ded7144d1578 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -26,6 +26,63 @@
> #include "smb2status.h"
> #include "smb2glob.h"
>
> +int
> +smb2_init_secmechs(struct TCP_Server_Info *server)
> +{
> + int rc = 0;
> +
> + if (server->dialect >= SMB30_PROT_ID &&
> + (server->capabilities & SMB2_GLOBAL_CAP_ENCRYPTION)) {
> + if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM ||
> + server->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
> + rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.enc);
> + if (!rc)
> + rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.dec);
> + } else {
> + rc = smb3_crypto_aead_allocate("ccm(aes)", &server->secmech.enc);
> + if (!rc)
> + rc = smb3_crypto_aead_allocate("ccm(aes)", &server->secmech.dec);
> + }
> +
> + if (rc)
> + return rc;
> + }
> +
> + if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
> + cifs_free_hash(&server->secmech.sign.shash);
> + cifs_free_hash(&server->secmech.verify.shash);
> +
> + rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.sign.aead);
> + if (!rc)
> + rc = smb3_crypto_aead_allocate("gcm(aes)", &server->secmech.verify.aead);
> + if (rc) {
> + smb3_crypto_aead_free(&server->secmech.sign.aead);
> + return rc;
> + }
> +
> + server->secmech.calc_signature = smb311_calc_signature;
> + } else {
> + char *shash_alg;
> +
> + if (server->dialect >= SMB30_PROT_ID)
> + shash_alg = "cmac(aes)";
> + else
> + shash_alg = "hmac(sha256)";
> +
> + rc = cifs_alloc_hash(shash_alg, &server->secmech.sign.shash);
> + if (!rc)
> + rc = cifs_alloc_hash(shash_alg, &server->secmech.verify.shash);
> + if (rc) {
> + cifs_free_hash(&server->secmech.sign.shash);
> + return rc;
> + }
> +
> + server->secmech.calc_signature = smb2_calc_signature;
> + }
> +
> + return rc;
> +}
> +
> static int
> smb3_setup_keys(struct TCP_Server_Info *server, u8 *sign_key, u8 *enc_key, u8 *dec_key)
> {
> @@ -42,44 +99,51 @@ smb3_setup_keys(struct TCP_Server_Info *server, u8 *sign_key, u8 *enc_key, u8 *d
> crypt_keylen = SMB3_GCM128_CRYPTKEY_SIZE;
>
> rc = crypto_aead_setkey(server->secmech.enc, enc_key, crypt_keylen);
> + if (!rc)
> + rc = crypto_aead_setkey(server->secmech.dec, dec_key, crypt_keylen);
> if (rc) {
> - cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption key, rc=%d\n",
> + cifs_server_dbg(VFS, "%s: Failed to set encryption/decryption key, rc=%d\n",
> __func__, rc);
> return rc;
> }
>
> rc = crypto_aead_setauthsize(server->secmech.enc, SMB2_SIGNATURE_SIZE);
> + if (!rc)
> + rc = crypto_aead_setauthsize(server->secmech.dec, SMB2_SIGNATURE_SIZE);
> if (rc) {
> - cifs_server_dbg(VFS, "%s: Failed to set AEAD encryption authsize, rc=%d\n",
> - __func__, rc);
> - return rc;
> - }
> -
> - rc = crypto_aead_setkey(server->secmech.dec, dec_key, crypt_keylen);
> - if (rc) {
> - cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption key, rc=%d\n",
> - __func__, rc);
> - return rc;
> - }
> -
> - rc = crypto_aead_setauthsize(server->secmech.dec, SMB2_SIGNATURE_SIZE);
> - if (rc) {
> - cifs_server_dbg(VFS, "%s: Failed to set AEAD decryption authsize, rc=%d\n",
> + cifs_server_dbg(VFS, "%s: Failed to set encryption/decryption authsize, rc=%d\n",
> __func__, rc);
> return rc;
> }
> setup_sign:
> - rc = crypto_shash_setkey(server->secmech.sign->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
> - if (rc) {
> - cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC signing key, rc=%d\n",
> - __func__, rc);
> - return rc;
> - }
> + if (server->signing_algorithm == SIGNING_ALG_AES_GMAC) {
> + rc = crypto_aead_setkey(server->secmech.sign.aead, sign_key, SMB3_SIGN_KEY_SIZE);
> + if (!rc)
> + rc = crypto_aead_setkey(server->secmech.verify.aead, sign_key,
> + SMB3_SIGN_KEY_SIZE);
> + if (rc) {
> + cifs_server_dbg(VFS, "%s: Failed to set AES-GMAC key, rc=%d\n",
> + __func__, rc);
> + return rc;
> + }
>
> - rc = crypto_shash_setkey(server->secmech.verify->tfm, sign_key, SMB3_SIGN_KEY_SIZE);
> - if (rc)
> - cifs_server_dbg(VFS, "%s: Failed to set AES-CMAC verify key, rc=%d\n",
> - __func__, rc);
> + rc = crypto_aead_setauthsize(server->secmech.sign.aead, SMB2_SIGNATURE_SIZE);
> + if (!rc)
> + rc = crypto_aead_setauthsize(server->secmech.verify.aead,
> + SMB2_SIGNATURE_SIZE);
> + if (rc)
> + cifs_server_dbg(VFS, "%s: Failed to set AES-GMAC authsize, rc=%d\n",
> + __func__, rc);
> + } else {
> + rc = crypto_shash_setkey(server->secmech.sign.shash->tfm, sign_key,
> + SMB3_SIGN_KEY_SIZE);
> + if (!rc)
> + rc = crypto_shash_setkey(server->secmech.verify.shash->tfm, sign_key,
> + SMB3_SIGN_KEY_SIZE);
> + if (rc)
> + cifs_server_dbg(VFS, "%s: Failed to set %s signing key, rc=%d\n", __func__,
> + crypto_shash_alg_name(server->secmech.sign.shash->tfm), rc);
> + }
>
> return rc;
> }
> @@ -171,9 +235,9 @@ smb2_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool
> memset(shdr->Signature, 0x0, SMB2_SIGNATURE_SIZE);
>
> if (verify)
> - shash = server->secmech.verify;
> + shash = server->secmech.verify.shash;
> else
> - shash = server->secmech.sign;
> + shash = server->secmech.sign.shash;
>
> rc = crypto_shash_init(shash);
> if (rc) {
> @@ -360,10 +424,6 @@ generate_smb3signingkey(struct cifs_ses *ses,
> if (rc)
> goto out_zero_keys;
>
> - rc = smb3_crypto_aead_allocate(server);
> - if (rc)
> - goto out_zero_keys;
> -
> rc = smb3_setup_keys(ses->server, sign_key, enc_key, dec_key);
> if (rc)
> goto out_zero_keys;
> @@ -471,6 +531,125 @@ generate_smb311signingkey(struct cifs_ses *ses,
> return generate_smb3signingkey(ses, server, &triplet);
> }
>
> +/*
> + * This function implements AES-GMAC signing for SMB2 messages as described in MS-SMB2
> + * specification. This algorithm is only supported on SMB 3.1.1.
> + *
> + * Note: even though Microsoft mentions RFC4543 in MS-SMB2, the mechanism used _must_ be the "raw"
> + * AES-128-GCM ("gcm(aes)"); RFC4543 is designed for IPsec and trying to use "rfc4543(gcm(aes)))"
> + * will fail the signature computation.
> + *
> + * MS-SMB2 3.1.4.1
> + */
> +int
> +smb311_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server, bool verify)
> +{
> + union {
> + struct {
> + /* for MessageId (8 bytes) */
> + __le64 mid;
> + /* for role (client or server) and if SMB2 CANCEL (4 bytes) */
> + __le32 role;
> + };
> + u8 buffer[12];
> + } __packed nonce;
> + u8 sig[SMB2_SIGNATURE_SIZE] = { 0 };
> + struct aead_request *aead_req = NULL;
> + struct crypto_aead *tfm = NULL;
> + struct scatterlist *sg = NULL;
> + unsigned long assoclen;
> + struct smb2_hdr *shdr = NULL;
> + struct crypto_wait *wait;
> + unsigned int save_npages = 0;
> + int rc = 0;
> +
> + if (verify) {
> + wait = &server->secmech.verify_wait;
> + tfm = server->secmech.verify.aead;
> + } else {
> + wait = &server->secmech.sign_wait;
> + tfm = server->secmech.sign.aead;
> + }
> +
> + if (completion_done(&wait->completion))
> + reinit_completion(&wait->completion);
> +
> + shdr = (struct smb2_hdr *)rqst->rq_iov[0].iov_base;
> +
> + memset(shdr->Signature, 0, SMB2_SIGNATURE_SIZE);
> + memset(&nonce, 0, SMB3_AES_GCM_NONCE);
> +
> + /* note that nonce must always be little endian */
> + nonce.mid = shdr->MessageId;
> + /* request is coming from the server, set LSB */
> + nonce.role |= shdr->Flags & SMB2_FLAGS_SERVER_TO_REDIR;
> + /* set penultimate LSB if SMB2_CANCEL command */
> + if (shdr->Command == SMB2_CANCEL)
> + nonce.role |= cpu_to_le32(1UL << 1);
> +
> + aead_req = aead_request_alloc(tfm, GFP_KERNEL);
> + if (!aead_req) {
> + cifs_dbg(VFS, "%s: Failed to alloc AEAD request\n", __func__);
> + return -ENOMEM;
> + }
> +
> + /* skip page data if non-success error status, as it will compute an invalid signature */
> + if (shdr->Status != 0 && rqst->rq_npages > 0) {
> + save_npages = rqst->rq_npages;
> + rqst->rq_npages = 0;
> + }
> +
> + assoclen = smb_rqst_len(server, rqst);
> +
> + sg = smb3_init_sg(1, rqst, sig);
> + if (!sg) {
> + cifs_dbg(VFS, "%s: Failed to init SG\n", __func__);
> + goto out_free_req;
> + }
> +
> + /* cryptlen == 0 because we're not encrypting anything */
> + aead_request_set_crypt(aead_req, sg, sg, 0, nonce.buffer);
> + aead_request_set_ad(aead_req, assoclen);
> + aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG, crypto_req_done, wait);
> +
> + /*
> + * Reminder: we must always use the encrypt function, as AES-GCM decrypt will internally
> + * try to match the authentication codes, where we pass a zeroed buffer, and the operation
> + * will fail with -EBADMSG (expectedly).
> + *
> + * Also note we can't use crypto_wait_req() here since it's not interruptible.
> + */
> + rc = crypto_aead_encrypt(aead_req);
> + if (!rc)
> + goto out;
> +
> + if (rc == -EINPROGRESS || rc == -EBUSY) {
> + rc = wait_for_completion_interruptible(&wait->completion);
> + if (!rc)
> + /* wait->err is set by crypto_req_done callback above */
> + rc = wait->err;
> + }
> +
> + if (rc) {
> + cifs_server_dbg(VFS, "%s: Failed to compute AES-GMAC signature, rc=%d\n",
> + __func__, rc);
> + goto out_free_sg;
> + }
> +
> +out:
> + memcpy(&shdr->Signature, sig, SMB2_SIGNATURE_SIZE);
> +out_free_sg:
> + kfree(sg);
> +out_free_req:
> + kfree(aead_req);
> +
> + /* restore rq_npages for further processing */
> + if (shdr->Status != 0 && save_npages > 0)
> + rqst->rq_npages = save_npages;
> +
> + return rc;
> +}
> +
> /* must be called with server->srv_mutex held */
> static int
> smb2_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server)
> @@ -497,12 +676,10 @@ smb2_sign_rqst(struct smb_rqst *rqst, struct TCP_Server_Info *server)
> return 0;
> }
> spin_unlock(&server->srv_lock);
> - if (!is_binding && !server->session_estab) {
> - strncpy(shdr->Signature, "BSRSPYL", 8);
> + if (!is_binding && !server->session_estab)
> return 0;
> - }
>
> - rc = server->ops->calc_signature(rqst, server, false);
> + rc = server->secmech.calc_signature(rqst, server, false);
>
> return rc;
> }
> @@ -518,6 +695,8 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
> if ((shdr->Command == SMB2_NEGOTIATE) ||
> (shdr->Command == SMB2_SESSION_SETUP) ||
> (shdr->Command == SMB2_OPLOCK_BREAK) ||
> + (shdr->MessageId == cpu_to_le64(0xFFFFFFFFFFFFFFFF)) ||
> + (shdr->Status == STATUS_PENDING) ||
> server->ignore_signature ||
> (!server->session_estab))
> return 0;
> @@ -527,20 +706,13 @@ smb2_verify_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
> * server does not send one? BB
> */
>
> - /* Do not need to verify session setups with signature "BSRSPYL " */
> - if (memcmp(shdr->Signature, "BSRSPYL ", 8) == 0)
> - cifs_dbg(FYI, "dummy signature received for smb command 0x%x\n",
> - shdr->Command);
> -
> /*
> * Save off the origiginal signature so we can modify the smb and check
> * our calculated signature against what the server sent.
> */
> memcpy(server_response_sig, shdr->Signature, SMB2_SIGNATURE_SIZE);
>
> - memset(shdr->Signature, 0, SMB2_SIGNATURE_SIZE);
> -
> - rc = server->ops->calc_signature(rqst, server, true);
> + rc = server->secmech.calc_signature(rqst, server, true);
>
> if (rc)
> return rc;
> @@ -693,6 +865,8 @@ smb2_setup_request(struct cifs_ses *ses, struct TCP_Server_Info *server,
> int rc;
> struct smb2_hdr *shdr =
> (struct smb2_hdr *)rqst->rq_iov[0].iov_base;
> + struct smb2_transform_hdr *trhdr =
> + (struct smb2_transform_hdr *)rqst->rq_iov[0].iov_base;
> struct mid_q_entry *mid;
>
> smb2_seq_num_into_buf(server, shdr);
> @@ -703,11 +877,22 @@ smb2_setup_request(struct cifs_ses *ses, struct TCP_Server_Info *server,
> return ERR_PTR(rc);
> }
>
> - rc = smb2_sign_rqst(rqst, server);
> - if (rc) {
> - revert_current_mid_from_hdr(server, shdr);
> - delete_mid(mid);
> - return ERR_PTR(rc);
> + /*
> + * Client must not sign the request if it's encrypted.
> + *
> + * Note: we can't rely on SMB2_SESSION_FLAG_ENCRYPT_DATA or SMB2_GLOBAL_CAP_ENCRYPTION
> + * here because they might be set, but not being actively used (e.g. not mounted with
> + * "seal"), so just check if header is a transform header.
> + *
> + * MS-SMB2 3.2.4.1.1
> + */
> + if (trhdr->ProtocolId != SMB2_TRANSFORM_PROTO_NUM) {
> + rc = smb2_sign_rqst(rqst, server);
> + if (rc) {
> + revert_current_mid_from_hdr(server, shdr);
> + delete_mid(mid);
> + return ERR_PTR(rc);
> + }
> }
>
> return mid;
> @@ -748,39 +933,29 @@ smb2_setup_async_request(struct TCP_Server_Info *server, struct smb_rqst *rqst)
> }
>
> int
> -smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
> +smb3_crypto_aead_allocate(const char *name, struct crypto_aead **tfm)
> {
> - struct crypto_aead *tfm;
> + if (unlikely(!tfm))
> + return -EIO;
>
> - if (!server->secmech.enc) {
> - 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 alloc encrypt aead\n",
> - __func__);
> - return PTR_ERR(tfm);
> - }
> - server->secmech.enc = tfm;
> - }
> + if (*tfm)
> + return 0;
>
> - if (!server->secmech.dec) {
> - 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)) {
> - crypto_free_aead(server->secmech.enc);
> - server->secmech.enc = NULL;
> - cifs_server_dbg(VFS, "%s: Failed to alloc decrypt aead\n",
> - __func__);
> - return PTR_ERR(tfm);
> - }
> - server->secmech.dec = tfm;
> + *tfm = crypto_alloc_aead(name, CRYPTO_ALG_TYPE_AEAD, 0);
> + if (IS_ERR(*tfm)) {
> + cifs_dbg(VFS, "%s: Failed to alloc %s crypto TFM, rc=%ld\n",
> + __func__, name, PTR_ERR(*tfm));
> + return PTR_ERR(*tfm);
> }
>
> return 0;
> }
> +
> +void smb3_crypto_aead_free(struct crypto_aead **tfm)
> +{
> + if (!tfm || !*tfm)
> + return;
> +
> + crypto_free_aead(*tfm);
> + *tfm = NULL;
> +}
> --
> 2.35.3
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
` (4 preceding siblings ...)
2022-09-29 1:56 ` [PATCH v3 5/8] cifs: introduce AES-GMAC signing support for SMB 3.1.1 Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 5:22 ` Steve French
2022-09-29 1:56 ` [PATCH v3 7/8] cifs: show signing algorithm name in DebugData Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer Enzo Matsumiya
7 siblings, 1 reply; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
Deprecate enable_negotiate_signing module parameter as it's irrelevant
since the requested dialect and server support will dictate which
algorithm will actually be used.
Send a negotiate signing context on every SMB 3.1.1 negotiation now.
AES-CMAC will still be used instead, iff, SMB 3.0.x negotiated or
SMB 3.1.1 negotiated, but server doesn't support AES-GMAC.
Warn the user if, for whatever reason, the module was loaded with
'enable_negotiate_signing=0'.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifsfs.c | 14 +++++++++++---
fs/cifs/cifsglob.h | 3 ++-
fs/cifs/smb2pdu.c | 11 ++++-------
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 8042d7280dec..c46dc9edf6ec 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -65,7 +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 */
+bool enable_negotiate_signing = true; /* deprecated -- always true now */
unsigned int global_secflags = CIFSSEC_DEF;
/* unsigned int ntlmv2_support = 0; */
unsigned int sign_CIFS_PDUs = 1;
@@ -133,8 +133,12 @@ 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_negotiate_signing, "Enable negotiating packet signing algorithm with server. Default: n/N/0");
+/* XXX: remove this at some point */
+module_param(enable_negotiate_signing, bool, 0);
+MODULE_PARM_DESC(enable_negotiate_signing,
+ "(deprecated) Enable negotiating packet signing algorithm with the server. "
+ "This parameter is ignored as cifs.ko will always try to negotiate the signing "
+ "algorithm on SMB 3.1.1 mounts.");
module_param(disable_legacy_dialects, bool, 0644);
MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be "
@@ -1712,6 +1716,10 @@ init_cifs(void)
goto out_init_cifs_idmap;
}
+ if (!enable_negotiate_signing)
+ pr_warn_once("ignoring deprecated module parameter 'enable_negotiate_signing=0', "
+ "will try to negotiate signing capabilities anyway...\n");
+
return 0;
out_init_cifs_idmap:
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 81a8eff06467..cadde6c451e5 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -2031,7 +2031,8 @@ 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 enable_negotiate_signing; /* request use of faster (GMAC) signing if available
+ XXX: deprecated, remove it at some point */
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 2c2bf28382bc..6c1d58492b18 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -609,13 +609,10 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
neg_context_count++;
}
- if (enable_negotiate_signing) {
- ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)
- pneg_ctxt);
- *total_len += ctxt_len;
- pneg_ctxt += ctxt_len;
- neg_context_count++;
- }
+ 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);
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param
2022-09-29 1:56 ` [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param Enzo Matsumiya
@ 2022-09-29 5:22 ` Steve French
2022-09-29 14:18 ` Enzo Matsumiya
0 siblings, 1 reply; 18+ messages in thread
From: Steve French @ 2022-09-29 5:22 UTC (permalink / raw)
To: Enzo Matsumiya; +Cc: linux-cifs, pc, ronniesahlberg, nspmangalore, tom, metze
could you fix these minor checkpatch warnings?
ERROR: code indent should use tabs where possible
#74: FILE: fs/cifs/cifsglob.h:2035:
+^I^I^I^I XXX: deprecated, remove it at some point */$
WARNING: Block comments use * on subsequent lines
#74: FILE: fs/cifs/cifsglob.h:2035:
+extern bool enable_negotiate_signing; /* request use of faster (GMAC)
signing if available
+ XXX: deprecated, remove it at some point */
WARNING: Block comments use a trailing */ on a separate line
#74: FILE: fs/cifs/cifsglob.h:2035:
+ XXX: deprecated, remove it at some point */
total: 1 errors, 5 warnings, 58 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
NOTE: Whitespace errors detected.
You may wish to use scripts/cleanpatch or scripts/cleanfile
0006-cifs-deprecate-enable_negotiate_signing-module-param.patch has
style problems, please review.
On Wed, Sep 28, 2022 at 8:57 PM Enzo Matsumiya <ematsumiya@suse.de> wrote:
>
> Deprecate enable_negotiate_signing module parameter as it's irrelevant
> since the requested dialect and server support will dictate which
> algorithm will actually be used.
>
> Send a negotiate signing context on every SMB 3.1.1 negotiation now.
> AES-CMAC will still be used instead, iff, SMB 3.0.x negotiated or
> SMB 3.1.1 negotiated, but server doesn't support AES-GMAC.
>
> Warn the user if, for whatever reason, the module was loaded with
> 'enable_negotiate_signing=0'.
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/cifsfs.c | 14 +++++++++++---
> fs/cifs/cifsglob.h | 3 ++-
> fs/cifs/smb2pdu.c | 11 ++++-------
> 3 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 8042d7280dec..c46dc9edf6ec 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -65,7 +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 */
> +bool enable_negotiate_signing = true; /* deprecated -- always true now */
> unsigned int global_secflags = CIFSSEC_DEF;
> /* unsigned int ntlmv2_support = 0; */
> unsigned int sign_CIFS_PDUs = 1;
> @@ -133,8 +133,12 @@ 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_negotiate_signing, "Enable negotiating packet signing algorithm with server. Default: n/N/0");
> +/* XXX: remove this at some point */
> +module_param(enable_negotiate_signing, bool, 0);
> +MODULE_PARM_DESC(enable_negotiate_signing,
> + "(deprecated) Enable negotiating packet signing algorithm with the server. "
> + "This parameter is ignored as cifs.ko will always try to negotiate the signing "
> + "algorithm on SMB 3.1.1 mounts.");
>
> module_param(disable_legacy_dialects, bool, 0644);
> MODULE_PARM_DESC(disable_legacy_dialects, "To improve security it may be "
> @@ -1712,6 +1716,10 @@ init_cifs(void)
> goto out_init_cifs_idmap;
> }
>
> + if (!enable_negotiate_signing)
> + pr_warn_once("ignoring deprecated module parameter 'enable_negotiate_signing=0', "
> + "will try to negotiate signing capabilities anyway...\n");
> +
> return 0;
>
> out_init_cifs_idmap:
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 81a8eff06467..cadde6c451e5 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -2031,7 +2031,8 @@ 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 enable_negotiate_signing; /* request use of faster (GMAC) signing if available
> + XXX: deprecated, remove it at some point */
> 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 2c2bf28382bc..6c1d58492b18 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -609,13 +609,10 @@ assemble_neg_contexts(struct smb2_negotiate_req *req,
> neg_context_count++;
> }
>
> - if (enable_negotiate_signing) {
> - ctxt_len = build_signing_ctxt((struct smb2_signing_capabilities *)
> - pneg_ctxt);
> - *total_len += ctxt_len;
> - pneg_ctxt += ctxt_len;
> - neg_context_count++;
> - }
> + 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);
> --
> 2.35.3
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param
2022-09-29 5:22 ` Steve French
@ 2022-09-29 14:18 ` Enzo Matsumiya
0 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 14:18 UTC (permalink / raw)
To: Steve French; +Cc: linux-cifs, pc, ronniesahlberg, nspmangalore, tom, metze
On 09/29, Steve French wrote:
>could you fix these minor checkpatch warnings?
Ugh, I ran checkpatch but forgot to fix things, sorry about that. Will
resubmit.
Cheers,
Enzo
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3 7/8] cifs: show signing algorithm name in DebugData
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
` (5 preceding siblings ...)
2022-09-29 1:56 ` [PATCH v3 6/8] cifs: deprecate 'enable_negotiate_signing' module param Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 1:56 ` [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer Enzo Matsumiya
7 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
Add smb2_signing_algo_str() helper in smb2glob.h
Show the algorithm name in DebugData, when signing is enabled.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/cifs_debug.c | 7 +++++--
fs/cifs/smb2glob.h | 10 ++++++++++
fs/cifs/smb2pdu.c | 4 ++--
3 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c
index c05477e28cff..86a5fb93f07f 100644
--- a/fs/cifs/cifs_debug.c
+++ b/fs/cifs/cifs_debug.c
@@ -13,6 +13,7 @@
#include <linux/uaccess.h>
#include "cifspdu.h"
#include "cifsglob.h"
+#include "smb2glob.h"
#include "cifsproto.h"
#include "cifs_debug.h"
#include "cifsfs.h"
@@ -354,7 +355,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
else if (server->compress_algorithm == SMB3_COMPRESS_LZ77_HUFF)
seq_printf(m, " COMPRESS_LZ77_HUFF");
if (server->sign)
- seq_printf(m, " signed");
+ seq_printf(m, " signed (%s)",
+ smb2_signing_algo_str(server->signing_algorithm));
if (server->posix_ext_supported)
seq_printf(m, " posix");
if (server->nosharesock)
@@ -405,7 +407,8 @@ static int cifs_debug_data_proc_show(struct seq_file *m, void *v)
if (ses->session_flags & SMB2_SESSION_FLAG_ENCRYPT_DATA)
seq_puts(m, " encrypted");
if (ses->sign)
- seq_puts(m, " signed");
+ seq_printf(m, " signed (%s)",
+ smb2_signing_algo_str(server->signing_algorithm));
seq_printf(m, "\n\tUser: %d Cred User: %d",
from_kuid(&init_user_ns, ses->linux_uid),
diff --git a/fs/cifs/smb2glob.h b/fs/cifs/smb2glob.h
index 82e916ad167c..3a3e81b1b8cb 100644
--- a/fs/cifs/smb2glob.h
+++ b/fs/cifs/smb2glob.h
@@ -41,4 +41,14 @@
#define END_OF_CHAIN 4
#define RELATED_REQUEST 8
+static inline const char *smb2_signing_algo_str(u16 algo)
+{
+ switch (algo) {
+ case SIGNING_ALG_HMAC_SHA256: return "HMAC-SHA256";
+ case SIGNING_ALG_AES_CMAC: return "AES-CMAC";
+ case SIGNING_ALG_AES_GMAC: return "AES-GMAC";
+ default: return "unknown algorithm";
+ }
+}
+
#endif /* _SMB2_GLOB_H */
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6c1d58492b18..6c22ead51feb 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -725,8 +725,8 @@ static void decode_signing_ctx(struct TCP_Server_Info *server,
server->signing_negotiated = true;
server->signing_algorithm = le16_to_cpu(pctxt->SigningAlgorithms[0]);
- cifs_dbg(FYI, "signing algorithm %d chosen\n",
- server->signing_algorithm);
+ cifs_dbg(FYI, "negotiated signing algorithm '%s'\n",
+ smb2_signing_algo_str(server->signing_algorithm));
}
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer
2022-09-29 1:56 [PATCH v3 0/8] cifs: introduce support for AES-GMAC signing Enzo Matsumiya
` (6 preceding siblings ...)
2022-09-29 1:56 ` [PATCH v3 7/8] cifs: show signing algorithm name in DebugData Enzo Matsumiya
@ 2022-09-29 1:56 ` Enzo Matsumiya
2022-09-29 5:45 ` Stefan Metzmacher
7 siblings, 1 reply; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 1:56 UTC (permalink / raw)
To: linux-cifs; +Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom, metze
AES-GMAC is more picky about buffers locality, alignment, and size, so
we can't use a stack-allocated buffer as padding (smb2_padding).
This commit drops smb2_padding and "reserves" the 8 last bytes of each
small buffer, which are slab-allocated, as the padding buffer space.
Introduce SMB2_PADDING_BUF(buf) macro to easily grab the padding buffer.
For now, only used in smb2_set_next_command().
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/cifs/smb2ops.c | 7 +++++--
fs/cifs/smb2pdu.c | 9 +++++----
fs/cifs/smb2pdu.h | 2 --
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 22b40d181bba..0b8497e1c747 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2323,7 +2323,10 @@ smb2_set_related(struct smb_rqst *rqst)
shdr->Flags |= SMB2_FLAGS_RELATED_OPERATIONS;
}
-char smb2_padding[7] = {0, 0, 0, 0, 0, 0, 0};
+/*
+ * Use the last 8 bytes of the small buf as the padding buffer, when necessary
+ */
+#define SMB2_PADDING_BUF(buf) (buf + MAX_CIFS_SMALL_BUFFER_SIZE - 8)
void
smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
@@ -2352,7 +2355,7 @@ smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
* If we do not have encryption then we can just add an extra
* iov for the padding.
*/
- rqst->rq_iov[rqst->rq_nvec].iov_base = smb2_padding;
+ rqst->rq_iov[rqst->rq_nvec].iov_base = SMB2_PADDING_BUF(rqst->rq_iov[0].iov_base);
rqst->rq_iov[rqst->rq_nvec].iov_len = num_padding;
rqst->rq_nvec++;
len += num_padding;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6c22ead51feb..fca1b580d57d 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -362,6 +362,9 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon,
/*
* smaller than SMALL_BUFFER_SIZE but bigger than fixed area of
* largest operations (Create)
+ *
+ * Note that the last 8 bytes of the small buffer are reserved for padding when required
+ * (see SMB2_PADDING_BUF in smb2ops.c)
*/
memset(buf, 0, 256);
@@ -2993,8 +2996,7 @@ SMB2_open_free(struct smb_rqst *rqst)
if (rqst && rqst->rq_iov) {
cifs_small_buf_release(rqst->rq_iov[0].iov_base);
for (i = 1; i < rqst->rq_nvec; i++)
- if (rqst->rq_iov[i].iov_base != smb2_padding)
- kfree(rqst->rq_iov[i].iov_base);
+ kfree(rqst->rq_iov[i].iov_base);
}
}
@@ -3187,8 +3189,7 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
if (rqst && rqst->rq_iov) {
cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
for (i = 1; i < rqst->rq_nvec; i++)
- if (rqst->rq_iov[i].iov_base != smb2_padding)
- kfree(rqst->rq_iov[i].iov_base);
+ kfree(rqst->rq_iov[i].iov_base);
}
}
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index f57881b8464f..689973a3acbb 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -371,8 +371,6 @@ struct smb2_file_id_extd_directory_info {
char FileName[1];
} __packed; /* level 60 */
-extern char smb2_padding[7];
-
/* equivalent of the contents of SMB3.1.1 POSIX open context response */
struct create_posix_rsp {
u32 nlink;
--
2.35.3
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer
2022-09-29 1:56 ` [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer Enzo Matsumiya
@ 2022-09-29 5:45 ` Stefan Metzmacher
2022-09-29 15:17 ` Enzo Matsumiya
0 siblings, 1 reply; 18+ messages in thread
From: Stefan Metzmacher @ 2022-09-29 5:45 UTC (permalink / raw)
To: Enzo Matsumiya, linux-cifs
Cc: smfrench, pc, ronniesahlberg, nspmangalore, tom
Am 29.09.22 um 03:56 schrieb Enzo Matsumiya:
> AES-GMAC is more picky about buffers locality, alignment, and size, so
> we can't use a stack-allocated buffer as padding (smb2_padding).
>
> This commit drops smb2_padding and "reserves" the 8 last bytes of each
> small buffer, which are slab-allocated, as the padding buffer space.
>
> Introduce SMB2_PADDING_BUF(buf) macro to easily grab the padding buffer.
> For now, only used in smb2_set_next_command().
>
> Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
> ---
> fs/cifs/smb2ops.c | 7 +++++--
> fs/cifs/smb2pdu.c | 9 +++++----
> fs/cifs/smb2pdu.h | 2 --
> 3 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 22b40d181bba..0b8497e1c747 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -2323,7 +2323,10 @@ smb2_set_related(struct smb_rqst *rqst)
> shdr->Flags |= SMB2_FLAGS_RELATED_OPERATIONS;
> }
>
> -char smb2_padding[7] = {0, 0, 0, 0, 0, 0, 0};
> +/*
> + * Use the last 8 bytes of the small buf as the padding buffer, when necessary
> + */
> +#define SMB2_PADDING_BUF(buf) (buf + MAX_CIFS_SMALL_BUFFER_SIZE - 8)
Do we need to expend the size of MAX_CIFS_SMALL_BUFFER_SIZE
in order to avoid reusing parts of the buffer used otherwise.
(But MAX_CIFS_SMALL_BUFFER_SIZE is confusing magic I don't really
understand, for me it's really hard to prove we never overflow
MAX_CIFS_SMALL_BUFFER_SIZE).
> void
> smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
> @@ -2352,7 +2355,7 @@ smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
> * If we do not have encryption then we can just add an extra
> * iov for the padding.
> */
> - rqst->rq_iov[rqst->rq_nvec].iov_base = smb2_padding;
> + rqst->rq_iov[rqst->rq_nvec].iov_base = SMB2_PADDING_BUF(rqst->rq_iov[0].iov_base);
> rqst->rq_iov[rqst->rq_nvec].iov_len = num_padding;
> rqst->rq_nvec++;
> len += num_padding;
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 6c22ead51feb..fca1b580d57d 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -362,6 +362,9 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon,
> /*
> * smaller than SMALL_BUFFER_SIZE but bigger than fixed area of
> * largest operations (Create)
> + *
> + * Note that the last 8 bytes of the small buffer are reserved for padding when required
> + * (see SMB2_PADDING_BUF in smb2ops.c)
> */
> memset(buf, 0, 256);
>
> @@ -2993,8 +2996,7 @@ SMB2_open_free(struct smb_rqst *rqst)
> if (rqst && rqst->rq_iov) {
> cifs_small_buf_release(rqst->rq_iov[0].iov_base);
> for (i = 1; i < rqst->rq_nvec; i++)
> - if (rqst->rq_iov[i].iov_base != smb2_padding)
> - kfree(rqst->rq_iov[i].iov_base);
> + kfree(rqst->rq_iov[i].iov_base);
> }
> }
>
> @@ -3187,8 +3189,7 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
> if (rqst && rqst->rq_iov) {
> cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
> for (i = 1; i < rqst->rq_nvec; i++)
> - if (rqst->rq_iov[i].iov_base != smb2_padding)
> - kfree(rqst->rq_iov[i].iov_base);
> + kfree(rqst->rq_iov[i].iov_base);
Don't we need to check against SMB2_PADDING_BUF(rqst->rq_iov[0].iov_base) here
and avoid passing an invalid pointer to kfree()?
metze
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH v3 8/8] cifs: use MAX_CIFS_SMALL_BUFFER_SIZE-8 as padding buffer
2022-09-29 5:45 ` Stefan Metzmacher
@ 2022-09-29 15:17 ` Enzo Matsumiya
0 siblings, 0 replies; 18+ messages in thread
From: Enzo Matsumiya @ 2022-09-29 15:17 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: linux-cifs, smfrench, pc, ronniesahlberg, nspmangalore, tom
On 09/29, Stefan Metzmacher wrote:
>Am 29.09.22 um 03:56 schrieb Enzo Matsumiya:
>>AES-GMAC is more picky about buffers locality, alignment, and size, so
>>we can't use a stack-allocated buffer as padding (smb2_padding).
>>
>>This commit drops smb2_padding and "reserves" the 8 last bytes of each
>>small buffer, which are slab-allocated, as the padding buffer space.
>>
>>Introduce SMB2_PADDING_BUF(buf) macro to easily grab the padding buffer.
>>For now, only used in smb2_set_next_command().
>>
>>Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
>>---
>> fs/cifs/smb2ops.c | 7 +++++--
>> fs/cifs/smb2pdu.c | 9 +++++----
>> fs/cifs/smb2pdu.h | 2 --
>> 3 files changed, 10 insertions(+), 8 deletions(-)
>>
>>diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
>>index 22b40d181bba..0b8497e1c747 100644
>>--- a/fs/cifs/smb2ops.c
>>+++ b/fs/cifs/smb2ops.c
>>@@ -2323,7 +2323,10 @@ smb2_set_related(struct smb_rqst *rqst)
>> shdr->Flags |= SMB2_FLAGS_RELATED_OPERATIONS;
>> }
>>-char smb2_padding[7] = {0, 0, 0, 0, 0, 0, 0};
>>+/*
>>+ * Use the last 8 bytes of the small buf as the padding buffer, when necessary
>>+ */
>>+#define SMB2_PADDING_BUF(buf) (buf + MAX_CIFS_SMALL_BUFFER_SIZE - 8)
>
>Do we need to expend the size of MAX_CIFS_SMALL_BUFFER_SIZE
>in order to avoid reusing parts of the buffer used otherwise.
>
>(But MAX_CIFS_SMALL_BUFFER_SIZE is confusing magic I don't really
>understand, for me it's really hard to prove we never overflow
>MAX_CIFS_SMALL_BUFFER_SIZE).
Yes, that was one concern I had. This is (yet another) part of the code
that I see where using hardcoded values hides their meanings and
reasoning, and the comments doesn't really help much understanding it.
So, based on my tests, that region (last 8 bytes) seems to not be ever used.
I didn't bother checking how much is actually being used, but for the
moment I'd say this is fine.
@Steve your clarification on the value for MAX_CIFS_SMALL_BUFFER_SIZE
would be appreciated, I guess.
>
>> void
>> smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
>>@@ -2352,7 +2355,7 @@ smb2_set_next_command(struct cifs_tcon *tcon, struct smb_rqst *rqst)
>> * If we do not have encryption then we can just add an extra
>> * iov for the padding.
>> */
>>- rqst->rq_iov[rqst->rq_nvec].iov_base = smb2_padding;
>>+ rqst->rq_iov[rqst->rq_nvec].iov_base = SMB2_PADDING_BUF(rqst->rq_iov[0].iov_base);
>> rqst->rq_iov[rqst->rq_nvec].iov_len = num_padding;
>> rqst->rq_nvec++;
>> len += num_padding;
>>diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
>>index 6c22ead51feb..fca1b580d57d 100644
>>--- a/fs/cifs/smb2pdu.c
>>+++ b/fs/cifs/smb2pdu.c
>>@@ -362,6 +362,9 @@ fill_small_buf(__le16 smb2_command, struct cifs_tcon *tcon,
>> /*
>> * smaller than SMALL_BUFFER_SIZE but bigger than fixed area of
>> * largest operations (Create)
>>+ *
>>+ * Note that the last 8 bytes of the small buffer are reserved for padding when required
>>+ * (see SMB2_PADDING_BUF in smb2ops.c)
>> */
>> memset(buf, 0, 256);
>>@@ -2993,8 +2996,7 @@ SMB2_open_free(struct smb_rqst *rqst)
>> if (rqst && rqst->rq_iov) {
>> cifs_small_buf_release(rqst->rq_iov[0].iov_base);
>> for (i = 1; i < rqst->rq_nvec; i++)
>>- if (rqst->rq_iov[i].iov_base != smb2_padding)
>>- kfree(rqst->rq_iov[i].iov_base);
>>+ kfree(rqst->rq_iov[i].iov_base);
>> }
>> }
>>@@ -3187,8 +3189,7 @@ SMB2_ioctl_free(struct smb_rqst *rqst)
>> if (rqst && rqst->rq_iov) {
>> cifs_small_buf_release(rqst->rq_iov[0].iov_base); /* request */
>> for (i = 1; i < rqst->rq_nvec; i++)
>>- if (rqst->rq_iov[i].iov_base != smb2_padding)
>>- kfree(rqst->rq_iov[i].iov_base);
>>+ kfree(rqst->rq_iov[i].iov_base);
>
>Don't we need to check against SMB2_PADDING_BUF(rqst->rq_iov[0].iov_base) here
>and avoid passing an invalid pointer to kfree()?
Ah good catch. Will fix it.
>metze
Cheers,
Enzo
^ permalink raw reply [flat|nested] 18+ messages in thread