2010-01-25 18:29:37

by Mikulas Patocka

[permalink] [raw]
Subject: [PATCH] dm-crypt: disable block encryption with arc4

Hi

When using arc4 to encrypt a block device, the resulting device is
unreliable. It reads garbage. That's because arc4 is a stream cipher, if
you write something, it advances its state and if you attempt to decrypt
the same sector, it uses new state that is different.

This patch disables the use of arc4 on block devices.

A question to crypto maintainers: Is there some general method how to
determine that the cipher is a stream cipher, changes its state as it
progresses and thus is unusable for block devices? I haven't found any
flag for that.

Mikulas

---

Disable arc4 for encrypting block device

Arc4 is a stream cipher, it's once initialized with a key, it outputs a stream
of bytes (that are xored with the data to be encrypted) and changes it's
internal state.

Because the cipher changes it's internal state, it is not useable for encrypting
block devices --- once someone encrypts a sector of data, the internal state
changes --- and further attempts to decrypt the same block of data use the new
internal state. Thus, the encrypted device returns garbage.

This patch disables the use of arc4 for dm-crypt.

If we wanted to use arc4, we would have to setup the key before encrypting each
sector. That is slow. Because arc4 works by xoring the bitstream with the data,
it is not suitable for encrypting block devices anyway: if the attacker obtains
two images of the same block device at two different times, he can xor them with
each other, eliminating the cipher and getting two xored plaintexts.

Signed-off-by: Mikulas Patocka <[email protected]>

---
drivers/md/dm-crypt.c | 5 +++++
1 file changed, 5 insertions(+)

Index: linux-2.6.32-devel/drivers/md/dm-crypt.c
===================================================================
--- linux-2.6.32-devel.orig/drivers/md/dm-crypt.c 2010-01-25 18:55:14.000000000 +0100
+++ linux-2.6.32-devel/drivers/md/dm-crypt.c 2010-01-25 18:57:02.000000000 +0100
@@ -1035,6 +1035,11 @@ static int crypt_ctr(struct dm_target *t
goto bad_cipher;
}

+ if (!strcmp(cc->cipher, "arc4")) {
+ ti->error = "Stream cipher arc4 not supported";
+ goto bad_cipher;
+ }
+
if (snprintf(cc->cipher, CRYPTO_MAX_ALG_NAME, "%s(%s)",
chainmode, cipher) >= CRYPTO_MAX_ALG_NAME) {
ti->error = "Chain mode + cipher name is too long";


2010-01-25 18:39:19

by Milan Broz

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On 01/25/2010 07:29 PM, Mikulas Patocka wrote:
> Hi
>
> When using arc4 to encrypt a block device, the resulting device is
> unreliable. It reads garbage. That's because arc4 is a stream cipher, if
> you write something, it advances its state and if you attempt to decrypt
> the same sector, it uses new state that is different.
>
> This patch disables the use of arc4 on block devices.

arc4 again. it is simply not a block cipher:-)

This should be solved inside cryptoAPI and not blacklist it in dm-crypt,
see that thread
http://article.gmane.org/gmane.linux.kernel.cryptoapi/3441

Milan

2010-01-25 18:39:56

by Mikulas Patocka

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

BTW. I created a script that tests all possible ciphers, keysizes,
chaining modes and iv modes for dm-crypt. arc4 is the only one that fails.
You can add it your regression testsuite if you want.

Mikulas


Attachments:
test-cipher (285.00 B)
test-cipher
cryptest (3.89 kB)
cryptest
Download all attachments
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

* Milan Broz | 2010-01-25 19:39:11 [+0100]:

>On 01/25/2010 07:29 PM, Mikulas Patocka wrote:
>> Hi
>>
>> When using arc4 to encrypt a block device, the resulting device is
>> unreliable. It reads garbage. That's because arc4 is a stream cipher, if
>> you write something, it advances its state and if you attempt to decrypt
>> the same sector, it uses new state that is different.
>>
>> This patch disables the use of arc4 on block devices.
>
>arc4 again. it is simply not a block cipher:-)
>
>This should be solved inside cryptoAPI and not blacklist it in dm-crypt,
>see that thread
>http://article.gmane.org/gmane.linux.kernel.cryptoapi/3441

I some how remember Herbert saying to test for block size > 1. Wouldn't
this be acceptable to block all stream cipher in one go?

>Milan

Sebastian

2010-01-26 10:49:20

by Milan Broz

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On 01/26/2010 10:22 AM, Sebastian Andrzej Siewior wrote:
> * Milan Broz | 2010-01-25 19:39:11 [+0100]:
>> On 01/25/2010 07:29 PM, Mikulas Patocka wrote:
>>> When using arc4 to encrypt a block device, the resulting device is
>>> unreliable. It reads garbage. That's because arc4 is a stream cipher, if
>>> you write something, it advances its state and if you attempt to decrypt
>>> the same sector, it uses new state that is different.
>>>
>>> This patch disables the use of arc4 on block devices.
>>
>> arc4 again. it is simply not a block cipher:-)
>>
>> This should be solved inside cryptoAPI and not blacklist it in dm-crypt,
>> see that thread
>> http://article.gmane.org/gmane.linux.kernel.cryptoapi/3441
>
> I some how remember Herbert saying to test for block size > 1. Wouldn't
> this be acceptable to block all stream cipher in one go?

