[PATCH v2 0/7] crypto: rc4 cleanup

[PATCH v2 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.

[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>

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                      | 106 ++------------------
 drivers/net/ppp/Kconfig            |   3 +-
 drivers/net/ppp/ppp_mppe.c         |  92 ++---------------
 fs/cifs/Kconfig                    |   2 +-
 fs/cifs/cifsencrypt.c              |  50 +++------
 include/crypto/arc4.h              |  13 +++
 lib/Makefile                       |   2 +-
 lib/crypto/Makefile                |   3 +
 lib/crypto/libarc4.c               |  74 ++++++++++++++
 net/mac80211/Kconfig               |   2 +-
 net/mac80211/cfg.c                 |   3 -
 net/mac80211/ieee80211_i.h         |   4 +-
 net/mac80211/key.h                 |   1 +
 net/mac80211/main.c                |  48 +--------
 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 |  42 +++-----
 net/wireless/lib80211_crypt_wep.c  |  43 ++------
 25 files changed, 176 insertions(+), 387 deletions(-)
 create mode 100644 lib/crypto/Makefile
 create mode 100644 lib/crypto/libarc4.c

-- 
2.20.1

[PATCH v2 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         | 74 +++-----------------
 include/crypto/arc4.h | 13 ++++
 lib/Makefile          |  2 +-
 lib/crypto/Makefile   |  3 +
 lib/crypto/libarc4.c  | 74 ++++++++++++++++++++
 7 files changed, 104 insertions(+), 67 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..310e2a5de59d 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
+	bool
+
 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..7f80623aa66a 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;
-	}
+	struct crypto_arc4_ctx *ctx = crypto_tfm_ctx(tfm);
 
-	return 0;
+	return crypto_arc4_set_key(ctx, in_key, key_len);
 }
 
 static int arc4_set_key_skcipher(struct crypto_skcipher *tfm, const u8 *in_key,
@@ -48,60 +27,23 @@ 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);
+	crypto_arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
 }
 
 static int ecb_arc4_crypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
-	struct arc4_ctx *ctx = crypto_skcipher_ctx(tfm);
+	struct crypto_arc4_ctx *ctx = crypto_skcipher_ctx(tfm);
 	struct skcipher_walk walk;
 	int err;
 
 	err = skcipher_walk_virt(&walk, req, false);
 
 	while (walk.nbytes > 0) {
-		arc4_crypt(ctx, walk.dst.virt.addr, walk.src.virt.addr,
-			   walk.nbytes);
+		crypto_arc4_crypt(ctx, walk.dst.virt.addr, walk.src.virt.addr,
+				  walk.nbytes);
 		err = skcipher_walk_done(&walk, 0);
 	}
 
