[PATCH v3 0/7] crypto: rc4 cleanup

[PATCH v3 0/7] crypto: rc4 cleanup

[Ard Biesheuvel]

This is a follow-up to, and supersedes [0], which moved some WEP code from
the cipher to the skcipher interface, in order to reduce the use of the bare
cipher interface in non-crypto subsystem code.

Since using the skcipher interface to invoke the generic C implementation of
an algorithm that is known at compile time is rather pointless, this series
moves those users to a new arc4 library interface instead, which is based on
the existing code.

Along the way, the arc4 cipher implementation is removed entirely, and only
the ecb(arc4) code is preserved, which is used in a number of places in the
kernel, and is known to be used by at least 'iwd' from user space via the
algif_skcipher API.

Changes since v2:
- drop the crypto_ prefix from the arc4 library functions and types
- rename the source file to arc4.c but keep the lib prefix for the actual
  module to prevent a clash with the crypto API driver
- preserve the existing behavior wrt the fips_enabled flag, which prevents
  any use of ARC4 (note that the fips_enabled flag evaluates to 'false' at
  compile time for kernels that lack the feature, so with these patches, we
  get rid of most of the runtime logic regarding FIPS for builds that don't
  have it enabled)

[0] https://lore.kernel.org/linux-crypto/20190607144944.13485-1-ard.biesheuvel@linaro.org/

Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Biggers <ebiggers@google.com>
Cc: Johannes Berg <johannes@sipsolutions.net>

Ard Biesheuvel (7):
  crypto: arc4 - refactor arc4 core code into separate library
  net/mac80211: move WEP handling to ARC4 library interface
  net/lib80211: move WEP handling to ARC4 library code
  net/lib80211: move TKIP handling to ARC4 library code
  crypto: arc4 - remove cipher implementation
  ppp: mppe: switch to RC4 library interface
  fs: cifs: switch to RC4 library interface

 MAINTAINERS                        |   1 +
 crypto/Kconfig                     |   4 +
 crypto/arc4.c                      | 104 +-------------------
 drivers/net/ppp/Kconfig            |   3 +-
 drivers/net/ppp/ppp_mppe.c         |  95 +++---------------
 fs/cifs/Kconfig                    |   2 +-
 fs/cifs/cifsencrypt.c              |  53 +++-------
 include/crypto/arc4.h              |  10 ++
 lib/Makefile                       |   2 +-
 lib/crypto/Makefile                |   4 +
 lib/crypto/arc4.c                  |  72 ++++++++++++++
 net/mac80211/Kconfig               |   2 +-
 net/mac80211/cfg.c                 |   3 -
 net/mac80211/ieee80211_i.h         |   4 +-
 net/mac80211/key.h                 |   1 +
 net/mac80211/main.c                |   6 +-
 net/mac80211/mlme.c                |   2 -
 net/mac80211/tkip.c                |   8 +-
 net/mac80211/tkip.h                |   4 +-
 net/mac80211/wep.c                 |  47 ++-------
 net/mac80211/wep.h                 |   4 +-
 net/mac80211/wpa.c                 |   4 +-
 net/wireless/Kconfig               |   1 +
 net/wireless/lib80211_crypt_tkip.c |  46 +++------
 net/wireless/lib80211_crypt_wep.c  |  49 +++------
 25 files changed, 179 insertions(+), 352 deletions(-)
 create mode 100644 lib/crypto/Makefile
 create mode 100644 lib/crypto/arc4.c

-- 
2.20.1

[PATCH v3 1/7] crypto: arc4 - refactor arc4 core code into separate library

[Ard Biesheuvel]

Refactor the core rc4 handling so we can move most users to a library
interface, permitting us to drop the cipher interface entirely in a
future patch. This is part of an effort to simplify the crypto API
and improve its robustness against incorrect use.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MAINTAINERS           |  1 +
 crypto/Kconfig        |  4 ++
 crypto/arc4.c         | 60 +---------------
 include/crypto/arc4.h | 10 +++
 lib/Makefile          |  2 +-
 lib/crypto/Makefile   |  4 ++
 lib/crypto/arc4.c     | 72 ++++++++++++++++++++
 7 files changed, 93 insertions(+), 60 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 57f496cff999..112f21066141 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4241,6 +4241,7 @@ F:	crypto/
 F:	drivers/crypto/
 F:	include/crypto/
 F:	include/linux/crypto*
+F:	lib/crypto/
 
 CRYPTOGRAPHIC RANDOM NUMBER GENERATOR
 M:	Neil Horman <nhorman@tuxdriver.com>
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 3d056e7da65f..5114b35ef3b4 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1230,9 +1230,13 @@ config CRYPTO_ANUBIS
 	  <https://www.cosic.esat.kuleuven.be/nessie/reports/>
 	  <http://www.larc.usp.br/~pbarreto/AnubisPage.html>
 
+config CRYPTO_LIB_ARC4
+	tristate
+
 config CRYPTO_ARC4
 	tristate "ARC4 cipher algorithm"
 	select CRYPTO_BLKCIPHER
+	select CRYPTO_LIB_ARC4
 	help
 	  ARC4 cipher algorithm.
 
diff --git a/crypto/arc4.c b/crypto/arc4.c
index a2120e06bf84..6974dba1b7b9 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -13,33 +13,12 @@
 #include <linux/init.h>
 #include <linux/module.h>
 
-struct arc4_ctx {
-	u32 S[256];
-	u32 x, y;
-};
-
 static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
 			unsigned int key_len)
 {
 	struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
-	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;
+	return arc4_setkey(ctx, in_key, key_len);
 }
 
 static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
@@ -48,43 +27,6 @@ static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
 	return arc4_set_key(&tfm->base, in_key, key_len);
 }
 