yes, I think it is better.
(...and I just forgot to add that test to dm-crypt after that suggestion.)

Milan

2010-01-26 12:27:47

by Mikulas Patocka

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

> >>> This patch disables the use of arc4 on block devices.
> >>
> >> arc4 again. it is simply not a block cipher:-)
> >>
> >> This should be solved inside cryptoAPI and not blacklist it in dm-crypt,
> >> see that thread
> >> http://article.gmane.org/gmane.linux.kernel.cryptoapi/3441
> >
> > I some how remember Herbert saying to test for block size > 1. Wouldn't
> > this be acceptable to block all stream cipher in one go?
>
> yes, I think it is better.
> (...and I just forgot to add that test to dm-crypt after that suggestion.)
>
> Milan

Hmm, there is salsa20 that has block size 1, larger initialization
vectors, and can be used to encrypt disks (although salsa20 doesn't
currently work with dm-crypt, because it doesn't accept "ecb(), cbc(),
etc." chaining modes --- but if you remove the chaining mode manually, it
works).

You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
cipher can't be used to encrypt disks.

Mikulas

2010-01-26 12:59:21

by Alasdair G Kergon

[permalink] [raw]
Subject: Re: [dm-devel] [PATCH] dm-crypt: disable block encryption with arc4

On Mon, Jan 25, 2010 at 07:39:11PM +0100, Milan Broz wrote:
> This should be solved inside cryptoAPI and not blacklist it in dm-crypt,
> see that thread

Agreed. I'm not going to apply a dm patch that maintains a hard-coded "broken"
list.

Alasdair


Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

* Mikulas Patocka | 2010-01-26 07:27:18 [-0500]:

>> yes, I think it is better.
>> (...and I just forgot to add that test to dm-crypt after that suggestion.)
>>
>> Milan
>
>Hmm, there is salsa20 that has block size 1, larger initialization
>vectors, and can be used to encrypt disks (although salsa20 doesn't
>currently work with dm-crypt, because it doesn't accept "ecb(), cbc(),
>etc." chaining modes --- but if you remove the chaining mode manually, it
>works).
>
>You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
>cipher can't be used to encrypt disks.

Just because it will work does not make it a good idea.

SALSA20 is a stream cipher not a block cipher.
Block ciphers are used to encrypt data.
Stream ciphers are used to create one time pads, a set of encryption
keys, ...
There are block modes like CTR which can turn a block cipher into a
stream cipher. Those should not be used for disk encryption as well.

>
>Mikulas

Sebastian

2010-01-26 17:12:16

by Mikulas Patocka

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On Tue, 26 Jan 2010, Sebastian Andrzej Siewior wrote:

> * Mikulas Patocka | 2010-01-26 07:27:18 [-0500]:
>
> >> yes, I think it is better.
> >> (...and I just forgot to add that test to dm-crypt after that suggestion.)
> >>
> >> Milan
> >
> >Hmm, there is salsa20 that has block size 1, larger initialization
> >vectors, and can be used to encrypt disks (although salsa20 doesn't
> >currently work with dm-crypt, because it doesn't accept "ecb(), cbc(),
> >etc." chaining modes --- but if you remove the chaining mode manually, it
> >works).
> >
> >You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
> >cipher can't be used to encrypt disks.
>
> Just because it will work does not make it a good idea.
>
> SALSA20 is a stream cipher not a block cipher.
> Block ciphers are used to encrypt data.
> Stream ciphers are used to create one time pads, a set of encryption
> keys, ...
> There are block modes like CTR which can turn a block cipher into a
> stream cipher. Those should not be used for disk encryption as well.

Salsa20 is unsuitable for disk encryption in most cases. It would be
suitable if we knew that the attacker can obtain the image of encrypted
device at most once --- it is OK to protect against laptop theft (it
happens just once), but it is not OK to protect against support technician
spying on your data (he can read them multiple times, if you have multiple
support requests).

Anyway, what I wanted to say, is that block_size <= 1 test is no less
hacky than !strcmp(cipher, "arc4") test.

Mikulas

2010-02-09 07:37:25

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

Mikulas Patocka <[email protected]> wrote:
>
> You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
> cipher can't be used to encrypt disks.

No, please see my reply in the previous thread. What we should
do is fix arc4. I just haven't got around to doing it yet.

As to blacklisting algorithms not suitable for disk encryption,
that is up to the dm-crypt maintainers to decide.

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-02-09 14:03:40

by Mikulas Patocka

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On Tue, 9 Feb 2010, Herbert Xu wrote:

> Mikulas Patocka <[email protected]> wrote:
> >
> > You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
> > cipher can't be used to encrypt disks.
>
> No, please see my reply in the previous thread. What we should
> do is fix arc4. I just haven't got around to doing it yet.

What is the fix for arc4? Copy the internal state after a key schedule and
restore it with every encryption?

> As to blacklisting algorithms not suitable for disk encryption,
> that is up to the dm-crypt maintainers to decide.
>
> Cheers,

I think blacklisting "arc4" is better, because it provides a fix now.
Otherwise, people will just keep on arguing what is the "clean" solution
and nothing gets done.

