Antoine,
> -----Original Message-----
> From: [email protected] <[email protected]>
> On Behalf Of Antoine Tenart
> Sent: Friday, July 26, 2019 2:20 PM
> To: Pascal van Leeuwen <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Pascal Van
> Leeuwen <[email protected]>
> Subject: Re: [PATCH 1/3] crypto: inside-secure - add support for
> authenc(hmac(sha1),cbc(des3_ede))
>
> Hi Pascal,
>
> On Fri, Jul 05, 2019 at 08:49:22AM +0200, Pascal van Leeuwen wrote:
> > Signed-off-by: Pascal van Leeuwen <[email protected]>
>
> Could you provide a commit message, explaining briefly what the patch is
> doing?
>
I initially figured that to be redundant if the subject already covered it
completely.
But now that I think of it, it's possible the subject does not end up in the
commit
at all ... if that is the case, would it work if I just copy-paste the relevant
part of the
subject message? Or do I need to be more verbose?
> > @@ -199,6 +201,15 @@ static int safexcel_aead_aes_setkey(struct crypto_aead
> > *ctfm, const u8 *key,
> > goto badkey;
> >
> > /* Encryption key */
> > + if (ctx->alg == SAFEXCEL_3DES) {
> > + flags = crypto_aead_get_flags(ctfm);
> > + err = __des3_verify_key(&flags, keys.enckey);
> > + crypto_aead_set_flags(ctfm, flags);
>
> You could use directly des3_verify_key() which does exactly this.
>
Actually, I couldn't due to des3_verify_key expecting a struct crypto_skcipher
as input,
and not a struct crypto_aead, that's why I had to do it this way ...
> > +struct safexcel_alg_template safexcel_alg_authenc_hmac_sha1_cbc_des3_ede =
> > {
> > + .type = SAFEXCEL_ALG_TYPE_AEAD,
>
> You either missed to fill .engines member of this struct, or this series
> is based on another one not merged yet.
>
Yes, that happened in the patchset of which v2 did not make it to the mailing
list ...
> > + .alg.aead = {
> > + .setkey = safexcel_aead_setkey,
> > + .encrypt = safexcel_aead_encrypt_3des,
> > + .decrypt = safexcel_aead_decrypt_3des,
> > + .ivsize = DES3_EDE_BLOCK_SIZE,
> > + .maxauthsize = SHA1_DIGEST_SIZE,
> > + .base = {
> > + .cra_name = "authenc(hmac(sha1),cbc(des3_ede))",
> > + .cra_driver_name =
> > "safexcel-authenc-hmac-sha1-cbc-des3_ede",
>
> You could drop "_ede" here, or s/_/-/.
>
Agree the underscore should not be there.
Our HW does not support any other form of 3DES so EDE doesn't
really add much here, therefore I will just remove "_ede" entirely.
> Apart from those small comments, the patch looks good.
>
> Thanks!
> Antoine
>
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
