Re: [PATCH 1/3] crypto: inside-secure - add support for authenc(hmac(sha1),cbc(des3_ede))

[Antoine Tenart <antoine.tenart@bootlin.com>]

Hi Pascal,

On Fri, Jul 05, 2019 at 08:49:22AM +0200, Pascal van Leeuwen wrote:
> Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>

Could you provide a commit message, explaining briefly what the patch is
doing?

> @@ -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.

> +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.

> +	.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/_/-/.

Apart from those small comments, the patch looks good.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com