Mikulas

Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

* Herbert Xu | 2010-02-09 18:37:18 [+1100]:

>Mikulas Patocka <[email protected]> wrote:
>>
>> You should rather add a flag CRYPTO_ALG_CHANGES_STATE to determine that a
>> cipher can't be used to encrypt disks.
>
>No, please see my reply in the previous thread. What we should
>do is fix arc4. I just haven't got around to doing it yet.
>
>As to blacklisting algorithms not suitable for disk encryption,
>that is up to the dm-crypt maintainers to decide.

Herbert, what happend to the "check for streamcipher" idea you had? Is
it gone? On the other hand it wouldn't be probably that bad to have a
seprate interface to grab a block cipher _or_ a stream cipher. So
something like this wouldn't happen. This is basically the "check for
stream cipher" without encrypt = decrypt plus with a struct the cra_u
union.
I can't imaging how you want to fix arc4 that it will work in dm-crypt.
The algorithm relies more or less on the fact that it envolves itself
during processing of data.
Salsa works with dm-crypt because the internal state is taken from the
IV and is never written back. dm-crypt always encrypts/decrypts a 512
block in one go. Splitting it in two and requesting two 256 block
encryptions would work with _every_ other block cipher but break salsa.

>
>Cheers,

Sebastian

2010-02-09 20:43:00

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On Tue, Feb 09, 2010 at 09:02:35AM -0500, Mikulas Patocka wrote:
>
> What is the fix for arc4? Copy the internal state after a key schedule and
> restore it with every encryption?

arc4 should be a blkcipher, not a cipher. Then it can have an IV
which is where thie should be stored.

> I think blacklisting "arc4" is better, because it provides a fix now.
> Otherwise, people will just keep on arguing what is the "clean" solution
> and nothing gets done.

The crypto layer makes no guarantee that every algorithm that is
available is suitable for a particular application such as disk
encryption. FWIW we also export an algorithm called null!

People should not be making uninformed choices on crypto algorithms.

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-02-09 20:45:23

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On Tue, Feb 09, 2010 at 03:57:05PM +0100, Sebastian Andrzej Siewior wrote:
>
> >As to blacklisting algorithms not suitable for disk encryption,
> >that is up to the dm-crypt maintainers to decide.
>
> Herbert, what happend to the "check for streamcipher" idea you had? Is
> it gone? On the other hand it wouldn't be probably that bad to have a

Well again whether that should be done is up to the dm-crypt
maintainers.

> seprate interface to grab a block cipher _or_ a stream cipher. So

Just because something isn't a stream cipher doesn't mean that
it is safe for disk encryption. People simply shouldn't be using
random algorithms for disk encryption.

> I can't imaging how you want to fix arc4 that it will work in dm-crypt.

I thought I've explained this before. Just turn it into a blkcipher
and add IV.

> The algorithm relies more or less on the fact that it envolves itself
> during processing of data.

This is no different to any stream cipher.

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] dm-crypt: disable block encryption with arc4

* Herbert Xu | 2010-02-10 07:45:19 [+1100]:

>> Herbert, what happend to the "check for streamcipher" idea you had? Is
>> it gone? On the other hand it wouldn't be probably that bad to have a
>
>Well again whether that should be done is up to the dm-crypt
>maintainers.
Milan liked that afaik.

>> seprate interface to grab a block cipher _or_ a stream cipher. So
>
>Just because something isn't a stream cipher doesn't mean that
>it is safe for disk encryption. People simply shouldn't be using
>random algorithms for disk encryption.
>
>> I can't imaging how you want to fix arc4 that it will work in dm-crypt.
>
>I thought I've explained this before. Just turn it into a blkcipher
>and add IV.
I beg your pardon. I probably mixed things up.

>> The algorithm relies more or less on the fact that it envolves itself
>> during processing of data.
>
>This is no different to any stream cipher.
Sure. So we fix arc4 and don't play mother . Okay I will into this :)

>
>Cheers,

Sebastian

2010-02-09 21:45:27

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] dm-crypt: disable block encryption with arc4

On Tue, Feb 09, 2010 at 10:12:38PM +0100, Sebastian Andrzej Siewior wrote:
>
> >This is no different to any stream cipher.
> Sure. So we fix arc4 and don't play mother . Okay I will into this :)

That would be awesome. 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

Subject: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

the state has been moved from ctx into iv. That way encrypt()/decrypt() can
deliver the same result for a given IV. This patch makes the cipher work with
dm-crypt not that it is a good thing. However, the performance may have
improved :)
The name is still ecb(aes) but since this is provided by the blkcipher itself,
I removed the select statement.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
I had it run with wireless and dm-crypt. No problems so far. Not sure if
it makes sense to rename it to arc4 and strip the ecb prefix. It would
make it consistent with salsa but would require another patch.

crypto/Kconfig | 2 +-
crypto/arc4.c | 120 ++++++++++++++++++++++++----------
crypto/testmgr.h | 3 +-
drivers/net/Kconfig | 1 -
drivers/net/wireless/hostap/Kconfig | 2 -
drivers/net/wireless/ipw2x00/Kconfig | 2 -
net/mac80211/Kconfig | 1 -
7 files changed, 87 insertions(+), 44 deletions(-)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 81c185a..5fab1c3 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -561,7 +561,7 @@ config CRYPTO_ANUBIS

