| Message ID | 20220222195819.2313913-1-a.fatoum@pengutronix.de |
|---|---|
| Headers | show |
| Series | KEYS: trusted: Introduce support for NXP CAAM-based trusted keys | expand |
On Tue, Feb 22, 2022 at 08:58:16PM +0100, Ahmad Fatoum wrote: > The two existing trusted key sources don't make use of the kernel RNG, > but instead let the hardware doing the sealing/unsealing also > generate the random key material. However, users may want to place > less trust into the quality of the trust source's random number > generator and instead use the kernel entropy pool, which can be > seeded from multiple entropy sources. > > Make this possible by adding a new trusted.kernel_rng parameter, > that will force use of the kernel RNG. In its absence, it's up > to the trust source to decide, which random numbers to use, > maintaining the existing behavior. > > Suggested-by: Jarkko Sakkinen <jarkko@kernel.org> > Acked-by: Sumit Garg <sumit.garg@linaro.org> > Reviewed-by: David Gstir <david@sigma-star.at> > Reviewed-by: Pankaj Gupta <pankaj.gupta@nxp.com> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > To: James Bottomley <jejb@linux.ibm.com> > To: Jarkko Sakkinen <jarkko@kernel.org> > To: Mimi Zohar <zohar@linux.ibm.com> > To: David Howells <dhowells@redhat.com> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: "Horia Geantă" <horia.geanta@nxp.com> > Cc: Aymen Sghaier <aymen.sghaier@nxp.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Jan Luebbe <j.luebbe@pengutronix.de> > Cc: Eric Biggers <ebiggers@kernel.org> > Cc: David Gstir <david@sigma-star.at> > Cc: Richard Weinberger <richard@nod.at> > Cc: Franck LENORMAND <franck.lenormand@nxp.com> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: Tim Harvey <tharvey@gateworks.com> > Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > Cc: keyrings@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > Cc: linux-integrity@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-security-module@vger.kernel.org > --- > .../admin-guide/kernel-parameters.txt | 10 ++++++ > .../security/keys/trusted-encrypted.rst | 20 ++++++----- > security/keys/trusted-keys/trusted_core.c | 35 ++++++++++++++++++- > 3 files changed, 56 insertions(+), 9 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index f5a27f067db9..844c883ca9d8 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -5880,6 +5880,16 @@ > first trust source as a backend which is initialized > successfully during iteration. > > + trusted.rng= [KEYS] > + Format: <string> > + The RNG used to generate key material for trusted keys. > + Can be one of: > + - "kernel" > + - the same value as trusted.source: "tpm" or "tee" > + - "default" > + If not specified, "default" is used. In this case, > + the RNG's choice is left to each individual trust source. > + > tsc= Disable clocksource stability checks for TSC. > Format: <string> > [x86] reliable: mark tsc clocksource as reliable, this > diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst > index 80d5a5af62a1..99cf34d7c025 100644 > --- a/Documentation/security/keys/trusted-encrypted.rst > +++ b/Documentation/security/keys/trusted-encrypted.rst > @@ -87,22 +87,26 @@ Key Generation > Trusted Keys > ------------ > > -New keys are created from random numbers generated in the trust source. They > -are encrypted/decrypted using a child key in the storage key hierarchy. > -Encryption and decryption of the child key must be protected by a strong > -access control policy within the trust source. > +New keys are created from random numbers. They are encrypted/decrypted using > +a child key in the storage key hierarchy. Encryption and decryption of the > +child key must be protected by a strong access control policy within the > +trust source. The random number generator in use differs according to the > +selected trust source: > > - * TPM (hardware device) based RNG > + * TPM: hardware device based RNG > > - Strength of random numbers may vary from one device manufacturer to > - another. > + Keys are generated within the TPM. Strength of random numbers may vary > + from one device manufacturer to another. > > - * TEE (OP-TEE based on Arm TrustZone) based RNG > + * TEE: OP-TEE based on Arm TrustZone based RNG > > RNG is customizable as per platform needs. It can either be direct output > from platform specific hardware RNG or a software based Fortuna CSPRNG > which can be seeded via multiple entropy sources. > > +Users may override this by specifying ``trusted.rng=kernel`` on the kernel > +command-line to override the used RNG with the kernel's random number pool. > + > Encrypted Keys > -------------- > > diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c > index 7cdbd16aed30..9235fb7d0ec9 100644 > --- a/security/keys/trusted-keys/trusted_core.c > +++ b/security/keys/trusted-keys/trusted_core.c > @@ -16,12 +16,17 @@ > #include <linux/key-type.h> > #include <linux/module.h> > #include <linux/parser.h> > +#include <linux/random.h> > #include <linux/rcupdate.h> > #include <linux/slab.h> > #include <linux/static_call.h> > #include <linux/string.h> > #include <linux/uaccess.h> > > +static char *trusted_rng = "default"; > +module_param_named(rng, trusted_rng, charp, 0); > +MODULE_PARM_DESC(rng, "Select trusted key RNG"); > + > static char *trusted_key_source; > module_param_named(source, trusted_key_source, charp, 0); > MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)"); > @@ -312,8 +317,14 @@ struct key_type key_type_trusted = { > }; > EXPORT_SYMBOL_GPL(key_type_trusted); > > +static int kernel_get_random(unsigned char *key, size_t key_len) > +{ > + return get_random_bytes_wait(key, key_len) ?: key_len; > +} > + > static int __init init_trusted(void) > { > + int (*get_random)(unsigned char *key, size_t key_len); > int i, ret = 0; > > for (i = 0; i < ARRAY_SIZE(trusted_key_sources); i++) { > @@ -322,6 +333,28 @@ static int __init init_trusted(void) > strlen(trusted_key_sources[i].name))) > continue; > > + /* > + * We always support trusted.rng="kernel" and "default" as > + * well as trusted.rng=$trusted.source if the trust source > + * defines its own get_random callback. > + */ > + get_random = trusted_key_sources[i].ops->get_random; > + if (trusted_rng && strcmp(trusted_rng, "default")) { > + if (!strcmp(trusted_rng, "kernel")) { > + get_random = kernel_get_random; > + } else if (strcmp(trusted_rng, trusted_key_sources[i].name) || > + !get_random) { > + pr_warn("Unsupported RNG. Supported: kernel"); > + if (get_random) > + pr_cont(", %s", trusted_key_sources[i].name); > + pr_cont(", default\n"); > + return -EINVAL; > + } > + } > + > + if (!get_random) > + get_random = kernel_get_random; > + > static_call_update(trusted_key_init, > trusted_key_sources[i].ops->init); > static_call_update(trusted_key_seal, > @@ -329,7 +362,7 @@ static int __init init_trusted(void) > static_call_update(trusted_key_unseal, > trusted_key_sources[i].ops->unseal); > static_call_update(trusted_key_get_random, > - trusted_key_sources[i].ops->get_random); > + get_random); > static_call_update(trusted_key_exit, > trusted_key_sources[i].ops->exit); > migratable = trusted_key_sources[i].ops->migratable; > -- > 2.30.2 > Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On 23.02.22 16:41, Jarkko Sakkinen wrote: > On Tue, Feb 22, 2022 at 08:58:18PM +0100, Ahmad Fatoum wrote: >> 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. >> >> Reviewed-by: David Gstir <david@sigma-star.at> >> Reviewed-by: Pankaj Gupta <pankaj.gupta@nxp.com> >> Tested-By: Tim Harvey <tharvey@gateworks.com> >> Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> --- >> To: "Horia Geantă" <horia.geanta@nxp.com> >> To: Aymen Sghaier <aymen.sghaier@nxp.com> >> To: Herbert Xu <herbert@gondor.apana.org.au> >> To: "David S. Miller" <davem@davemloft.net> >> Cc: James Bottomley <jejb@linux.ibm.com> >> Cc: Jarkko Sakkinen <jarkko@kernel.org> >> Cc: Mimi Zohar <zohar@linux.ibm.com> >> Cc: David Howells <dhowells@redhat.com> >> Cc: James Morris <jmorris@namei.org> >> Cc: Eric Biggers <ebiggers@kernel.org> >> Cc: "Serge E. Hallyn" <serge@hallyn.com> >> Cc: Jan Luebbe <j.luebbe@pengutronix.de> >> Cc: David Gstir <david@sigma-star.at> >> Cc: Richard Weinberger <richard@nod.at> >> Cc: Franck LENORMAND <franck.lenormand@nxp.com> >> Cc: Sumit Garg <sumit.garg@linaro.org> >> Cc: Tim Harvey <tharvey@gateworks.com> >> Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> >> Cc: Pankaj Gupta <pankaj.gupta@nxp.com> >> Cc: linux-integrity@vger.kernel.org >> Cc: keyrings@vger.kernel.org >> Cc: linux-crypto@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: linux-security-module@vger.kernel.org >> --- >> drivers/crypto/caam/Kconfig | 3 + >> drivers/crypto/caam/Makefile | 1 + >> drivers/crypto/caam/blob_gen.c | 230 +++++++++++++++++++++++++++++++++ >> include/soc/fsl/caam-blob.h | 56 ++++++++ >> 4 files changed, 290 insertions(+) >> create mode 100644 drivers/crypto/caam/blob_gen.c >> create mode 100644 include/soc/fsl/caam-blob.h >> >> diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig >> index 84ea7cba5ee5..ea9f8b1ae981 100644 >> --- a/drivers/crypto/caam/Kconfig >> +++ b/drivers/crypto/caam/Kconfig >> @@ -151,6 +151,9 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API >> Selecting this will register the SEC4 hardware rng to >> the hw_random API for supplying the kernel entropy pool. >> >> +config CRYPTO_DEV_FSL_CAAM_BLOB_GEN >> + bool >> + >> endif # CRYPTO_DEV_FSL_CAAM_JR >> >> endif # CRYPTO_DEV_FSL_CAAM >> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile >> index 3570286eb9ce..25f7ae5a4642 100644 >> --- a/drivers/crypto/caam/Makefile >> +++ b/drivers/crypto/caam/Makefile >> @@ -21,6 +21,7 @@ caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o pkc_desc.o >> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o >> >> caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o >> ifneq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),) >> diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c >> new file mode 100644 >> index 000000000000..513d3f90e438 >> --- /dev/null >> +++ b/drivers/crypto/caam/blob_gen.c >> @@ -0,0 +1,230 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (C) 2015 Pengutronix, Steffen Trumtrar <kernel@pengutronix.de> >> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de> >> + */ >> + >> +#include <linux/device.h> >> +#include <soc/fsl/caam-blob.h> >> + >> +#include "compat.h" >> +#include "desc_constr.h" >> +#include "desc.h" >> +#include "error.h" >> +#include "intern.h" >> +#include "jr.h" >> +#include "regs.h" >> + >> +struct caam_blob_priv { >> + struct device jrdev; >> +}; >> + >> +struct caam_blob_job_result { >> + int err; >> + struct completion completion; >> +}; >> + >> +static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err, void *context) >> +{ >> + struct caam_blob_job_result *res = context; >> + int ecode = 0; >> + >> + dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err); >> + >> + if (err) >> + ecode = caam_jr_strstatus(dev, err); >> + >> + res->err = ecode; >> + >> + /* >> + * Upon completion, desc points to a buffer containing a CAAM job >> + * descriptor which encapsulates data into an externally-storable >> + * blob. >> + */ >> + complete(&res->completion); >> +} >> + >> +static u32 *caam_blob_alloc_desc(size_t keymod_len) >> +{ >> + size_t len; >> + >> + /* header + (key mod immediate) + 2x pointers + op */ >> + len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + CAAM_PTR_SZ_MAX) + 4; > > Nit: the amount of magic numbers is overwhelming here. I neither understand > the statement nor how that comment should help me to understand it. header = 4 (key mod immediate) = (4 + ALIGN(keymod_len, 4)) 2x pointer = 2 * (4 + 4 + CAAM_PTR_SZ_MAX) op = 4 I haven't heard back from the CAAM maintainers yet since v1. Perhaps now is a good occasion to chime in? :-) @Jarkko, could you take a look at patch 5? That's the gist of the series and I have yet to get maintainer feedback on that one. Cheers, Ahmad > > BR, Jarkko >
> -----Original Message----- > From: Ahmad Fatoum <a.fatoum@pengutronix.de> > Sent: Wednesday, February 23, 2022 9:50 PM > To: Jarkko Sakkinen <jarkko@kernel.org> > Cc: Horia Geanta <horia.geanta@nxp.com>; Aymen Sghaier > <aymen.sghaier@nxp.com>; Herbert Xu <herbert@gondor.apana.org.au>; > David S. Miller <davem@davemloft.net>; kernel@pengutronix.de; David Gstir > <david@sigma-star.at>; Pankaj Gupta <pankaj.gupta@nxp.com>; > tharvey@gateworks.com; Matthias Schiffer <matthias.schiffer@ew.tq- > group.com>; James Bottomley <jejb@linux.ibm.com>; Mimi Zohar > <zohar@linux.ibm.com>; David Howells <dhowells@redhat.com>; James Morris > <jmorris@namei.org>; Eric Biggers <ebiggers@kernel.org>; Serge E. Hallyn > <serge@hallyn.com>; Jan Luebbe <j.luebbe@pengutronix.de>; Richard > Weinberger <richard@nod.at>; Franck Lenormand > <franck.lenormand@nxp.com>; Sumit Garg <sumit.garg@linaro.org>; linux- > integrity@vger.kernel.org; keyrings@vger.kernel.org; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org; linux-security- > module@vger.kernel.org > Subject: [EXT] Re: [PATCH v5 4/5] crypto: caam - add in-kernel interface for blob > generator > > Caution: EXT Email > > On 23.02.22 16:41, Jarkko Sakkinen wrote: > > On Tue, Feb 22, 2022 at 08:58:18PM +0100, Ahmad Fatoum wrote: > >> 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. > >> > >> Reviewed-by: David Gstir <david@sigma-star.at> > >> Reviewed-by: Pankaj Gupta <pankaj.gupta@nxp.com> > >> Tested-By: Tim Harvey <tharvey@gateworks.com> > >> Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > >> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > >> --- > >> To: "Horia Geantă" <horia.geanta@nxp.com> > >> To: Aymen Sghaier <aymen.sghaier@nxp.com> > >> To: Herbert Xu <herbert@gondor.apana.org.au> > >> To: "David S. Miller" <davem@davemloft.net> > >> Cc: James Bottomley <jejb@linux.ibm.com> > >> Cc: Jarkko Sakkinen <jarkko@kernel.org> > >> Cc: Mimi Zohar <zohar@linux.ibm.com> > >> Cc: David Howells <dhowells@redhat.com> > >> Cc: James Morris <jmorris@namei.org> > >> Cc: Eric Biggers <ebiggers@kernel.org> > >> Cc: "Serge E. Hallyn" <serge@hallyn.com> > >> Cc: Jan Luebbe <j.luebbe@pengutronix.de> > >> Cc: David Gstir <david@sigma-star.at> > >> Cc: Richard Weinberger <richard@nod.at> > >> Cc: Franck LENORMAND <franck.lenormand@nxp.com> > >> Cc: Sumit Garg <sumit.garg@linaro.org> > >> Cc: Tim Harvey <tharvey@gateworks.com> > >> Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > >> Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > >> Cc: linux-integrity@vger.kernel.org > >> Cc: keyrings@vger.kernel.org > >> Cc: linux-crypto@vger.kernel.org > >> Cc: linux-kernel@vger.kernel.org > >> Cc: linux-security-module@vger.kernel.org > >> --- > >> drivers/crypto/caam/Kconfig | 3 + > >> drivers/crypto/caam/Makefile | 1 + > >> drivers/crypto/caam/blob_gen.c | 230 > +++++++++++++++++++++++++++++++++ > >> include/soc/fsl/caam-blob.h | 56 ++++++++ > >> 4 files changed, 290 insertions(+) > >> create mode 100644 drivers/crypto/caam/blob_gen.c create mode > >> 100644 include/soc/fsl/caam-blob.h > >> > >> diff --git a/drivers/crypto/caam/Kconfig > >> b/drivers/crypto/caam/Kconfig index 84ea7cba5ee5..ea9f8b1ae981 100644 > >> --- a/drivers/crypto/caam/Kconfig > >> +++ b/drivers/crypto/caam/Kconfig > >> @@ -151,6 +151,9 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API > >> Selecting this will register the SEC4 hardware rng to > >> the hw_random API for supplying the kernel entropy pool. > >> > >> +config CRYPTO_DEV_FSL_CAAM_BLOB_GEN > >> + bool > >> + > >> endif # CRYPTO_DEV_FSL_CAAM_JR > >> > >> endif # CRYPTO_DEV_FSL_CAAM > >> diff --git a/drivers/crypto/caam/Makefile > >> b/drivers/crypto/caam/Makefile index 3570286eb9ce..25f7ae5a4642 > >> 100644 > >> --- a/drivers/crypto/caam/Makefile > >> +++ b/drivers/crypto/caam/Makefile > >> @@ -21,6 +21,7 @@ caam_jr- > $(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) > >> += caamalg_qi.o > >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o > >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o > >> caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o > >> pkc_desc.o > >> +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o > >> > >> caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o ifneq > >> ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),) > >> diff --git a/drivers/crypto/caam/blob_gen.c > >> b/drivers/crypto/caam/blob_gen.c new file mode 100644 index > >> 000000000000..513d3f90e438 > >> --- /dev/null > >> +++ b/drivers/crypto/caam/blob_gen.c > >> @@ -0,0 +1,230 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* > >> + * Copyright (C) 2015 Pengutronix, Steffen Trumtrar > >> +<kernel@pengutronix.de> > >> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum > >> +<kernel@pengutronix.de> */ > >> + > >> +#include <linux/device.h> > >> +#include <soc/fsl/caam-blob.h> > >> + > >> +#include "compat.h" > >> +#include "desc_constr.h" > >> +#include "desc.h" > >> +#include "error.h" > >> +#include "intern.h" > >> +#include "jr.h" > >> +#include "regs.h" > >> + > >> +struct caam_blob_priv { > >> + struct device jrdev; > >> +}; > >> + > >> +struct caam_blob_job_result { > >> + int err; > >> + struct completion completion; > >> +}; > >> + > >> +static void caam_blob_job_done(struct device *dev, u32 *desc, u32 > >> +err, void *context) { > >> + struct caam_blob_job_result *res = context; > >> + int ecode = 0; > >> + > >> + dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err); > >> + > >> + if (err) > >> + ecode = caam_jr_strstatus(dev, err); > >> + > >> + res->err = ecode; > >> + > >> + /* > >> + * Upon completion, desc points to a buffer containing a CAAM job > >> + * descriptor which encapsulates data into an externally-storable > >> + * blob. > >> + */ > >> + complete(&res->completion); > >> +} > >> + > >> +static u32 *caam_blob_alloc_desc(size_t keymod_len) { > >> + size_t len; > >> + > >> + /* header + (key mod immediate) + 2x pointers + op */ > >> + len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + > >> + CAAM_PTR_SZ_MAX) + 4; > > > > Nit: the amount of magic numbers is overwhelming here. I neither > > understand the statement nor how that comment should help me to > understand it. > > header = 4 > (key mod immediate) = (4 + ALIGN(keymod_len, 4)) > 2x pointer = 2 * (4 + 4 + CAAM_PTR_SZ_MAX) > op = 4 Instead of the function caam_blob_alloc_desc(), it will be great if the caller functions caam_encap_blob()/caam_encap_blob (), could add local array: uint32_t desc[CAAM_DESC_BYTES_MAX]; > > I haven't heard back from the CAAM maintainers yet since v1. Perhaps now is a > good occasion to chime in? :-) > > @Jarkko, could you take a look at patch 5? That's the gist of the series and I > have yet to get maintainer feedback on that one. > > Cheers, > Ahmad > > > > > > BR, Jarkko > > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pen > gutronix.de%2F&data=04%7C01%7Cpankaj.gupta%40nxp.com%7Cc97e9d4 > aaf304124407908d9f6e87101%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% > 7C0%7C637812300459173929%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wL > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&s > data=CvnfIXR68DPRCaYrOYQKSv2eSBSNSSJYx2BQJee4yLg%3D&reserved=0 > | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Wed, 2022-02-23 at 17:20 +0100, Ahmad Fatoum wrote: > On 23.02.22 16:41, Jarkko Sakkinen wrote: > > On Tue, Feb 22, 2022 at 08:58:18PM +0100, Ahmad Fatoum wrote: > > > 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. > > > > > > Reviewed-by: David Gstir <david@sigma-star.at> > > > Reviewed-by: Pankaj Gupta <pankaj.gupta@nxp.com> > > > Tested-By: Tim Harvey <tharvey@gateworks.com> > > > Tested-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de> > > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > > > --- > > > To: "Horia Geantă" <horia.geanta@nxp.com> > > > To: Aymen Sghaier <aymen.sghaier@nxp.com> > > > To: Herbert Xu <herbert@gondor.apana.org.au> > > > To: "David S. Miller" <davem@davemloft.net> > > > Cc: James Bottomley <jejb@linux.ibm.com> > > > Cc: Jarkko Sakkinen <jarkko@kernel.org> > > > Cc: Mimi Zohar <zohar@linux.ibm.com> > > > Cc: David Howells <dhowells@redhat.com> > > > Cc: James Morris <jmorris@namei.org> > > > Cc: Eric Biggers <ebiggers@kernel.org> > > > Cc: "Serge E. Hallyn" <serge@hallyn.com> > > > Cc: Jan Luebbe <j.luebbe@pengutronix.de> > > > Cc: David Gstir <david@sigma-star.at> > > > Cc: Richard Weinberger <richard@nod.at> > > > Cc: Franck LENORMAND <franck.lenormand@nxp.com> > > > Cc: Sumit Garg <sumit.garg@linaro.org> > > > Cc: Tim Harvey <tharvey@gateworks.com> > > > Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> > > > Cc: Pankaj Gupta <pankaj.gupta@nxp.com> > > > Cc: linux-integrity@vger.kernel.org > > > Cc: keyrings@vger.kernel.org > > > Cc: linux-crypto@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org > > > Cc: linux-security-module@vger.kernel.org > > > --- > > > drivers/crypto/caam/Kconfig | 3 + > > > drivers/crypto/caam/Makefile | 1 + > > > drivers/crypto/caam/blob_gen.c | 230 +++++++++++++++++++++++++++++++++ > > > include/soc/fsl/caam-blob.h | 56 ++++++++ > > > 4 files changed, 290 insertions(+) > > > create mode 100644 drivers/crypto/caam/blob_gen.c > > > create mode 100644 include/soc/fsl/caam-blob.h > > > > > > diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig > > > index 84ea7cba5ee5..ea9f8b1ae981 100644 > > > --- a/drivers/crypto/caam/Kconfig > > > +++ b/drivers/crypto/caam/Kconfig > > > @@ -151,6 +151,9 @@ config CRYPTO_DEV_FSL_CAAM_RNG_API > > > Selecting this will register the SEC4 hardware rng to > > > the hw_random API for supplying the kernel entropy pool. > > > > > > +config CRYPTO_DEV_FSL_CAAM_BLOB_GEN > > > + bool > > > + > > > endif # CRYPTO_DEV_FSL_CAAM_JR > > > > > > endif # CRYPTO_DEV_FSL_CAAM > > > diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile > > > index 3570286eb9ce..25f7ae5a4642 100644 > > > --- a/drivers/crypto/caam/Makefile > > > +++ b/drivers/crypto/caam/Makefile > > > @@ -21,6 +21,7 @@ caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += caamalg_qi.o > > > caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API) += caamhash.o > > > caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_RNG_API) += caamrng.o > > > caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_PKC_API) += caampkc.o pkc_desc.o > > > +caam_jr-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o > > > > > > caam-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI) += qi.o > > > ifneq ($(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_QI),) > > > diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c > > > new file mode 100644 > > > index 000000000000..513d3f90e438 > > > --- /dev/null > > > +++ b/drivers/crypto/caam/blob_gen.c > > > @@ -0,0 +1,230 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (C) 2015 Pengutronix, Steffen Trumtrar <kernel@pengutronix.de> > > > + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de> > > > + */ > > > + > > > +#include <linux/device.h> > > > +#include <soc/fsl/caam-blob.h> > > > + > > > +#include "compat.h" > > > +#include "desc_constr.h" > > > +#include "desc.h" > > > +#include "error.h" > > > +#include "intern.h" > > > +#include "jr.h" > > > +#include "regs.h" > > > + > > > +struct caam_blob_priv { > > > + struct device jrdev; > > > +}; > > > + > > > +struct caam_blob_job_result { > > > + int err; > > > + struct completion completion; > > > +}; > > > + > > > +static void caam_blob_job_done(struct device *dev, u32 *desc, u32 err, void *context) > > > +{ > > > + struct caam_blob_job_result *res = context; > > > + int ecode = 0; > > > + > > > + dev_dbg(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err); > > > + > > > + if (err) > > > + ecode = caam_jr_strstatus(dev, err); > > > + > > > + res->err = ecode; > > > + > > > + /* > > > + * Upon completion, desc points to a buffer containing a CAAM job > > > + * descriptor which encapsulates data into an externally-storable > > > + * blob. > > > + */ > > > + complete(&res->completion); > > > +} > > > + > > > +static u32 *caam_blob_alloc_desc(size_t keymod_len) > > > +{ > > > + size_t len; > > > + > > > + /* header + (key mod immediate) + 2x pointers + op */ > > > + len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + CAAM_PTR_SZ_MAX) + 4; > > > > Nit: the amount of magic numbers is overwhelming here. I neither understand > > the statement nor how that comment should help me to understand it. > > header = 4 > (key mod immediate) = (4 + ALIGN(keymod_len, 4)) > 2x pointer = 2 * (4 + 4 + CAAM_PTR_SZ_MAX) > op = 4 Please create a struct with the associated fields instead and then it is just sizeof that. BR, Jarkko
Hello Pankaj, On 25.02.22 13:20, Pankaj Gupta wrote: >>>> +static u32 *caam_blob_alloc_desc(size_t keymod_len) { >>>> + size_t len; >>>> + >>>> + /* header + (key mod immediate) + 2x pointers + op */ >>>> + len = 4 + (4 + ALIGN(keymod_len, 4)) + 2*(4 + 4 + >>>> + CAAM_PTR_SZ_MAX) + 4; >>> >>> Nit: the amount of magic numbers is overwhelming here. I neither >>> understand the statement nor how that comment should help me to >> understand it. >> >> header = 4 >> (key mod immediate) = (4 + ALIGN(keymod_len, 4)) >> 2x pointer = 2 * (4 + 4 + CAAM_PTR_SZ_MAX) >> op = 4 > > Instead of the function caam_blob_alloc_desc(), it will be great if the caller functions caam_encap_blob()/caam_encap_blob (), could add local array: > uint32_t desc[CAAM_DESC_BYTES_MAX]; I just looked into this and placing desc on stack is not possible as it is used for DMA inside caam_jr_enqueue(). Cheers, Ahmad
Series applies on top of current linux-tpmdd/master v4 was here: https://lore.kernel.org/linux-integrity/cover.8f40b6d1b93adc80aed2cac29a134f7a7fb5ee98.1633946449.git-series.a.fatoum@pengutronix.de v4 -> v5: - Collected Reviewed-by's and Tested-by's - Changed trusted.kernel_rng bool option into a string trusted.rng option (Jarkko) - Changed modifier to SECURE_KEY for compatibility with linux-imx (Matthias) - Typo fix in commit message (Jarkko) - Note in CAAM patch what CAAM is (Jarkko) v3 -> v4: - Collected Acked-by's, Reviewed-by's and Tested-by - Fixed typo spotted by David - Rebased on top of Andreas' regression fix and pulled Kconfig inflexibility fix back into series v2 -> v3: - Split off first Kconfig preparation patch. It fixes a regression, so sent that out, so it can be applied separately (Sumit) - Split off second key import patch. I'll send that out separately as it's a development aid and not required within the CAAM series - add MAINTAINERS entry v1 -> v2: - Added new commit to make trusted key Kconfig option independent of TPM and added new Kconfig file for trusted keys - Add new commit for importing existing key material - Allow users to force use of kernel RNG (Jarkko) - Enforce maximum keymod size (Horia) - Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr (Horia) - Make blobifier handle private to CAAM glue code file (Horia) - Extend trusted keys documentation for CAAM - Rebased and updated original cover letter: The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core built into many newer i.MX and QorIQ SoCs by NXP. Its blob mechanism can AES encrypt/decrypt user data using a unique never-disclosed device-specific key. There has been multiple discussions on how to represent this within the kernel: The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core built into many newer i.MX and QorIQ SoCs by NXP. Its blob mechanism can AES encrypt/decrypt user data using a unique never-disclosed device-specific key. There has been multiple discussions on how to represent this within the kernel: - [RFC] crypto: caam - add red blobifier Steffen implemented[1] a PoC sysfs driver to start a discussion on how to best integrate the blob mechanism. Mimi suggested that it could be used to implement trusted keys. Trusted keys back then were a TPM-only feature. - security/keys/secure_key: Adds the secure key support based on CAAM. Udit Agarwal added[2] a new "secure" key type with the CAAM as backend. The key material stays within the kernel only. Mimi and James agreed that this needs a generic interface, not specific to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as basis for TEE-backed keys. - [RFC] drivers: crypto: caam: key: Add caam_tk key type Franck added[3] a new "caam_tk" key type based on Udit's work. This time it uses CAAM "black blobs" instead of "red blobs", so key material stays within the CAAM and isn't exposed to kernel in plaintext. James voiced the opinion that there should be just one user-facing generic wrap/unwrap key type with multiple possible handlers. David suggested trusted keys. - Introduce TEE based Trusted Keys support Sumit reworked[4] trusted keys to support multiple possible backends with one chosen at boot time and added a new TEE backend along with TPM. This now sits in Jarkko's master branch to be sent out for v5.13 This patch series builds on top of Sumit's rework to have the CAAM as yet another trusted key backend. The CAAM bits are based on Steffen's initial patch from 2015. His work had been used in the field for some years now, so I preferred not to deviate too much from it. This series has been tested with dmcrypt[5] on an i.MX6DL. Looking forward to your feedback. Cheers, Ahmad [1]: https://lore.kernel.org/linux-crypto/1447082306-19946-2-git-send-email-s.trumtrar@pengutronix.de/ [2]: https://lore.kernel.org/linux-integrity/20180723111432.26830-1-udit.agarwal@nxp.com/ [3]: https://lore.kernel.org/lkml/1551456599-10603-2-git-send-email-franck.lenormand@nxp.com/ [4]: https://lore.kernel.org/lkml/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/ [5]: https://lore.kernel.org/linux-integrity/20210122084321.24012-2-a.fatoum@pengutronix.de/ --- To: Jarkko Sakkinen <jarkko@kernel.org> To: "Horia Geantă" <horia.geanta@nxp.com> To: Mimi Zohar <zohar@linux.ibm.com> To: Aymen Sghaier <aymen.sghaier@nxp.com> To: Herbert Xu <herbert@gondor.apana.org.au> To: "David S. Miller" <davem@davemloft.net> To: James Bottomley <jejb@linux.ibm.com> Cc: David Howells <dhowells@redhat.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de> Cc: Jan Luebbe <j.luebbe@pengutronix.de> Cc: David Gstir <david@sigma-star.at> Cc: Eric Biggers <ebiggers@kernel.org> Cc: Richard Weinberger <richard@nod.at> Cc: Franck LENORMAND <franck.lenormand@nxp.com> Cc: Sumit Garg <sumit.garg@linaro.org> Cc: Andreas Rammhold <andreas@rammhold.de> Cc: Tim Harvey <tharvey@gateworks.com> Cc: Matthias Schiffer <matthias.schiffer@ew.tq-group.com> Cc: Pankaj Gupta <pankaj.gupta@nxp.com> Cc: linux-integrity@vger.kernel.org Cc: keyrings@vger.kernel.org Cc: linux-crypto@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-security-module@vger.kernel.org Ahmad Fatoum (5): KEYS: trusted: allow use of TEE as backend without TCG_TPM support KEYS: trusted: allow users to use kernel RNG for key material KEYS: trusted: allow trust sources to use kernel RNG for key material crypto: caam - add in-kernel interface for blob generator KEYS: trusted: Introduce support for NXP CAAM-based trusted keys .../admin-guide/kernel-parameters.txt | 11 + .../security/keys/trusted-encrypted.rst | 60 ++++- MAINTAINERS | 9 + crypto/asymmetric_keys/Kconfig | 2 +- drivers/crypto/caam/Kconfig | 3 + drivers/crypto/caam/Makefile | 1 + drivers/crypto/caam/blob_gen.c | 230 ++++++++++++++++++ include/keys/trusted-type.h | 2 +- include/keys/trusted_caam.h | 11 + include/soc/fsl/caam-blob.h | 56 +++++ security/keys/Kconfig | 18 +- security/keys/trusted-keys/Kconfig | 38 +++ security/keys/trusted-keys/Makefile | 10 +- security/keys/trusted-keys/trusted_caam.c | 74 ++++++ security/keys/trusted-keys/trusted_core.c | 45 +++- 15 files changed, 540 insertions(+), 30 deletions(-) create mode 100644 drivers/crypto/caam/blob_gen.c create mode 100644 include/keys/trusted_caam.h create mode 100644 include/soc/fsl/caam-blob.h create mode 100644 security/keys/trusted-keys/Kconfig create mode 100644 security/keys/trusted-keys/trusted_caam.c