Hi Herbert,
>>I just noticed that the sha1 implementation in crypto4xx is
>>fundamentally broken. It stores the hash state in the context
>>of the tfm, instead of the context of the request.
>>This means that at any one time you can only have one entity
>>using the tfm, which is infeasible for an asynchronous hash.
I am assuming you are talking about the final hash result being stored in the ctx of the tfm and not the intermediate hash state.
In any case I think we should be safe here since we have a unique packet descriptor per request.
Each packet descriptor has a pointer for SA which holds a pointer for State record. This state record holds the IV and the final hash state.
In the driver’s crypto4xx_build_pd function we get a unique packet descriptor per request and a unique SA pointer location and a unique state record pointer.
The tfm’s context SA is now copied into the packet descriptor’s SA pointer and here we also assign the state record pointer to the SA.
if (iv_len || ctx->is_hash) {
ivlen = iv_len;
pd->sa = pd_uinfo->sa_pa;
sa = (struct dynamic_sa_ctl *) pd_uinfo->sa_va;
if (ctx->direction == DIR_INBOUND)
memcpy(sa, ctx->sa_in, ctx->sa_len * 4);
else
memcpy(sa, ctx->sa_out, ctx->sa_len * 4);
memcpy((void *) sa + ctx->offset_to_sr_ptr,
&pd_uinfo->sr_pa, 4);
If there exists an IV we then copy it to the state record.
if (iv_len)
crypto4xx_memcpy_le(pd_uinfo->sr_va, iv, iv_len);
The above applies for digest functions for hash algorithms.
For init/update/final operations in the init function we save the initial digest to the tfm ctx. We follow the same the procedure for update as in digest operations as well, where in we create a separate copy of the tfm ctx SA in the unique packet descriptor, and the result of each update function is copied into the tfm ctx SA ‘s initial digest again to be used for the next operation. So even for these operations we should be safe.
I hope we both are seeing the same problem here, if not it would be great if you could explain what's the exact problem that you are seeing.
Thanks,
-Shasi
-----Original Message-----
From: Herbert Xu [mailto:herbert@xxxxxxxxxxxxxxxxxxx]
Sent: Tuesday, July 14, 2009 6:06 AM
To: Rina Kagan; Shasi Pulijala
Cc: Linux Crypto Mailing List
Subject: Bogus sha1 implementation in crypto4xx
Hi:
I just noticed that the sha1 implementation in crypto4xx is
fundamentally broken. It stores the hash state in the context
of the tfm, instead of the context of the request.
This means that at any one time you can only have one entity
using the tfm, which is infeasible for an asynchronous hash.
So I'm going to disable the sha1 part of crypto4xx until this
is fixed.
This hasn't caused a problem before because we haven't started
using ahash yet, apart from tcrypt which is single-threaded.
I'm currently in the process of converting authenc (hence IPsec)
across, which means that we will soon rely on the fact that
you can have multiple hash operations ongoing at once.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
