| Message ID | 20190611134750.2974-8-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Tue, Jun 11, 2019 at 03:47:50PM +0200, Ard Biesheuvel wrote: > The CIFS code uses the sync skcipher API to invoke the ecb(arc4) skcipher, > of which only a single generic C code implementation exists. This means > that going through all the trouble of using scatterlists etc buys us > very little, and we're better off just invoking the arc4 library directly. > > Cc: linux-cifs@vger.kernel.org > Cc: Steve French <sfrench@samba.org> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > fs/cifs/Kconfig | 2 +- > fs/cifs/cifsencrypt.c | 53 ++++++-------------- > 2 files changed, 16 insertions(+), 39 deletions(-) > > diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig > index aae2b8b2adf5..523e9ea78a28 100644 > --- a/fs/cifs/Kconfig > +++ b/fs/cifs/Kconfig > @@ -10,7 +10,7 @@ config CIFS > select CRYPTO_SHA512 > select CRYPTO_CMAC > select CRYPTO_HMAC > - select CRYPTO_ARC4 > + select CRYPTO_LIB_ARC4 > select CRYPTO_AEAD2 > select CRYPTO_CCM > select CRYPTO_ECB Since the "arc4" module is no longer needed, the MODULE_SOFTDEP("pre: arc4"); in fs/cifs/cifsfs.c should be removed too. (Note that it doesn't need a soft dependency on libarc4 instead, since the cifs module will link directly to it.) > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index d2a05e46d6f5..3b7b5e83493d 100644 > --- a/fs/cifs/cifsencrypt.c > +++ b/fs/cifs/cifsencrypt.c > @@ -33,7 +33,8 @@ > #include <linux/ctype.h> > #include <linux/random.h> > #include <linux/highmem.h> > -#include <crypto/skcipher.h> > +#include <linux/fips.h> > +#include <crypto/arc4.h> > #include <crypto/aead.h> > > int __cifs_calc_signature(struct smb_rqst *rqst, > @@ -772,11 +773,12 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) > int > calc_seckey(struct cifs_ses *ses) > { > - int rc; > - struct crypto_skcipher *tfm_arc4; > - struct scatterlist sgin, sgout; > - struct skcipher_request *req; > + struct arc4_ctx *ctx_arc4; > unsigned char *sec_key; > + int rc = 0; > + > + if (fips_enabled) > + return -ENODEV; > > sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL); > if (sec_key == NULL) sec_key should be moved back to the stack now, basically reverting this commit: commit 5f4b55699aaff1028468e3f53853d781cdafedd6 Author: Sachin Prabhu <sprabhu@redhat.com> Date: Mon Oct 17 16:40:22 2016 -0400 CIFS: Fix BUG() in calc_seckey() It was only moved to the heap because it had to go in a scatterlist. > + arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE); > + arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, > + CIFS_CPHTXT_SIZE); > > /* make secondary_key/nonce as session key */ > memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE); > /* and make len as that of session key only */ > ses->auth_key.len = CIFS_SESS_KEY_SIZE; > > -out_free_cipher: > - crypto_free_skcipher(tfm_arc4); > out: > + kfree(ctx_arc4); Should be kzfree(). - Eric
diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig index aae2b8b2adf5..523e9ea78a28 100644 --- a/fs/cifs/Kconfig +++ b/fs/cifs/Kconfig @@ -10,7 +10,7 @@ config CIFS select CRYPTO_SHA512 select CRYPTO_CMAC select CRYPTO_HMAC - select CRYPTO_ARC4 + select CRYPTO_LIB_ARC4 select CRYPTO_AEAD2 select CRYPTO_CCM select CRYPTO_ECB diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c index d2a05e46d6f5..3b7b5e83493d 100644 --- a/fs/cifs/cifsencrypt.c +++ b/fs/cifs/cifsencrypt.c @@ -33,7 +33,8 @@ #include <linux/ctype.h> #include <linux/random.h> #include <linux/highmem.h> -#include <crypto/skcipher.h> +#include <linux/fips.h> +#include <crypto/arc4.h> #include <crypto/aead.h> int __cifs_calc_signature(struct smb_rqst *rqst, @@ -772,11 +773,12 @@ setup_ntlmv2_rsp(struct cifs_ses *ses, const struct nls_table *nls_cp) int calc_seckey(struct cifs_ses *ses) { - int rc; - struct crypto_skcipher *tfm_arc4; - struct scatterlist sgin, sgout; - struct skcipher_request *req; + struct arc4_ctx *ctx_arc4; unsigned char *sec_key; + int rc = 0; + + if (fips_enabled) + return -ENODEV; sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL); if (sec_key == NULL) @@ -784,49 +786,24 @@ calc_seckey(struct cifs_ses *ses) get_random_bytes(sec_key, CIFS_SESS_KEY_SIZE); - tfm_arc4 = crypto_alloc_skcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC); - if (IS_ERR(tfm_arc4)) { - rc = PTR_ERR(tfm_arc4); - cifs_dbg(VFS, "could not allocate crypto API arc4\n"); - goto out; - } - - rc = crypto_skcipher_setkey(tfm_arc4, ses->auth_key.response, - CIFS_SESS_KEY_SIZE); - if (rc) { - cifs_dbg(VFS, "%s: Could not set response as a key\n", - __func__); - goto out_free_cipher; - } - - req = skcipher_request_alloc(tfm_arc4, GFP_KERNEL); - if (!req) { + ctx_arc4 = kmalloc(sizeof(*ctx_arc4), GFP_KERNEL); + if (!ctx_arc4) { rc = -ENOMEM; - cifs_dbg(VFS, "could not allocate crypto API arc4 request\n"); - goto out_free_cipher; + cifs_dbg(VFS, "could not allocate arc4 context\n"); + goto out; } - sg_init_one(&sgin, sec_key, CIFS_SESS_KEY_SIZE); - sg_init_one(&sgout, ses->ntlmssp->ciphertext, CIFS_CPHTXT_SIZE); - - skcipher_request_set_callback(req, 0, NULL, NULL); - skcipher_request_set_crypt(req, &sgin, &sgout, CIFS_CPHTXT_SIZE, NULL); - - rc = crypto_skcipher_encrypt(req); - skcipher_request_free(req); - if (rc) { - cifs_dbg(VFS, "could not encrypt session key rc: %d\n", rc); - goto out_free_cipher; - } + arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE); + arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, + CIFS_CPHTXT_SIZE); /* make secondary_key/nonce as session key */ memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE); /* and make len as that of session key only */ ses->auth_key.len = CIFS_SESS_KEY_SIZE; -out_free_cipher: - crypto_free_skcipher(tfm_arc4); out: + kfree(ctx_arc4); kfree(sec_key); return rc; }
The CIFS code uses the sync skcipher API to invoke the ecb(arc4) skcipher, of which only a single generic C code implementation exists. This means that going through all the trouble of using scatterlists etc buys us very little, and we're better off just invoking the arc4 library directly. Cc: linux-cifs@vger.kernel.org Cc: Steve French <sfrench@samba.org> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- fs/cifs/Kconfig | 2 +- fs/cifs/cifsencrypt.c | 53 ++++++-------------- 2 files changed, 16 insertions(+), 39 deletions(-)