@@ -112,7 +54,7 @@ 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_ctxsize		=	sizeof(struct crypto_arc4_ctx),
 	.cra_module		=	THIS_MODULE,
 	.cra_u			=	{
 		.cipher = {
@@ -129,7 +71,7 @@ static struct skcipher_alg arc4_skcipher = {
 	.base.cra_name		=	"ecb(arc4)",
 	.base.cra_priority	=	100,
 	.base.cra_blocksize	=	ARC4_BLOCK_SIZE,
-	.base.cra_ctxsize	=	sizeof(struct arc4_ctx),
+	.base.cra_ctxsize	=	sizeof(struct crypto_arc4_ctx),
 	.base.cra_module	=	THIS_MODULE,
 	.min_keysize		=	ARC4_MIN_KEY_SIZE,
 	.max_keysize		=	ARC4_MAX_KEY_SIZE,
diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
index 5b2c24ab0139..62ac95ec6860 100644
--- a/include/crypto/arc4.h
+++ b/include/crypto/arc4.h
@@ -6,8 +6,21 @@
 #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 crypto_arc4_ctx {
+	u32 S[256];
+	u32 x, y;
+};
+
+int crypto_arc4_set_key(struct crypto_arc4_ctx *ctx, const u8 *in_key,
+			unsigned int key_len);
+
+void crypto_arc4_crypt(struct crypto_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..e375d150a547
--- /dev/null
+++ b/lib/crypto/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
diff --git a/lib/crypto/libarc4.c b/lib/crypto/libarc4.c
new file mode 100644
index 000000000000..b828af2cc03b
--- /dev/null
+++ b/lib/crypto/libarc4.c
@@ -0,0 +1,74 @@
+// 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 crypto_arc4_set_key(struct crypto_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(crypto_arc4_set_key);
+
+void crypto_arc4_crypt(struct crypto_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(crypto_arc4_crypt);
-- 
2.20.1

[PATCH v2 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.

Cc: linux-wireless@vger.kernel.org
Cc: Johannes Berg <johannes@sipsolutions.net>
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        | 48 ++------------------
 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, 26 insertions(+), 101 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..b2862e73f1fc 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 crypto_arc4_ctx wep_tx_ctx;
+	struct crypto_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..b339307035d2 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -733,8 +733,6 @@ 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_mfp = ieee80211_hw_check(&local->hw, MFP_CAPABLE);
 	int n_suites = 0, r = 0, w = 0;
 	u32 *suites;
@@ -757,31 +755,7 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 
 	if (ieee80211_hw_check(&local->hw, SW_CRYPTO_CONTROL) ||
 	    local->hw.wiphy->cipher_suites) {
-		/* If the driver advertises, or doesn't support SW crypto,
-		 * we only need to remove WEP if necessary.
-		 */
-		if (have_wep)
-			return 0;
-
-		/* well if it has _no_ ciphers ... fine */
-		if (!local->hw.wiphy->n_cipher_suites)
-			return 0;
-
-		/* Driver provides cipher suites, but we need to exclude WEP */
-		suites = kmemdup(local->hw.wiphy->cipher_suites,
-				 sizeof(u32) * local->hw.wiphy->n_cipher_suites,
-				 GFP_KERNEL);
-		if (!suites)
-			return -ENOMEM;
-
-		for (r = 0; r < local->hw.wiphy->n_cipher_suites; r++) {
-			u32 suite = local->hw.wiphy->cipher_suites[r];
-
-			if (suite == WLAN_CIPHER_SUITE_WEP40 ||
-			    suite == WLAN_CIPHER_SUITE_WEP104)
-				continue;
-			suites[w++] = suite;
-		}
+		return 0;
 	} else if (!local->hw.cipher_schemes) {
 		/* If the driver doesn't have cipher schemes, there's nothing
 		 * else to do other than assign the (software supported and
@@ -793,11 +767,6 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 		if (!have_mfp)
 			local->hw.wiphy->n_cipher_suites -= 4;
 
-		if (!have_wep) {
-			local->hw.wiphy->cipher_suites += 2;
-			local->hw.wiphy->n_cipher_suites -= 2;
-		}
-
 		/* not dynamically allocated, so just return */
 		return 0;
 	} else {
@@ -811,11 +780,7 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 		 * We start counting ciphers defined by schemes, TKIP, CCMP,
 		 * CCMP-256, GCMP, and GCMP-256
 		 */
-		n_suites = local->hw.n_cipher_schemes + 5;
-
-		/* check if we have WEP40 and WEP104 */
-		if (have_wep)
-			n_suites += 2;
+		n_suites = local->hw.n_cipher_schemes + 7;
 
 		/* check if we have AES_CMAC, BIP-CMAC-256, BIP-GMAC-128,
 		 * BIP-GMAC-256
@@ -832,11 +797,8 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 		suites[w++] = WLAN_CIPHER_SUITE_TKIP;
 		suites[w++] = WLAN_CIPHER_SUITE_GCMP;
 		suites[w++] = WLAN_CIPHER_SUITE_GCMP_256;
-
-		if (have_wep) {
-			suites[w++] = WLAN_CIPHER_SUITE_WEP40;
-			suites[w++] = WLAN_CIPHER_SUITE_WEP104;
-		}
+		suites[w++] = WLAN_CIPHER_SUITE_WEP40;
+		suites[w++] = WLAN_CIPHER_SUITE_WEP104;
 
 		if (have_mfp) {
 			suites[w++] = WLAN_CIPHER_SUITE_AES_CMAC;
@@ -1301,7 +1263,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 +1319,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..580b5c3e837e 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 crypto_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 crypto_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..42b300773c58 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 crypto_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 crypto_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..9aad40e28374 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 crypto_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);
+	crypto_arc4_set_key(ctx, rc4key, klen);
+	crypto_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 crypto_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);
+	crypto_arc4_set_key(ctx, rc4key, klen);
+	crypto_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..b63dda9dd442 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 crypto_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 crypto_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 v2 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.

Cc: linux-wireless@vger.kernel.org
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/wireless/Kconfig              |  1 +
 net/wireless/lib80211_crypt_wep.c | 43 ++++----------------
 2 files changed, 9 insertions(+), 35 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..3db1b2e6a25a 100644
--- a/net/wireless/lib80211_crypt_wep.c
+++ b/net/wireless/lib80211_crypt_wep.c
@@ -22,7 +22,7 @@
 
 #include <net/lib80211.h>
 
-#include <linux/crypto.h>
+#include <crypto/arc4.h>
 #include <linux/crc32.h>
 
 MODULE_AUTHOR("Jouni Malinen");
@@ -35,8 +35,8 @@ 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 crypto_arc4_ctx tx_ctx;
+	struct crypto_arc4_ctx rx_ctx;
 };
 
 static void *lib80211_wep_init(int keyidx)
@@ -45,41 +45,17 @@ static void *lib80211_wep_init(int keyidx)
 
 	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);
 }
 
@@ -160,10 +136,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);
+	crypto_arc4_set_key(&wep->tx_ctx, key, klen);
+	crypto_arc4_crypt(&wep->tx_ctx, pos, pos, len + 4);
 
 	return 0;
 }
@@ -202,9 +176,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);
+	crypto_arc4_set_key(&wep->rx_ctx, key, klen);
+	crypto_arc4_crypt(&wep->rx_ctx, pos, pos, plen + 4);
 
 	crc = ~crc32_le(~0, pos, plen);
 	icv[0] = crc;
-- 
2.20.1

[PATCH v2 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.

Cc: linux-wireless@vger.kernel.org
Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 net/wireless/lib80211_crypt_tkip.c | 42 ++++++--------------
 1 file changed, 12 insertions(+), 30 deletions(-)

diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index 11eaa5956f00..c19528a39563 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -29,6 +29,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 +65,9 @@ struct lib80211_tkip_data {
 
 	int key_idx;
 
-	struct crypto_cipher *rx_tfm_arc4;
+	struct crypto_arc4_ctx rx_ctx_arc4;
+	struct crypto_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) */
@@ -99,24 +100,12 @@ static void *lib80211_tkip_init(int key_idx)
 
 	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 +117,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 +129,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 +330,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 +354,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);
+	crypto_arc4_set_key(&tkey->tx_ctx_arc4, rc4key, 16);
+	crypto_arc4_crypt(&tkey->tx_ctx_arc4, pos, pos, len + 4);
+
 	return 0;
 }
 
@@ -400,7 +384,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 +436,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);
+	crypto_arc4_set_key(&tkey->rx_ctx_arc4, rc4key, 16);
+	crypto_arc4_crypt(&tkey->rx_ctx_arc4, pos, pos, plen + 4);
 
 	crc = ~crc32_le(~0, pos, plen);
 	icv[0] = crc;
@@ -640,17 +622,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 crypto_arc4_ctx *tfm2 = &tkey->tx_ctx_arc4;
 	struct crypto_shash *tfm3 = tkey->rx_tfm_michael;
-	struct crypto_cipher *tfm4 = tkey->rx_tfm_arc4;
+	struct crypto_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 v2 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 | 36 ++------------------
 1 file changed, 2 insertions(+), 34 deletions(-)

diff --git a/crypto/arc4.c b/crypto/arc4.c
index 7f80623aa66a..3cdfd12110ea 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -27,11 +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_one(struct crypto_tfm *tfm, u8 *out, const u8 *in)
-{
-	crypto_arc4_crypt(crypto_tfm_ctx(tfm), out, in, 1);
-}
-
 static int ecb_arc4_crypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
@@ -50,23 +45,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 crypto_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 +60,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 +74,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 v2 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 | 92 +++-----------------
 2 files changed, 12 insertions(+), 83 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..00be8143b0b6 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -42,8 +42,8 @@
  *                    deprecated in 2.6
  */
 
+#include <crypto/arc4.h>
 #include <crypto/hash.h>
-#include <crypto/skcipher.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -65,13 +65,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 +88,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 crypto_arc4_ctx arc4;
 	struct shash_desc *sha1;
 	unsigned char *sha1_digest;
 	unsigned char master_key[MPPE_MAX_KEY_LEN];
@@ -154,24 +147,12 @@ 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");
+		crypto_arc4_set_key(&state->arc4, state->sha1_digest,
+				    state->keylen);
+		crypto_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);
+	crypto_arc4_set_key(&state->arc4, state->session_key, state->keylen);
 }
 
 /*
@@ -204,12 +183,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 +223,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 +238,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 +337,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 +388,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf, ibuf, isize);
 
 	state->stats.unc_bytes += isize;
 	state->stats.unc_packets++;
@@ -480,10 +434,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 +562,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf, ibuf, 1);
 
 	/*
 	 * Do PFC decompression.
@@ -637,14 +577,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf + 1, ibuf + 1, isize - 1);
 
 	state->stats.unc_bytes += osize;
 	state->stats.unc_packets++;
@@ -654,8 +587,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 +659,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 v2 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 | 92 +++-----------------
 2 files changed, 12 insertions(+), 83 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..00be8143b0b6 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -42,8 +42,8 @@
  *                    deprecated in 2.6
  */
 
+#include <crypto/arc4.h>
 #include <crypto/hash.h>
-#include <crypto/skcipher.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -65,13 +65,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 +88,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 crypto_arc4_ctx arc4;
 	struct shash_desc *sha1;
 	unsigned char *sha1_digest;
 	unsigned char master_key[MPPE_MAX_KEY_LEN];
@@ -154,24 +147,12 @@ 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");
+		crypto_arc4_set_key(&state->arc4, state->sha1_digest,
+				    state->keylen);
+		crypto_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);
+	crypto_arc4_set_key(&state->arc4, state->session_key, state->keylen);
 }
 
 /*
@@ -204,12 +183,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 +223,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 +238,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 +337,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 +388,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf, ibuf, isize);
 
 	state->stats.unc_bytes += isize;
 	state->stats.unc_packets++;
@@ -480,10 +434,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 +562,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf, ibuf, 1);
 
 	/*
 	 * Do PFC decompression.
@@ -637,14 +577,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;
-	}
+	crypto_arc4_crypt(&state->arc4, obuf + 1, ibuf + 1, isize - 1);
 
 	state->stats.unc_bytes += osize;
 	state->stats.unc_packets++;
@@ -654,8 +587,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 +659,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 v2 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 | 50 +++++---------------
 2 files changed, 13 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..d0ab5a38e5d2 100644
--- a/fs/cifs/cifsencrypt.c
+++ b/fs/cifs/cifsencrypt.c
@@ -33,7 +33,7 @@
 #include <linux/ctype.h>
 #include <linux/random.h>
 #include <linux/highmem.h>
-#include <crypto/skcipher.h>
+#include <crypto/arc4.h>
 #include <crypto/aead.h>
 
 int __cifs_calc_signature(struct smb_rqst *rqst,
@@ -772,11 +772,9 @@ 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 crypto_arc4_ctx *ctx_arc4;
 	unsigned char *sec_key;
+	int rc = 0;
 
 	sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL);
 	if (sec_key == NULL)
@@ -784,49 +782,25 @@ 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;
-	}
+	crypto_arc4_set_key(ctx_arc4, ses->auth_key.response,
+			    CIFS_SESS_KEY_SIZE);
+	crypto_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 v2 0/7] crypto: rc4 cleanup

[Ard Biesheuvel]

(adding Johannes back to the cover letter cc)

On Sun, 9 Jun 2019 at 13:55, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> 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.
>
> [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>
>
> 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                      | 106 ++------------------
>  drivers/net/ppp/Kconfig            |   3 +-
>  drivers/net/ppp/ppp_mppe.c         |  92 ++---------------
>  fs/cifs/Kconfig                    |   2 +-
>  fs/cifs/cifsencrypt.c              |  50 +++------
>  include/crypto/arc4.h              |  13 +++
>  lib/Makefile                       |   2 +-
>  lib/crypto/Makefile                |   3 +
>  lib/crypto/libarc4.c               |  74 ++++++++++++++
>  net/mac80211/Kconfig               |   2 +-
>  net/mac80211/cfg.c                 |   3 -
>  net/mac80211/ieee80211_i.h         |   4 +-
>  net/mac80211/key.h                 |   1 +
>  net/mac80211/main.c                |  48 +--------
>  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 |  42 +++-----
>  net/wireless/lib80211_crypt_wep.c  |  43 ++------
>  25 files changed, 176 insertions(+), 387 deletions(-)
>  create mode 100644 lib/crypto/Makefile
>  create mode 100644 lib/crypto/libarc4.c
>
> --
> 2.20.1
>

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

[Johannes Berg]

Hi Ard,

In general, I have no objections to this.

However, with this

> -	select CRYPTO_ARC4
> +	select CRYPTO_LIB_ARC4

and this

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

there's one quirk that I worry about. Does this mean WEP is now *always*
available/enabled?

I had to dig in history a bit, but vaguely remembered this commit:

commit 3473187d2459a078e00e5fac8aafc30af69c57fa
Author: John W. Linville <linville@tuxdriver.com>
Date:   Wed Jul 7 15:07:49 2010 -0400

    mac80211: remove wep dependency


Since you create the new CRYPTO_LIB_ARC4 in patch 1, I wonder if
something is broken? I can't really seem to figure out how WEP is
disallowed in FIPS mode to start with though.


(This also is the reason for all the code that removes WEP/TKIP from the
list of permitted cipher suites when ARC4 isn't available...)

johannes

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

[Steve French]

Probably harmless to change this code path (build_ntlmssp_auth_blob is
called at session negotiation time so shouldn't have much of a
performance impact).

On the other hand if we can find optimizations in the encryption and
signing paths, that would be really helpful.   There was a lot of
focus on encryption performance at SambaXP last week.

Andreas from Redhat gave a talk on the improvements in Samba with TLS
implementation of AES-GCM.   I added the cifs client implementation of
AES-GCM and notice it is now faster to encrypt packets than sign them
(performance is about 2 to 3 times faster now with GCM).

On Sun, Jun 9, 2019 at 6:57 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> 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 | 50 +++++---------------
>  2 files changed, 13 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..d0ab5a38e5d2 100644
> --- a/fs/cifs/cifsencrypt.c
> +++ b/fs/cifs/cifsencrypt.c
> @@ -33,7 +33,7 @@
>  #include <linux/ctype.h>
>  #include <linux/random.h>
>  #include <linux/highmem.h>
> -#include <crypto/skcipher.h>
> +#include <crypto/arc4.h>
>  #include <crypto/aead.h>
>
>  int __cifs_calc_signature(struct smb_rqst *rqst,
> @@ -772,11 +772,9 @@ 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 crypto_arc4_ctx *ctx_arc4;
>         unsigned char *sec_key;
> +       int rc = 0;
>
>         sec_key = kmalloc(CIFS_SESS_KEY_SIZE, GFP_KERNEL);
>         if (sec_key == NULL)
> @@ -784,49 +782,25 @@ 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;
> -       }
> +       crypto_arc4_set_key(ctx_arc4, ses->auth_key.response,
> +                           CIFS_SESS_KEY_SIZE);
> +       crypto_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
>


-- 
Thanks,

Steve

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

[Ard Biesheuvel]

On Sun, 9 Jun 2019 at 22:09, Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Hi Ard,
>
> In general, I have no objections to this.
>
> However, with this
>
> > -     select CRYPTO_ARC4
> > +     select CRYPTO_LIB_ARC4
>
> and this
>
> >       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;
>
> there's one quirk that I worry about. Does this mean WEP is now *always*
> available/enabled?
>
> I had to dig in history a bit, but vaguely remembered this commit:
>
> commit 3473187d2459a078e00e5fac8aafc30af69c57fa
> Author: John W. Linville <linville@tuxdriver.com>
> Date:   Wed Jul 7 15:07:49 2010 -0400
>
>     mac80211: remove wep dependency
>
>
> Since you create the new CRYPTO_LIB_ARC4 in patch 1, I wonder if
> something is broken? I can't really seem to figure out how WEP is
> disallowed in FIPS mode to start with though.
>
>
> (This also is the reason for all the code that removes WEP/TKIP from the
> list of permitted cipher suites when ARC4 isn't available...)
>

Good point. And in the future, there may be other reasons besides FIPS
compliance to turn off WEP support, so it makes sense to retain this
logic.

I am still curious whether FIPS compliance actually requires us to do
this, or whether that code is simply there to work around the lack of
RC4 in the crypto API when FIPS support happens to be enabled.

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

[Eric Biggers]

Just some bike shedding:

On Sun, Jun 09, 2019 at 01:55:03PM +0200, Ard Biesheuvel wrote:
> diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
> index 5b2c24ab0139..62ac95ec6860 100644
> --- a/include/crypto/arc4.h
> +++ b/include/crypto/arc4.h
> @@ -6,8 +6,21 @@
>  #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 crypto_arc4_ctx {
> +	u32 S[256];
> +	u32 x, y;
> +};
> +
> +int crypto_arc4_set_key(struct crypto_arc4_ctx *ctx, const u8 *in_key,
> +			unsigned int key_len);
> +
> +void crypto_arc4_crypt(struct crypto_arc4_ctx *ctx, u8 *out, const u8 *in,
> +		       unsigned int len);

How about naming these just arc4_* instead of crypto_arc4_*?  The crypto_*
prefix is already used mostly for crypto API helper functions, i.e. functions
that take take one of the abstract crypto API types like crypto_skcipher,
shash_desc, etc.  For lib functions, the bare name seems more appropriate.  See
e.g. sha256_update() vs. crypto_sha256_update().

> +++ b/lib/crypto/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
> diff --git a/lib/crypto/libarc4.c b/lib/crypto/libarc4.c
> new file mode 100644
> index 000000000000..b828af2cc03b
> --- /dev/null
> +++ b/lib/crypto/libarc4.c
> @@ -0,0 +1,74 @@

How about arc4.c instead of libarc4.c?  The second "lib" is redundant, given
that the file is already in the lib/ directory.

- Eric

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

[Ard Biesheuvel]

On Mon, 10 Jun 2019 at 18:06, Eric Biggers <ebiggers@kernel.org> wrote:
>
> Just some bike shedding:
>
> On Sun, Jun 09, 2019 at 01:55:03PM +0200, Ard Biesheuvel wrote:
> > diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
> > index 5b2c24ab0139..62ac95ec6860 100644
> > --- a/include/crypto/arc4.h
> > +++ b/include/crypto/arc4.h
> > @@ -6,8 +6,21 @@
> >  #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 crypto_arc4_ctx {
> > +     u32 S[256];
> > +     u32 x, y;
> > +};
> > +
> > +int crypto_arc4_set_key(struct crypto_arc4_ctx *ctx, const u8 *in_key,
> > +                     unsigned int key_len);
> > +
> > +void crypto_arc4_crypt(struct crypto_arc4_ctx *ctx, u8 *out, const u8 *in,
> > +                    unsigned int len);
>
> How about naming these just arc4_* instead of crypto_arc4_*?  The crypto_*
> prefix is already used mostly for crypto API helper functions, i.e. functions
> that take take one of the abstract crypto API types like crypto_skcipher,
> shash_desc, etc.  For lib functions, the bare name seems more appropriate.  See
> e.g. sha256_update() vs. crypto_sha256_update().
>

That is also fine, although I am slightly concerned that we may run
into trouble with other algorithms in the future. But I do agree it
makes sense to make a clear distinction with the full blown API.

> > +++ b/lib/crypto/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
> > diff --git a/lib/crypto/libarc4.c b/lib/crypto/libarc4.c
> > new file mode 100644
> > index 000000000000..b828af2cc03b
> > --- /dev/null
> > +++ b/lib/crypto/libarc4.c
> > @@ -0,0 +1,74 @@
>
> How about arc4.c instead of libarc4.c?  The second "lib" is redundant, given
> that the file is already in the lib/ directory.
>

The problem here is that we'll end up with two modules named arc4.ko,
one in crypto/ and one in lib/crypto/. Perhaps we should rename the
other one? (especially once it implements ecb(arc4) only.)

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

[Eric Biggers]

Hi Steve,

On Sun, Jun 09, 2019 at 05:03:25PM -0500, Steve French wrote:
> Probably harmless to change this code path (build_ntlmssp_auth_blob is
> called at session negotiation time so shouldn't have much of a
> performance impact).
> 
> On the other hand if we can find optimizations in the encryption and
> signing paths, that would be really helpful.   There was a lot of
> focus on encryption performance at SambaXP last week.
> 
> Andreas from Redhat gave a talk on the improvements in Samba with TLS
> implementation of AES-GCM.   I added the cifs client implementation of
> AES-GCM and notice it is now faster to encrypt packets than sign them
> (performance is about 2 to 3 times faster now with GCM).
> 
> On Sun, Jun 9, 2019 at 6:57 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > The CIFS code uses the sync skcipher API to invoke the ecb(arc4) skcipher,
> > of which only a single generic C code implementation exists. This means
> > that going through all the trouble of using scatterlists etc buys us
> > very little, and we're better off just invoking the arc4 library directly.

This patch only changes RC4 encryption, and to be clear it actually *improves*
the performance of the RC4 encryption, since it removes unnecessary
abstractions.  I'd really hope that people wouldn't care either way, though,
since RC4 is insecure and should not be used.

Also it seems that fs/cifs/ supports AES-CCM but not AES-GCM?

/* pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;*/ /* not supported yet */
        pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_CCM;

