Subject: Convert arc4 from a cipher into a block cipher

This patch series converts arc4 into a block cipher and converts all its
users (except those in staging) to use it. The first two patches ensure
that two implementations can coexist, the following patches convert each
user so we remain bisectable.
- lib80211_crypt_tkip was tested with ipw2200
- mac80211 was tested with zd1211rw

Sebastian



2010-04-05 09:25:36

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 2/7] crypto: add blkcipher implementation of ARC4

On Sat, Apr 03, 2010 at 09:49:24AM +0200, Sebastian Andrzej Siewior wrote:
>
> +#include <crypto/arc4.h>

This file doesn't seem to exist in the kernel tree or your patch-set?

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Subject: [PATCH 1/7] crypto: rename arc4

The blk version of arc4 is comming. The rename ensures that the request
for arc4 loads both modules: this one and the new blk edition.

Cc: <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
crypto/Makefile | 2 +-
crypto/{arc4.c => arc4cip.c} | 1 +
2 files changed, 2 insertions(+), 1 deletions(-)
rename crypto/{arc4.c => arc4cip.c} (98%)

diff --git a/crypto/Makefile b/crypto/Makefile
index 9e8f619..1f15112 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -67,7 +67,7 @@ obj-$(CONFIG_CRYPTO_AES) += aes_generic.o
obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
-obj-$(CONFIG_CRYPTO_ARC4) += arc4.o
+obj-$(CONFIG_CRYPTO_ARC4) += arc4cip.o
obj-$(CONFIG_CRYPTO_TEA) += tea.o
obj-$(CONFIG_CRYPTO_KHAZAD) += khazad.o
obj-$(CONFIG_CRYPTO_ANUBIS) += anubis.o
diff --git a/crypto/arc4.c b/crypto/arc4cip.c
similarity index 98%
rename from crypto/arc4.c
rename to crypto/arc4cip.c
index 8be47e1..bf04659 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4cip.c
@@ -102,3 +102,4 @@ module_exit(arc4_exit);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("ARC4 Cipher Algorithm");
MODULE_AUTHOR("Jon Oberheide <[email protected]>");
+MODULE_ALIAS("arc4");
--
1.6.6


2010-04-05 17:30:09

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 6/7] net/mac80211: convert wep from arc4 to arc4blk

On Sat, Apr 03, 2010 at 09:49:28AM +0200, Sebastian Andrzej Siewior wrote:
> ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself.
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>

Seems ok to me...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

Subject: [PATCH 2/7] crypto: add blkcipher implementation of ARC4

This is a pure blkcipher implementation of ARC4. The internal state is
saved within an IV which is supplied by the user. The goal is that the
cipher does not change its internal state now, only the iv changes during
encryption.

Cc: <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
crypto/Kconfig | 13 +++++
crypto/Makefile | 1 +
crypto/arc4blk.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
crypto/testmgr.h | 3 +-
4 files changed, 166 insertions(+), 1 deletions(-)
create mode 100644 crypto/arc4blk.c

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 81c185a..be9add2 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -570,6 +570,19 @@ config CRYPTO_ARC4
WEP, but it should not be for other purposes because of the
weakness of the algorithm.