-static void 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;
-}
-
 static void arc4_crypt_one(struct crypto_tfm *tfm, u8 *out, const u8 *in)
 {
 	arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
index 5b2c24ab0139..f3c22fe01704 100644
--- a/include/crypto/arc4.h
+++ b/include/crypto/arc4.h
@@ -6,8 +6,18 @@
 #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 arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
+void arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
+
 #endif /* _CRYPTO_ARC4_H */
diff --git a/lib/Makefile b/lib/Makefile
index fb7697031a79..d3daedf93c5a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -102,7 +102,7 @@ endif
 obj-$(CONFIG_DEBUG_INFO_REDUCED) += debug_info.o
 CFLAGS_debug_info.o += $(call cc-option, -femit-struct-debug-detailed=any)
 
-obj-y += math/
+obj-y += math/ crypto/
 
 obj-$(CONFIG_GENERIC_IOMAP) += iomap.o
 obj-$(CONFIG_GENERIC_PCI_IOMAP) += pci_iomap.o
diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
new file mode 100644
index 000000000000..88195c34932d
--- /dev/null
+++ b/lib/crypto/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
+libarc4-y := arc4.o
diff --git a/lib/crypto/arc4.c b/lib/crypto/arc4.c
new file mode 100644
index 000000000000..fa589eba3d50
--- /dev/null
+++ b/lib/crypto/arc4.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Cryptographic API
+ *
+ * ARC4 Cipher Algorithm
+ *
+ * Jon Oberheide <jon@oberheide.org>
+ */
+
+#include <crypto/arc4.h>
+#include <linux/module.h>
+
+int 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(arc4_setkey);
+
+void 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(arc4_crypt);
-- 
2.20.1

[PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Ard Biesheuvel]

The WEP code in the mac80211 subsystem currently uses the crypto
API to access the arc4 (RC4) cipher, which is overly complicated,
and doesn't really have an upside in this particular case, since
ciphers are always synchronous and therefore always implemented in
software. Given that we have no accelerated software implementations
either, it is much more straightforward to invoke a generic library
interface directly.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/mac80211/Kconfig       |  2 +-
 net/mac80211/cfg.c         |  3 --
 net/mac80211/ieee80211_i.h |  4 +-
 net/mac80211/key.h         |  1 +
 net/mac80211/main.c        |  6 +--
 net/mac80211/mlme.c        |  2 -
 net/mac80211/tkip.c        |  8 ++--
 net/mac80211/tkip.h        |  4 +-
 net/mac80211/wep.c         | 47 ++++----------------
 net/mac80211/wep.h         |  4 +-
 net/mac80211/wpa.c         |  4 +-
 11 files changed, 24 insertions(+), 61 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index 0227cce9685e..0c93b1b7a826 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -3,7 +3,7 @@ config MAC80211
 	tristate "Generic IEEE 802.11 Networking Stack (mac80211)"
 	depends on CFG80211
 	select CRYPTO
-	select CRYPTO_ARC4
+	select CRYPTO_LIB_ARC4
 	select CRYPTO_AES
 	select CRYPTO_CCM
 	select CRYPTO_GCM
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a1973a26c7fc..9d8a8878a487 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -402,9 +402,6 @@ static int ieee80211_add_key(struct wiphy *wiphy, struct net_device *dev,
 	case WLAN_CIPHER_SUITE_WEP40:
 	case WLAN_CIPHER_SUITE_TKIP:
 	case WLAN_CIPHER_SUITE_WEP104:
-		if (IS_ERR(local->wep_tx_tfm))
-			return -EINVAL;
-		break;
 	case WLAN_CIPHER_SUITE_CCMP:
 	case WLAN_CIPHER_SUITE_CCMP_256:
 	case WLAN_CIPHER_SUITE_AES_CMAC:
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 073a8235ae1b..412da8cfbc36 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1258,8 +1258,8 @@ struct ieee80211_local {
 
 	struct rate_control_ref *rate_ctrl;
 
-	struct crypto_cipher *wep_tx_tfm;
-	struct crypto_cipher *wep_rx_tfm;
+	struct arc4_ctx wep_tx_ctx;
+	struct arc4_ctx wep_rx_ctx;
 	u32 wep_iv;
 
 	/* see iface.c */
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index f06fbd03d235..6c5bbaebd02c 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -14,6 +14,7 @@
 #include <linux/list.h>
 #include <linux/crypto.h>
 #include <linux/rcupdate.h>
+#include <crypto/arc4.h>
 #include <net/mac80211.h>
 
 #define NUM_DEFAULT_KEYS 4
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 2b608044ae23..93c4a2d0623e 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -13,6 +13,7 @@
 
 #include <net/mac80211.h>
 #include <linux/module.h>
+#include <linux/fips.h>
 #include <linux/init.h>
 #include <linux/netdevice.h>
 #include <linux/types.h>
@@ -733,8 +734,7 @@ EXPORT_SYMBOL(ieee80211_alloc_hw_nm);
 
 static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 {
-	bool have_wep = !(IS_ERR(local->wep_tx_tfm) ||
-			  IS_ERR(local->wep_rx_tfm));
+	bool have_wep = !fips_enabled; /* FIPS does not permit the use of RC4 */
 	bool have_mfp = ieee80211_hw_check(&local->hw, MFP_CAPABLE);
 	int n_suites = 0, r = 0, w = 0;
 	u32 *suites;
@@ -1301,7 +1301,6 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
  fail_rate:
 	rtnl_unlock();
 	ieee80211_led_exit(local);
-	ieee80211_wep_free(local);
  fail_flows:
 	destroy_workqueue(local->workqueue);
  fail_workqueue:
@@ -1358,7 +1357,6 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 
 	destroy_workqueue(local->workqueue);
 	wiphy_unregister(local->hw.wiphy);
-	ieee80211_wep_free(local);
 	ieee80211_led_exit(local);
 	kfree(local->int_scan_req);
 }
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b7a9fe3d5fcb..cf8b87cfd619 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5038,8 +5038,6 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
 		auth_alg = WLAN_AUTH_OPEN;
 		break;
 	case NL80211_AUTHTYPE_SHARED_KEY:
-		if (IS_ERR(local->wep_tx_tfm))
-			return -EOPNOTSUPP;
 		auth_alg = WLAN_AUTH_SHARED_KEY;
 		break;
 	case NL80211_AUTHTYPE_FT:
diff --git a/net/mac80211/tkip.c b/net/mac80211/tkip.c
index b3622823bad2..96b87fc7122e 100644
--- a/net/mac80211/tkip.c
+++ b/net/mac80211/tkip.c
@@ -222,7 +222,7 @@ EXPORT_SYMBOL(ieee80211_get_tkip_p2k);
  * @payload_len is the length of payload (_not_ including IV/ICV length).
  * @ta is the transmitter addresses.
  */
-int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
+int ieee80211_tkip_encrypt_data(struct arc4_ctx *ctx,
 				struct ieee80211_key *key,
 				struct sk_buff *skb,
 				u8 *payload, size_t payload_len)
@@ -231,7 +231,7 @@ int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
 
 	ieee80211_get_tkip_p2k(&key->conf, skb, rc4key);
 
-	return ieee80211_wep_encrypt_data(tfm, rc4key, 16,
+	return ieee80211_wep_encrypt_data(ctx, rc4key, 16,
 					  payload, payload_len);
 }
 
@@ -239,7 +239,7 @@ int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
  * beginning of the buffer containing IEEE 802.11 header payload, i.e.,
  * including IV, Ext. IV, real data, Michael MIC, ICV. @payload_len is the
  * length of payload, including IV, Ext. IV, MIC, ICV.  */
-int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
+int ieee80211_tkip_decrypt_data(struct arc4_ctx *ctx,
 				struct ieee80211_key *key,
 				u8 *payload, size_t payload_len, u8 *ta,
 				u8 *ra, int only_iv, int queue,
@@ -297,7 +297,7 @@ int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
 
 	tkip_mixing_phase2(tk, &rx_ctx->ctx, iv16, rc4key);
 
-	res = ieee80211_wep_decrypt_data(tfm, rc4key, 16, pos, payload_len - 12);
+	res = ieee80211_wep_decrypt_data(ctx, rc4key, 16, pos, payload_len - 12);
  done:
 	if (res == TKIP_DECRYPT_OK) {
 		/*
diff --git a/net/mac80211/tkip.h b/net/mac80211/tkip.h
index a1bcbfbefe7c..798583056201 100644
--- a/net/mac80211/tkip.h
+++ b/net/mac80211/tkip.h
@@ -13,7 +13,7 @@
 #include <linux/crypto.h>
 #include "key.h"
 
-int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
+int ieee80211_tkip_encrypt_data(struct arc4_ctx *ctx,
 				struct ieee80211_key *key,
 				struct sk_buff *skb,
 				u8 *payload, size_t payload_len);
@@ -24,7 +24,7 @@ enum {
 	TKIP_DECRYPT_INVALID_KEYIDX = -2,
 	TKIP_DECRYPT_REPLAY = -3,
 };
-int ieee80211_tkip_decrypt_data(struct crypto_cipher *tfm,
+int ieee80211_tkip_decrypt_data(struct arc4_ctx *ctx,
 				struct ieee80211_key *key,
 				u8 *payload, size_t payload_len, u8 *ta,
 				u8 *ra, int only_iv, int queue,
diff --git a/net/mac80211/wep.c b/net/mac80211/wep.c
index bfe9ed9f4c48..4373c62ecce2 100644
--- a/net/mac80211/wep.c
+++ b/net/mac80211/wep.c
@@ -30,30 +30,9 @@ int ieee80211_wep_init(struct ieee80211_local *local)
 	/* start WEP IV from a random value */
 	get_random_bytes(&local->wep_iv, IEEE80211_WEP_IV_LEN);
 
-	local->wep_tx_tfm = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(local->wep_tx_tfm)) {
-		local->wep_rx_tfm = ERR_PTR(-EINVAL);
-		return PTR_ERR(local->wep_tx_tfm);
-	}
-
-	local->wep_rx_tfm = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(local->wep_rx_tfm)) {
-		crypto_free_cipher(local->wep_tx_tfm);
-		local->wep_tx_tfm = ERR_PTR(-EINVAL);
-		return PTR_ERR(local->wep_rx_tfm);
-	}
-
 	return 0;
 }
 
-void ieee80211_wep_free(struct ieee80211_local *local)
-{
-	if (!IS_ERR(local->wep_tx_tfm))
-		crypto_free_cipher(local->wep_tx_tfm);
-	if (!IS_ERR(local->wep_rx_tfm))
-		crypto_free_cipher(local->wep_rx_tfm);
-}
-
 static inline bool ieee80211_wep_weak_iv(u32 iv, int keylen)
 {
 	/*
@@ -131,21 +110,16 @@ static void ieee80211_wep_remove_iv(struct ieee80211_local *local,
 /* Perform WEP encryption using given key. data buffer must have tailroom
  * for 4-byte ICV. data_len must not include this ICV. Note: this function
  * does _not_ add IV. data = RC4(data | CRC32(data)) */
-int ieee80211_wep_encrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
+int ieee80211_wep_encrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
 			       size_t klen, u8 *data, size_t data_len)
 {
 	__le32 icv;
-	int i;
-
-	if (IS_ERR(tfm))
-		return -1;
 
 	icv = cpu_to_le32(~crc32_le(~0, data, data_len));
 	put_unaligned(icv, (__le32 *)(data + data_len));
 
-	crypto_cipher_setkey(tfm, rc4key, klen);
-	for (i = 0; i < data_len + IEEE80211_WEP_ICV_LEN; i++)
-		crypto_cipher_encrypt_one(tfm, data + i, data + i);
+	arc4_setkey(ctx, rc4key, klen);
+	arc4_crypt(ctx, data, data, data_len + IEEE80211_WEP_ICV_LEN);
 
 	return 0;
 }
@@ -184,7 +158,7 @@ int ieee80211_wep_encrypt(struct ieee80211_local *local,
 	/* Add room for ICV */
 	skb_put(skb, IEEE80211_WEP_ICV_LEN);
 
-	return ieee80211_wep_encrypt_data(local->wep_tx_tfm, rc4key, keylen + 3,
+	return ieee80211_wep_encrypt_data(&local->wep_tx_ctx, rc4key, keylen + 3,
 					  iv + IEEE80211_WEP_IV_LEN, len);
 }
 
@@ -192,18 +166,13 @@ int ieee80211_wep_encrypt(struct ieee80211_local *local,
 /* Perform WEP decryption using given key. data buffer includes encrypted
  * payload, including 4-byte ICV, but _not_ IV. data_len must not include ICV.
  * Return 0 on success and -1 on ICV mismatch. */
-int ieee80211_wep_decrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
+int ieee80211_wep_decrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
 			       size_t klen, u8 *data, size_t data_len)
 {
 	__le32 crc;
-	int i;
-
-	if (IS_ERR(tfm))
-		return -1;
 
-	crypto_cipher_setkey(tfm, rc4key, klen);
-	for (i = 0; i < data_len + IEEE80211_WEP_ICV_LEN; i++)
-		crypto_cipher_decrypt_one(tfm, data + i, data + i);
+	arc4_setkey(ctx, rc4key, klen);
+	arc4_crypt(ctx, data, data, data_len + IEEE80211_WEP_ICV_LEN);
 
 	crc = cpu_to_le32(~crc32_le(~0, data, data_len));
 	if (memcmp(&crc, data + data_len, IEEE80211_WEP_ICV_LEN) != 0)
@@ -256,7 +225,7 @@ static int ieee80211_wep_decrypt(struct ieee80211_local *local,
 	/* Copy rest of the WEP key (the secret part) */
 	memcpy(rc4key + 3, key->conf.key, key->conf.keylen);
 
-	if (ieee80211_wep_decrypt_data(local->wep_rx_tfm, rc4key, klen,
+	if (ieee80211_wep_decrypt_data(&local->wep_rx_ctx, rc4key, klen,
 				       skb->data + hdrlen +
 				       IEEE80211_WEP_IV_LEN, len))
 		ret = -1;
diff --git a/net/mac80211/wep.h b/net/mac80211/wep.h
index 9615749d1f65..f752f5b78713 100644
--- a/net/mac80211/wep.h
+++ b/net/mac80211/wep.h
@@ -18,12 +18,12 @@
 
 int ieee80211_wep_init(struct ieee80211_local *local);
 void ieee80211_wep_free(struct ieee80211_local *local);
-int ieee80211_wep_encrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
+int ieee80211_wep_encrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
 				size_t klen, u8 *data, size_t data_len);
 int ieee80211_wep_encrypt(struct ieee80211_local *local,
 			  struct sk_buff *skb,
 			  const u8 *key, int keylen, int keyidx);
-int ieee80211_wep_decrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
+int ieee80211_wep_decrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
 			       size_t klen, u8 *data, size_t data_len);
 
 ieee80211_rx_result
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 58d0b258b684..02e8ab7b2b4c 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -242,7 +242,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 	/* Add room for ICV */
 	skb_put(skb, IEEE80211_TKIP_ICV_LEN);
 
-	return ieee80211_tkip_encrypt_data(tx->local->wep_tx_tfm,
+	return ieee80211_tkip_encrypt_data(&tx->local->wep_tx_ctx,
 					   key, skb, pos, len);
 }
 
@@ -293,7 +293,7 @@ ieee80211_crypto_tkip_decrypt(struct ieee80211_rx_data *rx)
 	if (status->flag & RX_FLAG_DECRYPTED)
 		hwaccel = 1;
 
-	res = ieee80211_tkip_decrypt_data(rx->local->wep_rx_tfm,
+	res = ieee80211_tkip_decrypt_data(&rx->local->wep_rx_ctx,
 					  key, skb->data + hdrlen,
 					  skb->len - hdrlen, rx->sta->sta.addr,
 					  hdr->addr1, hwaccel, rx->security_idx,
-- 
2.20.1

[PATCH v3 3/7] net/lib80211: move WEP handling to ARC4 library code

[Ard Biesheuvel]

The crypto API abstraction is not very useful for invoking ciphers
directly, especially in the case of arc4, which only has a generic
implementation in C. So let's invoke the library code directly.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/wireless/Kconfig              |  1 +
 net/wireless/lib80211_crypt_wep.c | 49 +++++---------------
 2 files changed, 13 insertions(+), 37 deletions(-)

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 6310ddede220..6d9c48cea07e 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -213,6 +213,7 @@ config LIB80211
 
 config LIB80211_CRYPT_WEP
 	tristate
+	select CRYPTO_LIB_ARC4
 
 config LIB80211_CRYPT_CCMP
 	tristate
diff --git a/net/wireless/lib80211_crypt_wep.c b/net/wireless/lib80211_crypt_wep.c
index 20c1ad63ad44..9a4e4653fe64 100644
--- a/net/wireless/lib80211_crypt_wep.c
+++ b/net/wireless/lib80211_crypt_wep.c
@@ -11,6 +11,7 @@
  */
 
 #include <linux/err.h>
+#include <linux/fips.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -22,7 +23,7 @@
 
 #include <net/lib80211.h>
 
-#include <linux/crypto.h>
+#include <crypto/arc4.h>
 #include <linux/crc32.h>
 
 MODULE_AUTHOR("Jouni Malinen");
@@ -35,51 +36,30 @@ struct lib80211_wep_data {
 	u8 key[WEP_KEY_LEN + 1];
 	u8 key_len;
 	u8 key_idx;
-	struct crypto_cipher *tx_tfm;
-	struct crypto_cipher *rx_tfm;
+	struct arc4_ctx tx_ctx;
+	struct arc4_ctx rx_ctx;
 };
 
 static void *lib80211_wep_init(int keyidx)
 {
 	struct lib80211_wep_data *priv;
 
+	if (fips_enabled)
+		return NULL;
+
 	priv = kzalloc(sizeof(*priv), GFP_ATOMIC);
 	if (priv == NULL)
-		goto fail;
+		return NULL;
 	priv->key_idx = keyidx;
 
-	priv->tx_tfm = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(priv->tx_tfm)) {
-		priv->tx_tfm = NULL;
-		goto fail;
-	}
-
-	priv->rx_tfm = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(priv->rx_tfm)) {
-		priv->rx_tfm = NULL;
-		goto fail;
-	}
 	/* start WEP IV from a random value */
 	get_random_bytes(&priv->iv, 4);
 
 	return priv;
-
-      fail:
-	if (priv) {
-		crypto_free_cipher(priv->tx_tfm);
-		crypto_free_cipher(priv->rx_tfm);
-		kfree(priv);
-	}
-	return NULL;
 }
 
 static void lib80211_wep_deinit(void *priv)
 {
-	struct lib80211_wep_data *_priv = priv;
-	if (_priv) {
-		crypto_free_cipher(_priv->tx_tfm);
-		crypto_free_cipher(_priv->rx_tfm);
-	}
 	kfree(priv);
 }
 
@@ -132,7 +112,6 @@ static int lib80211_wep_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	u32 crc, klen, len;
 	u8 *pos, *icv;
 	u8 key[WEP_KEY_LEN + 3];
-	int i;
 
 	/* other checks are in lib80211_wep_build_iv */
 	if (skb_tailroom(skb) < 4)
@@ -160,10 +139,8 @@ static int lib80211_wep_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	icv[2] = crc >> 16;
 	icv[3] = crc >> 24;
 
-	crypto_cipher_setkey(wep->tx_tfm, key, klen);
-
-	for (i = 0; i < len + 4; i++)
-		crypto_cipher_encrypt_one(wep->tx_tfm, pos + i, pos + i);
+	arc4_setkey(&wep->tx_ctx, key, klen);
+	arc4_crypt(&wep->tx_ctx, pos, pos, len + 4);
 
 	return 0;
 }
@@ -181,7 +158,6 @@ static int lib80211_wep_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	u32 crc, klen, plen;
 	u8 key[WEP_KEY_LEN + 3];
 	u8 keyidx, *pos, icv[4];
-	int i;
 
 	if (skb->len < hdr_len + 8)
 		return -1;
@@ -202,9 +178,8 @@ static int lib80211_wep_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	/* Apply RC4 to data and compute CRC32 over decrypted data */
 	plen = skb->len - hdr_len - 8;
 
-	crypto_cipher_setkey(wep->rx_tfm, key, klen);
-	for (i = 0; i < plen + 4; i++)
-		crypto_cipher_decrypt_one(wep->rx_tfm, pos + i, pos + i);
+	arc4_setkey(&wep->rx_ctx, key, klen);
+	arc4_crypt(&wep->rx_ctx, pos, pos, plen + 4);
 
 	crc = ~crc32_le(~0, pos, plen);
 	icv[0] = crc;
-- 
2.20.1

[PATCH v3 4/7] net/lib80211: move TKIP handling to ARC4 library code

[Ard Biesheuvel]

The crypto API abstraction is not very useful for invoking ciphers
directly, especially in the case of arc4, which only has a generic
implementation in C. So let's invoke the library code directly.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/wireless/lib80211_crypt_tkip.c | 46 +++++++-------------
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index 11eaa5956f00..02841825bf73 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -13,6 +13,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/err.h>
+#include <linux/fips.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/slab.h>
@@ -29,6 +30,7 @@
 #include <linux/ieee80211.h>
 #include <net/iw_handler.h>
 
+#include <crypto/arc4.h>
 #include <crypto/hash.h>
 #include <linux/crypto.h>
 #include <linux/crc32.h>
@@ -64,9 +66,9 @@ struct lib80211_tkip_data {
 
 	int key_idx;
 
-	struct crypto_cipher *rx_tfm_arc4;
+	struct arc4_ctx rx_ctx_arc4;
+	struct arc4_ctx tx_ctx_arc4;
 	struct crypto_shash *rx_tfm_michael;
-	struct crypto_cipher *tx_tfm_arc4;
 	struct crypto_shash *tx_tfm_michael;
 
 	/* scratch buffers for virt_to_page() (crypto API) */
@@ -93,30 +95,21 @@ static void *lib80211_tkip_init(int key_idx)
 {
 	struct lib80211_tkip_data *priv;
 
+	if (fips_enabled)
+		return NULL;
+
 	priv = kzalloc(sizeof(*priv), GFP_ATOMIC);
 	if (priv == NULL)
 		goto fail;
 
 	priv->key_idx = key_idx;
 
-	priv->tx_tfm_arc4 = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(priv->tx_tfm_arc4)) {
-		priv->tx_tfm_arc4 = NULL;
-		goto fail;
-	}
-
 	priv->tx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
 	if (IS_ERR(priv->tx_tfm_michael)) {
 		priv->tx_tfm_michael = NULL;
 		goto fail;
 	}
 
-	priv->rx_tfm_arc4 = crypto_alloc_cipher("arc4", 0, 0);
-	if (IS_ERR(priv->rx_tfm_arc4)) {
-		priv->rx_tfm_arc4 = NULL;
-		goto fail;
-	}
-
 	priv->rx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
 	if (IS_ERR(priv->rx_tfm_michael)) {
 		priv->rx_tfm_michael = NULL;
@@ -128,9 +121,7 @@ static void *lib80211_tkip_init(int key_idx)
       fail:
 	if (priv) {
 		crypto_free_shash(priv->tx_tfm_michael);
-		crypto_free_cipher(priv->tx_tfm_arc4);
 		crypto_free_shash(priv->rx_tfm_michael);
-		crypto_free_cipher(priv->rx_tfm_arc4);
 		kfree(priv);
 	}
 
@@ -142,9 +133,7 @@ static void lib80211_tkip_deinit(void *priv)
 	struct lib80211_tkip_data *_priv = priv;
 	if (_priv) {
 		crypto_free_shash(_priv->tx_tfm_michael);
-		crypto_free_cipher(_priv->tx_tfm_arc4);
 		crypto_free_shash(_priv->rx_tfm_michael);
-		crypto_free_cipher(_priv->rx_tfm_arc4);
 	}
 	kfree(priv);
 }
@@ -345,7 +334,6 @@ static int lib80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	int len;
 	u8 rc4key[16], *pos, *icv;
 	u32 crc;
-	int i;
 
 	if (tkey->flags & IEEE80211_CRYPTO_TKIP_COUNTERMEASURES) {
 		struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
@@ -370,9 +358,9 @@ static int lib80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	icv[2] = crc >> 16;
 	icv[3] = crc >> 24;
 
-	crypto_cipher_setkey(tkey->tx_tfm_arc4, rc4key, 16);
-	for (i = 0; i < len + 4; i++)
-		crypto_cipher_encrypt_one(tkey->tx_tfm_arc4, pos + i, pos + i);
+	arc4_setkey(&tkey->tx_ctx_arc4, rc4key, 16);
+	arc4_crypt(&tkey->tx_ctx_arc4, pos, pos, len + 4);
+
 	return 0;
 }
 
@@ -400,7 +388,6 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 	u8 icv[4];
 	u32 crc;
 	int plen;
-	int i;
 
 	hdr = (struct ieee80211_hdr *)skb->data;
 
@@ -453,9 +440,8 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
 
 	plen = skb->len - hdr_len - 12;
 
-	crypto_cipher_setkey(tkey->rx_tfm_arc4, rc4key, 16);
-	for (i = 0; i < plen + 4; i++)
-		crypto_cipher_decrypt_one(tkey->rx_tfm_arc4, pos + i, pos + i);
+	arc4_setkey(&tkey->rx_ctx_arc4, rc4key, 16);
+	arc4_crypt(&tkey->rx_ctx_arc4, pos, pos, plen + 4);
 
 	crc = ~crc32_le(~0, pos, plen);
 	icv[0] = crc;
@@ -640,17 +626,17 @@ static int lib80211_tkip_set_key(void *key, int len, u8 * seq, void *priv)
 	struct lib80211_tkip_data *tkey = priv;
 	int keyidx;
 	struct crypto_shash *tfm = tkey->tx_tfm_michael;
-	struct crypto_cipher *tfm2 = tkey->tx_tfm_arc4;
+	struct arc4_ctx *tfm2 = &tkey->tx_ctx_arc4;
 	struct crypto_shash *tfm3 = tkey->rx_tfm_michael;
-	struct crypto_cipher *tfm4 = tkey->rx_tfm_arc4;
+	struct arc4_ctx *tfm4 = &tkey->rx_ctx_arc4;
 
 	keyidx = tkey->key_idx;
 	memset(tkey, 0, sizeof(*tkey));
 	tkey->key_idx = keyidx;
 	tkey->tx_tfm_michael = tfm;
-	tkey->tx_tfm_arc4 = tfm2;
+	tkey->tx_ctx_arc4 = *tfm2;
 	tkey->rx_tfm_michael = tfm3;
-	tkey->rx_tfm_arc4 = tfm4;
+	tkey->rx_ctx_arc4 = *tfm4;
 	if (len == TKIP_KEY_LEN) {
 		memcpy(tkey->key, key, TKIP_KEY_LEN);
 		tkey->key_set = 1;
-- 
2.20.1

[PATCH v3 5/7] crypto: arc4 - remove cipher implementation

[Ard Biesheuvel]

There are no remaining users of the cipher implementation, and there
are no meaningful ways in which the arc4 cipher can be combined with
templates other than ECB (and the way we do provide that combination
is highly dubious to begin with).

So let's drop the arc4 cipher altogether, and only keep the ecb(arc4)
skcipher, which is used in various places in the kernel.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 crypto/arc4.c | 46 ++------------------
 1 file changed, 4 insertions(+), 42 deletions(-)

diff --git a/crypto/arc4.c b/crypto/arc4.c
index 6974dba1b7b9..79a51e9f90ae 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -13,23 +13,12 @@
 #include <linux/init.h>
 #include <linux/module.h>
 
-static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
-			unsigned int key_len)
-{
-	struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
-
-	return arc4_setkey(ctx, in_key, key_len);
-}
-
 static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
 				 unsigned int key_len)
 {
-	return arc4_set_key(&tfm->base, in_key, key_len);
-}
+	struct arc4_ctx *ctx = crypto_tfm_ctx(&tfm->base);
 
-static void arc4_crypt_one(struct crypto_tfm *tfm, u8 *out, const u8 *in)
-{
-	arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
+	return arc4_setkey(ctx, in_key, key_len);
 }
 
 static int ecb_arc4_crypt(struct skcipher_request *req)
@@ -50,23 +39,6 @@ static int ecb_arc4_crypt(struct skcipher_request *req)
 	return err;
 }
 
-static struct crypto_alg arc4_cipher = {
-	.cra_name		=	"arc4",
-	.cra_flags		=	CRYPTO_ALG_TYPE_CIPHER,
-	.cra_blocksize		=	ARC4_BLOCK_SIZE,
-	.cra_ctxsize		=	sizeof(struct arc4_ctx),
-	.cra_module		=	THIS_MODULE,
-	.cra_u			=	{
-		.cipher = {
-			.cia_min_keysize	=	ARC4_MIN_KEY_SIZE,
-			.cia_max_keysize	=	ARC4_MAX_KEY_SIZE,
-			.cia_setkey		=	arc4_set_key,
-			.cia_encrypt		=	arc4_crypt_one,
-			.cia_decrypt		=	arc4_crypt_one,
-		},
-	},
-};
-
 static struct skcipher_alg arc4_skcipher = {
 	.base.cra_name		=	"ecb(arc4)",
 	.base.cra_priority	=	100,
@@ -82,21 +54,11 @@ static struct skcipher_alg arc4_skcipher = {
 
 static int __init arc4_init(void)
 {
-	int err;
-
-	err = crypto_register_alg(&arc4_cipher);
-	if (err)
-		return err;
-
-	err = crypto_register_skcipher(&arc4_skcipher);
-	if (err)
-		crypto_unregister_alg(&arc4_cipher);
-	return err;
+	return crypto_register_skcipher(&arc4_skcipher);
 }
 
 static void __exit arc4_exit(void)
 {
-	crypto_unregister_alg(&arc4_cipher);
 	crypto_unregister_skcipher(&arc4_skcipher);
 }
 
@@ -106,4 +68,4 @@ module_exit(arc4_exit);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("ARC4 Cipher Algorithm");
 MODULE_AUTHOR("Jon Oberheide <jon@oberheide.org>");
-MODULE_ALIAS_CRYPTO("arc4");
+MODULE_ALIAS_CRYPTO("ecb(arc4)");
-- 
2.20.1

[PATCH v3 6/7] ppp: mppe: switch to RC4 library interface

[Ard Biesheuvel]

The MPPE code uses the sync skcipher 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.

Note that the SHA1 shash used by this driver has several accelerated
implementations for various architectures, so retaining that part does
make sense.

Cc: linux-ppp@vger.kernel.org
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/net/ppp/Kconfig    |  3 +-
 drivers/net/ppp/ppp_mppe.c | 95 +++-----------------
 2 files changed, 14 insertions(+), 84 deletions(-)

diff --git a/drivers/net/ppp/Kconfig b/drivers/net/ppp/Kconfig
index bf395df3bb37..1a2e2f7629f3 100644
--- a/drivers/net/ppp/Kconfig
+++ b/drivers/net/ppp/Kconfig
@@ -87,8 +87,7 @@ config PPP_MPPE
 	depends on PPP
 	select CRYPTO
 	select CRYPTO_SHA1
-	select CRYPTO_ARC4
-	select CRYPTO_ECB
+	select CRYPTO_LIB_ARC4
 	---help---
 	  Support for the MPPE Encryption protocol, as employed by the
 	  Microsoft Point-to-Point Tunneling Protocol.
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index ff61dd8748de..a2040a156f40 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -42,9 +42,10 @@
  *                    deprecated in 2.6
  */
 
+#include <crypto/arc4.h>
 #include <crypto/hash.h>
-#include <crypto/skcipher.h>
 #include <linux/err.h>
+#include <linux/fips.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -65,13 +66,6 @@ MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE));
 MODULE_VERSION("1.0.2");
 
-static unsigned int
-setup_sg(struct scatterlist *sg, const void *address, unsigned int length)
-{
-	sg_set_buf(sg, address, length);
-	return length;
-}
-
 #define SHA1_PAD_SIZE 40
 
 /*
@@ -95,7 +89,7 @@ static inline void sha_pad_init(struct sha_pad *shapad)
  * State for an MPPE (de)compressor.
  */
 struct ppp_mppe_state {
-	struct crypto_sync_skcipher *arc4;
+	struct arc4_ctx arc4;
 	struct shash_desc *sha1;
 	unsigned char *sha1_digest;
 	unsigned char master_key[MPPE_MAX_KEY_LEN];
@@ -154,24 +148,11 @@ static void get_new_key_from_sha(struct ppp_mppe_state * state)
  */
 static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
 {
-	struct scatterlist sg_in[1], sg_out[1];
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-
 	get_new_key_from_sha(state);
 	if (!initial_key) {
-		crypto_sync_skcipher_setkey(state->arc4, state->sha1_digest,
-					    state->keylen);
-		sg_init_table(sg_in, 1);
-		sg_init_table(sg_out, 1);
-		setup_sg(sg_in, state->sha1_digest, state->keylen);
-		setup_sg(sg_out, state->session_key, state->keylen);
-		skcipher_request_set_crypt(req, sg_in, sg_out, state->keylen,
-					   NULL);
-		if (crypto_skcipher_encrypt(req))
-    		    printk(KERN_WARNING "mppe_rekey: cipher_encrypt failed\n");
+		arc4_setkey(&state->arc4, state->sha1_digest, state->keylen);
+		arc4_crypt(&state->arc4, state->session_key, state->sha1_digest,
+			   state->keylen);
 	} else {
 		memcpy(state->session_key, state->sha1_digest, state->keylen);
 	}
@@ -181,9 +162,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
 		state->session_key[1] = 0x26;
 		state->session_key[2] = 0x9e;
 	}
-	crypto_sync_skcipher_setkey(state->arc4, state->session_key,
-				    state->keylen);
-	skcipher_request_zero(req);
+	arc4_setkey(&state->arc4, state->session_key, state->keylen);
 }
 
 /*
@@ -196,7 +175,8 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 	unsigned int digestsize;
 
 	if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
-	    options[0] != CI_MPPE || options[1] != CILEN_MPPE)
+	    options[0] != CI_MPPE || options[1] != CILEN_MPPE ||
+	    fips_enabled)
 		goto out;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
@@ -204,12 +184,6 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 		goto out;
 
 
-	state->arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-	if (IS_ERR(state->arc4)) {
-		state->arc4 = NULL;
-		goto out_free;
-	}
-
 	shash = crypto_alloc_shash("sha1", 0, 0);
 	if (IS_ERR(shash))
 		goto out_free;
@@ -250,7 +224,6 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 		crypto_free_shash(state->sha1->tfm);
 		kzfree(state->sha1);
 	}
-	crypto_free_sync_skcipher(state->arc4);
 	kfree(state);
 out:
 	return NULL;
@@ -266,7 +239,6 @@ static void mppe_free(void *arg)
 		kfree(state->sha1_digest);
 		crypto_free_shash(state->sha1->tfm);
 		kzfree(state->sha1);
-		crypto_free_sync_skcipher(state->arc4);
 		kfree(state);
 	}
 }
@@ -366,10 +338,7 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 	      int isize, int osize)
 {
 	struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
 	int proto;
-	int err;
-	struct scatterlist sg_in[1], sg_out[1];
 
 	/*
 	 * Check that the protocol is in the range we handle.
@@ -420,21 +389,7 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 	ibuf += 2;		/* skip to proto field */
 	isize -= 2;
 
-	/* Encrypt packet */
-	sg_init_table(sg_in, 1);
-	sg_init_table(sg_out, 1);
-	setup_sg(sg_in, ibuf, isize);
-	setup_sg(sg_out, obuf, osize);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-	skcipher_request_set_crypt(req, sg_in, sg_out, isize, NULL);
-	err = crypto_skcipher_encrypt(req);
-	skcipher_request_zero(req);
-	if (err) {
-		printk(KERN_DEBUG "crypto_cypher_encrypt failed\n");
-		return -1;
-	}
+	arc4_crypt(&state->arc4, obuf, ibuf, isize);
 
 	state->stats.unc_bytes += isize;
 	state->stats.unc_packets++;
@@ -480,10 +435,8 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 		int osize)
 {
 	struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
 	unsigned ccount;
 	int flushed = MPPE_BITS(ibuf) & MPPE_BIT_FLUSHED;
-	struct scatterlist sg_in[1], sg_out[1];
 
 	if (isize <= PPP_HDRLEN + MPPE_OVHD) {
 		if (state->debug)
@@ -610,19 +563,7 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	 * Decrypt the first byte in order to check if it is
 	 * a compressed or uncompressed protocol field.
 	 */
-	sg_init_table(sg_in, 1);
-	sg_init_table(sg_out, 1);
-	setup_sg(sg_in, ibuf, 1);
-	setup_sg(sg_out, obuf, 1);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-	skcipher_request_set_crypt(req, sg_in, sg_out, 1, NULL);
-	if (crypto_skcipher_decrypt(req)) {
-		printk(KERN_DEBUG "crypto_cypher_decrypt failed\n");
-		osize = DECOMP_ERROR;
-		goto out_zap_req;
-	}
+	arc4_crypt(&state->arc4, obuf, ibuf, 1);
 
 	/*
 	 * Do PFC decompression.
@@ -637,14 +578,7 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	}
 
 	/* And finally, decrypt the rest of the packet. */
-	setup_sg(sg_in, ibuf + 1, isize - 1);
-	setup_sg(sg_out, obuf + 1, osize - 1);
-	skcipher_request_set_crypt(req, sg_in, sg_out, isize - 1, NULL);
-	if (crypto_skcipher_decrypt(req)) {
-		printk(KERN_DEBUG "crypto_cypher_decrypt failed\n");
-		osize = DECOMP_ERROR;
-		goto out_zap_req;
-	}
+	arc4_crypt(&state->arc4, obuf + 1, ibuf + 1, isize - 1);
 
 	state->stats.unc_bytes += osize;
 	state->stats.unc_packets++;
@@ -654,8 +588,6 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	/* good packet credit */
 	state->sanity_errors >>= 1;
 
-out_zap_req:
-	skcipher_request_zero(req);
 	return osize;
 
 sanity_error:
@@ -728,8 +660,7 @@ static struct compressor ppp_mppe = {
 static int __init ppp_mppe_init(void)
 {
 	int answer;
-	if (!(crypto_has_skcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC) &&
-	      crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC)))
+	if (!crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC))
 		return -ENODEV;
 
 	sha_pad = kmalloc(sizeof(struct sha_pad), GFP_KERNEL);
-- 
2.20.1

[PATCH v3 6/7] ppp: mppe: switch to RC4 library interface

[Ard Biesheuvel]

The MPPE code uses the sync skcipher 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.

Note that the SHA1 shash used by this driver has several accelerated
implementations for various architectures, so retaining that part does
make sense.

Cc: linux-ppp@vger.kernel.org
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/net/ppp/Kconfig    |  3 +-
 drivers/net/ppp/ppp_mppe.c | 95 +++-----------------
 2 files changed, 14 insertions(+), 84 deletions(-)

diff --git a/drivers/net/ppp/Kconfig b/drivers/net/ppp/Kconfig
index bf395df3bb37..1a2e2f7629f3 100644
--- a/drivers/net/ppp/Kconfig
+++ b/drivers/net/ppp/Kconfig
@@ -87,8 +87,7 @@ config PPP_MPPE
 	depends on PPP
 	select CRYPTO
 	select CRYPTO_SHA1
-	select CRYPTO_ARC4
-	select CRYPTO_ECB
+	select CRYPTO_LIB_ARC4
 	---help---
 	  Support for the MPPE Encryption protocol, as employed by the
 	  Microsoft Point-to-Point Tunneling Protocol.
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index ff61dd8748de..a2040a156f40 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -42,9 +42,10 @@
  *                    deprecated in 2.6
  */
 
+#include <crypto/arc4.h>
 #include <crypto/hash.h>
-#include <crypto/skcipher.h>
 #include <linux/err.h>
+#include <linux/fips.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
@@ -65,13 +66,6 @@ MODULE_LICENSE("Dual BSD/GPL");
 MODULE_ALIAS("ppp-compress-" __stringify(CI_MPPE));
 MODULE_VERSION("1.0.2");
 
-static unsigned int
-setup_sg(struct scatterlist *sg, const void *address, unsigned int length)
-{
-	sg_set_buf(sg, address, length);
-	return length;
-}
-
 #define SHA1_PAD_SIZE 40
 
 /*
@@ -95,7 +89,7 @@ static inline void sha_pad_init(struct sha_pad *shapad)
  * State for an MPPE (de)compressor.
  */
 struct ppp_mppe_state {
-	struct crypto_sync_skcipher *arc4;
+	struct arc4_ctx arc4;
 	struct shash_desc *sha1;
 	unsigned char *sha1_digest;
 	unsigned char master_key[MPPE_MAX_KEY_LEN];
@@ -154,24 +148,11 @@ static void get_new_key_from_sha(struct ppp_mppe_state * state)
  */
 static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
 {
-	struct scatterlist sg_in[1], sg_out[1];
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-
 	get_new_key_from_sha(state);
 	if (!initial_key) {
-		crypto_sync_skcipher_setkey(state->arc4, state->sha1_digest,
-					    state->keylen);
-		sg_init_table(sg_in, 1);
-		sg_init_table(sg_out, 1);
-		setup_sg(sg_in, state->sha1_digest, state->keylen);
-		setup_sg(sg_out, state->session_key, state->keylen);
-		skcipher_request_set_crypt(req, sg_in, sg_out, state->keylen,
-					   NULL);
-		if (crypto_skcipher_encrypt(req))
-    		    printk(KERN_WARNING "mppe_rekey: cipher_encrypt failed\n");
+		arc4_setkey(&state->arc4, state->sha1_digest, state->keylen);
+		arc4_crypt(&state->arc4, state->session_key, state->sha1_digest,
+			   state->keylen);
 	} else {
 		memcpy(state->session_key, state->sha1_digest, state->keylen);
 	}
@@ -181,9 +162,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
 		state->session_key[1] = 0x26;
 		state->session_key[2] = 0x9e;
 	}
-	crypto_sync_skcipher_setkey(state->arc4, state->session_key,
-				    state->keylen);
-	skcipher_request_zero(req);
+	arc4_setkey(&state->arc4, state->session_key, state->keylen);
 }
 
 /*
@@ -196,7 +175,8 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 	unsigned int digestsize;
 
 	if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
-	    options[0] != CI_MPPE || options[1] != CILEN_MPPE)
+	    options[0] != CI_MPPE || options[1] != CILEN_MPPE ||
+	    fips_enabled)
 		goto out;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
@@ -204,12 +184,6 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 		goto out;
 
 
-	state->arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-	if (IS_ERR(state->arc4)) {
-		state->arc4 = NULL;
-		goto out_free;
-	}
-
 	shash = crypto_alloc_shash("sha1", 0, 0);
 	if (IS_ERR(shash))
 		goto out_free;
@@ -250,7 +224,6 @@ static void *mppe_alloc(unsigned char *options, int optlen)
 		crypto_free_shash(state->sha1->tfm);
 		kzfree(state->sha1);
 	}
-	crypto_free_sync_skcipher(state->arc4);
 	kfree(state);
 out:
 	return NULL;
@@ -266,7 +239,6 @@ static void mppe_free(void *arg)
 		kfree(state->sha1_digest);
 		crypto_free_shash(state->sha1->tfm);
 		kzfree(state->sha1);
-		crypto_free_sync_skcipher(state->arc4);
 		kfree(state);
 	}
 }
@@ -366,10 +338,7 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 	      int isize, int osize)
 {
 	struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
 	int proto;
-	int err;
-	struct scatterlist sg_in[1], sg_out[1];
 
 	/*
 	 * Check that the protocol is in the range we handle.
@@ -420,21 +389,7 @@ mppe_compress(void *arg, unsigned char *ibuf, unsigned char *obuf,
 	ibuf += 2;		/* skip to proto field */
 	isize -= 2;
 
-	/* Encrypt packet */
-	sg_init_table(sg_in, 1);
-	sg_init_table(sg_out, 1);
-	setup_sg(sg_in, ibuf, isize);
-	setup_sg(sg_out, obuf, osize);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-	skcipher_request_set_crypt(req, sg_in, sg_out, isize, NULL);
-	err = crypto_skcipher_encrypt(req);
-	skcipher_request_zero(req);
-	if (err) {
-		printk(KERN_DEBUG "crypto_cypher_encrypt failed\n");
-		return -1;
-	}
+	arc4_crypt(&state->arc4, obuf, ibuf, isize);
 
 	state->stats.unc_bytes += isize;
 	state->stats.unc_packets++;
@@ -480,10 +435,8 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 		int osize)
 {
 	struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
-	SYNC_SKCIPHER_REQUEST_ON_STACK(req, state->arc4);
 	unsigned ccount;
 	int flushed = MPPE_BITS(ibuf) & MPPE_BIT_FLUSHED;
-	struct scatterlist sg_in[1], sg_out[1];
 
 	if (isize <= PPP_HDRLEN + MPPE_OVHD) {
 		if (state->debug)
@@ -610,19 +563,7 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	 * Decrypt the first byte in order to check if it is
 	 * a compressed or uncompressed protocol field.
 	 */
-	sg_init_table(sg_in, 1);
-	sg_init_table(sg_out, 1);
-	setup_sg(sg_in, ibuf, 1);
-	setup_sg(sg_out, obuf, 1);
-
-	skcipher_request_set_sync_tfm(req, state->arc4);
-	skcipher_request_set_callback(req, 0, NULL, NULL);
-	skcipher_request_set_crypt(req, sg_in, sg_out, 1, NULL);
-	if (crypto_skcipher_decrypt(req)) {
-		printk(KERN_DEBUG "crypto_cypher_decrypt failed\n");
-		osize = DECOMP_ERROR;
-		goto out_zap_req;
-	}
+	arc4_crypt(&state->arc4, obuf, ibuf, 1);
 
 	/*
 	 * Do PFC decompression.
@@ -637,14 +578,7 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	}
 
 	/* And finally, decrypt the rest of the packet. */
-	setup_sg(sg_in, ibuf + 1, isize - 1);
-	setup_sg(sg_out, obuf + 1, osize - 1);
-	skcipher_request_set_crypt(req, sg_in, sg_out, isize - 1, NULL);
-	if (crypto_skcipher_decrypt(req)) {
-		printk(KERN_DEBUG "crypto_cypher_decrypt failed\n");
-		osize = DECOMP_ERROR;
-		goto out_zap_req;
-	}
+	arc4_crypt(&state->arc4, obuf + 1, ibuf + 1, isize - 1);
 
 	state->stats.unc_bytes += osize;
 	state->stats.unc_packets++;
@@ -654,8 +588,6 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
 	/* good packet credit */
 	state->sanity_errors >>= 1;
 
-out_zap_req:
-	skcipher_request_zero(req);
 	return osize;
 
 sanity_error:
@@ -728,8 +660,7 @@ static struct compressor ppp_mppe = {
 static int __init ppp_mppe_init(void)
 {
 	int answer;
-	if (!(crypto_has_skcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC) &&
-	      crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC)))
+	if (!crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC))
 		return -ENODEV;
 
 	sha_pad = kmalloc(sizeof(struct sha_pad), GFP_KERNEL);
-- 
2.20.1

[PATCH v3 7/7] fs: cifs: switch to RC4 library interface

[Ard Biesheuvel]

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
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;
 }
-- 
2.20.1

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Johannes Berg]

On Tue, 2019-06-11 at 15:47 +0200, Ard Biesheuvel wrote:
> 
> +++ b/net/mac80211/mlme.c
> @@ -5038,8 +5038,6 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
>  		auth_alg = WLAN_AUTH_OPEN;
>  		break;
>  	case NL80211_AUTHTYPE_SHARED_KEY:
> -		if (IS_ERR(local->wep_tx_tfm))
> -			return -EOPNOTSUPP;
>  		auth_alg = WLAN_AUTH_SHARED_KEY;

This bit is probably not right, we directly use the WEP functions for
shared key authentication.

johannes

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Ard Biesheuvel]

On Tue, 11 Jun 2019 at 15:52, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Tue, 2019-06-11 at 15:47 +0200, Ard Biesheuvel wrote:
> >
> > +++ b/net/mac80211/mlme.c
> > @@ -5038,8 +5038,6 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
> >               auth_alg = WLAN_AUTH_OPEN;
> >               break;
> >       case NL80211_AUTHTYPE_SHARED_KEY:
> > -             if (IS_ERR(local->wep_tx_tfm))
> > -                     return -EOPNOTSUPP;
> >               auth_alg = WLAN_AUTH_SHARED_KEY;
>
> This bit is probably not right, we directly use the WEP functions for
> shared key authentication.
>

OK. So we need to change this test to

> > -             if (fips_enabled)
> > -                     return -EOPNOTSUPP;

Does this also apply to

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a1973a26c7fc..9d8a8878a487 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -402,9 +402,6 @@ static int ieee80211_add_key(struct wiphy *wiphy,
struct net_device *dev,
  case WLAN_CIPHER_SUITE_WEP40:
  case WLAN_CIPHER_SUITE_TKIP:
  case WLAN_CIPHER_SUITE_WEP104:
-     if (IS_ERR(local->wep_tx_tfm))
-         return -EINVAL;
-     break;
  case WLAN_CIPHER_SUITE_CCMP:
  case WLAN_CIPHER_SUITE_CCMP_256:
  case WLAN_CIPHER_SUITE_AES_CMAC:

?

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Johannes Berg]

On Tue, 2019-06-11 at 15:53 +0200, Ard Biesheuvel wrote:
> On Tue, 11 Jun 2019 at 15:52, Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > On Tue, 2019-06-11 at 15:47 +0200, Ard Biesheuvel wrote:
> > > 
> > > +++ b/net/mac80211/mlme.c
> > > @@ -5038,8 +5038,6 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
> > >               auth_alg = WLAN_AUTH_OPEN;
> > >               break;
> > >       case NL80211_AUTHTYPE_SHARED_KEY:
> > > -             if (IS_ERR(local->wep_tx_tfm))
> > > -                     return -EOPNOTSUPP;
> > >               auth_alg = WLAN_AUTH_SHARED_KEY;
> > 
> > This bit is probably not right, we directly use the WEP functions for
> > shared key authentication.
> > 
> 
> OK. So we need to change this test to
> 
> > > -             if (fips_enabled)
> > > -                     return -EOPNOTSUPP;

Right.

> Does this also apply to
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index a1973a26c7fc..9d8a8878a487 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -402,9 +402,6 @@ static int ieee80211_add_key(struct wiphy *wiphy,
> struct net_device *dev,
>   case WLAN_CIPHER_SUITE_WEP40:
>   case WLAN_CIPHER_SUITE_TKIP:
>   case WLAN_CIPHER_SUITE_WEP104:
> -     if (IS_ERR(local->wep_tx_tfm))
> -         return -EINVAL;
> -     break;

This shouldn't be possible because in cfg80211 we should be checking
that only ciphers are allowed that are in the cipher list (which we
built with the "have_wep" thing before), but perhaps better to be safe
here just in case we forgot something in cfg80211 - I could see e.g. the
old WEXT compatibility code not checking too carefully...

johannes

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Ard Biesheuvel]

On Tue, 11 Jun 2019 at 15:55, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Tue, 2019-06-11 at 15:53 +0200, Ard Biesheuvel wrote:
> > On Tue, 11 Jun 2019 at 15:52, Johannes Berg <johannes@sipsolutions.net> wrote:
> > >
> > > On Tue, 2019-06-11 at 15:47 +0200, Ard Biesheuvel wrote:
> > > >
> > > > +++ b/net/mac80211/mlme.c
> > > > @@ -5038,8 +5038,6 @@ int ieee80211_mgd_auth(struct ieee80211_sub_if_data *sdata,
> > > >               auth_alg = WLAN_AUTH_OPEN;
> > > >               break;
> > > >       case NL80211_AUTHTYPE_SHARED_KEY:
> > > > -             if (IS_ERR(local->wep_tx_tfm))
> > > > -                     return -EOPNOTSUPP;
> > > >               auth_alg = WLAN_AUTH_SHARED_KEY;
> > >
> > > This bit is probably not right, we directly use the WEP functions for
> > > shared key authentication.
> > >
> >
> > OK. So we need to change this test to
> >
> > > > -             if (fips_enabled)
> > > > -                     return -EOPNOTSUPP;
>
> Right.
>
> > Does this also apply to
> >
> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> > index a1973a26c7fc..9d8a8878a487 100644
> > --- a/net/mac80211/cfg.c
> > +++ b/net/mac80211/cfg.c
> > @@ -402,9 +402,6 @@ static int ieee80211_add_key(struct wiphy *wiphy,
> > struct net_device *dev,
> >   case WLAN_CIPHER_SUITE_WEP40:
> >   case WLAN_CIPHER_SUITE_TKIP:
> >   case WLAN_CIPHER_SUITE_WEP104:
> > -     if (IS_ERR(local->wep_tx_tfm))
> > -         return -EINVAL;
> > -     break;
>
> This shouldn't be possible because in cfg80211 we should be checking
> that only ciphers are allowed that are in the cipher list (which we
> built with the "have_wep" thing before), but perhaps better to be safe
> here just in case we forgot something in cfg80211 - I could see e.g. the
> old WEXT compatibility code not checking too carefully...
>

OK. I could make it a WARN_ONCE(fips_enabled) perhaps?

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Johannes Berg]

On Tue, 2019-06-11 at 15:56 +0200, Ard Biesheuvel wrote:
> 
> > > +++ b/net/mac80211/cfg.c
> > > @@ -402,9 +402,6 @@ static int ieee80211_add_key(struct wiphy *wiphy,
> > > struct net_device *dev,
> > >   case WLAN_CIPHER_SUITE_WEP40:
> > >   case WLAN_CIPHER_SUITE_TKIP:
> > >   case WLAN_CIPHER_SUITE_WEP104:
> > > -     if (IS_ERR(local->wep_tx_tfm))
> > > -         return -EINVAL;
> > > -     break;
> > 
> > This shouldn't be possible because in cfg80211 we should be checking
> > that only ciphers are allowed that are in the cipher list (which we
> > built with the "have_wep" thing before), but perhaps better to be safe
> > here just in case we forgot something in cfg80211 - I could see e.g. the
> > old WEXT compatibility code not checking too carefully...
> > 
> 
> OK. I could make it a WARN_ONCE(fips_enabled) perhaps?

Sounds good to me.

johannes

Re: [PATCH v3 5/7] crypto: arc4 - remove cipher implementation

[Eric Biggers]

On Tue, Jun 11, 2019 at 03:47:48PM +0200, Ard Biesheuvel wrote:
> There are no remaining users of the cipher implementation, and there
> are no meaningful ways in which the arc4 cipher can be combined with
> templates other than ECB (and the way we do provide that combination
> is highly dubious to begin with).
> 
> So let's drop the arc4 cipher altogether, and only keep the ecb(arc4)
> skcipher, which is used in various places in the kernel.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  crypto/arc4.c | 46 ++------------------
>  1 file changed, 4 insertions(+), 42 deletions(-)
> 
> diff --git a/crypto/arc4.c b/crypto/arc4.c
> index 6974dba1b7b9..79a51e9f90ae 100644
> --- a/crypto/arc4.c
> +++ b/crypto/arc4.c
> @@ -13,23 +13,12 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  
> -static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
> -			unsigned int key_len)
> -{
> -	struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
> -
> -	return arc4_setkey(ctx, in_key, key_len);
> -}
> -
>  static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
>  				 unsigned int key_len)
>  {
> -	return arc4_set_key(&tfm->base, in_key, key_len);
> -}
> +	struct arc4_ctx *ctx = crypto_tfm_ctx(&tfm->base);
>  
> -static void arc4_crypt_one(struct crypto_tfm *tfm, u8 *out, const u8 *in)
> -{
> -	arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
> +	return arc4_setkey(ctx, in_key, key_len);
>  }
>  
>  static int ecb_arc4_crypt(struct skcipher_request *req)

Can you clean up the naming here?

	arc4_set_key_skcipher() => crypto_arc4_setkey()
	ecb_arc4_crypt() => crypto_arc4_crypt()

The current names were intended to distinguish the "skcipher" functions from the
"cipher" functions, but that will no longer be needed.

Also, crypto_arc4_setkey() should use crypto_skcipher_ctx() rather than
crypto_tfm_ctx(), now that it only handles "skcipher".

> @@ -50,23 +39,6 @@ static int ecb_arc4_crypt(struct skcipher_request *req)
>  	return err;
>  }
>  
> -static struct crypto_alg arc4_cipher = {
> -	.cra_name		=	"arc4",
> -	.cra_flags		=	CRYPTO_ALG_TYPE_CIPHER,
> -	.cra_blocksize		=	ARC4_BLOCK_SIZE,
> -	.cra_ctxsize		=	sizeof(struct arc4_ctx),
> -	.cra_module		=	THIS_MODULE,
> -	.cra_u			=	{
> -		.cipher = {
> -			.cia_min_keysize	=	ARC4_MIN_KEY_SIZE,
> -			.cia_max_keysize	=	ARC4_MAX_KEY_SIZE,
> -			.cia_setkey		=	arc4_set_key,
> -			.cia_encrypt		=	arc4_crypt_one,
> -			.cia_decrypt		=	arc4_crypt_one,
> -		},
> -	},
> -};
> -
>  static struct skcipher_alg arc4_skcipher = {

Similarly this could be renamed from arc4_skcipher to arc4_alg, now that the
skcipher algorithm doesn't need to be distinguished from the cipher algorithm.

>  	.base.cra_name		=	"ecb(arc4)",

Given the confusion this name causes, can you leave a comment?  Like:

        /*
         * For legacy reasons, this is named "ecb(arc4)", not "arc4".
         * Nevertheless it's actually a stream cipher, not a block cipher.
         */
	 .base.cra_name          =       "ecb(arc4)",


Also, due to removing the cipher algorithm, we need the following testmgr change
so that the comparison self-tests consider the generic implementation of this
algorithm to be itself rather than "ecb(arc4-generic)":

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 658a7eeebab28..5d3eb8577605f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4125,6 +4125,7 @@ static const struct alg_test_desc alg_test_descs[] = {
 		}
 	}, {
 		.alg = "ecb(arc4)",
+		.generic_driver = "ecb(arc4)-generic",
 		.test = alg_test_skcipher,
 		.suite = {
 			.cipher = __VECS(arc4_tv_template)

- Eric

Re: [PATCH v3 2/7] net/mac80211: move WEP handling to ARC4 library interface

[Eric Biggers]

On Tue, Jun 11, 2019 at 03:47:45PM +0200, Ard Biesheuvel wrote:
>  
> -void ieee80211_wep_free(struct ieee80211_local *local)
> -{
> -	if (!IS_ERR(local->wep_tx_tfm))
> -		crypto_free_cipher(local->wep_tx_tfm);
> -	if (!IS_ERR(local->wep_rx_tfm))
> -		crypto_free_cipher(local->wep_rx_tfm);
> -}
> -

This function was removed, but its declaration in net/mac80211/wep.h was not.

>  static inline bool ieee80211_wep_weak_iv(u32 iv, int keylen)
>  {
>  	/*
> @@ -131,21 +110,16 @@ static void ieee80211_wep_remove_iv(struct ieee80211_local *local,
>  /* Perform WEP encryption using given key. data buffer must have tailroom
>   * for 4-byte ICV. data_len must not include this ICV. Note: this function
>   * does _not_ add IV. data = RC4(data | CRC32(data)) */
> -int ieee80211_wep_encrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
> +int ieee80211_wep_encrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
>  			       size_t klen, u8 *data, size_t data_len)
>  {
>  	__le32 icv;
> -	int i;
> -
> -	if (IS_ERR(tfm))
> -		return -1;
>  
>  	icv = cpu_to_le32(~crc32_le(~0, data, data_len));
>  	put_unaligned(icv, (__le32 *)(data + data_len));
>  
> -	crypto_cipher_setkey(tfm, rc4key, klen);
> -	for (i = 0; i < data_len + IEEE80211_WEP_ICV_LEN; i++)
> -		crypto_cipher_encrypt_one(tfm, data + i, data + i);
> +	arc4_setkey(ctx, rc4key, klen);
> +	arc4_crypt(ctx, data, data, data_len + IEEE80211_WEP_ICV_LEN);

How about adding:

	memzero_explicit(ctx, sizeof(*ctx));

> -int ieee80211_wep_decrypt_data(struct crypto_cipher *tfm, u8 *rc4key,
> +int ieee80211_wep_decrypt_data(struct arc4_ctx *ctx, u8 *rc4key,
>  			       size_t klen, u8 *data, size_t data_len)
>  {
>  	__le32 crc;
> -	int i;
> -
> -	if (IS_ERR(tfm))
> -		return -1;
>  
> -	crypto_cipher_setkey(tfm, rc4key, klen);
> -	for (i = 0; i < data_len + IEEE80211_WEP_ICV_LEN; i++)
> -		crypto_cipher_decrypt_one(tfm, data + i, data + i);
> +	arc4_setkey(ctx, rc4key, klen);
> +	arc4_crypt(ctx, data, data, data_len + IEEE80211_WEP_ICV_LEN);

Same here.

- Eric

Re: [PATCH v3 3/7] net/lib80211: move WEP handling to ARC4 library code

[Eric Biggers]

On Tue, Jun 11, 2019 at 03:47:46PM +0200, Ard Biesheuvel wrote:
> The crypto API abstraction is not very useful for invoking ciphers
> directly, especially in the case of arc4, which only has a generic
> implementation in C. So let's invoke the library code directly.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  net/wireless/Kconfig              |  1 +
>  net/wireless/lib80211_crypt_wep.c | 49 +++++---------------
>  2 files changed, 13 insertions(+), 37 deletions(-)
> 
> diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
> index 6310ddede220..6d9c48cea07e 100644
> --- a/net/wireless/Kconfig
> +++ b/net/wireless/Kconfig
> @@ -213,6 +213,7 @@ config LIB80211
>  
>  config LIB80211_CRYPT_WEP
>  	tristate
> +	select CRYPTO_LIB_ARC4
>  
>  config LIB80211_CRYPT_CCMP
>  	tristate
> diff --git a/net/wireless/lib80211_crypt_wep.c b/net/wireless/lib80211_crypt_wep.c
> index 20c1ad63ad44..9a4e4653fe64 100644
> --- a/net/wireless/lib80211_crypt_wep.c
> +++ b/net/wireless/lib80211_crypt_wep.c
> @@ -11,6 +11,7 @@
>   */
>  
>  #include <linux/err.h>
> +#include <linux/fips.h>
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> @@ -22,7 +23,7 @@
>  
>  #include <net/lib80211.h>
>  
> -#include <linux/crypto.h>
> +#include <crypto/arc4.h>
>  #include <linux/crc32.h>
>  
>  MODULE_AUTHOR("Jouni Malinen");
> @@ -35,51 +36,30 @@ struct lib80211_wep_data {
>  	u8 key[WEP_KEY_LEN + 1];
>  	u8 key_len;
>  	u8 key_idx;
> -	struct crypto_cipher *tx_tfm;
> -	struct crypto_cipher *rx_tfm;
> +	struct arc4_ctx tx_ctx;
> +	struct arc4_ctx rx_ctx;
>  };
>  
>  static void *lib80211_wep_init(int keyidx)
>  {
>  	struct lib80211_wep_data *priv;
>  
> +	if (fips_enabled)
> +		return NULL;
> +
>  	priv = kzalloc(sizeof(*priv), GFP_ATOMIC);
>  	if (priv == NULL)
> -		goto fail;
> +		return NULL;
>  	priv->key_idx = keyidx;
>  
> -	priv->tx_tfm = crypto_alloc_cipher("arc4", 0, 0);
> -	if (IS_ERR(priv->tx_tfm)) {
> -		priv->tx_tfm = NULL;
> -		goto fail;
> -	}
> -
> -	priv->rx_tfm = crypto_alloc_cipher("arc4", 0, 0);
> -	if (IS_ERR(priv->rx_tfm)) {
> -		priv->rx_tfm = NULL;
> -		goto fail;
> -	}
>  	/* start WEP IV from a random value */
>  	get_random_bytes(&priv->iv, 4);
>  
>  	return priv;
> -
> -      fail:
> -	if (priv) {
> -		crypto_free_cipher(priv->tx_tfm);
> -		crypto_free_cipher(priv->rx_tfm);
> -		kfree(priv);
> -	}
> -	return NULL;
>  }
>  
>  static void lib80211_wep_deinit(void *priv)
>  {
> -	struct lib80211_wep_data *_priv = priv;
> -	if (_priv) {
> -		crypto_free_cipher(_priv->tx_tfm);
> -		crypto_free_cipher(_priv->rx_tfm);
> -	}
>  	kfree(priv);
>  }

How about changing lib80211_wep_deinit() to use kzfree()?  As a result of
changing to the ARC4 library, the arc4_ctx is no longer zeroed.  Of course it's
not any worse than it was before since the raw key is already in
lib80211_wep_data too, but it really ought to be kzfree()...

- Eric

Re: [PATCH v3 6/7] ppp: mppe: switch to RC4 library interface

[Eric Biggers]

On Tue, Jun 11, 2019 at 03:47:49PM +0200, Ard Biesheuvel wrote:
> @@ -728,8 +660,7 @@ static struct compressor ppp_mppe = {
>  static int __init ppp_mppe_init(void)
>  {
>  	int answer;
> -	if (!(crypto_has_skcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC) &&
> -	      crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC)))
> +	if (!crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC))
>  		return -ENODEV;

Doesn't fips_enabled need to be checked here too?

- Eric

Re: [PATCH v3 6/7] ppp: mppe: switch to RC4 library interface

[Eric Biggers]

On Tue, Jun 11, 2019 at 03:47:49PM +0200, Ard Biesheuvel wrote:
> @@ -728,8 +660,7 @@ static struct compressor ppp_mppe = {
>  static int __init ppp_mppe_init(void)
>  {
>  	int answer;
> -	if (!(crypto_has_skcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC) &&
> -	      crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC)))
> +	if (!crypto_has_ahash("sha1", 0, CRYPTO_ALG_ASYNC))
>  		return -ENODEV;

Doesn't fips_enabled need to be checked here too?

- Eric

Re: [PATCH v3 7/7] fs: cifs: switch to RC4 library interface

[Eric Biggers]

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

Re: [PATCH v3 5/7] crypto: arc4 - remove cipher implementation

[Eric Biggers]

On Tue, Jun 11, 2019 at 10:39:39AM -0700, Eric Biggers wrote:
> > -
> >  static struct skcipher_alg arc4_skcipher = {
> 
> Similarly this could be renamed from arc4_skcipher to arc4_alg, now that the
> skcipher algorithm doesn't need to be distinguished from the cipher algorithm.
> 
> >  	.base.cra_name		=	"ecb(arc4)",
> 
> Given the confusion this name causes, can you leave a comment?  Like:
> 
>         /*
>          * For legacy reasons, this is named "ecb(arc4)", not "arc4".
>          * Nevertheless it's actually a stream cipher, not a block cipher.
>          */
> 	 .base.cra_name          =       "ecb(arc4)",
> 
> 
> Also, due to removing the cipher algorithm, we need the following testmgr change
> so that the comparison self-tests consider the generic implementation of this
> algorithm to be itself rather than "ecb(arc4-generic)":
> 
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 658a7eeebab28..5d3eb8577605f 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -4125,6 +4125,7 @@ static const struct alg_test_desc alg_test_descs[] = {
>  		}
>  	}, {
>  		.alg = "ecb(arc4)",
> +		.generic_driver = "ecb(arc4)-generic",
>  		.test = alg_test_skcipher,
>  		.suite = {
>  			.cipher = __VECS(arc4_tv_template)
> 
> - Eric

Hi Ard, did you see these comments?  They weren't addressed in v4.  We need at
least the testmgr change, otherwise there's a warning when booting with
CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y:

[    0.542610] alg: skcipher: skipping comparison tests for ecb(arc4)-generic because ecb(arc4-generic) is unavailable

- Eric

Re: [PATCH v3 5/7] crypto: arc4 - remove cipher implementation

[Ard Biesheuvel]

On Wed, 12 Jun 2019 at 17:33, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jun 11, 2019 at 10:39:39AM -0700, Eric Biggers wrote:
> > > -
> > >  static struct skcipher_alg arc4_skcipher = {
> >
> > Similarly this could be renamed from arc4_skcipher to arc4_alg, now that the
> > skcipher algorithm doesn't need to be distinguished from the cipher algorithm.
> >
> > >     .base.cra_name          =       "ecb(arc4)",
> >
> > Given the confusion this name causes, can you leave a comment?  Like:
> >
> >         /*
> >          * For legacy reasons, this is named "ecb(arc4)", not "arc4".
> >          * Nevertheless it's actually a stream cipher, not a block cipher.
> >          */
> >        .base.cra_name          =       "ecb(arc4)",
> >
> >
> > Also, due to removing the cipher algorithm, we need the following testmgr change
> > so that the comparison self-tests consider the generic implementation of this
> > algorithm to be itself rather than "ecb(arc4-generic)":
> >
> > diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> > index 658a7eeebab28..5d3eb8577605f 100644
> > --- a/crypto/testmgr.c
> > +++ b/crypto/testmgr.c
> > @@ -4125,6 +4125,7 @@ static const struct alg_test_desc alg_test_descs[] = {
> >               }
> >       }, {
> >               .alg = "ecb(arc4)",
> > +             .generic_driver = "ecb(arc4)-generic",
> >               .test = alg_test_skcipher,
> >               .suite = {
> >                       .cipher = __VECS(arc4_tv_template)
> >
> > - Eric
>
> Hi Ard, did you see these comments?  They weren't addressed in v4.  We need at
> least the testmgr change, otherwise there's a warning when booting with
> CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y:
>
> [    0.542610] alg: skcipher: skipping comparison tests for ecb(arc4)-generic because ecb(arc4-generic) is unavailable
>

Oops, no, I didn't

I'll fix it up and resend. I forgot to add a MODULE_LICENSE() to
libarc4.ko as well, so I needed to do this anyway.