AES-GCM is usually faster than AES-CCM, so if you want to improve performance,
switching from CCM to GCM would do that.

- Eric

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

[Eric Biggers]

On Mon, Jun 10, 2019 at 06:10:41PM +0200, Ard Biesheuvel wrote:
> On Mon, 10 Jun 2019 at 18:06, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Just some bike shedding:
> >
> > On Sun, Jun 09, 2019 at 01:55:03PM +0200, Ard Biesheuvel wrote:
> > > diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
> > > index 5b2c24ab0139..62ac95ec6860 100644
> > > --- a/include/crypto/arc4.h
> > > +++ b/include/crypto/arc4.h
> > > @@ -6,8 +6,21 @@
> > >  #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 crypto_arc4_ctx {
> > > +     u32 S[256];
> > > +     u32 x, y;
> > > +};
> > > +
> > > +int crypto_arc4_set_key(struct crypto_arc4_ctx *ctx, const u8 *in_key,
> > > +                     unsigned int key_len);
> > > +
> > > +void crypto_arc4_crypt(struct crypto_arc4_ctx *ctx, u8 *out, const u8 *in,
> > > +                    unsigned int len);
> >
> > How about naming these just arc4_* instead of crypto_arc4_*?  The crypto_*
> > prefix is already used mostly for crypto API helper functions, i.e. functions
> > that take take one of the abstract crypto API types like crypto_skcipher,
> > shash_desc, etc.  For lib functions, the bare name seems more appropriate.  See
> > e.g. sha256_update() vs. crypto_sha256_update().
> >
> 
> That is also fine, although I am slightly concerned that we may run
> into trouble with other algorithms in the future. But I do agree it
> makes sense to make a clear distinction with the full blown API.
> 
> > > +++ b/lib/crypto/Makefile
> > > @@ -0,0 +1,3 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
> > > diff --git a/lib/crypto/libarc4.c b/lib/crypto/libarc4.c
> > > new file mode 100644
> > > index 000000000000..b828af2cc03b
> > > --- /dev/null
> > > +++ b/lib/crypto/libarc4.c
> > > @@ -0,0 +1,74 @@
> >
> > How about arc4.c instead of libarc4.c?  The second "lib" is redundant, given
> > that the file is already in the lib/ directory.
> >
> 
> The problem here is that we'll end up with two modules named arc4.ko,
> one in crypto/ and one in lib/crypto/. Perhaps we should rename the
> other one? (especially once it implements ecb(arc4) only.)

