2010-01-30 22:17:31

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.32: padlock_sha1 and hmac broken?

On Sat, Jan 30, 2010 at 05:34:45PM +0100, Wolfgang Walter wrote:
>
> > Please also try "modprobe tcrypt mode=101".
>
> [ 474.947508] alg: hash: Failed to load transform for hmac(sha1): -2
> [ 474.952660] alg: hash: Failed to load transform for hmac(sha1): -2
> [ 474.952737] tcrypt: one or more tests failed!

Oops, it looks like this has been broken ever since we added
prehashing to hmac.

Please try this patch and let me know whether it makes it work
again.

diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index 0af8057..a1180ca 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -57,6 +57,20 @@ static int padlock_sha_update(struct shash_desc *desc,
return crypto_shash_update(&dctx->fallback, data, length);
}

+static int padlock_sha_export(struct shash_desc *desc, void *out)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+
+ return crypto_shash_export(&dctx->fallback, out);
+}
+
+static int padlock_sha_import(struct shash_desc *desc, const void *in)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+
+ return crypto_shash_import(&dctx->fallback, in);
+}
+
static inline void padlock_output_block(uint32_t *src,
uint32_t *dst, size_t count)
{
@@ -235,7 +249,10 @@ static struct shash_alg sha1_alg = {
.update = padlock_sha_update,
.finup = padlock_sha1_finup,
.final = padlock_sha1_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha1_state),
.base = {
.cra_name = "sha1",
.cra_driver_name = "sha1-padlock",
@@ -256,7 +273,10 @@ static struct shash_alg sha256_alg = {
.update = padlock_sha_update,
.finup = padlock_sha256_finup,
.final = padlock_sha256_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha256_state),
.base = {
.cra_name = "sha256",
.cra_driver_name = "sha256-padlock",

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-01-31 02:11:22

by Wolfgang Walter

[permalink] [raw]
Subject: Re: 2.6.32: padlock_sha1 and hmac broken?

Am Samstag, 30. Januar 2010 schrieb Herbert Xu:
> On Sat, Jan 30, 2010 at 05:34:45PM +0100, Wolfgang Walter wrote:
> > > Please also try "modprobe tcrypt mode=101".
> >
> > [ 474.947508] alg: hash: Failed to load transform for hmac(sha1): -2
> > [ 474.952660] alg: hash: Failed to load transform for hmac(sha1): -2
> > [ 474.952737] tcrypt: one or more tests failed!
>
> Oops, it looks like this has been broken ever since we added
> prehashing to hmac.
>
> Please try this patch and let me know whether it makes it work
> again.
>
> diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
> index 0af8057..a1180ca 100644
> --- a/drivers/crypto/padlock-sha.c
> +++ b/drivers/crypto/padlock-sha.c
> @@ -57,6 +57,20 @@ static int padlock_sha_update(struct shash_desc *desc,
> return crypto_shash_update(&dctx->fallback, data, length);
> }
>
> +static int padlock_sha_export(struct shash_desc *desc, void *out)
> +{
> + struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
> +
> + return crypto_shash_export(&dctx->fallback, out);
> +}
> +
> +static int padlock_sha_import(struct shash_desc *desc, const void *in)
> +{
> + struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
> +
> + return crypto_shash_import(&dctx->fallback, in);
> +}
> +
> static inline void padlock_output_block(uint32_t *src,
> uint32_t *dst, size_t count)
> {
> @@ -235,7 +249,10 @@ static struct shash_alg sha1_alg = {
> .update = padlock_sha_update,
> .finup = padlock_sha1_finup,
> .final = padlock_sha1_final,
> + .export = padlock_sha_export,
> + .import = padlock_sha_import,
> .descsize = sizeof(struct padlock_sha_desc),
> + .statesize = sizeof(struct sha1_state),
> .base = {
> .cra_name = "sha1",
> .cra_driver_name = "sha1-padlock",
> @@ -256,7 +273,10 @@ static struct shash_alg sha256_alg = {
> .update = padlock_sha_update,
> .finup = padlock_sha256_finup,
> .final = padlock_sha256_final,
> + .export = padlock_sha_export,
> + .import = padlock_sha_import,
> .descsize = sizeof(struct padlock_sha_desc),
> + .statesize = sizeof(struct sha256_state),
> .base = {
> .cra_name = "sha256",
> .cra_driver_name = "sha256-padlock",
>
> Thanks,

Not sure.

When I do

modprobe tcrypt mode=101

I get a kernel oops:

[ 113.074210] BUG: unable to handle kernel NULL pointer dereference at
00000034
[ 113.074375] IP: [<dfc92042>] padlock_sha_import+0xa/0x15 [padlock_sha]
[ 113.074493] *pde = 00000000
[ 113.074590] Oops: 0000 [#1] PREEMPT
[ 113.074727] last sysfs file: /sys/module/vt/parameters/default_utf8
[ 113.074792] Modules linked in: tcrypt(+) padlock_sha nf_conntrack_tftp
nf_conntrack_sip nf_conntrack_sane nf_conntrack_ftp xt_connlimit xt_connbytes
xt_CONNMARK xt_connmark xt_helper xt_NOTRACK xt_conntrack nf_conntrack_ipv4
nf_conntrack nf_defrag_ipv4
[ 113.075595]
[ 113.075653] Pid: 1701, comm: cryptomgr_test Not tainted (2.6.32.7 #1)
[ 113.075722] EIP: 0060:[<dfc92042>] EFLAGS: 00010246 CPU: 0
[ 113.075789] EIP is at padlock_sha_import+0xa/0x15 [padlock_sha]
[ 113.075855] EAX: 00000000 EBX: d74619f8 ECX: dfc925d4 EDX: cd280238
[ 113.075923] ESI: dc0d2a2c EDI: d74619f0 EBP: cdb77d80 ESP: cdb77d7c
[ 113.075990] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
[ 113.076057] Process cryptomgr_test (pid: 1701, ti=cdb76000 task=d97d3b80
task.ti=cdb76000)
[ 113.076133] Stack:
[ 113.076187] cd280238 cdb77d94 c01ec148 00000000 d74619c0 d74619e8 cdb77d9c
c01ec165
[ 113.076671] <0> cdb77db4 c01e9581 00000008 d74619c0 00000000 00000000
cdb77dbc c01e95b6
[ 113.076671] <0> cdb77dd4 c01e89c5 c01e959c c056fdd8 00000000 cd489008
cdb77ddc c01e89dd
[ 113.076671] Call Trace:
[ 113.076671] [<c01ec148>] ? hmac_import+0x3b/0x40
[ 113.076671] [<c01ec165>] ? hmac_init+0x18/0x1a
[ 113.076671] [<c01e9581>] ? shash_ahash_digest+0x8c/0xa7
[ 113.076671] [<c01e95b6>] ? shash_async_digest+0x1a/0x1c
[ 113.076671] [<c01e89c5>] ? crypto_ahash_op+0x8f/0x99
[ 113.076671] [<c01e959c>] ? shash_async_digest+0x0/0x1c
[ 113.076671] [<c01e89dd>] ? crypto_ahash_digest+0xe/0x10
[ 113.076671] [<c01ea4cb>] ? test_hash+0x17c/0x4fd
[ 113.076671] [<c01e4643>] ? crypto_larval_lookup+0x30/0xfe
[ 113.076671] [<c01e4278>] ? crypto_alloc_tfm+0x3d/0x71
[ 113.076671] [<c01e9114>] ? crypto_alloc_shash+0x10/0x12
[ 113.076671] [<dfc92429>] ? padlock_cra_init+0x1c/0x4b [padlock_sha]
[ 113.076671] [<c01e420c>] ? crypto_create_tfm+0x59/0x88
[ 113.076671] [<c01e4eb4>] ? crypto_spawn_tfm2+0x20/0x37
[ 113.076671] [<c01ec551>] ? hmac_init_tfm+0x1b/0x46
[ 113.076671] [<c01e420c>] ? crypto_create_tfm+0x59/0x88
[ 113.076671] [<c01ea88b>] ? alg_test_hash+0x3f/0x55
[ 113.076671] [<c01ec06b>] ? alg_test+0x168/0x1e9
[ 113.076671] [<c011c37e>] ? pick_next_task_fair+0x8b/0xb5
[ 113.076671] [<c0440b4e>] ? schedule+0x1db/0x382
[ 113.076671] [<c01e9a0e>] ? cryptomgr_test+0x0/0x3e
[ 113.076671] [<c01e9a30>] ? cryptomgr_test+0x22/0x3e
[ 113.076671] [<c01308ef>] ? kthread+0x60/0x65
[ 113.076671] [<c013088f>] ? kthread+0x0/0x65
[ 113.076671] [<c0103147>] ? kernel_thread_helper+0x7/0x10
[ 113.076671] Code: 41 04 89 c8 8b 52 34 ff 52 d4 5d c3 55 89 e5 53 8d 58 08
8b 40 08 8b 48 34 89 d8 ff 51 e8 5b 5d c3 55 89 e5 53 8d 58 08 8b 40 08 <8b>
48 34 89 d8 ff 51 ec 5b 5d c3 55 89 e5 53 8d 58 08 8b 40 04
[ 113.076671] EIP: [<dfc92042>] padlock_sha_import+0xa/0x15 [padlock_sha]
SS:ESP 0068:cdb77d7c
[ 113.076671] CR2: 0000000000000034
[ 113.092662] ---[ end trace d25b6d64215b111e ]---
[ 123.233370] alg: hash: Failed to load transform for hmac(sha1): -4
[ 123.233472] alg: hash: Failed to load transform for hmac(sha1): -4
[ 123.233538] tcrypt: one or more tests failed!



Regards,
--
Wolfgang Walter
Studentenwerk M?nchen
Anstalt des ?ffentlichen Rechts

2010-01-31 09:22:25

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.32: padlock_sha1 and hmac broken?

On Sun, Jan 31, 2010 at 03:11:22AM +0100, Wolfgang Walter wrote:
>
> Not sure.
>
> When I do
>
> modprobe tcrypt mode=101
>
> I get a kernel oops:

Sorry, this one should have a better chance at working.

diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index 0af8057..d3a27e0 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -57,6 +57,23 @@ static int padlock_sha_update(struct shash_desc *desc,
return crypto_shash_update(&dctx->fallback, data, length);
}

+static int padlock_sha_export(struct shash_desc *desc, void *out)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+
+ return crypto_shash_export(&dctx->fallback, out);
+}
+
+static int padlock_sha_import(struct shash_desc *desc, const void *in)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+ struct padlock_sha_ctx *ctx = crypto_shash_ctx(desc->tfm);
+
+ dctx->fallback.tfm = ctx->fallback;
+ dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
+ return crypto_shash_import(&dctx->fallback, in);
+}
+
static inline void padlock_output_block(uint32_t *src,
uint32_t *dst, size_t count)
{
@@ -235,7 +252,10 @@ static struct shash_alg sha1_alg = {
.update = padlock_sha_update,
.finup = padlock_sha1_finup,
.final = padlock_sha1_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha1_state),
.base = {
.cra_name = "sha1",
.cra_driver_name = "sha1-padlock",
@@ -256,7 +276,10 @@ static struct shash_alg sha256_alg = {
.update = padlock_sha_update,
.finup = padlock_sha256_finup,
.final = padlock_sha256_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha256_state),
.base = {
.cra_name = "sha256",
.cra_driver_name = "sha256-padlock",

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-01-31 13:37:57

by Wolfgang Walter

[permalink] [raw]
Subject: Re: 2.6.32: padlock_sha1 and hmac broken?

Am Sonntag, 31. Januar 2010 schrieb Herbert Xu:
> On Sun, Jan 31, 2010 at 03:11:22AM +0100, Wolfgang Walter wrote:
> > Not sure.
> >
> > When I do
> >
> > modprobe tcrypt mode=101
> >
> > I get a kernel oops:
>
> Sorry, this one should have a better chance at working.
>
> diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
> index 0af8057..d3a27e0 100644
> --- a/drivers/crypto/padlock-sha.c
> +++ b/drivers/crypto/padlock-sha.c
> @@ -57,6 +57,23 @@ static int padlock_sha_update(struct shash_desc *desc,
> return crypto_shash_update(&dctx->fallback, data, length);
> }
>
> +static int padlock_sha_export(struct shash_desc *desc, void *out)
> +{
> + struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
> +
> + return crypto_shash_export(&dctx->fallback, out);
> +}
> +
> +static int padlock_sha_import(struct shash_desc *desc, const void *in)
> +{
> + struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
> + struct padlock_sha_ctx *ctx = crypto_shash_ctx(desc->tfm);
> +
> + dctx->fallback.tfm = ctx->fallback;
> + dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
> + return crypto_shash_import(&dctx->fallback, in);
> +}
> +
> static inline void padlock_output_block(uint32_t *src,
> uint32_t *dst, size_t count)
> {
> @@ -235,7 +252,10 @@ static struct shash_alg sha1_alg = {
> .update = padlock_sha_update,
> .finup = padlock_sha1_finup,
> .final = padlock_sha1_final,
> + .export = padlock_sha_export,
> + .import = padlock_sha_import,
> .descsize = sizeof(struct padlock_sha_desc),
> + .statesize = sizeof(struct sha1_state),
> .base = {
> .cra_name = "sha1",
> .cra_driver_name = "sha1-padlock",
> @@ -256,7 +276,10 @@ static struct shash_alg sha256_alg = {
> .update = padlock_sha_update,
> .finup = padlock_sha256_finup,
> .final = padlock_sha256_final,
> + .export = padlock_sha_export,
> + .import = padlock_sha_import,
> .descsize = sizeof(struct padlock_sha_desc),
> + .statesize = sizeof(struct sha256_state),
> .base = {
> .cra_name = "sha256",
> .cra_driver_name = "sha256-padlock",
>
> Thanks,

This patch works. /proc/crypto shows

name : authenc(hmac(sha1),cbc(aes))
driver : authenc(hmac(sha1-padlock),cbc-aes-padlock)
module : kernel
priority : 4300
refcnt : 85
selftest : passed
type : aead
async : yes
blocksize : 16
ivsize : 16
maxauthsize : 20
geniv : <built-in>

....
....

and

modprobe tcrypt mode=101

logs nothing.

Thanks a lot,
--
Wolfgang Walter
Studentenwerk M?nchen
Anstalt des ?ffentlichen Rechts

2010-01-31 23:18:09

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.32: padlock_sha1 and hmac broken?

On Sun, Jan 31, 2010 at 02:37:55PM +0100, Wolfgang Walter wrote:
>
> This patch works. /proc/crypto shows
>
> name : authenc(hmac(sha1),cbc(aes))
> driver : authenc(hmac(sha1-padlock),cbc-aes-padlock)
> module : kernel
> priority : 4300
> refcnt : 85
> selftest : passed
> type : aead
> async : yes
> blocksize : 16
> ivsize : 16
> maxauthsize : 20
> geniv : <built-in>

Thank you for checking!

I've added this patch into crypto-2.6 and will push to stable
when it is merged upstream.

commit 137bb21e8f03758eeffb464a5033338f34446b4e
Author: Herbert Xu <[email protected]>
Date: Mon Feb 1 09:17:56 2010 +1100

crypto: padlock-sha - Add import/export support

As the padlock driver for SHA uses a software fallback to perform
partial hashing, it must implement custom import/export functions.
Otherwise hmac which depends on import/export for prehashing will
not work with padlock-sha.

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

diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index 0af8057..d3a27e0 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -57,6 +57,23 @@ static int padlock_sha_update(struct shash_desc *desc,
return crypto_shash_update(&dctx->fallback, data, length);
}

+static int padlock_sha_export(struct shash_desc *desc, void *out)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+
+ return crypto_shash_export(&dctx->fallback, out);
+}
+
+static int padlock_sha_import(struct shash_desc *desc, const void *in)
+{
+ struct padlock_sha_desc *dctx = shash_desc_ctx(desc);
+ struct padlock_sha_ctx *ctx = crypto_shash_ctx(desc->tfm);
+
+ dctx->fallback.tfm = ctx->fallback;
+ dctx->fallback.flags = desc->flags & CRYPTO_TFM_REQ_MAY_SLEEP;
+ return crypto_shash_import(&dctx->fallback, in);
+}
+
static inline void padlock_output_block(uint32_t *src,
uint32_t *dst, size_t count)
{
@@ -235,7 +252,10 @@ static struct shash_alg sha1_alg = {
.update = padlock_sha_update,
.finup = padlock_sha1_finup,
.final = padlock_sha1_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha1_state),
.base = {
.cra_name = "sha1",
.cra_driver_name = "sha1-padlock",
@@ -256,7 +276,10 @@ static struct shash_alg sha256_alg = {
.update = padlock_sha_update,
.finup = padlock_sha256_finup,
.final = padlock_sha256_final,
+ .export = padlock_sha_export,
+ .import = padlock_sha_import,
.descsize = sizeof(struct padlock_sha_desc),
+ .statesize = sizeof(struct sha256_state),
.base = {
.cra_name = "sha256",
.cra_driver_name = "sha256-padlock",


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