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 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 ~{PmVHI~} herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 ~{PmVHI~} herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 ~{PmVHI~} herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