Another option is to do:

obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
libarc4-y := arc4.o

Also, CONFIG_CRYPTO_LIB_ARC4 needs to actually be a tristate.
It seems you accidentally made it a bool:

> +config CRYPTO_LIB_ARC4
> +	bool
> +

- Eric

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

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On Mon, Jun 10, 2019 at 11:17 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> Hi Steve,
>
> On Sun, Jun 09, 2019 at 05:03:25PM -0500, Steve French wrote:
> > Probably harmless to change this code path (build_ntlmssp_auth_blob is
> > called at session negotiation time so shouldn't have much of a
> > performance impact).
> >
> > On the other hand if we can find optimizations in the encryption and
> > signing paths, that would be really helpful.   There was a lot of
> > focus on encryption performance at SambaXP last week.
> >
> > Andreas from Redhat gave a talk on the improvements in Samba with TLS
> > implementation of AES-GCM.   I added the cifs client implementation of
> > AES-GCM and notice it is now faster to encrypt packets than sign them
> > (performance is about 2 to 3 times faster now with GCM).
> >
> > On Sun, Jun 9, 2019 at 6:57 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > The CIFS code uses the sync skcipher API to invoke the ecb(arc4) skcipher,
> > > of which only a single generic C code implementation exists. This means
> > > that going through all the trouble of using scatterlists etc buys us
> > > very little, and we're better off just invoking the arc4 library directly.
>
> This patch only changes RC4 encryption, and to be clear it actually *improves*
> the performance of the RC4 encryption, since it removes unnecessary
> abstractions.  I'd really hope that people wouldn't care either way, though,
> since RC4 is insecure and should not be used.
>
> Also it seems that fs/cifs/ supports AES-CCM but not AES-GCM?
>
> /* pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;*/ /* not supported yet */
>         pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_CCM;
>
> AES-GCM is usually faster than AES-CCM, so if you want to improve performance,
> switching from CCM to GCM would do that.
>
> - Eric

