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.
Signed-off-by: Nikolaus Voss <[email protected]>
---
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)
{
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");
+
/*
* 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
u32 status; /* CSTA - CAAM Status */
u64 rsvd3;
--
2.34.1
On Wed, Oct 19, 2022 at 02:44:56PM +0200, Nikolaus Voss wrote:
>
> @@ -62,11 +63,13 @@ int caam_process_blob(struct caam_blob_priv *priv,
> struct caam_blob_info *info, bool encap)
> {
> struct caam_blob_job_result testres;
> + const struct caam_drv_private *ctrlpriv;
Please keep the declarations sorted by length in reverse.
> @@ -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");
Who is the audience of this warning? Is it possible for this warning
to be too frequent?
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
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 <[email protected]>
> ---
> 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!"
> +
> /*
> * 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.
> u32 status; /* CSTA - CAAM Status */
> u64 rsvd3;
>
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, 4 Nov 2022, Herbert Xu wrote:
> On Wed, Oct 19, 2022 at 02:44:56PM +0200, Nikolaus Voss wrote:
>>
>> @@ -62,11 +63,13 @@ int caam_process_blob(struct caam_blob_priv *priv,
>> struct caam_blob_info *info, bool encap)
>> {
>> struct caam_blob_job_result testres;
>> + const struct caam_drv_private *ctrlpriv;
>
> Please keep the declarations sorted by length in reverse.
ok!
>
>> @@ -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");
>
> Who is the audience of this warning? Is it possible for this warning
> to be too frequent?
The warning is issued every time the non-volatile hardware test key is
used to seal/unseal data. This way, the association to the operation is
strong and is less likely to be ignored by mistake. Further, the system
security monitor can transition to a non-secure state any time, so one
warning during boot-up doesn't suffice. IMO, on a typical system, the
warning occurs with very low frequency, only a few times during
e.g. authenication or mounting of encrypted volumes.
The audience is the developer who forgets to enable secure boot (HAB) or
even doesn't know she has to enable HAB to employ the device's unique
hardware key instead of the test key.
Niko
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 <[email protected]>
>> ---
>> 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