[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAH2r5mu-JMKZrj1RiohyPeyBL7npcvXyVZYFQxX+yBmz4cV=VQ@mail.gmail.com>
Date: Sun, 7 Sep 2025 16:20:15 -0500
From: Steve French <smfrench@...il.com>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-cifs@...r.kernel.org, Steve French <sfrench@...ba.org>,
Paulo Alcantara <pc@...guebit.org>, Namjae Jeon <linkinjeon@...nel.org>,
Ronnie Sahlberg <ronniesahlberg@...il.com>, Shyam Prasad N <sprasad@...rosoft.com>,
Tom Talpey <tom@...pey.com>, Bharath SM <bharathsm@...rosoft.com>,
Sergey Senozhatsky <senozhatsky@...omium.org>, linux-crypto@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] smb: Use arc4 library instead of duplicate arc4 code
tentatively put in cifs-2.6.git for-next pending any additional review/testing
I did some simple tests and worked fine for me.
On Sat, Sep 6, 2025 at 10:22 PM Eric Biggers <ebiggers@...nel.org> wrote:
>
> fs/smb/common/cifs_arc4.c has an implementation of ARC4, but a copy of
> this same code is also present in lib/crypto/arc4.c to serve the other
> users of this legacy algorithm in the kernel. Remove the duplicate
> implementation in fs/smb/, which seems to have been added because of a
> misunderstanding, and just use the lib/crypto/ one.
>
> Signed-off-by: Eric Biggers <ebiggers@...nel.org>
> ---
> fs/smb/client/Kconfig | 1 +
> fs/smb/client/cifsencrypt.c | 8 ++--
> fs/smb/common/Makefile | 1 -
> fs/smb/common/arc4.h | 23 ------------
> fs/smb/common/cifs_arc4.c | 75 -------------------------------------
> fs/smb/server/Kconfig | 1 +
> fs/smb/server/auth.c | 9 ++---
> 7 files changed, 10 insertions(+), 108 deletions(-)
> delete mode 100644 fs/smb/common/arc4.h
> delete mode 100644 fs/smb/common/cifs_arc4.c
>
> diff --git a/fs/smb/client/Kconfig b/fs/smb/client/Kconfig
> index 9f05f94e265a6..a4c02199fef48 100644
> --- a/fs/smb/client/Kconfig
> +++ b/fs/smb/client/Kconfig
> @@ -13,10 +13,11 @@ config CIFS
> select CRYPTO_AEAD2
> select CRYPTO_CCM
> select CRYPTO_GCM
> select CRYPTO_ECB
> select CRYPTO_AES
> + select CRYPTO_LIB_ARC4
> select KEYS
> select DNS_RESOLVER
> select ASN1
> select OID_REGISTRY
> select NETFS_SUPPORT
> diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
> index 3cc6862469087..7b7c8c38fdd08 100644
> --- a/fs/smb/client/cifsencrypt.c
> +++ b/fs/smb/client/cifsencrypt.c
> @@ -20,12 +20,12 @@
> #include <linux/ctype.h>
> #include <linux/random.h>
> #include <linux/highmem.h>
> #include <linux/fips.h>
> #include <linux/iov_iter.h>
> -#include "../common/arc4.h"
> #include <crypto/aead.h>
> +#include <crypto/arc4.h>
>
> static size_t cifs_shash_step(void *iter_base, size_t progress, size_t len,
> void *priv, void *priv2)
> {
> struct shash_desc *shash = priv;
> @@ -723,13 +723,13 @@ calc_seckey(struct cifs_ses *ses)
> if (!ctx_arc4) {
> cifs_dbg(VFS, "Could not allocate arc4 context\n");
> 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);
> /* and make len as that of session key only */
> ses->auth_key.len = CIFS_SESS_KEY_SIZE;
> diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
> index c66dbbc1469c3..9e0730a385fb1 100644
> --- a/fs/smb/common/Makefile
> +++ b/fs/smb/common/Makefile
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> #
> # 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 12e71ec033a18..0000000000000
> --- 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 df360ca47826a..0000000000000
> --- a/fs/smb/common/cifs_arc4.c
> +++ /dev/null
> @@ -1,75 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * Cryptographic API
> - *
> - * ARC4 Cipher Algorithm
> - *
> - * Jon Oberheide <jon@...rheide.org>
> - */
> -
> -#include <linux/module.h>
> -#include "arc4.h"
> -
> -MODULE_DESCRIPTION("ARC4 Cipher Algorithm");
> -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/Kconfig b/fs/smb/server/Kconfig
> index 4a23a5e7e8fec..098cac98d31e6 100644
> --- a/fs/smb/server/Kconfig
> +++ b/fs/smb/server/Kconfig
> @@ -8,10 +8,11 @@ config SMB_SERVER
> select NLS_UCS2_UTILS
> select CRYPTO
> select CRYPTO_MD5
> select CRYPTO_HMAC
> select CRYPTO_ECB
> + select CRYPTO_LIB_ARC4
> select CRYPTO_LIB_DES
> select CRYPTO_LIB_SHA256
> select CRYPTO_SHA256
> select CRYPTO_CMAC
> select CRYPTO_SHA512
> diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
> index d99871c214518..b4020bb55a268 100644
> --- a/fs/smb/server/auth.c
> +++ b/fs/smb/server/auth.c
> @@ -18,20 +18,20 @@
>
> #include "auth.h"
> #include "glob.h"
>
> #include <linux/fips.h>
> +#include <crypto/arc4.h>
> #include <crypto/des.h>
>
> #include "server.h"
> #include "smb_common.h"
> #include "connection.h"
> #include "mgmt/user_session.h"
> #include "mgmt/user_config.h"
> #include "crypto_ctx.h"
> #include "transport_ipc.h"
> -#include "../common/arc4.h"
>
> /*
> * Fixed format data defining GSS header and fixed string
> * "not_defined_in_RFC4178@...ase_ignore".
> * So sec blob data in neg phase could be generated statically.
> @@ -363,14 +363,13 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
>
> ctx_arc4 = kmalloc(sizeof(*ctx_arc4), KSMBD_DEFAULT_GFP);
> if (!ctx_arc4)
> return -ENOMEM;
>
> - cifs_arc4_setkey(ctx_arc4, sess->sess_key,
> - SMB2_NTLMV2_SESSKEY_SIZE);
> - cifs_arc4_crypt(ctx_arc4, sess->sess_key,
> - (char *)authblob + sess_key_off, sess_key_len);
> + arc4_setkey(ctx_arc4, sess->sess_key, SMB2_NTLMV2_SESSKEY_SIZE);
> + arc4_crypt(ctx_arc4, sess->sess_key,
> + (char *)authblob + sess_key_off, sess_key_len);
> kfree_sensitive(ctx_arc4);
> }
>
> return ret;
> }
>
> base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
> --
> 2.50.1
>
>
--
Thanks,
Steve
Powered by blists - more mailing lists