Yes - when I tested the GCM code in cifs.ko last week (the two patches
are currently
in the cifs-2.6.git for-next branch and thus in linux-next and are
attached), I was astounded
at the improvement - encryption with GCM is now faster than signing,
and copy is more
than twice as fast as encryption was before with CCM (assuming a fast
enough network so
that the network is not the bottleneck).  We see more benefit in the write path
(copy to server) than the read path (copy from server) but even the
read path gets
80% benefit in my tests, and with the addition of multichannel support
in the next
few months, we should see copy from server on SMB3.1.1 mounts
improving even more.

Any other ideas how to improve the encryption or signing in the read
or write path
in cifs.ko would be appreciated.   We still are slower than Windows, probably in
part due to holding mutexes longer in sending the frame across the network
(including signing or encryption) which limits parallelism somewhat.

-- 
Thanks,

Steve

[-- Attachment #2: 0001-SMB3-Add-SMB3.1.1-GCM-to-negotiated-crypto-algorigth.patch --]
[-- Type: application/octet-stream, Size: 2973 bytes --]

From 24277abb8bd80aaea66b7edd55b4ea80dc648db3 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Fri, 7 Jun 2019 08:59:40 -0500
Subject: [PATCH 1/2] SMB3: Add SMB3.1.1 GCM to negotiated crypto algorigthms

GCM is faster. Request it during negotiate protocol.
Followon patch will add callouts to GCM crypto

Signed-off-by: Steve French <stfrench@microsoft.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2ops.c | 4 ++--
 fs/cifs/smb2pdu.c | 8 ++++----
 fs/cifs/smb2pdu.h | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index e921e6511728..7fa95929c8fc 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3333,7 +3333,7 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM;
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
-	get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CMM_NONCE);
+	get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
 	memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8);
 }
 
