Acked-by: Steve French <stfrench@xxxxxxxxxxxxx> On Wed, Jun 12, 2019 at 1:06 PM Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> 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. > > This also reverts commit 5f4b55699aaf ("CIFS: Fix BUG() in calc_seckey()"), > since it is no longer necessary to allocate sec_key on the heap. > > Cc: linux-cifs@xxxxxxxxxxxxxxx > Cc: Steve French <sfrench@xxxxxxxxx> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > --- > fs/cifs/Kconfig | 2 +- > fs/cifs/cifsencrypt.c | 62 +++++--------------- > fs/cifs/cifsfs.c | 1 - > 3 files changed, 17 insertions(+), 48 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 > diff --git a/fs/cifs/cifsencrypt.c b/fs/cifs/cifsencrypt.c > index d2a05e46d6f5..97b7497c13ef 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,63 +773,32 @@ 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; > - unsigned char *sec_key; > + unsigned char sec_key[CIFS_SESS_KEY_SIZE]; /* a nonce */ > + struct arc4_ctx *ctx_arc4; > > - sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL); > - if (sec_key == NULL) > - return -ENOMEM; > + if (fips_enabled) > + return -ENODEV; > > 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) { > - rc = -ENOMEM; > - cifs_dbg(VFS, "could not allocate crypto API arc4 request\n"); > - goto out_free_cipher; > + ctx_arc4 = kmalloc(sizeof(*ctx_arc4), GFP_KERNEL); > + if (!ctx_arc4) { > + cifs_dbg(VFS, "could not allocate arc4 context\n"); > + return -ENOMEM; > } > > - 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(sec_key); > - return rc; > + memzero_explicit(sec_key, CIFS_SESS_KEY_SIZE); > + kzfree(ctx_arc4); > + return 0; > } > > void > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > index f5fcd6360056..e55afaf9e5a3 100644 > --- a/fs/cifs/cifsfs.c > +++ b/fs/cifs/cifsfs.c > @@ -1590,7 +1590,6 @@ MODULE_DESCRIPTION > ("VFS to access SMB3 servers e.g. Samba, Macs, Azure and Windows (and " > "also older servers complying with the SNIA CIFS Specification)"); > MODULE_VERSION(CIFS_VERSION); > -MODULE_SOFTDEP("pre: arc4"); > MODULE_SOFTDEP("pre: des"); > MODULE_SOFTDEP("pre: ecb"); > MODULE_SOFTDEP("pre: hmac"); > -- > 2.20.1 > -- Thanks, Steve