Re: [PATCH 1/4] crypto: add eboiv as a crypto API template

Eric Biggers
2020-10-26T18:26:33Z

On Mon, Oct 26, 2020 at 11:24:50AM -0700, Eric Biggers wrote:
> > +static int eboiv_create(struct crypto_template *tmpl, struct rtattr **tb)
> > +{
> > + struct crypto_attr_type *algt;
> > + const char *inner_cipher_name;
> > + struct skcipher_instance *skcipher_inst = NULL;
> > + struct crypto_instance *inst;
> > + struct crypto_alg *base, *block_base;
> > + struct eboiv_instance_ctx *ictx;
> > + struct skcipher_alg *skcipher_alg = NULL;
> > + int ivsize;
> > + u32 mask;
> > + int err;
> > +
> > + algt = crypto_get_attr_type(tb);
> > + if (IS_ERR(algt))
> > + return PTR_ERR(algt);
>
> Need to check that the algorithm is being instantiated as skcipher.
> crypto_check_attr_type() should be used.
>
> > +
> > + inner_cipher_name = crypto_attr_alg_name(tb[1]);
> > + if (IS_ERR(inner_cipher_name))
> > + return PTR_ERR(inner_cipher_name);
>
> The result of crypto_attr_alg_name() can be passed directly to
> crypto_grab_skcipher().
>
> > + mask = crypto_algt_inherited_mask(algt);
> > +
> > + skcipher_inst = kzalloc(sizeof(*skcipher_inst) + sizeof(*ictx), GFP_KERNEL);
> > + if (!skcipher_inst)
> > + return -ENOMEM;
> > +
> > + inst = skcipher_crypto_instance(skcipher_inst);
> > + base = &skcipher_inst->alg.base;
> > + ictx = crypto_instance_ctx(inst);
> > +
> > + /* Symmetric cipher, e.g., "cbc(aes)" */
> > + err = crypto_grab_skcipher(&ictx->skcipher_spawn, inst, inner_cipher_name, 0, mask);
> > + if (err)
> > + goto out_free_inst;
> > +
> > + skcipher_alg = crypto_spawn_skcipher_alg(&ictx->skcipher_spawn);
> > + block_base = &skcipher_alg->base;
> > + ivsize = crypto_skcipher_alg_ivsize(skcipher_alg);
> > +
> > + if (ivsize != block_base->cra_blocksize)
> > + goto out_drop_skcipher;
>
> Shouldn't it be verified that the underlying algorithm is actually cbc?
>
> > + skcipher_inst->alg.chunksize = crypto_skcipher_alg_chunksize(skcipher_alg);
> > + skcipher_inst->alg.walksize = crypto_skcipher_alg_walksize(skcipher_alg);
>
> Setting these isn't necessary.
>
> > +
> > + skcipher_inst->free = eboiv_skcipher_free_instance;
> > +
> > + err = skcipher_register_instance(tmpl, skcipher_inst);
> > +
> > + if (err)
> > + goto out_drop_skcipher;
> > +
> > + return 0;
> > +
> > +out_drop_skcipher:
> > + crypto_drop_skcipher(&ictx->skcipher_spawn);
> > +out_free_inst:
> > + kfree(skcipher_inst);
> > + return err;
> > +}
>
> eboiv_skcipher_free_instance() can be called on the error path.

Here's the version of eboiv_create() I recommend (untested):

static int eboiv_create(struct crypto_template *tmpl, struct rtattr **tb)
{
struct skcipher_instance *inst;
struct eboiv_instance_ctx *ictx;
struct skcipher_alg *alg;
u32 mask;
int err;

err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_SKCIPHER, &mask);
if (err)
return err;

inst = kzalloc(sizeof(*inst) + sizeof(*ictx), GFP_KERNEL);
if (!inst)
return -ENOMEM;
ictx = skcipher_instance_ctx(inst);

err = crypto_grab_skcipher(&ictx->skcipher_spawn,
skcipher_crypto_instance(inst),
crypto_attr_alg_name(tb[1]), 0, mask);
if (err)
goto err_free_inst;
alg = crypto_spawn_skcipher_alg(&ictx->skcipher_spawn);

err = -EINVAL;
if (strncmp(alg->base.cra_name, "cbc(", 4) ||
crypto_skcipher_alg_ivsize(alg) != alg->base.cra_blocksize)
goto err_free_inst;

err = -ENAMETOOLONG;
if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME, "eboiv(%s)",
alg->base.cra_name) >= CRYPTO_MAX_ALG_NAME)
goto err_free_inst;

if (snprintf(inst->alg.base.cra_driver_name, CRYPTO_MAX_ALG_NAME,
"eboiv(%s)", alg->base.cra_driver_name) >=
CRYPTO_MAX_ALG_NAME)
goto err_free_inst;

inst->alg.base.cra_blocksize = alg->base.cra_blocksize;
inst->alg.base.cra_ctxsize = sizeof(struct eboiv_tfm_ctx);
inst->alg.base.cra_alignmask = alg->base.cra_alignmask;
inst->alg.base.cra_priority = alg->base.cra_priority;

inst->alg.setkey = eboiv_skcipher_setkey;
inst->alg.encrypt = eboiv_skcipher_encrypt;
inst->alg.decrypt = eboiv_skcipher_decrypt;
inst->alg.init = eboiv_skcipher_init_tfm;
inst->alg.exit = eboiv_skcipher_exit_tfm;

inst->alg.min_keysize = crypto_skcipher_alg_min_keysize(alg);
inst->alg.max_keysize = crypto_skcipher_alg_max_keysize(alg);
inst->alg.ivsize = crypto_skcipher_alg_ivsize(alg);

inst->free = eboiv_skcipher_free_instance;

err = skcipher_register_instance(tmpl, inst);
if (err) {
err_free_inst:
eboiv_skcipher_free_instance(inst);
}
return err;
}