+config CRYPTO_ARC4BLK
+ tristate "ARC4 cipher algorithm (alternative implemenation)"
+ select CRYPTO_BLKCIPHER
+ help
+ ARC4 cipher algorithm. This is an alternative ARC4 implementation which
+ will replace the other ARC4 implementation once all in-kernel users are
+ converted.
+
+ ARC4 is a stream cipher using keys ranging from 8 bits to 2048
+ bits in length. This algorithm is required for driver-based
+ WEP, but it should not be for other purposes because of the
+ weakness of the algorithm.
+
config CRYPTO_BLOWFISH
tristate "Blowfish cipher algorithm"
select CRYPTO_ALGAPI
diff --git a/crypto/Makefile b/crypto/Makefile
index 1f15112..11300e3 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
obj-$(CONFIG_CRYPTO_ARC4) += arc4cip.o
+obj-$(CONFIG_CRYPTO_ARC4BLK) += arc4blk.o
obj-$(CONFIG_CRYPTO_TEA) += tea.o
obj-$(CONFIG_CRYPTO_KHAZAD) += khazad.o
obj-$(CONFIG_CRYPTO_ANUBIS) += anubis.o
diff --git a/crypto/arc4blk.c b/crypto/arc4blk.c
new file mode 100644
index 0000000..bdf938a
--- /dev/null
+++ b/crypto/arc4blk.c
@@ -0,0 +1,150 @@
+/*
+ * Cryptographic API
+ *
+ * ARC4 Cipher Algorithm
+ *
+ * Jon Oberheide <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <crypto/algapi.h>
+#include <crypto/arc4.h>
+
+#define ARC4_MIN_KEY_SIZE 1
+#define ARC4_MAX_KEY_SIZE 256
+#define ARC4_BLOCK_SIZE 1
+
+static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
+ unsigned int key_len)
+{
+ /*
+ * ARC4 is special: The user should supply an IV as struct arc4_iv and
+ * fill either the key or the iv.
+ */
+ return 0;
+}
+
+static void arc4_key_to_iv(const u8 *in_key, u32 key_len, struct arc4_iv *iv)
+{
+ int i, j = 0, k = 0;
+
+ iv->iv.x = 1;
+ iv->iv.y = 0;
+
+ for (i = 0; i < 256; i++)
+ iv->iv.S[i] = i;
+
+ for (i = 0; i < 256; i++)
+ {
+ u8 a = iv->iv.S[i];
+ j = (j + in_key[k] + a) & 0xff;
+ iv->iv.S[i] = iv->iv.S[j];
+ iv->iv.S[j] = a;
+ if (++k >= key_len)
+ k = 0;
+ }
+}
+
+static void arc4_ivsetup(struct arc4_iv *iv)
+{
+ struct arc4_iv tmp_iv;
+
+ if (iv->type == ARC4_TYPE_IV)
+ return;
+
+ memcpy(&tmp_iv, iv, sizeof(tmp_iv));
+ arc4_key_to_iv(tmp_iv.key.key, tmp_iv.key.key_len, iv);
+ iv->type = ARC4_TYPE_IV;
+}
+
+static int arc4_crypt(struct blkcipher_desc *desc, struct scatterlist *dst,
+ struct scatterlist *src, unsigned int nbytes)
+{
+ struct blkcipher_walk walk;
+ struct arc4_iv *aiv;
+ u8 *S;
+ u8 x;
+ u8 y;
+ u8 a, b;
+ int ret;
+
+ blkcipher_walk_init(&walk, dst, src, nbytes);
+ ret = blkcipher_walk_virt(desc, &walk);
+ if (ret)
+ return ret;
+
+ aiv = (struct arc4_iv *)walk.iv;
+ arc4_ivsetup(aiv);
+
+ S = aiv->iv.S;
+ x = aiv->iv.x;
+ y = aiv->iv.y;
+
+ while (walk.nbytes) {
+ u8 *in = walk.src.virt.addr;
+ u8 *out = walk.dst.virt.addr;
+ u32 i;
+
+ for (i = 0; i < walk.nbytes; i++) {
+ a = S[x];
+ y = (y + a) & 0xff;
+ b = S[y];
+ S[x] = b;
+ S[y] = a;
+ x = (x + 1) & 0xff;
+ *out = *in ^ S[(a + b) & 0xff];
+
+ in++;
+ out++;
+ }
+ ret = blkcipher_walk_done(desc, &walk, 0);
+ WARN_ON(ret < 0);
+ }
+
+ aiv->iv.x = x;
+ aiv->iv.y = y;
+ return ret;
+}
+
+static struct crypto_alg arc4_alg = {
+ .cra_name = "arc4",
+ .cra_priority = 100,
+ .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
+ .cra_blocksize = ARC4_BLOCK_SIZE,
+ .cra_ctxsize = 0,
+ .cra_type = &crypto_blkcipher_type,
+ .cra_alignmask = 3,
+ .cra_module = THIS_MODULE,
+ .cra_list = LIST_HEAD_INIT(arc4_alg.cra_list),
+ .cra_u = { .blkcipher = {
+ .min_keysize = 0,
+ .max_keysize = 0,
+ .ivsize = sizeof(struct arc4_iv),
+ .setkey = arc4_set_key,
+ .encrypt = arc4_crypt,
+ .decrypt = arc4_crypt } }
+};
+
+static int __init arc4_init(void)
+{
+ return crypto_register_alg(&arc4_alg);
+}
+
+static void __exit arc4_exit(void)
+{
+ crypto_unregister_alg(&arc4_alg);
+}
+
+module_init(arc4_init);
+module_exit(arc4_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ARC4 Cipher Algorithm");
+MODULE_AUTHOR("Jon Oberheide <[email protected]>");
+MODULE_ALIAS("arc4");
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index fb76517..423cf86 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -24,7 +24,8 @@
#define MAX_TAP 8

#define MAX_KEYLEN 56
-#define MAX_IVLEN 32
+/* sizeof arc4_iv */
+#define MAX_IVLEN 260

struct hash_testvec {
/* only used with keyed hash algorithms */
--
1.6.6


2010-04-07 09:29:21

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 3/7] crypto/testmgr: add testing for arc4 based on ecb(arc4)

On Wed, Apr 07, 2010 at 02:29:53AM -0400, Pavel Roskin wrote:
>
> Can we avoid those special cases? If the goal is "to make arc4
> compliant with the crypto API", this looks like a step in a wrong
> direction.
>
> The same applies to many other changes in the series.
>
> I do realize that the original arc4 is not a block cipher at all.

I don't like this bit either.

Sebastian, how about precomputing the IV and provide them directly
as a hex array?

To test arc4_setup_iv itself, you can add an alg_test_arc4 function
(like alg_test_crc32) that tests IV generation specifically.

Alternatively, just add an alg_test_arc4 that computes the IV
before calling alg_test_skcipher.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

* Pavel Roskin | 2010-04-07 02:19:55 [-0400]:

>On Mon, 2010-04-05 at 19:04 +0200, Sebastian Andrzej Siewior wrote:
>
>> +module_init(arc4_init);
>> +module_exit(arc4_exit);
>
>I'm feelings uneasy about using the same module init/exit functions
>names in arc4blk.c and arc4cip.c.
>
>Even though it doesn't break for me on x86_64 (whether I'm compiling
>modules or a solid kernel), and even though the potential name conflict
Take a look at
- sd_mod_init
- via_init
- watchdog_init

just to name a few. There is no conflict because those functions are not
global. The only problem you have is in the backtrace since you can't
distinguish them.

Sebastian

Subject: [PATCH 3/7] crypto/testmgr: add testing for arc4 based on ecb(arc4)

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
crypto/testmgr.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 7620bfc..c471e04 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -20,6 +20,7 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <crypto/rng.h>
+#include <crypto/arc4.h>

#include "internal.h"
#include "testmgr.h"
@@ -44,6 +45,7 @@
/*
* Used by test_cipher()
*/
+#define CRYPT_ARC4 2
#define ENCRYPT 1
#define DECRYPT 0

@@ -717,7 +719,7 @@ out_nobuf:
return ret;
}