config CRYPTO_ARC4
tristate "ARC4 cipher algorithm"
- select CRYPTO_ALGAPI
+ select CRYPTO_BLKCIPHER
help
ARC4 cipher algorithm.

diff --git a/crypto/arc4.c b/crypto/arc4.c
index 8be47e1..b67b656 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -1,4 +1,4 @@
-/*
+/*
* Cryptographic API
*
* ARC4 Cipher Algorithm
@@ -13,76 +13,124 @@
*/
#include <linux/module.h>
#include <linux/init.h>
-#include <linux/crypto.h>
+#include <crypto/algapi.h>

#define ARC4_MIN_KEY_SIZE 1
#define ARC4_MAX_KEY_SIZE 256
#define ARC4_BLOCK_SIZE 1

-struct arc4_ctx {
+struct arc4_iv {
u8 S[256];
u8 x, y;
};

+struct arc4_ctx {
+ struct arc4_iv iv;
+ u8 new_key;
+};
+
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;
+ ctx->iv.x = 1;
+ ctx->iv.y = 0;

- for(i = 0; i < 256; i++)
- ctx->S[i] = i;
+ for (i = 0; i < 256; i++)
+ ctx->iv.S[i] = i;

- for(i = 0; i < 256; i++)
+ for (i = 0; i < 256; i++)
{
- u8 a = ctx->S[i];
+ u8 a = ctx->iv.S[i];
j = (j + in_key[k] + a) & 0xff;
- ctx->S[i] = ctx->S[j];
- ctx->S[j] = a;
- if(++k >= key_len)
+ ctx->iv.S[i] = ctx->iv.S[j];
+ ctx->iv.S[j] = a;
+ if (++k >= key_len)
k = 0;
}
-
+ ctx->new_key = 1;
return 0;
}

-static void arc4_crypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
+static void arc4_ivsetup(struct arc4_ctx *ctx, u8 *iv)
{
- struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
+ if (unlikely(!ctx->new_key))
+ return;
+ memcpy(iv, &ctx->iv, sizeof(ctx->iv));
+ ctx->new_key = 0;
+}

- u8 *const S = ctx->S;
- u8 x = ctx->x;
- u8 y = ctx->y;
+static int arc4_crypt(struct blkcipher_desc *desc, struct scatterlist *dst,
+ struct scatterlist *src, unsigned int nbytes)
+{
+ struct blkcipher_walk walk;
+ struct crypto_blkcipher *tfm = desc->tfm;
+ struct arc4_ctx *ctx = crypto_blkcipher_ctx(tfm);
+ struct arc4_iv *iv;
+ 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;
+
+ arc4_ivsetup(ctx, walk.iv);
+
+ iv = (struct arc4_iv *)walk.iv;
+
+ S = iv->S;
+ x = iv->x;
+ y = 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);
+ }

- 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];
-
- ctx->x = x;
- ctx->y = y;
+ iv->x = x;
+ iv->y = y;
+ return ret;
}

static struct crypto_alg arc4_alg = {
- .cra_name = "arc4",
- .cra_flags = CRYPTO_ALG_TYPE_CIPHER,
+ .cra_name = "ecb(arc4)",
+ .cra_priority = 100,
+ .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
.cra_blocksize = ARC4_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct arc4_ctx),
+ .cra_type = &crypto_blkcipher_type,
+ .cra_alignmask = 3,
.cra_module = THIS_MODULE,
.cra_list = LIST_HEAD_INIT(arc4_alg.cra_list),
- .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,
- .cia_decrypt = arc4_crypt } }
+ .cra_u = { .blkcipher = {
+ .min_keysize = ARC4_MIN_KEY_SIZE,
+ .max_keysize = ARC4_MAX_KEY_SIZE,
+ .ivsize = sizeof(struct arc4_iv),
+ .setkey = arc4_set_key,
+ .encrypt = arc4_crypt,
+ .decrypt = arc4_crypt } }
};

static int __init arc4_init(void)
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/drivers/net/Kconfig b/drivers/net/Kconfig
index dd9a09c..ddce826 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3076,7 +3076,6 @@ config PPP_MPPE
select CRYPTO
select CRYPTO_SHA1
select CRYPTO_ARC4
- select CRYPTO_ECB
---help---
Support for the MPPE Encryption protocol, as employed by the
Microsoft Point-to-Point Tunneling Protocol.
diff --git a/drivers/net/wireless/hostap/Kconfig b/drivers/net/wireless/hostap/Kconfig
index 287d827..2548b56 100644
--- a/drivers/net/wireless/hostap/Kconfig
+++ b/drivers/net/wireless/hostap/Kconfig
@@ -5,10 +5,8 @@ config HOSTAP
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..2f2b7d9 100644
--- a/drivers/net/wireless/ipw2x00/Kconfig
+++ b/drivers/net/wireless/ipw2x00/Kconfig
@@ -159,10 +159,8 @@ config LIBIPW
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/mac80211/Kconfig b/net/mac80211/Kconfig
index a10d508..7925f44 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -2,7 +2,6 @@ config MAC80211
tristate "Generic IEEE 802.11 Networking Stack (mac80211)"
depends on CFG80211
select CRYPTO
- select CRYPTO_ECB
select CRYPTO_ARC4
select CRYPTO_AES
select CRC32
--
1.6.6.1


