Re: [PATCH] crypto: caam: blob_gen.c: warn if key is insecure

[Nikolaus Voss]

On Fri, 4 Nov 2022, Ahmad Fatoum wrote:

> Hello Nikolaus,
>
> On 19.10.22 14:44, Nikolaus Voss wrote:
> > If CAAM is not in "trusted" or "secure" state, a fixed non-volatile key
> > is used instead of the unique device key. This is the default mode of
> > operation without secure boot (HAB). In this scenario, CAAM encrypted
> > blobs should be used only for testing but not in a production
> > environment, so issue a warning.
>
> Thanks for your patch.
>
> > Signed-off-by: Nikolaus Voss <nikolaus.voss@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/crypto/caam/blob_gen.c | 8 ++++++++
> >  drivers/crypto/caam/regs.h     | 3 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
> > index 6345c7269eb0..f3e741393f65 100644
> > --- a/drivers/crypto/caam/blob_gen.c
> > +++ b/drivers/crypto/caam/blob_gen.c
> > @@ -6,6 +6,7 @@
> >
> >  #define pr_fmt(fmt) "caam blob_gen: " fmt
> >
> > +#include <linux/bitfield.h>
> >  #include <linux/device.h>
> >  #include <soc/fsl/caam-blob.h>
> >
> > @@ -62,11 +63,13 @@ int caam_process_blob(struct caam_blob_priv *priv,
> >  		      struct caam_blob_info *info, bool encap)
>
> I agree with Herbert that this may not be the best place. I think
> a single warning during caam_blob_gen_init() would suffice.
>
> >  {
> >  	struct caam_blob_job_result testres;
> > +	const struct caam_drv_private *ctrlpriv;
> >  	struct device *jrdev = &priv->jrdev;
> >  	dma_addr_t dma_in, dma_out;
> >  	int op = OP_PCLID_BLOB;
> >  	size_t output_len;
> >  	u32 *desc;
> > +	u32 moo;
> >  	int ret;
> >
> >  	if (info->key_mod_len > CAAM_BLOB_KEYMOD_LENGTH)
> > @@ -100,6 +103,11 @@ int caam_process_blob(struct caam_blob_priv *priv,
> >  		goto out_unmap_in;
> >  	}
> >
> > +	ctrlpriv = dev_get_drvdata(jrdev->parent);
> > +	moo = FIELD_GET(CSTA_MOO, ctrlpriv->ctrl->perfmon.status);
> > +	if (moo != CSTA_MOO_SECURE && moo != CSTA_MOO_TRUSTED)
> > +		dev_warn(jrdev, "using insecure test key!\n");
>
> I'd make the warning a bit more verbose, e.g.
>
>  "device not configured for trusted/secure mode: using insecure test key!"

I agree.

> > +
> >  	/*
> >  	 * A data blob is encrypted using a blob key (BK); a random number.
> >  	 * The BK is used as an AES-CCM key. The initial block (B0) and the
> > diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
> > index 66d6dad841bb..b7de5fb5e056 100644
> > --- a/drivers/crypto/caam/regs.h
> > +++ b/drivers/crypto/caam/regs.h
> > @@ -426,6 +426,9 @@ struct caam_perfmon {
> >  	u32 rsvd2;
> >  #define CSTA_PLEND		BIT(10)
> >  #define CSTA_ALT_PLEND		BIT(18)
> > +#define CSTA_MOO		GENMASK(9, 8)
> > +#define CSTA_MOO_SECURE	1
> > +#define CSTA_MOO_TRUSTED	3
>
> I just checked the i.MX6 and LS1046 security reference manuals and both
> have Trusted as 2 (10b). 3 is fail. Does you SoC differ? Either way, please
> note what SoC you were testing on in the commit message.

You're right, I will correct that.

Niko