-static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
+static int test_skcipher(struct crypto_ablkcipher *tfm, int mode,
struct cipher_testvec *template, unsigned int tcount)
{
const char *algo =
@@ -736,7 +738,7 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;

- if (enc == ENCRYPT)
+ if (mode == ENCRYPT)
e = "encryption";
else
e = "decryption";
@@ -775,7 +777,11 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
crypto_ablkcipher_set_flags(
tfm, CRYPTO_TFM_REQ_WEAK_KEY);

- ret = crypto_ablkcipher_setkey(tfm, template[i].key,
+ if (mode == CRYPT_ARC4)
+ arc4_setup_iv((struct arc4_iv *)iv,
+ template[i].key, template[i].klen);
+ else
+ ret = crypto_ablkcipher_setkey(tfm, template[i].key,
template[i].klen);
if (!ret == template[i].fail) {
printk(KERN_ERR "alg: skcipher: setkey failed "
@@ -789,7 +795,7 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,

ablkcipher_request_set_crypt(req, sg, sg,
template[i].ilen, iv);
- ret = enc ?
+ ret = mode ?
crypto_ablkcipher_encrypt(req) :
crypto_ablkcipher_decrypt(req);

@@ -839,7 +845,11 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
crypto_ablkcipher_set_flags(
tfm, CRYPTO_TFM_REQ_WEAK_KEY);

- ret = crypto_ablkcipher_setkey(tfm, template[i].key,
+ if (mode == CRYPT_ARC4)
+ arc4_setup_iv((struct arc4_iv *)iv,
+ template[i].key, template[i].klen);
+ else
+ ret = crypto_ablkcipher_setkey(tfm, template[i].key,
template[i].klen);
if (!ret == template[i].fail) {
printk(KERN_ERR "alg: skcipher: setkey failed "
@@ -876,7 +886,7 @@ static int test_skcipher(struct crypto_ablkcipher *tfm, int enc,
ablkcipher_request_set_crypt(req, sg, sg,
template[i].ilen, iv);

- ret = enc ?
+ ret = mode ?
crypto_ablkcipher_encrypt(req) :
crypto_ablkcipher_decrypt(req);

@@ -1316,11 +1326,12 @@ out:
return err;
}

-static int alg_test_skcipher(const struct alg_test_desc *desc,
- const char *driver, u32 type, u32 mask)
+static int _alg_test_skcipher(const struct alg_test_desc *desc,
+ const char *driver, u32 type, u32 mask, u32 arc4)
{
struct crypto_ablkcipher *tfm;
int err = 0;
+ int mode;

tfm = crypto_alloc_ablkcipher(driver, type, mask);
if (IS_ERR(tfm)) {
@@ -1329,15 +1340,17 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
return PTR_ERR(tfm);
}

+ mode = arc4 ? CRYPT_ARC4 : ENCRYPT;
if (desc->suite.cipher.enc.vecs) {
- err = test_skcipher(tfm, ENCRYPT, desc->suite.cipher.enc.vecs,
+ err = test_skcipher(tfm, mode , desc->suite.cipher.enc.vecs,
desc->suite.cipher.enc.count);
if (err)
goto out;
}

+ mode = arc4 ? CRYPT_ARC4 : DECRYPT;
if (desc->suite.cipher.dec.vecs)
- err = test_skcipher(tfm, DECRYPT, desc->suite.cipher.dec.vecs,
+ err = test_skcipher(tfm, mode, desc->suite.cipher.dec.vecs,
desc->suite.cipher.dec.count);

out:
@@ -1345,6 +1358,18 @@ out:
return err;
}

+static int alg_test_skcipher(const struct alg_test_desc *desc,
+ const char *driver, u32 type, u32 mask)
+{
+ return _alg_test_skcipher(desc, driver, type, mask, 0);
+}
+
+static int alg_test_arc4_skcipher(const struct alg_test_desc *desc,
+ const char *driver, u32 type, u32 mask)
+{
+ return _alg_test_skcipher(desc, driver, type, mask, 1);
+}
+
static int alg_test_comp(const struct alg_test_desc *desc, const char *driver,
u32 type, u32 mask)
{
@@ -1490,6 +1515,21 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}
}, {
+ .alg = "arc4",
+ .test = alg_test_arc4_skcipher,
+ .suite = {
+ .cipher = {
+ .enc = {
+ .vecs = arc4_enc_tv_template,
+ .count = ARC4_ENC_TEST_VECTORS
+ },
+ .dec = {
+ .vecs = arc4_dec_tv_template,
+ .count = ARC4_DEC_TEST_VECTORS
+ }
+ }
+ }
+ }, {
.alg = "cbc(aes)",
.test = alg_test_skcipher,
.fips_allowed = 1,
--
1.6.6


2010-04-06 12:44:15

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

On Mon, Apr 05, 2010 at 07:04:06PM +0200, Sebastian Andrzej Siewior wrote:
>
> +static void arc4_key_to_iv(const u8 *in_key, u32 key_len, struct arc4_iv *iv)
> +{
> + int i, j = 0, k = 0;
> +
> + iv->iv.x = 1;
> + iv->iv.y = 0;
> +
> + for (i = 0; i < 256; i++)
> + iv->iv.S[i] = i;
> +
> + for (i = 0; i < 256; i++)
> + {
> + u8 a = iv->iv.S[i];
> + j = (j + in_key[k] + a) & 0xff;
> + iv->iv.S[i] = iv->iv.S[j];
> + iv->iv.S[j] = a;
> + if (++k >= key_len)
> + k = 0;
> + }
> +}
> +
> +static void arc4_ivsetup(struct arc4_iv *iv)
> +{
> + struct arc4_iv tmp_iv;
> +
> + if (iv->type == ARC4_TYPE_IV)
> + return;
> +
> + memcpy(&tmp_iv, iv, sizeof(tmp_iv));
> + arc4_key_to_iv(tmp_iv.key.key, tmp_iv.key.key_len, iv);
> + iv->type = ARC4_TYPE_IV;
> +}

We need to verify that 1 <= key_len <= 256.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2010-04-07 09:25:07

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

On Wed, Apr 07, 2010 at 10:23:00AM +0200, Sebastian Andrzej Siewior wrote:
>
> So arc4_setup_iv() should do what the internal arc4_ivsetup() does and
> we change void to int and check the keysize in there right? The problem
> here is that we are bounded to *this* implementation of the algorithm
> and are not able to replace it with a different implementation. Not that
> this is likely to happen for RC4 but it may be true for other stream
> ciphers.

By setting an IV we're already requiring the other implementations
use the IV format used by our arc4. So they would always work with
this arc4_ivsetup anyway.

If and when we do have a piece of hardware that cannot do this
(which I doubt would ever happen, considering how fast arc4 is
already), then we can talk about changing this.

> >I'm pretty sure testmgr will call setkey even for keylen == 0, no?
> Prior patch #3 it has no test case so it should not test it at all.
> Patch #3 adds a flag in order to distinguish it. You want to look at
> patch #3 now :)

I see.

But still we should at least not crash when crypto_blkcipher_setkey
is called. This might happen in future when we get a user-space
API.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2010-04-07 06:19:57

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

On Mon, 2010-04-05 at 19:04 +0200, Sebastian Andrzej Siewior wrote:

> +module_init(arc4_init);
> +module_exit(arc4_exit);

I'm feelings uneasy about using the same module init/exit functions
names in arc4blk.c and arc4cip.c.

Even though it doesn't break for me on x86_64 (whether I'm compiling
modules or a solid kernel), and even though the potential name conflict
is temporary until arc4cip.c is removed, it could break on some other
architecture or maybe with another linker.

Let's use arc4blk_init and arc4blk_exit.

--
Regards,
Pavel Roskin

2010-04-05 17:15:09

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 5/7] net/wireless: switch lib80211_crypt_wep from arc4 to arc4blk

On Sat, Apr 03, 2010 at 09:49:27AM +0200, Sebastian Andrzej Siewior wrote:
> ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself. The
> required selects are now pulled in by LIB80211_CRYPT_WEP instead of
> selecting it by every driver. Since there is no dependency on ecb and arc4
> therr are removed from the idividual driver.
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>

Fine by me...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-04-05 20:33:47

by Pavel Roskin

[permalink] [raw]
Subject: Re: Convert arc4 from a cipher into a block cipher

On Sat, 2010-04-03 at 09:49 +0200, Sebastian Andrzej Siewior wrote:
> This patch series converts arc4 into a block cipher and converts all its
> users (except those in staging) to use it. The first two patches ensure
> that two implementations can coexist, the following patches convert each
> user so we remain bisectable.
> - lib80211_crypt_tkip was tested with ipw2200
> - mac80211 was tested with zd1211rw

Are you trying to speed up arc4? Or you want to simplify the code? Or
maybe you are trying to make arc4 unsuitable for anything other than WEP
and TKIP? The later should be fine, actually, considering the known
security issues.

--
Regards,
Pavel Roskin

Subject: Re: [PATCH 3/7] crypto/testmgr: add testing for arc4 based on ecb(arc4)

* Herbert Xu | 2010-04-07 17:29:07 [+0800]:

>Sebastian, how about precomputing the IV and provide them directly
>as a hex array?
>
>To test arc4_setup_iv itself, you can add an alg_test_arc4 function
>(like alg_test_crc32) that tests IV generation specifically.
>
>Alternatively, just add an alg_test_arc4 that computes the IV
>before calling alg_test_skcipher.

I take a look at this.

>Cheers,

Sebastian

Subject: [PATCH 5/7] net/wireless: switch lib80211_crypt_wep from arc4 to arc4blk

ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself. The
required selects are now pulled in by LIB80211_CRYPT_WEP instead of
selecting it by every driver. Since there is no dependency on ecb and arc4
therr are removed from the idividual driver.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
drivers/net/wireless/hostap/Kconfig | 3 ---
drivers/net/wireless/ipw2x00/Kconfig | 3 ---
net/wireless/Kconfig | 2 ++
net/wireless/lib80211_crypt_wep.c | 11 +++++++----
4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/hostap/Kconfig b/drivers/net/wireless/hostap/Kconfig
index 287d827..dab2c6b 100644
--- a/drivers/net/wireless/hostap/Kconfig
+++ b/drivers/net/wireless/hostap/Kconfig
@@ -4,11 +4,8 @@ config HOSTAP
select WEXT_SPY
select WEXT_PRIV
select CRYPTO
- select CRYPTO_ARC4
- select CRYPTO_ECB
select CRYPTO_AES
select CRYPTO_MICHAEL_MIC
- select CRYPTO_ECB
select CRC32
select LIB80211
select LIB80211_CRYPT_WEP
diff --git a/drivers/net/wireless/ipw2x00/Kconfig b/drivers/net/wireless/ipw2x00/Kconfig
index 2715b10..6fe1995 100644
--- a/drivers/net/wireless/ipw2x00/Kconfig
+++ b/drivers/net/wireless/ipw2x00/Kconfig
@@ -158,11 +158,8 @@ config LIBIPW
select WIRELESS_EXT
select WEXT_SPY
select CRYPTO
- select CRYPTO_ARC4
- select CRYPTO_ECB
select CRYPTO_AES
select CRYPTO_MICHAEL_MIC
- select CRYPTO_ECB
select CRC32
select LIB80211
select LIB80211_CRYPT_WEP
diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 119f1eb..267eb42 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -141,6 +141,8 @@ config LIB80211
you want this built into your kernel.

config LIB80211_CRYPT_WEP
+ select CRYPTO
+ select CRYPTO_ARC4BLK
tristate

config LIB80211_CRYPT_CCMP
diff --git a/net/wireless/lib80211_crypt_wep.c b/net/wireless/lib80211_crypt_wep.c
index 6d41e05..3759e46 100644
--- a/net/wireless/lib80211_crypt_wep.c
+++ b/net/wireless/lib80211_crypt_wep.c
@@ -23,6 +23,7 @@
#include <net/lib80211.h>

#include <linux/crypto.h>
+#include <crypto/arc4.h>
#include <linux/crc32.h>

MODULE_AUTHOR("Jouni Malinen");
@@ -48,7 +49,7 @@ static void *lib80211_wep_init(int keyidx)
goto fail;
priv->key_idx = keyidx;

- priv->tx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
+ priv->tx_tfm = crypto_alloc_blkcipher("arc4", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->tx_tfm)) {
printk(KERN_DEBUG "lib80211_crypt_wep: could not allocate "
"crypto API arc4\n");
@@ -56,7 +57,7 @@ static void *lib80211_wep_init(int keyidx)
goto fail;
}

- priv->rx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
+ priv->rx_tfm = crypto_alloc_blkcipher("arc4", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->rx_tfm)) {
printk(KERN_DEBUG "lib80211_crypt_wep: could not allocate "
"crypto API arc4\n");
@@ -139,6 +140,7 @@ static int lib80211_wep_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct lib80211_wep_data *wep = priv;
struct blkcipher_desc desc = { .tfm = wep->tx_tfm };
+ struct arc4_iv *iv = crypto_blkcipher_crt(wep->tx_tfm)->iv;
u32 crc, klen, len;
u8 *pos, *icv;
struct scatterlist sg;
@@ -170,7 +172,7 @@ static int lib80211_wep_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
icv[2] = crc >> 16;
icv[3] = crc >> 24;

- crypto_blkcipher_setkey(wep->tx_tfm, key, klen);
+ arc4_setup_iv(iv, key, klen);
sg_init_one(&sg, pos, len + 4);
return crypto_blkcipher_encrypt(&desc, &sg, &sg, len + 4);
}
@@ -186,6 +188,7 @@ static int lib80211_wep_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct lib80211_wep_data *wep = priv;
struct blkcipher_desc desc = { .tfm = wep->rx_tfm };
+ struct arc4_iv *iv = crypto_blkcipher_crt(wep->rx_tfm)->iv;
u32 crc, klen, plen;
u8 key[WEP_KEY_LEN + 3];
u8 keyidx, *pos, icv[4];
@@ -210,7 +213,7 @@ 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_blkcipher_setkey(wep->rx_tfm, key, klen);
+ arc4_setup_iv(iv, key, klen);
sg_init_one(&sg, pos, plen + 4);
if (crypto_blkcipher_decrypt(&desc, &sg, &sg, plen + 4))
return -7;
--
1.6.6


2010-04-07 16:31:56

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

On Wed, 2010-04-07 at 10:29 +0200, Sebastian Andrzej Siewior wrote:
> * Pavel Roskin | 2010-04-07 02:19:55 [-0400]:
>
> >On Mon, 2010-04-05 at 19:04 +0200, Sebastian Andrzej Siewior wrote:
> >
> >> +module_init(arc4_init);
> >> +module_exit(arc4_exit);
> >
> >I'm feelings uneasy about using the same module init/exit functions
> >names in arc4blk.c and arc4cip.c.
> >
> >Even though it doesn't break for me on x86_64 (whether I'm compiling
> >modules or a solid kernel), and even though the potential name conflict
> Take a look at
> - sd_mod_init
> - via_init
> - watchdog_init
>
> just to name a few. There is no conflict because those functions are not
> global. The only problem you have is in the backtrace since you can't
> distinguish them.

Touché :-)

--
Regards,
Pavel Roskin

Subject: [PATCH v2] crypto: add blkcipher implementation of ARC4

This is a pure blkcipher implementation of ARC4. The internal state is
saved within an IV which is supplied by the user. The goal is that the
cipher does not change its internal state now, only the iv changes during
encryption.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
crypto/Kconfig | 13 ++++
crypto/Makefile | 1 +
crypto/arc4blk.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++++
crypto/testmgr.h | 3 +-
include/crypto/arc4.h | 26 +++++++++
5 files changed, 192 insertions(+), 1 deletions(-)
create mode 100644 crypto/arc4blk.c
create mode 100644 include/crypto/arc4.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 81c185a..be9add2 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -570,6 +570,19 @@ config CRYPTO_ARC4
WEP, but it should not be for other purposes because of the
weakness of the algorithm.

+config CRYPTO_ARC4BLK
+ tristate "ARC4 cipher algorithm (alternative implemenation)"
+ select CRYPTO_BLKCIPHER
+ help
+ ARC4 cipher algorithm. This is an alternative ARC4 implementation which
+ will replace the other ARC4 implementation once all in-kernel users are
+ converted.
+
+ ARC4 is a stream cipher using keys ranging from 8 bits to 2048
+ bits in length. This algorithm is required for driver-based
+ WEP, but it should not be for other purposes because of the
+ weakness of the algorithm.
+
config CRYPTO_BLOWFISH
tristate "Blowfish cipher algorithm"
select CRYPTO_ALGAPI
diff --git a/crypto/Makefile b/crypto/Makefile
index 1f15112..11300e3 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
obj-$(CONFIG_CRYPTO_ARC4) += arc4cip.o
+obj-$(CONFIG_CRYPTO_ARC4BLK) += arc4blk.o
obj-$(CONFIG_CRYPTO_TEA) += tea.o
obj-$(CONFIG_CRYPTO_KHAZAD) += khazad.o
obj-$(CONFIG_CRYPTO_ANUBIS) += anubis.o
diff --git a/crypto/arc4blk.c b/crypto/arc4blk.c
new file mode 100644
index 0000000..bdf938a
--- /dev/null
+++ b/crypto/arc4blk.c
@@ -0,0 +1,150 @@
+/*
+ * Cryptographic API
+ *
+ * ARC4 Cipher Algorithm
+ *
+ * Jon Oberheide <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <crypto/algapi.h>
+#include <crypto/arc4.h>
+
+#define ARC4_MIN_KEY_SIZE 1
+#define ARC4_MAX_KEY_SIZE 256
+#define ARC4_BLOCK_SIZE 1
+
+static int arc4_set_key(struct crypto_tfm *tfm, const u8 *in_key,
+ unsigned int key_len)
+{
+ /*
+ * ARC4 is special: The user should supply an IV as struct arc4_iv and
+ * fill either the key or the iv.
+ */
+ return 0;
+}
+
+static void arc4_key_to_iv(const u8 *in_key, u32 key_len, struct arc4_iv *iv)
+{
+ int i, j = 0, k = 0;
+
+ iv->iv.x = 1;
+ iv->iv.y = 0;
+
+ for (i = 0; i < 256; i++)
+ iv->iv.S[i] = i;
+
+ for (i = 0; i < 256; i++)
+ {
+ u8 a = iv->iv.S[i];
+ j = (j + in_key[k] + a) & 0xff;
+ iv->iv.S[i] = iv->iv.S[j];
+ iv->iv.S[j] = a;
+ if (++k >= key_len)
+ k = 0;
+ }
+}
+
+static void arc4_ivsetup(struct arc4_iv *iv)
+{
+ struct arc4_iv tmp_iv;
+
+ if (iv->type == ARC4_TYPE_IV)
+ return;
+
+ memcpy(&tmp_iv, iv, sizeof(tmp_iv));
+ arc4_key_to_iv(tmp_iv.key.key, tmp_iv.key.key_len, iv);
+ iv->type = ARC4_TYPE_IV;
+}
+
+static int arc4_crypt(struct blkcipher_desc *desc, struct scatterlist *dst,
+ struct scatterlist *src, unsigned int nbytes)
+{
+ struct blkcipher_walk walk;
+ struct arc4_iv *aiv;
+ u8 *S;
+ u8 x;
+ u8 y;
+ u8 a, b;
+ int ret;
+
+ blkcipher_walk_init(&walk, dst, src, nbytes);
+ ret = blkcipher_walk_virt(desc, &walk);
+ if (ret)
+ return ret;
+
+ aiv = (struct arc4_iv *)walk.iv;
+ arc4_ivsetup(aiv);
+
+ S = aiv->iv.S;
+ x = aiv->iv.x;
+ y = aiv->iv.y;
+
+ while (walk.nbytes) {
+ u8 *in = walk.src.virt.addr;
+ u8 *out = walk.dst.virt.addr;
+ u32 i;
+
+ for (i = 0; i < walk.nbytes; i++) {
+ a = S[x];
+ y = (y + a) & 0xff;
+ b = S[y];
+ S[x] = b;
+ S[y] = a;
+ x = (x + 1) & 0xff;
+ *out = *in ^ S[(a + b) & 0xff];
+
+ in++;
+ out++;
+ }
+ ret = blkcipher_walk_done(desc, &walk, 0);
+ WARN_ON(ret < 0);
+ }
+
+ aiv->iv.x = x;
+ aiv->iv.y = y;
+ return ret;
+}
+
+static struct crypto_alg arc4_alg = {
+ .cra_name = "arc4",
+ .cra_priority = 100,
+ .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
+ .cra_blocksize = ARC4_BLOCK_SIZE,
+ .cra_ctxsize = 0,
+ .cra_type = &crypto_blkcipher_type,
+ .cra_alignmask = 3,
+ .cra_module = THIS_MODULE,
+ .cra_list = LIST_HEAD_INIT(arc4_alg.cra_list),
+ .cra_u = { .blkcipher = {
+ .min_keysize = 0,
+ .max_keysize = 0,
+ .ivsize = sizeof(struct arc4_iv),
+ .setkey = arc4_set_key,
+ .encrypt = arc4_crypt,
+ .decrypt = arc4_crypt } }
+};
+
+static int __init arc4_init(void)
+{
+ return crypto_register_alg(&arc4_alg);
+}
+
+static void __exit arc4_exit(void)
+{
+ crypto_unregister_alg(&arc4_alg);
+}
+
+module_init(arc4_init);
+module_exit(arc4_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ARC4 Cipher Algorithm");
+MODULE_AUTHOR("Jon Oberheide <[email protected]>");
+MODULE_ALIAS("arc4");
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index fb76517..423cf86 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -24,7 +24,8 @@
#define MAX_TAP 8

#define MAX_KEYLEN 56
-#define MAX_IVLEN 32
+/* sizeof arc4_iv */
+#define MAX_IVLEN 260

struct hash_testvec {
/* only used with keyed hash algorithms */
diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
new file mode 100644
index 0000000..1423c92
--- /dev/null
+++ b/include/crypto/arc4.h
@@ -0,0 +1,26 @@
+#ifndef __CRYPTO_ARC4_H__
+#define __CRYPTO_ARC4_H__
+
+struct arc4_iv {
+ union {
+ struct arc4_key {
+ u8 key[256];
+ u16 key_len;
+ } key;
+ struct arc4_riv {
+ u8 S[256];
+ u8 x, y;
+ } iv;
+ };
+#define ARC4_TYPE_KEY 0
+#define ARC4_TYPE_IV 1
+ u8 type;
+};
+
+static inline void arc4_setup_iv(struct arc4_iv *iv, const u8 *key, u32 len)
+{
+ memcpy(iv->key.key, key, len);
+ iv->key.key_len = len;
+ iv->type = ARC4_TYPE_KEY;
+}
+#endif
--
1.7.0.3


Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

* Herbert Xu | 2010-04-06 20:44:12 [+0800]:

>On Mon, Apr 05, 2010 at 07:04:06PM +0200, Sebastian Andrzej Siewior wrote:
>>
>> +static void arc4_key_to_iv(const u8 *in_key, u32 key_len, struct arc4_iv *iv)
>> +{
>> + int i, j = 0, k = 0;
>> +
>> + iv->iv.x = 1;
>> + iv->iv.y = 0;
>> +
>> + for (i = 0; i < 256; i++)
>> + iv->iv.S[i] = i;
>> +
>> + for (i = 0; i < 256; i++)
>> + {
>> + u8 a = iv->iv.S[i];
>> + j = (j + in_key[k] + a) & 0xff;
>> + iv->iv.S[i] = iv->iv.S[j];
>> + iv->iv.S[j] = a;
>> + if (++k >= key_len)
>> + k = 0;
>> + }
>> +}
>> +
>> +static void arc4_ivsetup(struct arc4_iv *iv)
>> +{
>> + struct arc4_iv tmp_iv;
>> +
>> + if (iv->type == ARC4_TYPE_IV)
>> + return;
>> +
>> + memcpy(&tmp_iv, iv, sizeof(tmp_iv));
>> + arc4_key_to_iv(tmp_iv.key.key, tmp_iv.key.key_len, iv);
>> + iv->type = ARC4_TYPE_IV;
>> +}
>
>We need to verify that 1 <= key_len <= 256.
Good point. All arc4 users don't care about return value of setkey so I
think that I just change void to int add the check for the valid key
length.

While we are here, the .setkey() callback could be removed, couldn't it?
It returns 0 even it is doing nothing what looks kinda wrong. However it
shouldn't be called at all since min/max key is 0. Any objections on
that?

>
>Cheers,

Sebastian

2010-04-05 17:15:09

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 4/7] net/wireless: switch lib80211_crypt_tkip from arc4 to arc4blk

On Sat, Apr 03, 2010 at 09:49:26AM +0200, Sebastian Andrzej Siewior wrote:
> ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself. The
> required selects are now pulled in by LIB80211_CRYPT_TKIP instead of
> selecting it by every driver.
>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>

Fine by me...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-04-07 06:29:55

by Pavel Roskin

[permalink] [raw]
Subject: Re: [PATCH 3/7] crypto/testmgr: add testing for arc4 based on ecb(arc4)

On Sat, 2010-04-03 at 09:49 +0200, Sebastian Andrzej Siewior wrote:

> + if (mode == CRYPT_ARC4)
> + arc4_setup_iv((struct arc4_iv *)iv,
> + template[i].key, template[i].klen);
> + else
> + ret = crypto_ablkcipher_setkey(tfm, template[i].key,
> template[i].klen);

Can we avoid those special cases? If the goal is "to make arc4
compliant with the crypto API", this looks like a step in a wrong
direction.

The same applies to many other changes in the series.

I do realize that the original arc4 is not a block cipher at all.

--
Regards,
Pavel Roskin

Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

* Herbert Xu | 2010-04-07 08:31:09 [+0800]:

>On Tue, Apr 06, 2010 at 10:30:02PM +0200, Sebastian Andrzej Siewior wrote:
>>
>> Good point. All arc4 users don't care about return value of setkey so I
>> think that I just change void to int add the check for the valid key
>> length.
>
>Actually, how about getting arc4_setup_iv to do all the legwork
>and turn it into a real IV? Then we don't need any checks on the
>data path.
So arc4_setup_iv() should do what the internal arc4_ivsetup() does and
we change void to int and check the keysize in there right? The problem
here is that we are bounded to *this* implementation of the algorithm
and are not able to replace it with a different implementation. Not that
this is likely to happen for RC4 but it may be true for other stream
ciphers.

>> While we are here, the .setkey() callback could be removed, couldn't it?
>> It returns 0 even it is doing nothing what looks kinda wrong. However it
>> shouldn't be called at all since min/max key is 0. Any objections on
>> that?
>
>I'm pretty sure testmgr will call setkey even for keylen == 0, no?
Prior patch #3 it has no test case so it should not test it at all.
Patch #3 adds a flag in order to distinguish it. You want to look at
patch #3 now :)

>
>Thanks,

Sebastian

Subject: [PATCH 4/7] net/wireless: switch lib80211_crypt_tkip from arc4 to arc4blk

ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself. The
required selects are now pulled in by LIB80211_CRYPT_TKIP instead of
selecting it by every driver.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
net/wireless/Kconfig | 2 ++
net/wireless/lib80211_crypt_tkip.c | 11 +++++++----
2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index 90e93a5..119f1eb 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -147,6 +147,8 @@ config LIB80211_CRYPT_CCMP
tristate

config LIB80211_CRYPT_TKIP
+ select CRYPTO
+ select CRYPTO_ARC4BLK
tristate

config LIB80211_DEBUG
diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index c362873..089f84f 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -28,6 +28,7 @@
#include <net/iw_handler.h>

#include <linux/crypto.h>
+#include <crypto/arc4.h>
#include <linux/crc32.h>

#include <net/lib80211.h>
@@ -94,7 +95,7 @@ static void *lib80211_tkip_init(int key_idx)

priv->key_idx = key_idx;

- priv->tx_tfm_arc4 = crypto_alloc_blkcipher("ecb(arc4)", 0,
+ priv->tx_tfm_arc4 = crypto_alloc_blkcipher("arc4", 0,
CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->tx_tfm_arc4)) {
printk(KERN_DEBUG "lib80211_crypt_tkip: could not allocate "
@@ -112,7 +113,7 @@ static void *lib80211_tkip_init(int key_idx)
goto fail;
}

- priv->rx_tfm_arc4 = crypto_alloc_blkcipher("ecb(arc4)", 0,
+ priv->rx_tfm_arc4 = crypto_alloc_blkcipher("arc4", 0,
CRYPTO_ALG_ASYNC);
if (IS_ERR(priv->rx_tfm_arc4)) {
printk(KERN_DEBUG "lib80211_crypt_tkip: could not allocate "
@@ -360,6 +361,7 @@ static int lib80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct lib80211_tkip_data *tkey = priv;
struct blkcipher_desc desc = { .tfm = tkey->tx_tfm_arc4 };
+ struct arc4_iv *iv = crypto_blkcipher_crt(tkey->tx_tfm_arc4)->iv;
int len;
u8 rc4key[16], *pos, *icv;
u32 crc;
@@ -392,7 +394,7 @@ static int lib80211_tkip_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
icv[2] = crc >> 16;
icv[3] = crc >> 24;

- crypto_blkcipher_setkey(tkey->tx_tfm_arc4, rc4key, 16);
+ arc4_setup_iv(iv, rc4key, 16);
sg_init_one(&sg, pos, len + 4);
return crypto_blkcipher_encrypt(&desc, &sg, &sg, len + 4);
}
@@ -414,6 +416,7 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)
{
struct lib80211_tkip_data *tkey = priv;
struct blkcipher_desc desc = { .tfm = tkey->rx_tfm_arc4 };
+ struct arc4_iv *iv = crypto_blkcipher_crt(tkey->rx_tfm_arc4)->iv;
u8 rc4key[16];
u8 keyidx, *pos;
u32 iv32;
@@ -485,7 +488,7 @@ static int lib80211_tkip_decrypt(struct sk_buff *skb, int hdr_len, void *priv)

plen = skb->len - hdr_len - 12;

- crypto_blkcipher_setkey(tkey->rx_tfm_arc4, rc4key, 16);
+ arc4_setup_iv(iv, rc4key, 16);
sg_init_one(&sg, pos, plen + 4);
if (crypto_blkcipher_decrypt(&desc, &sg, &sg, plen + 4)) {
if (net_ratelimit()) {
--
1.6.6


2010-04-07 00:31:13

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH v2] crypto: add blkcipher implementation of ARC4

On Tue, Apr 06, 2010 at 10:30:02PM +0200, Sebastian Andrzej Siewior wrote:
>
> Good point. All arc4 users don't care about return value of setkey so I
> think that I just change void to int add the check for the valid key
> length.

Actually, how about getting arc4_setup_iv to do all the legwork
and turn it into a real IV? Then we don't need any checks on the
data path.

> While we are here, the .setkey() callback could be removed, couldn't it?
> It returns 0 even it is doing nothing what looks kinda wrong. However it
> shouldn't be called at all since min/max key is 0. Any objections on
> that?

I'm pretty sure testmgr will call setkey even for keylen == 0, no?

Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2010-04-06 00:39:51

by Herbert Xu

[permalink] [raw]
Subject: Re: Convert arc4 from a cipher into a block cipher

On Mon, Apr 05, 2010 at 04:33:45PM -0400, Pavel Roskin wrote:
>
> Are you trying to speed up arc4? Or you want to simplify the code? Or
> maybe you are trying to make arc4 unsuitable for anything other than WEP
> and TKIP? The later should be fine, actually, considering the known
> security issues.

No the point is to make arc4 compliant with the crypto API by
being reentrant.

Functionality-wise there is no difference.

Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Subject: [PATCH 6/7] net/mac80211: convert wep from arc4 to arc4blk

ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
net/mac80211/Kconfig | 3 +--
net/mac80211/wep.c | 11 +++++++----
2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index a10d508..d14fe06 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -2,8 +2,7 @@ config MAC80211
tristate "Generic IEEE 802.11 Networking Stack (mac80211)"
depends on CFG80211
select CRYPTO
- select CRYPTO_ECB
- select CRYPTO_ARC4
+ select CRYPTO_ARC4BLK
select CRYPTO_AES
select CRC32
---help---
diff --git a/net/mac80211/wep.c b/net/mac80211/wep.c
index 247123f..4314b50 100644
--- a/net/mac80211/wep.c
+++ b/net/mac80211/wep.c
@@ -17,6 +17,7 @@
#include <linux/err.h>
#include <linux/mm.h>
#include <linux/scatterlist.h>
+#include <crypto/arc4.h>
#include <asm/unaligned.h>

#include <net/mac80211.h>
@@ -29,12 +30,12 @@ int ieee80211_wep_init(struct ieee80211_local *local)
/* start WEP IV from a random value */
get_random_bytes(&local->wep_iv, WEP_IV_LEN);

- local->wep_tx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0,
+ local->wep_tx_tfm = crypto_alloc_blkcipher("arc4", 0,
CRYPTO_ALG_ASYNC);
if (IS_ERR(local->wep_tx_tfm))
return PTR_ERR(local->wep_tx_tfm);

- local->wep_rx_tfm = crypto_alloc_blkcipher("ecb(arc4)", 0,
+ local->wep_rx_tfm = crypto_alloc_blkcipher("arc4", 0,
CRYPTO_ALG_ASYNC);
if (IS_ERR(local->wep_rx_tfm)) {
crypto_free_blkcipher(local->wep_tx_tfm);
@@ -125,13 +126,14 @@ void ieee80211_wep_encrypt_data(struct crypto_blkcipher *tfm, u8 *rc4key,
size_t klen, u8 *data, size_t data_len)
{
struct blkcipher_desc desc = { .tfm = tfm };
+ struct arc4_iv *iv = crypto_blkcipher_crt(tfm)->iv;
struct scatterlist sg;
__le32 icv;

icv = cpu_to_le32(~crc32_le(~0, data, data_len));
put_unaligned(icv, (__le32 *)(data + data_len));

- crypto_blkcipher_setkey(tfm, rc4key, klen);
+ arc4_setup_iv(iv, rc4key, klen);
sg_init_one(&sg, data, data_len + WEP_ICV_LEN);
crypto_blkcipher_encrypt(&desc, &sg, &sg, sg.length);
}
@@ -181,10 +183,11 @@ int ieee80211_wep_decrypt_data(struct crypto_blkcipher *tfm, u8 *rc4key,
size_t klen, u8 *data, size_t data_len)
{
struct blkcipher_desc desc = { .tfm = tfm };
+ struct arc4_iv *iv = crypto_blkcipher_crt(tfm)->iv;
struct scatterlist sg;
__le32 crc;

- crypto_blkcipher_setkey(tfm, rc4key, klen);
+ arc4_setup_iv(iv, rc4key, klen);
sg_init_one(&sg, data, data_len + WEP_ICV_LEN);
crypto_blkcipher_decrypt(&desc, &sg, &sg, sg.length);

--
1.6.6


Subject: [PATCH 7/7] net/ppp_mppe: convert from arc4 to arc4blk

ecb(arc4) is getting replaced by arc4 which is a blkcipher by itself

Cc: <[email protected]>
Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
drivers/net/Kconfig | 3 +--
drivers/net/ppp_mppe.c | 12 ++++++------
2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index dd9a09c..4b5dd86 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3075,8 +3075,7 @@ config PPP_MPPE
depends on PPP && EXPERIMENTAL
select CRYPTO
select CRYPTO_SHA1
- select CRYPTO_ARC4
- select CRYPTO_ECB
+ select CRYPTO_ARC4BLK
---help---
Support for the MPPE Encryption protocol, as employed by the
Microsoft Point-to-Point Tunneling Protocol.
diff --git a/drivers/net/ppp_mppe.c b/drivers/net/ppp_mppe.c
index 6d1a1b8..4deaf70 100644
--- a/drivers/net/ppp_mppe.c
+++ b/drivers/net/ppp_mppe.c
@@ -42,7 +42,6 @@
* MOD_DEC_USAGE_COUNT/MOD_INC_USAGE_COUNT which are
* deprecated in 2.6
*/
-
#include <linux/err.h>
#include <linux/module.h>
#include <linux/kernel.h>
@@ -55,6 +54,7 @@
#include <linux/ppp_defs.h>
#include <linux/ppp-comp.h>
#include <linux/scatterlist.h>
+#include <crypto/arc4.h>

#include "ppp_mppe.h"

@@ -162,11 +162,11 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
{
struct scatterlist sg_in[1], sg_out[1];
struct blkcipher_desc desc = { .tfm = state->arc4 };
+ struct arc4_iv *iv = crypto_blkcipher_crt(state->arc4)->iv;

get_new_key_from_sha(state);
if (!initial_key) {
- crypto_blkcipher_setkey(state->arc4, state->sha1_digest,
- state->keylen);
+ arc4_setup_iv(iv, 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);
@@ -184,7 +184,7 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
state->session_key[1] = 0x26;
state->session_key[2] = 0x9e;
}
- crypto_blkcipher_setkey(state->arc4, state->session_key, state->keylen);
+ arc4_setup_iv(iv, state->session_key, state->keylen);
}

/*
@@ -204,7 +204,7 @@ static void *mppe_alloc(unsigned char *options, int optlen)
goto out;


- state->arc4 = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
+ state->arc4 = crypto_alloc_blkcipher("arc4", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(state->arc4)) {
state->arc4 = NULL;
goto out_free;
@@ -712,7 +712,7 @@ static struct compressor ppp_mppe = {
static int __init ppp_mppe_init(void)
{
int answer;
- if (!(crypto_has_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC) &&
+ if (!(crypto_has_blkcipher("arc4", 0, CRYPTO_ALG_ASYNC) &&
crypto_has_hash("sha1", 0, CRYPTO_ALG_ASYNC)))
return -ENODEV;

--
1.6.6