2009-08-03 07:44:46

by Huang, Ying

[permalink] [raw]
Subject: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

When doing "modeprobe tcrypt mode=10", the following error will show
in dmesg.

alg: skcipher: Failed to load transform for ctr(aes): -22
alg: skcipher: Failed to load transform for ctr(aes): -22
tcrypt: one or more tests failed!

Because ctr(aes) testing code will allocate ctr(aes) with geniv, but
defualt geniv mode may not work with ctr(aes). As that of rfc3686,
this is fixed via specifying geniv mode to "seqiv".

Signed-off-by: Huang Ying <[email protected]>
---
crypto/ctr.c | 2 ++
1 file changed, 2 insertions(+)

--- a/crypto/ctr.c
+++ b/crypto/ctr.c
@@ -219,6 +219,8 @@ static struct crypto_instance *crypto_ct
inst->alg.cra_blkcipher.encrypt = crypto_ctr_crypt;
inst->alg.cra_blkcipher.decrypt = crypto_ctr_crypt;

+ inst->alg.cra_blkcipher.geniv = "seqiv";
+
out:
crypto_mod_put(alg);
return inst;


2009-08-05 09:45:08

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Mon, Aug 03, 2009 at 03:44:43PM +0800, Huang Ying wrote:
> When doing "modeprobe tcrypt mode=10", the following error will show
> in dmesg.
>
> alg: skcipher: Failed to load transform for ctr(aes): -22
> alg: skcipher: Failed to load transform for ctr(aes): -22
> tcrypt: one or more tests failed!

I can't reproduce it here:

etch1:~# modprobe tcrypt mode=10
FATAL: Error inserting tcrypt (/lib/modules/2.6.30/kernel/crypto/tcrypt.ko): Resource temporarily unavailable
etch1:~# dmesg|tail
alg: No test for digest_null (digest_null-generic)
alg: No test for compress_null (compress_null-generic)
padlock: VIA PadLock Hash Engine not detected.
padlock: VIA PadLock Hash Engine not detected.
eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
Initializing XFRM netlink socket
NET: Registered protocol family 17
NET: Registered protocol family 10
lo: Disabled Privacy Extensions
eth0: no IPv6 routers present
etch1:~#

The tcrypt test shouldn't use geniv at all.

What kernel version was this?

> Because ctr(aes) testing code will allocate ctr(aes) with geniv, but
> defualt geniv mode may not work with ctr(aes). As that of rfc3686,
> this is fixed via specifying geniv mode to "seqiv".
>
> Signed-off-by: Huang Ying <[email protected]>

I think this patch makes sense as the default IV generator is not
designed to guarantee uniqueness (although repetitions would be
very rare).

But I'd like to get to the bottom of the failure that you were
seeing so that we don't commit this with a potentially misleading
message.

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

2009-08-06 02:12:50

by Huang, Ying

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Wed, 2009-08-05 at 17:45 +0800, Herbert Xu wrote:
> On Mon, Aug 03, 2009 at 03:44:43PM +0800, Huang Ying wrote:
> > When doing "modeprobe tcrypt mode=10", the following error will show
> > in dmesg.
> >
> > alg: skcipher: Failed to load transform for ctr(aes): -22
> > alg: skcipher: Failed to load transform for ctr(aes): -22
> > tcrypt: one or more tests failed!
>
> I can't reproduce it here:
>
> etch1:~# modprobe tcrypt mode=10
> FATAL: Error inserting tcrypt (/lib/modules/2.6.30/kernel/crypto/tcrypt.ko): Resource temporarily unavailable
> etch1:~# dmesg|tail
> alg: No test for digest_null (digest_null-generic)
> alg: No test for compress_null (compress_null-generic)
> padlock: VIA PadLock Hash Engine not detected.
> padlock: VIA PadLock Hash Engine not detected.
> eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
> Initializing XFRM netlink socket
> NET: Registered protocol family 17
> NET: Registered protocol family 10
> lo: Disabled Privacy Extensions
> eth0: no IPv6 routers present
> etch1:~#
>
> The tcrypt test shouldn't use geniv at all.
>
> What kernel version was this?

