Re: [PATCH] fs/smb: using crypto lib instead cifs_arc4

Steve French
2023-10-22T19:41:02Z

Adding Ronnie to cc: since he may have additional context why arc4 was
moved out of crypto to cifs common code

On Sun, Oct 22, 2023 at 2:38 PM Steve French <smfrench@gmail.com> wrote:
>
> I thought that the whole point of kernel crypto guys was the reverse -
> ie arc4 must be moved to cifs.ko since cifs/smb3 mounts had the only
> approved use case. Ronnie may have additional context, but there was
> a push to remove arc4 and md4 (see e.g. the emails threads about:
> "crypto: remove MD4 generic shash"). I also want to be careful that
> we don't accidentally disable smb3.1.1 mounts (which are highly
> secure) because they have small dependencies on old algorithms (even
> if that doesn't cause problems with typical reasonable length password
> cases)
>
> commit 71c02863246167b3d1639b8278681ca8ebedcb4e
> Author: Ronnie Sahlberg <lsahlber@redhat.com>
> Date: Thu Aug 19 20:34:59 2021 +1000
>
> cifs: fork arc4 and create a separate module for it for cifs and other users
>
> We can not drop ARC4 and basically destroy CIFS connectivity for
> almost all CIFS users so create a new forked ARC4 module that CIFS and other
> subsystems that have a hard dependency on ARC4 can use.
>
> On Sun, Oct 22, 2023 at 1:39 PM John Sanpe <sanpeqf@gmail.com> wrote:
> >
> > Replace internal logic with an independent arc4 library.
> >
> > Signed-off-by: John Sanpe <sanpeqf@gmail.com>
> > ---
> > fs/smb/Kconfig | 1 +
> > fs/smb/client/cifsencrypt.c | 7 ++--
> > fs/smb/common/Makefile | 1 -
> > fs/smb/common/arc4.h | 23 ------------
> > fs/smb/common/cifs_arc4.c | 74 -------------------------------------
> > fs/smb/server/auth.c | 6 +--
> > 6 files changed, 7 insertions(+), 105 deletions(-)
> > delete mode 100644 fs/smb/common/arc4.h
> > delete mode 100644 fs/smb/common/cifs_arc4.c
> >
> > diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
> > index ef425789fa6a..65e5a437898b 100644
> > --- a/fs/smb/Kconfig
> > +++ b/fs/smb/Kconfig
> > @@ -7,5 +7,6 @@ source "fs/smb/server/Kconfig"
> >
> > config SMBFS
> > tristate
> > + select CRYPTO_LIB_ARC4
> > default y if CIFS=y || SMB_SERVER=y
> > default m if CIFS=m || SMB_SERVER=m
> > diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
> > index ef4c2e3c9fa6..d8754c406b5f 100644
> > --- a/fs/smb/client/cifsencrypt.c
> > +++ b/fs/smb/client/cifsencrypt.c
> > @@ -21,7 +21,7 @@
> > #include <linux/random.h>
> > #include <linux/highmem.h>
> > #include <linux/fips.h>
> > -#include "../common/arc4.h"
> > +#include <crypto/arc4.h>
> > #include <crypto/aead.h>
> >
> > /*
> > @@ -826,9 +826,8 @@ calc_seckey(struct cifs_ses *ses)
> > return -ENOMEM;
> > }
> >
> > - cifs_arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> > - cifs_arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key,
> > - CIFS_CPHTXT_SIZE);
> > + 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);
> > diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
> > index c66dbbc1469c..9e0730a385fb 100644
> > --- a/fs/smb/common/Makefile
> > +++ b/fs/smb/common/Makefile
> > @@ -3,5 +3,4 @@
> > # Makefile for Linux filesystem routines that are shared by client and server.
> > #
> >
> > -obj-$(CONFIG_SMBFS) += cifs_arc4.o
> > obj-$(CONFIG_SMBFS) += cifs_md4.o
> > diff --git a/fs/smb/common/arc4.h b/fs/smb/common/arc4.h
> > deleted file mode 100644
> > index 12e71ec033a1..000000000000
> > --- a/fs/smb/common/arc4.h
> > +++ /dev/null
> > @@ -1,23 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0+ */
> > -/*
> > - * Common values for ARC4 Cipher Algorithm
> > - */
> > -
> > -#ifndef _CRYPTO_ARC4_H
> > -#define _CRYPTO_ARC4_H
> > -
> > -#include <linux/types.h>
> > -
> > -#define ARC4_MIN_KEY_SIZE 1
> > -#define ARC4_MAX_KEY_SIZE 256
> > -#define ARC4_BLOCK_SIZE 1
> > -
> > -struct arc4_ctx {
> > - u32 S[256];
> > - u32 x, y;
> > -};
> > -
> > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
> > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
> > -
> > -#endif /* _CRYPTO_ARC4_H */
> > diff --git a/fs/smb/common/cifs_arc4.c b/fs/smb/common/cifs_arc4.c
> > deleted file mode 100644
> > index 043e4cb839fa..000000000000
> > --- a/fs/smb/common/cifs_arc4.c
> > +++ /dev/null
> > @@ -1,74 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0-or-later
> > -/*
> > - * Cryptographic API
> > - *
> > - * ARC4 Cipher Algorithm
> > - *
> > - * Jon Oberheide <jon@oberheide.org>
> > - */
> > -
> > -#include <linux/module.h>
> > -#include "arc4.h"
> > -
> > -MODULE_LICENSE("GPL");
> > -
> > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len)
> > -{
> > - int i, j = 0, k = 0;
> > -
> > - ctx->x = 1;
> > - ctx->y = 0;
> > -
> > - for (i = 0; i < 256; i++)
> > - ctx->S[i] = i;
> > -
> > - for (i = 0; i < 256; i++) {
> > - u32 a = ctx->S[i];
> > -
> > - j = (j + in_key[k] + a) & 0xff;
> > - ctx->S[i] = ctx->S[j];
> > - ctx->S[j] = a;
> > - if (++k >= key_len)
> > - k = 0;
> > - }
> > -
> > - return 0;
> > -}
> > -EXPORT_SYMBOL_GPL(cifs_arc4_setkey);
> > -
> > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
> > -{
> > - u32 *const S = ctx->S;
> > - u32 x, y, a, b;
> > - u32 ty, ta, tb;
> > -
> > - if (len == 0)
> > - return;
> > -
> > - x = ctx->x;
> > - y = ctx->y;
> > -
> > - a = S[x];
> > - y = (y + a) & 0xff;
> > - b = S[y];
> > -
> > - do {
> > - S[y] = a;
> > - a = (a + b) & 0xff;
> > - S[x] = b;
> > - x = (x + 1) & 0xff;
> > - ta = S[x];
> > - ty = (y + ta) & 0xff;
> > - tb = S[ty];
> > - *out++ = *in++ ^ S[a];
> > - if (--len == 0)
> > - break;
> > - y = ty;
> > - a = ta;
> > - b = tb;
> > - } while (true);
> > -
> > - ctx->x = x;
> > - ctx->y = y;
> > -}
> > -EXPORT_SYMBOL_GPL(cifs_arc4_crypt);
> > diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
> > index 229a6527870d..5640196b313f 100644
> > --- a/fs/smb/server/auth.c
> > +++ b/fs/smb/server/auth.c
> > @@ -29,7 +29,7 @@
> > #include "mgmt/user_config.h"
> > #include "crypto_ctx.h"
> > #include "transport_ipc.h"
> > -#include "../common/arc4.h"
> > +#include <crypto/arc4.h>
> >
> > /*
> > * Fixed format data defining GSS header and fixed string
> > @@ -362,9 +362,9 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
> > if (!ctx_arc4)
> > return -ENOMEM;
> >
> > - cifs_arc4_setkey(ctx_arc4, sess->sess_key,
> > + arc4_setkey(ctx_arc4, sess->sess_key,
> > SMB2_NTLMV2_SESSKEY_SIZE);
> > - cifs_arc4_crypt(ctx_arc4, sess->sess_key,
> > + arc4_crypt(ctx_arc4, sess->sess_key,
> > (char *)authblob + sess_key_off, sess_key_len);
> > kfree_sensitive(ctx_arc4);
> > }
> > --
> > 2.41.0
> >
>
>
> --
> Thanks,
>
> Steve

--
Thanks,

Steve