@@ -3492,7 +3492,7 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 		goto free_sg;
 	}
 	iv[0] = 3;
-	memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES128CMM_NONCE);
+	memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
 
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
 	aead_request_set_ad(req, assoc_data_len);
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index ab8dc73d2282..ab3300a39071 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -489,10 +489,10 @@ static void
 build_encrypt_ctxt(struct smb2_encryption_neg_context *pneg_ctxt)
 {
 	pneg_ctxt->ContextType = SMB2_ENCRYPTION_CAPABILITIES;
-	pneg_ctxt->DataLength = cpu_to_le16(4); /* Cipher Count + le16 cipher */
-	pneg_ctxt->CipherCount = cpu_to_le16(1);
-/* pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;*/ /* not supported yet */
-	pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_CCM;
+	pneg_ctxt->DataLength = cpu_to_le16(6); /* Cipher Count + two ciphers */
+	pneg_ctxt->CipherCount = cpu_to_le16(2);
+	pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;
+	pneg_ctxt->Ciphers[1] = SMB2_ENCRYPTION_AES128_CCM;
 }
 
 static void
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index c7d5813bebd8..d3a64cf812d9 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -123,7 +123,7 @@ struct smb2_sync_pdu {
 	__le16 StructureSize2; /* size of wct area (varies, request specific) */
 } __packed;
 