The version is: v2.6.30-6814-gab30046. Kernel config is attached too.

Best Regards,
Huang Ying


Attachments:
.config (41.93 kB)

2009-08-12 02:27:00

by Huang, Ying

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Thu, 2009-08-06 at 10:12 +0800, Huang Ying wrote:
> On Wed, 2009-08-05 at 17:45 +0800, Herbert Xu wrote:
> > On Mon, Aug 03, 2009 at 03:44:43PM +0800, Huang Ying wrote:
> > > When doing "modeprobe tcrypt mode=10", the following error will show
> > > in dmesg.
> > >
> > > alg: skcipher: Failed to load transform for ctr(aes): -22
> > > alg: skcipher: Failed to load transform for ctr(aes): -22
> > > tcrypt: one or more tests failed!
> >
> > I can't reproduce it here:
> >
> > etch1:~# modprobe tcrypt mode=10
> > FATAL: Error inserting tcrypt (/lib/modules/2.6.30/kernel/crypto/tcrypt.ko): Resource temporarily unavailable
> > etch1:~# dmesg|tail
> > alg: No test for digest_null (digest_null-generic)
> > alg: No test for compress_null (compress_null-generic)
> > padlock: VIA PadLock Hash Engine not detected.
> > padlock: VIA PadLock Hash Engine not detected.
> > eth0: link up, 100Mbps, full-duplex, lpa 0x05E1
> > Initializing XFRM netlink socket
> > NET: Registered protocol family 17
> > NET: Registered protocol family 10
> > lo: Disabled Privacy Extensions
> > eth0: no IPv6 routers present
> > etch1:~#
> >
> > The tcrypt test shouldn't use geniv at all.
> >
> > What kernel version was this?
>
> The version is: v2.6.30-6814-gab30046. Kernel config is attached too.

Any follow-up for this patch? Still can not re-produce?

Best Regards,
Huang Ying



2009-08-13 04:53:02

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Wed, Aug 12, 2009 at 10:27:00AM +0800, Huang Ying wrote:
>
> Any follow-up for this patch? Still can not re-produce?

I can now that I'm on a 32-bit machine. I wonder if it's 32-bit
specific or perhaps I was just using an old kernel.

I'll look into it.

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

2009-08-13 07:39:12

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Thu, Aug 13, 2009 at 02:53:00PM +1000, Herbert Xu wrote:
>
> I'll look into it.

Oh I see what's going on. It's the switch from chainiv to eseqiv
that created the error. I'll apply your patch.

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

2009-08-13 13:12:54

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Thu, Aug 13, 2009 at 05:39:10PM +1000, Herbert Xu wrote:
>
> Oh I see what's going on. It's the switch from chainiv to eseqiv
> that created the error. I'll apply your patch.

Actually we can't use seqiv on raw counter mode because it cannot
guarantee IV uniqueness. I think reverting to chainiv is the safer
option.

commit aef27136b8b5e526f2e96ca1caa30a6d07e70f42
Author: Herbert Xu <[email protected]>
Date: Thu Aug 13 23:10:39 2009 +1000

crypto: ctr - Use chainiv on raw counter mode

Raw counter mode only works with chainiv, which is no longer
the default IV generator on SMP machines. This broke raw counter
mode as it can no longer instantiate as a givcipher.

This patch fixes it by always picking chainiv on raw counter
mode. This is based on the diagnosis and a patch by Huang
Ying.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/ctr.c b/crypto/ctr.c
index 2d7425f..6c3bfab 100644
--- a/crypto/ctr.c
+++ b/crypto/ctr.c
@@ -219,6 +219,8 @@ static struct crypto_instance *crypto_ctr_alloc(struct rtattr **tb)
inst->alg.cra_blkcipher.encrypt = crypto_ctr_crypt;
inst->alg.cra_blkcipher.decrypt = crypto_ctr_crypt;