2010-02-12 09:42:08

by Adrian-Ken Rueegsegger

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

Hi,

Sebastian Andrzej Siewior schrieb:
> the state has been moved from ctx into iv. That way encrypt()/decrypt() can
> deliver the same result for a given IV. This patch makes the cipher work with
> dm-crypt not that it is a good thing. However, the performance may have
> improved :)
> The name is still ecb(aes) but since this is provided by the blkcipher itself,

Just to avoid any confusion you meant ecb(arc4) not ecb(aes) here right?

-Adrian

Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

* Adrian-Ken Rueegsegger | 2010-02-12 10:34:27 [+0100]:

>Hi,
Hi,

>Sebastian Andrzej Siewior schrieb:
>> The name is still ecb(aes) but since this is provided by the blkcipher itself,
>Just to avoid any confusion you meant ecb(arc4) not ecb(aes) here right?

Yes, I do. Not sure how I got aes in there. I did not mix it up in the
patch :)

>
>-Adrian

Sebastian

Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

* Sebastian Andrzej Siewior | 2010-02-12 09:42:28 [+0100]:

>+static void arc4_ivsetup(struct arc4_ctx *ctx, u8 *iv)
> {
>- struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
>+ if (unlikely(!ctx->new_key))

That should be likely(). Do you want me resend the whole thing? Haven't
noticed anything else :)

>+ return;
>+ memcpy(iv, &ctx->iv, sizeof(ctx->iv));
>+ ctx->new_key = 0;
>+}

Sebastian

2010-02-15 00:10:15

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Sun, Feb 14, 2010 at 09:42:54PM +0100, Sebastian Andrzej Siewior wrote:
> * Sebastian Andrzej Siewior | 2010-02-12 09:42:28 [+0100]:
>
> >+static void arc4_ivsetup(struct arc4_ctx *ctx, u8 *iv)
> > {
> >- struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
> >+ if (unlikely(!ctx->new_key))
>
> That should be likely(). Do you want me resend the whole thing? Haven't
> noticed anything else :)

How about we just remove it? It's not on a hot path anyway.

I can do this when integrating the patch so you don't have to
resend.

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

Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

* Herbert Xu | 2010-02-15 08:10:08 [+0800]:

>How about we just remove it? It's not on a hot path anyway.
Sure.

>I can do this when integrating the patch so you don't have to
>resend.
Okay, thanks.

>Thanks,

Sebastian

2010-02-16 12:51:34

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Fri, Feb 12, 2010 at 09:42:28AM +0100, Sebastian Andrzej Siewior wrote:
>
> -static void arc4_crypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
> +static void arc4_ivsetup(struct arc4_ctx *ctx, u8 *iv)
> {
> - struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
> + if (unlikely(!ctx->new_key))
> + return;
> + memcpy(iv, &ctx->iv, sizeof(ctx->iv));
> + ctx->new_key = 0;

Sorry, but this doesn't work.

A ctx is supposed to be reentrant. That is, while one thread
is working away with a given ctx I should be able to use that
same ctx in a different thread without them clobbering each
other.

So that means (in general) you must not modify the ctx in any
function other than setkey.

This also brings up the bigger question of how we transition to
this new arc4. I don't think we need to maintain exactly the
same behaviour as the existing ecb(arc4).

So what we could do is simply add a new blkcipher arc4, alongside
the existing cipher arc4. Then we can convert the existing users
across, and finally remove the old arc4.

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-02-22 00:45:54

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Sun, Feb 21, 2010 at 09:01:40PM +0100, Sebastian Andrzej Siewior wrote:
>
> I also destroy the user supplied IV. You don't care about that? :)
> So I have to know that someone called setkey() on this ctx but I can't
> leave hints.

How about this? You extend the IV by one more byte, and use that
byte as a boolean flag to indicate whether the IV is valid. All
users that cannot supply their own IVs can then set the IV to zero.

When you see the zero flag in the IV, you reinitialise the IV per
the key.

> salsa also does not stick to plan here. ctx->input[6-9] is initialized
> in encrypt() path. So two threads sharing a ctx are going to clobber
> their state.

Salsa should also be fixed.

> What about a new api for the stream cipher? We would merge the ctx part
> and the iv into one handle. So the user would call setup_iv() instead of
> setkey(). The difference would be that I can access the iv from within
> setkey(). And the algorithm can fully express himself since he is no
> longer trapped in the wrong body :)

There is some merit in that, but as the current API can be made to do
the same thing (see above) I'm not convinced that this is worth the
cost for the moment.

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-02-22 00:52:22

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Mon, Feb 22, 2010 at 08:45:47AM +0800, Herbert Xu wrote:
>
> How about this? You extend the IV by one more byte, and use that
> byte as a boolean flag to indicate whether the IV is valid. All
> users that cannot supply their own IVs can then set the IV to zero.
>
> When you see the zero flag in the IV, you reinitialise the IV per
> the key.

