2022-05-03 20:32:18

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH v8 3/6] crypto: caam - add in-kernel interface for blob generator

Hi,

> The NXP Cryptographic Acceleration and Assurance Module (CAAM)
> can be used to protect user-defined data across system reboot:
>
> - When the system is fused and boots into secure state, the master
> key is a unique never-disclosed device-specific key
> - random key is encrypted by key derived from master key
> - data is encrypted using the random key
> - encrypted data and its encrypted random key are stored alongside
> - This blob can now be safely stored in non-volatile memory
>
> On next power-on:
> - blob is loaded into CAAM
> - CAAM writes decrypted data either into memory or key register
>
> Add functions to realize encrypting and decrypting into memory alongside
> the CAAM driver.
>
> They will be used in a later commit as a source for the trusted key
> seal/unseal mechanism.

Thanks for the work on this and I'm excited to try this. I'm currently
playing with this and one thing I've noticed is that an export restricted
CAAM isn't handled properly.

That is, there are CAAM's which aren't fully featured. Normally, the
caam driver will take care of it. For example, see commit f20311cc9c58
("crypto: caam - disable pkc for non-E SoCs"). For the trusted keys case,
it would be nice if the kernel will not even probe (or similar).

Right now, everything seems to work fine, but once I try to add a new key,
I'll get the following errros:

# keyctl add trusted mykey "new 32" @u
add_key: Invalid argument
[ 23.138714] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.
[ 23.138740] trusted_key: key_seal failed (-22)

Again this is expected, because I run it on a non-E version. IMHO, it
should be that the trusted keys shouldn't be enabled at all. Like it is
for example if an unknown rng is given:

trusted_key: Unsupported RNG. Supported: kernel, default

Thanks,
-michael


2022-05-04 09:24:41

by Ahmad Fatoum

[permalink] [raw]
Subject: Re: [PATCH v8 3/6] crypto: caam - add in-kernel interface for blob generator

Hello Michael,

On 03.05.22 20:24, Michael Walle wrote:
>> Add functions to realize encrypting and decrypting into memory alongside
>> the CAAM driver.
>>
>> They will be used in a later commit as a source for the trusted key
>> seal/unseal mechanism.
>
> Thanks for the work on this and I'm excited to try this. I'm currently
> playing with this and one thing I've noticed is that an export restricted
> CAAM isn't handled properly.

I didn't know there are still crypto export restrictions in place ;o

> That is, there are CAAM's which aren't fully featured. Normally, the
> caam driver will take care of it. For example, see commit f20311cc9c58
> ("crypto: caam - disable pkc for non-E SoCs"). For the trusted keys case,
> it would be nice if the kernel will not even probe (or similar).
>
> Right now, everything seems to work fine, but once I try to add a new key,
> I'll get the following errros:
>
> # keyctl add trusted mykey "new 32" @u
> add_key: Invalid argument
> [ 23.138714] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.
> [ 23.138740] trusted_key: key_seal failed (-22)

Trusted key core will attempt TPM and TEE if enabled before trying CAAM unless
CAAM was explicitly requested. Silently failing in this case would not be
helpful to users. I think an info message (not error, as it'd be annoying to
see it every time booting a restricted SoC) is a good idea.
Thanks for the feedback.

> Again this is expected, because I run it on a non-E version. IMHO, it
> should be that the trusted keys shouldn't be enabled at all. Like it is
> for example if an unknown rng is given:
>
> trusted_key: Unsupported RNG. Supported: kernel, default

Other backends return -ENODEV and Trusted key core will ignore and try next
in list. Please give below patch a try. I tested it on normal unrestricted
i.MX6. If that's what you had in mind, I can incorporate it into v9.
If you have any Tested-by's or the like you want me to add, please tell. :)

Cheers,
Ahmad

------------------------------ 8< ------------------------------

diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
index d0b1a0015308..1d07e056a5dd 100644
--- a/drivers/crypto/caam/blob_gen.c
+++ b/drivers/crypto/caam/blob_gen.c
@@ -4,6 +4,8 @@
* Copyright (C) 2021 Pengutronix, Ahmad Fatoum <[email protected]>
*/