-#define SMB3_AES128CMM_NONCE 11
+#define SMB3_AES128CCM_NONCE 11
 #define SMB3_AES128GCM_NONCE 12
 
 struct smb2_transform_hdr {
@@ -293,7 +293,7 @@ struct smb2_encryption_neg_context {
 	__le16	DataLength;
 	__le32	Reserved;
 	__le16	CipherCount; /* AES-128-GCM and AES-128-CCM */
-	__le16	Ciphers[1]; /* Ciphers[0] since only one used now */
+	__le16	Ciphers[2];
 } __packed;
 
 /* See MS-SMB2 2.2.3.1.3 */
-- 
2.17.1.windows.2


[-- Attachment #3: 0002-Add-SMB3.1.1-GCM-crypto-to-the-encrypt-and-decrypt-f.patch --]
[-- Type: application/octet-stream, Size: 3671 bytes --]

From 120ae85c0e041d5c6ed2ca5adb370226bdd984e1 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Fri, 7 Jun 2019 15:16:10 -0500
Subject: [PATCH 2/2] Add SMB3.1.1 GCM crypto to the encrypt and decrypt
 functions

SMB3.1.1 GCM performs much better than the older CCM default:
more than twice as fast in the write patch (copy to the Samba
server on localhost for example) and 80% faster on the read
patch (copy from the server).

Signed-off-by: Steve French <stfrench@microsoft.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/smb2ops.c       | 18 +++++++++++++-----
 fs/cifs/smb2transport.c | 10 ++++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 7fa95929c8fc..a8e28b955c69 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -3324,7 +3324,7 @@ smb2_dir_needs_close(struct cifsFileInfo *cfile)
 
 static void
 fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
-		   struct smb_rqst *old_rq)
+		   struct smb_rqst *old_rq, struct TCP_Server_Info *server)
 {
 	struct smb2_sync_hdr *shdr =
 			(struct smb2_sync_hdr *)old_rq->rq_iov[0].iov_base;
@@ -3333,7 +3333,10 @@ fill_transform_hdr(struct smb2_transform_hdr *tr_hdr, unsigned int orig_len,
 	tr_hdr->ProtocolId = SMB2_TRANSFORM_PROTO_NUM;
 	tr_hdr->OriginalMessageSize = cpu_to_le32(orig_len);
 	tr_hdr->Flags = cpu_to_le16(0x01);
-	get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
+	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES128GCM_NONCE);
+	else
+		get_random_bytes(&tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
 	memcpy(&tr_hdr->SessionId, &shdr->SessionId, 8);
 }
 
@@ -3491,8 +3494,13 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 		rc = -ENOMEM;
 		goto free_sg;
 	}
-	iv[0] = 3;
-	memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
+
+	if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+		memcpy(iv, (char *)tr_hdr->Nonce, SMB3_AES128GCM_NONCE);
+	else {
+		iv[0] = 3;
+		memcpy(iv + 1, (char *)tr_hdr->Nonce, SMB3_AES128CCM_NONCE);
+	}
 
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
 	aead_request_set_ad(req, assoc_data_len);
@@ -3592,7 +3600,7 @@ smb3_init_transform_rq(struct TCP_Server_Info *server, int num_rqst,
 	}
 
 	/* fill the 1st iov with a transform header */
-	fill_transform_hdr(tr_hdr, orig_len, old_rq);
+	fill_transform_hdr(tr_hdr, orig_len, old_rq, server);
 
 	rc = crypt_message(server, num_rqst, new_rq, 1);
 	cifs_dbg(FYI, "Encrypt message returned %d\n", rc);
diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index d1181572758b..1ccbcf9c2c3b 100644
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -734,7 +734,10 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	struct crypto_aead *tfm;
 
 	if (!server->secmech.ccmaesencrypt) {
-		tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
+		else
+			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
 			cifs_dbg(VFS, "%s: Failed to alloc encrypt aead\n",
 				 __func__);
@@ -744,7 +747,10 @@ smb3_crypto_aead_allocate(struct TCP_Server_Info *server)
 	}
 
 	if (!server->secmech.ccmaesdecrypt) {
-		tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
+		if (server->cipher_type == SMB2_ENCRYPTION_AES128_GCM)
+			tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
+		else
+			tfm = crypto_alloc_aead("ccm(aes)", 0, 0);
 		if (IS_ERR(tfm)) {
 			crypto_free_aead(server->secmech.ccmaesencrypt);
 			server->secmech.ccmaesencrypt = NULL;
-- 
2.17.1.windows.2

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

[Ard Biesheuvel]

On Mon, 10 Jun 2019 at 19:54, Steve French <smfrench@gmail.com> wrote:
>
> On Mon, Jun 10, 2019 at 11:17 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Hi Steve,
> >
> > On Sun, Jun 09, 2019 at 05:03:25PM -0500, Steve French wrote:
> > > Probably harmless to change this code path (build_ntlmssp_auth_blob is
> > > called at session negotiation time so shouldn't have much of a
> > > performance impact).
> > >
> > > On the other hand if we can find optimizations in the encryption and
> > > signing paths, that would be really helpful.   There was a lot of
> > > focus on encryption performance at SambaXP last week.
> > >
> > > Andreas from Redhat gave a talk on the improvements in Samba with TLS
> > > implementation of AES-GCM.   I added the cifs client implementation of
> > > AES-GCM and notice it is now faster to encrypt packets than sign them
> > > (performance is about 2 to 3 times faster now with GCM).
> > >
> > > On Sun, Jun 9, 2019 at 6:57 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > The CIFS code uses the sync skcipher API to invoke the ecb(arc4) skcipher,
> > > > of which only a single generic C code implementation exists. This means
> > > > that going through all the trouble of using scatterlists etc buys us
> > > > very little, and we're better off just invoking the arc4 library directly.
> >
> > This patch only changes RC4 encryption, and to be clear it actually *improves*
> > the performance of the RC4 encryption, since it removes unnecessary
> > abstractions.  I'd really hope that people wouldn't care either way, though,
> > since RC4 is insecure and should not be used.
> >
> > Also it seems that fs/cifs/ supports AES-CCM but not AES-GCM?
> >
> > /* pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_GCM;*/ /* not supported yet */
> >         pneg_ctxt->Ciphers[0] = SMB2_ENCRYPTION_AES128_CCM;
> >
> > AES-GCM is usually faster than AES-CCM, so if you want to improve performance,
> > switching from CCM to GCM would do that.
> >
> > - Eric
>
> Yes - when I tested the GCM code in cifs.ko last week (the two patches
> are currently
> in the cifs-2.6.git for-next branch and thus in linux-next and are
> attached), I was astounded
> at the improvement - encryption with GCM is now faster than signing,
> and copy is more
> than twice as fast as encryption was before with CCM (assuming a fast
> enough network so
> that the network is not the bottleneck).  We see more benefit in the write path
> (copy to server) than the read path (copy from server) but even the
> read path gets
> 80% benefit in my tests, and with the addition of multichannel support
> in the next
> few months, we should see copy from server on SMB3.1.1 mounts
> improving even more.
>

I assume this was tested on high end x86 silicon? The CBCMAC path in
CCM is strictly sequential, so on systems with deep pipelines, you
lose a lot of speed there. For arm64, I implemented a CCM driver that
does the encryption and authentication in parallel, which mitigates
most of the performance hit, but even then, you will still be running
with a underutilized pipeline (unless you are using low end silicon
which only has a 2 cycle latency for AES instructions)

> Any other ideas how to improve the encryption or signing in the read
> or write path
> in cifs.ko would be appreciated.   We still are slower than Windows, probably in
> part due to holding mutexes longer in sending the frame across the network
> (including signing or encryption) which limits parallelism somewhat.
>

It all depends on whether crypto is still the bottleneck in this case.

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

[Steve French]

On Mon, Jun 10, 2019 at 1:02 PM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On Mon, 10 Jun 2019 at 19:54, Steve French <smfrench@gmail.com> wrote:
> > Yes - when I tested the GCM code in cifs.ko last week (the two patches
> > are currently
> > in the cifs-2.6.git for-next branch and thus in linux-next and are
> > attached), I was astounded
> > at the improvement - encryption with GCM is now faster than signing,
<snip>
> I assume this was tested on high end x86 silicon? The CBCMAC path in
> CCM is strictly sequential, so on systems with deep pipelines, you
> lose a lot of speed there. For arm64, I implemented a CCM driver that
> does the encryption and authentication in parallel, which mitigates
> most of the performance hit, but even then, you will still be running
> with a underutilized pipeline (unless you are using low end silicon
> which only has a 2 cycle latency for AES instructions)

Was doing most of my testing on an i7-7820HQ 2.9GHz
(passmark score 9231)

> > Any other ideas how to improve the encryption or signing in the read
> > or write path
> > in cifs.ko would be appreciated.   We still are slower than Windows, probably in
> > part due to holding mutexes longer in sending the frame across the network
> > (including signing or encryption) which limits parallelism somewhat.
> >
>
> It all depends on whether crypto is still the bottleneck in this case.

Trying to wade through traces to see.  Unfortunately lots of different
configurations (especially varying network latency and network
copy offload) to experiment with.



-- 
Thanks,

Steve