In fact for arc4 we could just drop the key altogether since it
plays no part after setting the initial state.

> > salsa also does not stick to plan here. ctx->input[6-9] is initialized
> > in encrypt() path. So two threads sharing a ctx are going to clobber
> > their state.
>
> Salsa should also be fixed.

For Salsa on the other hand the key is rather useful since all
we need is a two-byte IV that's just a sequence number.

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] crypto/arc4: convert this stream cipher into a block cipher

* Herbert Xu | 2010-02-22 08:52:17 [+0800]:

>On Mon, Feb 22, 2010 at 08:45:47AM +0800, Herbert Xu wrote:
>>
>> How about this? You extend the IV by one more byte, and use that
>> byte as a boolean flag to indicate whether the IV is valid. All
So I trick the crypto api to allocate more bytes than ->ivsize says.

>> users that cannot supply their own IVs can then set the IV to zero.
which works fine with kzalloc()

>> When you see the zero flag in the IV, you reinitialise the IV per
>> the key.
Okay. When we have to re-key and the user calls setkey() without
re-allocating thr cipher then I would not notice this. So I need a
counter. And all this will make it work but I still think it is fishy.
Plus we waste 258bytes.

>In fact for arc4 we could just drop the key altogether since it
>plays no part after setting the initial state.
Since I'm not allowed to kfree() the ctx in encrypt() are you proposing
tfm->setup_iv(iv, key)?

>> > salsa also does not stick to plan here. ctx->input[6-9] is initialized
>> > in encrypt() path. So two threads sharing a ctx are going to clobber
>> > their state.
>>
>> Salsa should also be fixed.
I saw that comming. And I complaind back then that the assembly code was
not pretty enough... and removing the assembly is probably not option :)

>For Salsa on the other hand the key is rather useful since all
>we need is a two-byte IV that's just a sequence number.
No it's 8 bytes. Berstein's U8TO32_LITTLE() is actually a cpu_to_be32().
Not sure if he knows it :)
However I'm not sure where you going with this. salsa is fine besides
the clobber thing, isn't it?

>Cheers,

Sebastian

2010-02-23 00:15:28

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Mon, Feb 22, 2010 at 02:40:49PM -0500, Mikulas Patocka wrote:
>
> > So what we could do is simply add a new blkcipher arc4, alongside
> > the existing cipher arc4. Then we can convert the existing users
> > across, and finally remove the old arc4.
>
> arc4 can't be used as a block cipher --- see this paper
> http://www.wisdom.weizmann.ac.il/~itsik/RC4/Papers/Rc4_ksa.ps , it says
> that initialization vectors on RC4 are unreliable, if you use (unknown key
> concatenated with known IV) or (known IV concatenated with unknown key) as
> a RC4 key, the RC4 state can be exposed and the cipher is broken.

What we call a blkcipher is not really a block cipher. In fact,
what we call "cipher" is really a block cipher. So we're actually
changing arc4 so that it doesn't get used as a block cipher, i.e.,
you will no longer be able to say "cbc(arc4)" or some such.

I know it's confusing and perhaps one day we will rename blkcipher
to skcipher and cipher to blkcipher.

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-02-23 00:35:38

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Mon, Feb 22, 2010 at 11:08:35PM +0100, Sebastian Andrzej Siewior wrote:
> * Herbert Xu | 2010-02-22 08:52:17 [+0800]:
>
> >On Mon, Feb 22, 2010 at 08:45:47AM +0800, Herbert Xu wrote:
> >>
> >> How about this? You extend the IV by one more byte, and use that
> >> byte as a boolean flag to indicate whether the IV is valid. All
> So I trick the crypto api to allocate more bytes than ->ivsize says.

No tricks needed, just add the flag to the struct.

struct arc4_iv {
u8 S[256];
u8 x, y;
u8 valid;
};

> Okay. When we have to re-key and the user calls setkey() without
> re-allocating thr cipher then I would not notice this. So I need a
> counter. And all this will make it work but I still think it is fishy.
> Plus we waste 258bytes.

No you don't need to refresh the IV when the key changes. The
key should only be consulted when the valid flag in the IV is zero.

You need the 258 + flag bytes because that's just the amount of
state carried between the encrypt/decrypt operation. So it isn't
really wasted.

If you can find a way that allows arc4 to be used by multiple
threads at the same time while storing less than 258 bytes in
each thread, please let me know :)

> >In fact for arc4 we could just drop the key altogether since it
> >plays no part after setting the initial state.
> Since I'm not allowed to kfree() the ctx in encrypt() are you proposing
> tfm->setup_iv(iv, key)?

No, what you could do is structure the IV differently based on the
flag:

struct arc4_iv {
union {
struct key {
u8 key[256];
u16 keylen;
};
struct iv {
u8 S[256];
u8 x, y;
};
};
u8 type;
};

This relies on the fact that we never use more than 256 bytes in
the key so limiting its length is OK.

> >> Salsa should also be fixed.
> I saw that comming. And I complaind back then that the assembly code was
> not pretty enough... and removing the assembly is probably not option :)

Well if nobody steps in to fix the assembly then removing it is the
only option.