+ inst->alg.cra_blkcipher.geniv = "chainiv";
+
out:
crypto_mod_put(alg);
return inst;

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

2009-08-14 01:01:13

by Huang, Ying

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Thu, 2009-08-13 at 21:12 +0800, Herbert Xu wrote:
> On Thu, Aug 13, 2009 at 05:39:10PM +1000, Herbert Xu wrote:
> >
> > Oh I see what's going on. It's the switch from chainiv to eseqiv
> > that created the error. I'll apply your patch.
>
> Actually we can't use seqiv on raw counter mode because it cannot
> guarantee IV uniqueness. I think reverting to chainiv is the safer
> option.


I see seqiv is used in rfc3686 mode, it means seqiv can not be used on
raw counter mode but can be used for rfc3686?

Best Regards,
Huang Ying


2009-08-14 01:05:28

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Fri, Aug 14, 2009 at 09:01:07AM +0800, Huang Ying wrote:
>
> I see seqiv is used in rfc3686 mode, it means seqiv can not be used on
> raw counter mode but can be used for rfc3686?

Yeah, with rfc3686 a portion of the counter is available for
counting bytes within each request. This allows a sequential
IV to be safely used as each IV is essentially 2^32 blocks apart.

With raw counter mode as soon as you process two blocks in one
request the next IV would collide with one that has alreay been
used.

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

2009-08-14 13:02:43

by Herbert Xu

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Thu, Aug 13, 2009 at 11:12:53PM +1000, Herbert Xu wrote:
>
> Actually we can't use seqiv on raw counter mode because it cannot
> guarantee IV uniqueness. I think reverting to chainiv is the safer
> option.

In fact this is needed by all stream ciphers, not just ctr.

commit 63b5ac286d5d7f668da537cc53a552578f7674a2
Author: Herbert Xu <[email protected]>
Date: Fri Aug 14 22:55:35 2009 +1000

crypto: blkcipher - Do not use eseqiv on stream ciphers

Recently we switched to using eseqiv on SMP machines in preference
over chainiv. However, eseqiv does not support stream ciphers so
they should still default to chainiv.

This patch applies the same check as done by eseqiv to weed out
the stream ciphers. In particular, all algorithms where the IV
size is not equal to the block size will now default to chainiv.

Signed-off-by: Herbert Xu <[email protected]>

diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
index 03fb5fa..f6f0833 100644
--- a/crypto/ablkcipher.c
+++ b/crypto/ablkcipher.c
@@ -183,6 +183,12 @@ EXPORT_SYMBOL_GPL(crypto_givcipher_type);

const char *crypto_default_geniv(const struct crypto_alg *alg)
{
+ if (((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
+ CRYPTO_ALG_TYPE_BLKCIPHER ? alg->cra_blkcipher.ivsize :
+ alg->cra_ablkcipher.ivsize) !=
+ alg->cra_blocksize)
+ return "chainiv";
+
return alg->cra_flags & CRYPTO_ALG_ASYNC ?
"eseqiv" : skcipher_default_geniv;
}

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

2009-08-14 13:50:23

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [BUGFIX] crypto: Fix ctr(aes) testing by specifying geniv

On Fri, 2009-08-14 at 23:02 +1000, Herbert Xu wrote:
> diff --git a/crypto/ablkcipher.c b/crypto/ablkcipher.c
> index 03fb5fa..f6f0833 100644
> --- a/crypto/ablkcipher.c
> +++ b/crypto/ablkcipher.c
> @@ -183,6 +183,12 @@ EXPORT_SYMBOL_GPL(crypto_givcipher_type);
>
> const char *crypto_default_geniv(const struct crypto_alg *alg)
> {
> + if (((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
> + CRYPTO_ALG_TYPE_BLKCIPHER ? alg->cra_blkcipher.ivsize :
> + alg->cra_ablkcipher.ivsize) !=
> + alg->cra_blocksize)
> + return "chainiv";

I guess you used this cryptic construct instead of something more
readable because this is about a crypto algorithms? ;-)

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)