+#define pr_fmt(fmt) "caam blob_gen: " fmt
+
#include <linux/device.h>
#include <soc/fsl/caam-blob.h>

@@ -147,11 +149,27 @@ EXPORT_SYMBOL(caam_process_blob);

struct caam_blob_priv *caam_blob_gen_init(void)
{
+ struct caam_drv_private *ctrlpriv;
struct device *jrdev;

+ /*
+ * caam_blob_gen_init() may expectedly fail with -ENODEV, e.g. when
+ * CAAM driver didn't probe or when SoC lacks BLOB support. An
+ * error would be harsh in this case, so we stick to info level.
+ */
+
jrdev = caam_jr_alloc();
- if (IS_ERR(jrdev))
- return ERR_CAST(jrdev);
+ if (IS_ERR(jrdev)) {
+ pr_info("no job ring available\n");
+ return ERR_PTR(-ENODEV);
+ }
+
+ ctrlpriv = dev_get_drvdata(jrdev->parent);
+ if (!ctrlpriv->blob_present) {
+ dev_info(jrdev, "no hardware blob generation support\n");
+ caam_jr_free(jrdev);
+ return ERR_PTR(-ENODEV);
+ }

return container_of(jrdev, struct caam_blob_priv, jrdev);
}
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ca0361b2dbb0..a0a622ca5dd4 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -660,6 +660,10 @@ static int caam_probe(struct platform_device *pdev)

caam_little_end = !(bool)(rd_reg32(&ctrl->perfmon.status) &
(CSTA_PLEND | CSTA_ALT_PLEND));
+
+ comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ls);
+ ctrlpriv->blob_present = !!(comp_params & CTPR_LS_BLOB);
+
comp_params = rd_reg32(&ctrl->perfmon.comp_parms_ms);
if (comp_params & CTPR_MS_PS && rd_reg32(&ctrl->mcr) & MCFGR_LONG_PTR)
caam_ptr_sz = sizeof(u64);
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index 7d45b21bd55a..e92210e2ab76 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -92,6 +92,7 @@ struct caam_drv_private {
*/
u8 total_jobrs; /* Total Job Rings in device */
u8 qi_present; /* Nonzero if QI present in device */
+ u8 blob_present; /* Nonzero if BLOB support present in device */
u8 mc_en; /* Nonzero if MC f/w is active */
int secvio_irq; /* Security violation interrupt number */
int virt_en; /* Virtualization enabled in CAAM */
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 3738625c0250..b829066f5063 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -414,6 +414,7 @@ struct caam_perfmon {
#define CTPR_MS_PG_SZ_MASK 0x10
#define CTPR_MS_PG_SZ_SHIFT 4
u32 comp_parms_ms; /* CTPR - Compile Parameters Register */
+#define CTPR_LS_BLOB BIT(1)
u32 comp_parms_ls; /* CTPR - Compile Parameters Register */
u64 rsvd1[2];

diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
index ec57eec4f2d2..8e821bd56e54 100644
--- a/include/soc/fsl/caam-blob.h
+++ b/include/soc/fsl/caam-blob.h
@@ -38,8 +38,9 @@ struct caam_blob_info {

/**
* caam_blob_gen_init - initialize blob generation
- * Return: pointer to new caam_blob_priv instance on success
- * and error pointer otherwise
+ * Return: pointer to new &struct caam_blob_priv instance on success
+ * and ``ERR_PTR(-ENODEV)`` if CAAM has no hardware blobbing support
+ * or no job ring could be allocated.
*/
struct caam_blob_priv *caam_blob_gen_init(void);

diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index 46cb2484ec36..e3415c520c0a 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -55,10 +55,8 @@ static int trusted_caam_init(void)
int ret;

blobifier = caam_blob_gen_init();
- if (IS_ERR(blobifier)) {
- pr_err("Job Ring Device allocation for transform failed\n");
+ if (IS_ERR(blobifier))
return PTR_ERR(blobifier);
- }

ret = register_key_type(&key_type_trusted);
if (ret)
--
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 |