> >For Salsa on the other hand the key is rather useful since all
> >we need is a two-byte IV that's just a sequence number.
> No it's 8 bytes. Berstein's U8TO32_LITTLE() is actually a cpu_to_be32().
> Not sure if he knows it :)

Right.

> However I'm not sure where you going with this. salsa is fine besides
> the clobber thing, isn't it?

I don't know of any other problems.

Basically salsa should look pretty much like CTR from the outside
when it's fixed.

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] crypto/arc4: convert this stream cipher into a block cipher

* Herbert Xu | 2010-02-16 20:51:25 [+0800]:

>On Fri, Feb 12, 2010 at 09:42:28AM +0100, Sebastian Andrzej Siewior wrote:
>>
>> -static void arc4_crypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
>> +static void arc4_ivsetup(struct arc4_ctx *ctx, u8 *iv)
>> {
>> - struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
>> + if (unlikely(!ctx->new_key))
>> + return;
>> + memcpy(iv, &ctx->iv, sizeof(ctx->iv));
>> + ctx->new_key = 0;
>
>Sorry, but this doesn't work.
>
>A ctx is supposed to be reentrant. That is, while one thread
>is working away with a given ctx I should be able to use that
>same ctx in a different thread without them clobbering each
>other.
I also destroy the user supplied IV. You don't care about that? :)
So I have to know that someone called setkey() on this ctx but I can't
leave hints.
salsa also does not stick to plan here. ctx->input[6-9] is initialized
in encrypt() path. So two threads sharing a ctx are going to clobber
their state.

What about a new api for the stream cipher? We would merge the ctx part
and the iv into one handle. So the user would call setup_iv() instead of
setkey(). The difference would be that I can access the iv from within
setkey(). And the algorithm can fully express himself since he is no
longer trapped in the wrong body :)

>So that means (in general) you must not modify the ctx in any
>function other than setkey.
That is hard because I have a new state after encryption which I am only
allowed to save in the iv. And the new state may be reset in setkey()
where I can't touch the iv.
salsa does not keep/update its state. So the input[6-9] problem could be
fixed. Who/where is it used anyway? I can't see any user besides the
possible once (i.e. dm-crypt/ipsec).

>This also brings up the bigger question of how we transition to
>this new arc4. I don't think we need to maintain exactly the
>same behaviour as the existing ecb(arc4).
>
>So what we could do is simply add a new blkcipher arc4, alongside
>the existing cipher arc4. Then we can convert the existing users
>across, and finally remove the old arc4.
This has worked out before, lets stick to this :)

>Cheers,

Sebastian

Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

* Herbert Xu | 2010-02-23 08:32:39 [+0800]:

>If you can find a way that allows arc4 to be used by multiple
>threads at the same time while storing less than 258 bytes in
>each thread, please let me know :)
:)

>No, what you could do is structure the IV differently based on the
>flag:
>
>struct arc4_iv {
> union {
> struct key {
> u8 key[256];
> u16 keylen;
> };
> struct iv {
> u8 S[256];
> u8 x, y;
> };
> };
> u8 type;
>};
>
>This relies on the fact that we never use more than 256 bytes in
>the key so limiting its length is OK.

Okay. So so are we talking about something like that below then? This is
untested and I break other users bexcept lib80211_crypt_tkip.

the state has been moved from ctx into iv. That way encrypt()/decrypt() can
deliver the same result for a given IV. If the IV is supplied as a plain
key then it wil be converted into a different internal state.
The name is now arc4.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
---
crypto/Kconfig | 2 +-
crypto/arc4.c | 131 +++++++++++++++++++++++-----------
crypto/testmgr.h | 3 +-
drivers/net/Kconfig | 1 -
drivers/net/wireless/hostap/Kconfig | 2 -
drivers/net/wireless/ipw2x00/Kconfig | 2 -
include/crypto/arc4.h | 26 +++++++
net/mac80211/Kconfig | 1 -
net/wireless/lib80211_crypt_tkip.c | 10 ++-
9 files changed, 123 insertions(+), 55 deletions(-)
create mode 100644 include/crypto/arc4.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 81c185a..5fab1c3 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -561,7 +561,7 @@ config CRYPTO_ANUBIS

config CRYPTO_ARC4
tristate "ARC4 cipher algorithm"
- select CRYPTO_ALGAPI
+ select CRYPTO_BLKCIPHER
help
ARC4 cipher algorithm.

diff --git a/crypto/arc4.c b/crypto/arc4.c
index 8be47e1..1b20463 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -1,4 +1,4 @@
-/*
+/*
* Cryptographic API
*
* ARC4 Cipher Algorithm
@@ -13,76 +13,122 @@
*/
#include <linux/module.h>
#include <linux/init.h>
-#include <linux/crypto.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

