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

[Steve French]

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@xxxxxxxxx> 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@xxxxxxxxxx>
> 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@xxxxxxxxx> wrote:
> >
> > Replace internal logic with an independent arc4 library.
> >
> > Signed-off-by: John Sanpe <sanpeqf@xxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxxxx>
> > - */
> > -
> > -#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