-struct arc4_ctx {
- u8 S[256];
- u8 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);
+ /*
+ * ARC4 is special: The user should supply an IV as struct arc4_iv and
+ * fill either the key or the iv.
+ */
+ return -EOPNOTSUPP;
+}
+
+static void arc4_key_to_iv(const u8 *in_key, u32 key_len, struct arc4_iv *iv)
+{
int i, j = 0, k = 0;

- ctx->x = 1;
- ctx->y = 0;
+ iv->iv.x = 1;
+ iv->iv.y = 0;

- for(i = 0; i < 256; i++)
- ctx->S[i] = i;
+ for (i = 0; i < 256; i++)
+ iv->iv.S[i] = i;

- for(i = 0; i < 256; i++)
+ for (i = 0; i < 256; i++)
{
- u8 a = ctx->S[i];
+ u8 a = iv->iv.S[i];
j = (j + in_key[k] + a) & 0xff;
- ctx->S[i] = ctx->S[j];
- ctx->S[j] = a;
- if(++k >= key_len)
+ iv->iv.S[i] = iv->iv.S[j];
+ iv->iv.S[j] = a;
+ if (++k >= key_len)
k = 0;
}
-
- return 0;
}

-static void arc4_crypt(struct crypto_tfm *tfm, u8 *out, const u8 *in)
+static void arc4_ivsetup(struct arc4_iv *iv)
{
- struct arc4_ctx *ctx = crypto_tfm_ctx(tfm);
+ struct arc4_iv tmp_iv;

- u8 *const S = ctx->S;
- u8 x = ctx->x;
- u8 y = ctx->y;
- u8 a, b;
+ if (iv->type == ARC4_TYPE_IV)
+ return;

- 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];
+ 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);
+ }

- ctx->x = x;
- ctx->y = y;
+ aiv->iv.x = x;
+ aiv->iv.y = y;
+ return ret;
}

static struct crypto_alg arc4_alg = {
.cra_name = "arc4",
- .cra_flags = CRYPTO_ALG_TYPE_CIPHER,
+ .cra_priority = 100,
+ .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER,
.cra_blocksize = ARC4_BLOCK_SIZE,
- .cra_ctxsize = sizeof(struct arc4_ctx),
+ .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 = { .cipher = {
- .cia_min_keysize = ARC4_MIN_KEY_SIZE,
- .cia_max_keysize = ARC4_MAX_KEY_SIZE,
- .cia_setkey = arc4_set_key,
- .cia_encrypt = arc4_crypt,
- .cia_decrypt = arc4_crypt } }
+ .cra_u = { .blkcipher = {
+ .min_keysize = ARC4_MIN_KEY_SIZE,
+ .max_keysize = ARC4_MAX_KEY_SIZE,
+ .ivsize = sizeof(struct arc4_iv),
+ .setkey = arc4_set_key,
+ .encrypt = arc4_crypt,
+ .decrypt = arc4_crypt } }
};

static int __init arc4_init(void)
@@ -90,7 +136,6 @@ static int __init arc4_init(void)
return crypto_register_alg(&arc4_alg);
}

-
static void __exit arc4_exit(void)
{
crypto_unregister_alg(&arc4_alg);
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/drivers/net/Kconfig b/drivers/net/Kconfig
index dd9a09c..ddce826 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -3076,7 +3076,6 @@ config PPP_MPPE
select CRYPTO
select CRYPTO_SHA1
select CRYPTO_ARC4
- select CRYPTO_ECB
---help---
Support for the MPPE Encryption protocol, as employed by the
Microsoft Point-to-Point Tunneling Protocol.
diff --git a/drivers/net/wireless/hostap/Kconfig b/drivers/net/wireless/hostap/Kconfig
index 287d827..2548b56 100644
--- a/drivers/net/wireless/hostap/Kconfig
+++ b/drivers/net/wireless/hostap/Kconfig
@@ -5,10 +5,8 @@ config HOSTAP
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..2f2b7d9 100644
--- a/drivers/net/wireless/ipw2x00/Kconfig
+++ b/drivers/net/wireless/ipw2x00/Kconfig
@@ -159,10 +159,8 @@ config LIBIPW
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/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
diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index a10d508..7925f44 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -2,7 +2,6 @@ config MAC80211
tristate "Generic IEEE 802.11 Networking Stack (mac80211)"
depends on CFG80211
select CRYPTO
- select CRYPTO_ECB
select CRYPTO_ARC4
select CRYPTO_AES
select CRC32
diff --git a/net/wireless/lib80211_crypt_tkip.c b/net/wireless/lib80211_crypt_tkip.c
index c362873..4c1290d 100644
--- a/net/wireless/lib80211_crypt_tkip.c
+++ b/net/wireless/lib80211_crypt_tkip.c
@@ -94,7 +94,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 +112,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 +360,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 +393,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 +415,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 +487,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-03-14 09:06:32

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto/arc4: convert this stream cipher into a block cipher

On Sun, Mar 14, 2010 at 09:24:32AM +0100, Sebastian Andrzej Siewior wrote:
>
> Okay. So so are we talking about something like that below then? This is

Pretty much.

> untested and I break other users bexcept lib80211_crypt_tkip.

For the sake of compatibility please do this as a 3-step dance.
First add a new arc4 blkcipher, then convert the users, and
finally delete the old arc4.

> 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);
> + /*
> + * ARC4 is special: The user should supply an IV as struct arc4_iv and
> + * fill either the key or the iv.
> + */
> + return -EOPNOTSUPP;
> +}

You should return 0 here. You should also set the min/max key
size to zero. The new arc4 no longer has a key as far as the
API is concerned.

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