linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
@ 2021-03-16 17:01 Ahmad Fatoum
  2021-03-16 17:01 ` [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator Ahmad Fatoum
                   ` (4 more replies)
  0 siblings, 5 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-16 17:01 UTC (permalink / raw)
  To: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley
  Cc: kernel, David Howells, James Morris, Serge E. Hallyn,
	Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck LENORMAND, Sumit Garg, linux-integrity, keyrings,
	linux-crypto, linux-kernel, linux-security-module

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 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. The key
   material stays within the kernel only, but can optionally be user-set
   instead of coming from RNG. 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: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@penutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
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 (3):
  crypto: caam - add in-kernel interface for blob generator
  KEYS: trusted: implement fallback to kernel RNG
  KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

 Documentation/admin-guide/kernel-parameters.txt |   1 +-
 drivers/crypto/caam/Kconfig                     |   4 +-
 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                     |  54 ++++-
 security/keys/trusted-keys/Makefile             |   1 +-
 security/keys/trusted-keys/trusted_caam.c       |  74 +++++-
 security/keys/trusted-keys/trusted_core.c       |  17 +-
 10 files changed, 392 insertions(+), 3 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/trusted_caam.c

base-commit: 8a3fa8ade8a35d8f7c178f5680f07f223da41b87
-- 
git-series 0.9.1

^ permalink raw reply	[flat|nested] 71+ messages in thread

* [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator
  2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
@ 2021-03-16 17:01 ` Ahmad Fatoum
  2021-03-21 20:46   ` Horia Geantă
  2021-03-16 17:01 ` [PATCH v1 2/3] KEYS: trusted: implement fallback to kernel RNG Ahmad Fatoum
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-16 17:01 UTC (permalink / raw)
  To: Horia Geantă, Aymen Sghaier, Herbert Xu, David S. Miller
  Cc: kernel, Ahmad Fatoum, James Bottomley, Jarkko Sakkinen,
	Mimi Zohar, David Howells, James Morris, Serge E. Hallyn,
	Udit Agarwal, Jan Luebbe, David Gstir, Franck LENORMAND,
	Sumit Garg, linux-integrity, keyrings, linux-crypto,
	linux-kernel, linux-security-module

The 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.

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: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@penutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
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    |   4 +-
 drivers/crypto/caam/Makefile   |   1 +-
 drivers/crypto/caam/blob_gen.c | 230 ++++++++++++++++++++++++++++++++++-
 include/soc/fsl/caam-blob.h    |  54 ++++++++-
 4 files changed, 289 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..8f7df1299f3d 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -151,6 +151,10 @@ 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
+	default y if TRUSTED_KEYS
+
 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..21e83ad0f226 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM) += caam.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_JR) += caam_jr.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_CRYPTO_API_DESC) += caamalg_desc.o
 obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC) += caamhash_desc.o
+obj-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += blob_gen.o
 
 caam-y := ctrl.o
 caam_jr-y := jr.o key_gen.o
diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
new file mode 100644
index 000000000000..6a17941c9504
--- /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;
+
+	if (len > CAAM_DESC_BYTES_MAX)
+		return NULL;
+
+	return kzalloc(len, GFP_KERNEL | GFP_DMA);
+}
+
+int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
+		    void *input, void *output, size_t length)
+{
+	u32 *desc;
+	struct device *jrdev = &priv->jrdev;
+	dma_addr_t dma_in, dma_out;
+	struct caam_blob_job_result testres;
+	size_t keymod_len = strlen(keymod);
+	int ret;
+
+	if (length <= CAAM_BLOB_OVERHEAD)
+		return -EINVAL;
+
+	desc = caam_blob_alloc_desc(keymod_len);
+	if (!desc) {
+		dev_err(jrdev, "unable to allocate desc\n");
+		return -ENOMEM;
+	}
+
+	dma_in = dma_map_single(jrdev, input, length - CAAM_BLOB_OVERHEAD, DMA_TO_DEVICE);
+	if (dma_mapping_error(jrdev, dma_in)) {
+		dev_err(jrdev, "unable to map input DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	dma_out = dma_map_single(jrdev, output, length,	DMA_FROM_DEVICE);
+	if (dma_mapping_error(jrdev, dma_out)) {
+		dev_err(jrdev, "unable to map output DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_unmap_in;
+	}
+
+	/*
+	 * 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
+	 * initial counter (Ctr0) are generated automatically and stored in
+	 * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
+	 * Class 1 Key Register. Operation Mode is set to AES-CCM.
+	 */
+
+	init_job_desc(desc, 0);
+	append_key_as_imm(desc, keymod, keymod_len, keymod_len,
+			  CLASS_2 | KEY_DEST_CLASS_REG);
+	append_seq_in_ptr(desc, dma_in, length - CAAM_BLOB_OVERHEAD, 0);
+	append_seq_out_ptr(desc, dma_out, length, 0);
+	append_operation(desc, OP_TYPE_ENCAP_PROTOCOL | OP_PCLID_BLOB);
+
+	print_hex_dump_debug("data@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, input,
+			     length - CAAM_BLOB_OVERHEAD, false);
+	print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, desc,
+			     desc_bytes(desc), false);
+
+	testres.err = 0;
+	init_completion(&testres.completion);
+
+	ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
+	if (ret == -EINPROGRESS) {
+		wait_for_completion(&testres.completion);
+		ret = testres.err;
+		print_hex_dump_debug("output@"__stringify(__LINE__)": ",
+				     DUMP_PREFIX_ADDRESS, 16, 1, output,
+				     length, false);
+	}
+
+	dma_unmap_single(jrdev, dma_out, length, DMA_FROM_DEVICE);
+out_unmap_in:
+	dma_unmap_single(jrdev, dma_in, length - CAAM_BLOB_OVERHEAD, DMA_TO_DEVICE);
+out_free:
+	kfree(desc);
+
+	return ret;
+}
+EXPORT_SYMBOL(caam_encap_blob);
+
+int caam_decap_blob(struct caam_blob_priv *priv, const char *keymod,
+		    void *input, void *output, size_t length)
+{
+	u32 *desc;
+	struct device *jrdev = &priv->jrdev;
+	dma_addr_t dma_in, dma_out;
+	struct caam_blob_job_result testres;
+	size_t keymod_len = strlen(keymod);
+	int ret;
+
+	if (length <= CAAM_BLOB_OVERHEAD)
+		return -EINVAL;
+
+	desc = caam_blob_alloc_desc(keymod_len);
+	if (!desc) {
+		dev_err(jrdev, "unable to allocate desc\n");
+		return -ENOMEM;
+	}
+
+	dma_in = dma_map_single(jrdev, input, length, DMA_TO_DEVICE);
+	if (dma_mapping_error(jrdev, dma_in)) {
+		dev_err(jrdev, "unable to map input DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_free;
+	}
+
+	dma_out = dma_map_single(jrdev, output, length - CAAM_BLOB_OVERHEAD, DMA_FROM_DEVICE);
+	if (dma_mapping_error(jrdev, dma_out)) {
+		dev_err(jrdev, "unable to map output DMA buffer\n");
+		ret = -ENOMEM;
+		goto out_unmap_in;
+	}
+
+	/*
+	 * 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
+	 * initial counter (Ctr0) are generated automatically and stored in
+	 * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
+	 * Class 1 Key Register. Operation Mode is set to AES-CCM.
+	 */
+
+	init_job_desc(desc, 0);
+	append_key_as_imm(desc, keymod, keymod_len, keymod_len,
+			  CLASS_2 | KEY_DEST_CLASS_REG);
+	append_seq_in_ptr(desc, dma_in, length, 0);
+	append_seq_out_ptr(desc, dma_out, length - CAAM_BLOB_OVERHEAD, 0);
+	append_operation(desc, OP_TYPE_DECAP_PROTOCOL | OP_PCLID_BLOB);
+
+	print_hex_dump_debug("data@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, input,
+			     length, false);
+	print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
+			     DUMP_PREFIX_ADDRESS, 16, 1, desc,
+			     desc_bytes(desc), false);
+
+	testres.err = 0;
+	init_completion(&testres.completion);
+
+	ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
+	if (ret == -EINPROGRESS) {
+		wait_for_completion(&testres.completion);
+		ret = testres.err;
+		print_hex_dump_debug("output@"__stringify(__LINE__)": ",
+				     DUMP_PREFIX_ADDRESS, 16, 1, output,
+				     length - CAAM_BLOB_OVERHEAD, false);
+	}
+
+	dma_unmap_single(jrdev, dma_out, length - CAAM_BLOB_OVERHEAD, DMA_FROM_DEVICE);
+out_unmap_in:
+	dma_unmap_single(jrdev, dma_in, length, DMA_TO_DEVICE);
+out_free:
+	kfree(desc);
+
+	return ret;
+}
+EXPORT_SYMBOL(caam_decap_blob);
+
+struct caam_blob_priv *caam_blob_gen_init(void)
+{
+	struct device *jrdev;
+
+	jrdev = caam_jr_alloc();
+	if (IS_ERR(jrdev))
+		return ERR_CAST(jrdev);
+
+	return container_of(jrdev, struct caam_blob_priv, jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_init);
+
+void caam_blob_gen_exit(struct caam_blob_priv *priv)
+{
+	caam_jr_free(&priv->jrdev);
+}
+EXPORT_SYMBOL(caam_blob_gen_exit);
diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
new file mode 100644
index 000000000000..7eea0f543832
--- /dev/null
+++ b/include/soc/fsl/caam-blob.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
+ */
+
+#ifndef __CAAM_BLOB_GEN
+#define __CAAM_BLOB_GEN
+
+#include <linux/types.h>
+
+#define CAAM_BLOB_KEYMOD_LENGTH		16
+#define CAAM_BLOB_OVERHEAD		(32 + 16)
+#define CAAM_BLOB_MAX_LEN		4096
+
+struct caam_blob_priv;
+
+/** caam_blob_gen_init - initialize blob generation
+ *
+ * returns either pointer to new caam_blob_priv instance
+ * or error pointer
+ */
+struct caam_blob_priv *caam_blob_gen_init(void);
+
+/** caam_blob_gen_init - free blob generation resources
+ *
+ * @priv: instance returned by caam_blob_gen_init
+ */
+void caam_blob_gen_exit(struct caam_blob_priv *priv);
+
+/** caam_encap_blob - encapsulate blob
+ *
+ * @priv:   instance returned by caam_blob_gen_init
+ * @keymod: string to use as key modifier for blob encapsulation
+ * @input:  buffer which CAAM will DMA from
+ * @output: buffer which CAAM will DMA to
+ * @length: buffer length including blob overhead
+ *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
+ */
+int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
+		    void *input, void *output, size_t length);
+
+/** caam_decap_blob - decapsulate blob
+ *
+ * @priv:   instance returned by caam_blob_gen_init
+ * @keymod: string to use as key modifier for blob decapsulation
+ * @input:  buffer which CAAM will DMA from
+ * @output: buffer which CAAM will DMA to
+ * @length: buffer length including blob overhead
+ *          CAAM_BLOB_OVERHEAD < length <= CAAM_BLOB_MAX_LEN
+ */
+int caam_decap_blob(struct caam_blob_priv *priv, const char *keymod,
+		    void *input, void *output, size_t length);
+
+#endif
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v1 2/3] KEYS: trusted: implement fallback to kernel RNG
  2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
  2021-03-16 17:01 ` [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator Ahmad Fatoum
@ 2021-03-16 17:01 ` Ahmad Fatoum
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-16 17:01 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells
  Cc: kernel, Ahmad Fatoum, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	linux-crypto, linux-integrity, linux-kernel,
	linux-security-module

For cases a trusted key source already sources the kernel RNG, we can
use get_random_bytes_wait to get the random data for key material.

Make the get_random callback optional to allow sources to make use of
this.

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: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@penutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
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
---
 include/keys/trusted-type.h               |  2 +-
 security/keys/trusted-keys/trusted_core.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index d89fa2579ac0..4eb64548a74f 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -64,7 +64,7 @@ struct trusted_key_ops {
 	/* Unseal a key. */
 	int (*unseal)(struct trusted_key_payload *p, char *datablob);
 
-	/* Get a randomized key. */
+	/* Optional: Get a randomized key. */
 	int (*get_random)(unsigned char *key, size_t key_len);
 
 	/* Exit key interface. */
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index ec3a066a4b42..5f92323efedf 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -16,6 +16,7 @@
 #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>
@@ -310,8 +311,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++) {
@@ -320,6 +327,8 @@ static int __init init_trusted(void)
 			    strlen(trusted_key_sources[i].name)))
 			continue;
 
+		get_random = trusted_key_sources[i].ops->get_random ?: kernel_get_random;
+
 		static_call_update(trusted_key_init,
 				   trusted_key_sources[i].ops->init);
 		static_call_update(trusted_key_seal,
@@ -327,7 +336,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;
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
  2021-03-16 17:01 ` [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator Ahmad Fatoum
  2021-03-16 17:01 ` [PATCH v1 2/3] KEYS: trusted: implement fallback to kernel RNG Ahmad Fatoum
@ 2021-03-16 17:01 ` Ahmad Fatoum
  2021-03-16 19:22   ` Jarkko Sakkinen
                     ` (3 more replies)
  2021-03-16 23:10 ` [PATCH v1 0/3] " Richard Weinberger
  2021-03-21 20:01 ` Horia Geantă
  4 siblings, 4 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-16 17:01 UTC (permalink / raw)
  To: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar
  Cc: kernel, Ahmad Fatoum, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
built into many newer i.MX and QorIQ SoCs by NXP.

The CAAM does crypto acceleration, hardware number generation and
has a blob mechanism for encapsulation/decapsulation of sensitive material.

This blob mechanism depends on a device specific random 256-bit One Time
Programmable Master Key that is fused in each SoC at manufacturing
time. This key is unreadable and can only be used by the CAAM for AES
encryption/decryption of user data.

This makes it a suitable backend (source) for kernel trusted keys.

Previous commits generalized trusted keys to support multiple backends
and added an API to access the CAAM blob mechanism. Based on these,
provide the necessary glue to use the CAAM for trusted keys.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
To: Jonathan Corbet <corbet@lwn.net>
To: David Howells <dhowells@redhat.com>
To: Jarkko Sakkinen <jarkko@kernel.org>
To: James Bottomley <jejb@linux.ibm.com>
To: Mimi Zohar <zohar@linux.ibm.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: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@penutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
---
 Documentation/admin-guide/kernel-parameters.txt |  1 +-
 include/keys/trusted_caam.h                     | 11 +++-
 security/keys/trusted-keys/Makefile             |  1 +-
 security/keys/trusted-keys/trusted_caam.c       | 74 ++++++++++++++++++-
 security/keys/trusted-keys/trusted_core.c       |  6 +-
 5 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 include/keys/trusted_caam.h
 create mode 100644 security/keys/trusted-keys/trusted_caam.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index c8bad1762cba..382e911389aa 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5469,6 +5469,7 @@
 			sources:
 			- "tpm"
 			- "tee"
+			- "caam"
 			If not specified then it defaults to iterating through
 			the trust source list starting with TPM and assigns the
 			first trust source as a backend which is initialized
diff --git a/include/keys/trusted_caam.h b/include/keys/trusted_caam.h
new file mode 100644
index 000000000000..2fba0996b0b0
--- /dev/null
+++ b/include/keys/trusted_caam.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
+ */
+
+#ifndef __CAAM_TRUSTED_KEY_H
+#define __CAAM_TRUSTED_KEY_H
+
+extern struct trusted_key_ops caam_trusted_key_ops;
+
+#endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index feb8b6c3cc79..050370690abd 100644
--- a/security/keys/trusted-keys/Makefile
+++ b/security/keys/trusted-keys/Makefile
@@ -12,3 +12,4 @@ trusted-y += trusted_tpm2.o
 trusted-y += tpm2key.asn1.o
 
 trusted-$(CONFIG_TEE) += trusted_tee.o
+trusted-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += trusted_caam.o
diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
new file mode 100644
index 000000000000..fc2e3dde9e06
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
+ */
+
+#include <keys/trusted_caam.h>
+#include <keys/trusted-type.h>
+#include <linux/build_bug.h>
+#include <linux/key-type.h>
+#include <soc/fsl/caam-blob.h>
+
+struct caam_blob_priv *blobifier;
+
+#define KEYMOD "kernel:trusted"
+
+static_assert(MAX_KEY_SIZE + CAAM_BLOB_OVERHEAD <= CAAM_BLOB_MAX_LEN);
+static_assert(MAX_BLOB_SIZE <= CAAM_BLOB_MAX_LEN);
+
+static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
+{
+	int length = p->key_len + CAAM_BLOB_OVERHEAD;
+	int ret;
+
+	ret = caam_encap_blob(blobifier, KEYMOD, p->key, p->blob, length);
+	if (ret)
+		return ret;
+
+	p->blob_len = length;
+	return 0;
+}
+
+static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
+{
+	int length = p->blob_len;
+	int ret;
+
+	ret = caam_decap_blob(blobifier, KEYMOD, p->blob, p->key, length);
+	if (ret)
+		return ret;
+
+	p->key_len = length - CAAM_BLOB_OVERHEAD;
+	return 0;
+}
+
+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");
+		return PTR_ERR(blobifier);
+	}
+
+	ret = register_key_type(&key_type_trusted);
+	if (ret)
+		caam_blob_gen_exit(blobifier);
+
+	return ret;
+}
+
+static void trusted_caam_exit(void)
+{
+	unregister_key_type(&key_type_trusted);
+	caam_blob_gen_exit(blobifier);
+}
+
+struct trusted_key_ops caam_trusted_key_ops = {
+	.migratable = 0, /* non-migratable */
+	.init = trusted_caam_init,
+	.seal = trusted_caam_seal,
+	.unseal = trusted_caam_unseal,
+	.exit = trusted_caam_exit,
+};
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index 5f92323efedf..e9bfb1bbc014 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -9,6 +9,7 @@
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
 #include <keys/trusted_tee.h>
+#include <keys/trusted_caam.h>
 #include <keys/trusted_tpm.h>
 #include <linux/capability.h>
 #include <linux/err.h>
@@ -25,7 +26,7 @@
 
 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)");
+MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
 
 static const struct trusted_key_source trusted_key_sources[] = {
 #if defined(CONFIG_TCG_TPM)
@@ -34,6 +35,9 @@ static const struct trusted_key_source trusted_key_sources[] = {
 #if defined(CONFIG_TEE)
 	{ "tee", &trusted_key_tee_ops },
 #endif
+#if defined(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN)
+	{ "caam", &caam_trusted_key_ops },
+#endif
 };
 
 DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
-- 
git-series 0.9.1

^ permalink raw reply related	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
@ 2021-03-16 19:22   ` Jarkko Sakkinen
  2021-03-17 13:58     ` Ahmad Fatoum
  2021-03-16 23:14   ` Richard Weinberger
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-03-16 19:22 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jonathan Corbet, David Howells, James Bottomley, Mimi Zohar,
	kernel, James Morris, Serge E. Hallyn, Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Tue, Mar 16, 2021 at 06:01:18PM +0100, Ahmad Fatoum wrote:
> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
> built into many newer i.MX and QorIQ SoCs by NXP.
> 
> The CAAM does crypto acceleration, hardware number generation and
> has a blob mechanism for encapsulation/decapsulation of sensitive material.
> 
> This blob mechanism depends on a device specific random 256-bit One Time
> Programmable Master Key that is fused in each SoC at manufacturing
> time. This key is unreadable and can only be used by the CAAM for AES
> encryption/decryption of user data.
> 
> This makes it a suitable backend (source) for kernel trusted keys.
> 
> Previous commits generalized trusted keys to support multiple backends
> and added an API to access the CAAM blob mechanism. Based on these,
> provide the necessary glue to use the CAAM for trusted keys.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> To: Jonathan Corbet <corbet@lwn.net>
> To: David Howells <dhowells@redhat.com>
> To: Jarkko Sakkinen <jarkko@kernel.org>
> To: James Bottomley <jejb@linux.ibm.com>
> To: Mimi Zohar <zohar@linux.ibm.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: Udit Agarwal <udit.agarwal@nxp.com>
> Cc: Jan Luebbe <j.luebbe@penutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: Franck LENORMAND <franck.lenormand@nxp.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: keyrings@vger.kernel.org
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-integrity@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  1 +-
>  include/keys/trusted_caam.h                     | 11 +++-
>  security/keys/trusted-keys/Makefile             |  1 +-
>  security/keys/trusted-keys/trusted_caam.c       | 74 ++++++++++++++++++-
>  security/keys/trusted-keys/trusted_core.c       |  6 +-
>  5 files changed, 92 insertions(+), 1 deletion(-)
>  create mode 100644 include/keys/trusted_caam.h
>  create mode 100644 security/keys/trusted-keys/trusted_caam.c
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index c8bad1762cba..382e911389aa 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5469,6 +5469,7 @@
>  			sources:
>  			- "tpm"
>  			- "tee"
> +			- "caam"
>  			If not specified then it defaults to iterating through
>  			the trust source list starting with TPM and assigns the
>  			first trust source as a backend which is initialized
> diff --git a/include/keys/trusted_caam.h b/include/keys/trusted_caam.h
> new file mode 100644
> index 000000000000..2fba0996b0b0
> --- /dev/null
> +++ b/include/keys/trusted_caam.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
> + */
> +
> +#ifndef __CAAM_TRUSTED_KEY_H
> +#define __CAAM_TRUSTED_KEY_H
> +
> +extern struct trusted_key_ops caam_trusted_key_ops;
> +
> +#endif
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index feb8b6c3cc79..050370690abd 100644
> --- a/security/keys/trusted-keys/Makefile
> +++ b/security/keys/trusted-keys/Makefile
> @@ -12,3 +12,4 @@ trusted-y += trusted_tpm2.o
>  trusted-y += tpm2key.asn1.o
>  
>  trusted-$(CONFIG_TEE) += trusted_tee.o
> +trusted-$(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN) += trusted_caam.o
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> new file mode 100644
> index 000000000000..fc2e3dde9e06
> --- /dev/null
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
> + */
> +
> +#include <keys/trusted_caam.h>
> +#include <keys/trusted-type.h>
> +#include <linux/build_bug.h>
> +#include <linux/key-type.h>
> +#include <soc/fsl/caam-blob.h>
> +
> +struct caam_blob_priv *blobifier;
> +
> +#define KEYMOD "kernel:trusted"
> +
> +static_assert(MAX_KEY_SIZE + CAAM_BLOB_OVERHEAD <= CAAM_BLOB_MAX_LEN);
> +static_assert(MAX_BLOB_SIZE <= CAAM_BLOB_MAX_LEN);
> +
> +static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
> +{
> +	int length = p->key_len + CAAM_BLOB_OVERHEAD;
> +	int ret;
> +
> +	ret = caam_encap_blob(blobifier, KEYMOD, p->key, p->blob, length);
> +	if (ret)
> +		return ret;
> +
> +	p->blob_len = length;
> +	return 0;
> +}
> +
> +static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
> +{
> +	int length = p->blob_len;
> +	int ret;
> +
> +	ret = caam_decap_blob(blobifier, KEYMOD, p->blob, p->key, length);
> +	if (ret)
> +		return ret;
> +
> +	p->key_len = length - CAAM_BLOB_OVERHEAD;
> +	return 0;
> +}
> +
> +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");
> +		return PTR_ERR(blobifier);
> +	}
> +
> +	ret = register_key_type(&key_type_trusted);
> +	if (ret)
> +		caam_blob_gen_exit(blobifier);
> +
> +	return ret;
> +}
> +
> +static void trusted_caam_exit(void)
> +{
> +	unregister_key_type(&key_type_trusted);
> +	caam_blob_gen_exit(blobifier);
> +}
> +
> +struct trusted_key_ops caam_trusted_key_ops = {
> +	.migratable = 0, /* non-migratable */
> +	.init = trusted_caam_init,
> +	.seal = trusted_caam_seal,
> +	.unseal = trusted_caam_unseal,
> +	.exit = trusted_caam_exit,
> +};
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index 5f92323efedf..e9bfb1bbc014 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -9,6 +9,7 @@
>  #include <keys/user-type.h>
>  #include <keys/trusted-type.h>
>  #include <keys/trusted_tee.h>
> +#include <keys/trusted_caam.h>
>  #include <keys/trusted_tpm.h>
>  #include <linux/capability.h>
>  #include <linux/err.h>
> @@ -25,7 +26,7 @@
>  
>  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)");
> +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
>  
>  static const struct trusted_key_source trusted_key_sources[] = {
>  #if defined(CONFIG_TCG_TPM)
> @@ -34,6 +35,9 @@ static const struct trusted_key_source trusted_key_sources[] = {
>  #if defined(CONFIG_TEE)
>  	{ "tee", &trusted_key_tee_ops },
>  #endif
> +#if defined(CONFIG_CRYPTO_DEV_FSL_CAAM_BLOB_GEN)
> +	{ "caam", &caam_trusted_key_ops },
> +#endif
>  };
>  
>  DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
> -- 
> git-series 0.9.1
> 

Too early to ack, as I've not included the TEE thing to any PR yet.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
@ 2021-03-16 23:10 ` Richard Weinberger
  2021-03-17 14:08   ` Ahmad Fatoum
  2021-03-21 20:01 ` Horia Geantă
  4 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-03-16 23:10 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, Sumit Garg, linux-integrity,
	keyrings, Linux Crypto Mailing List, LKML, LSM

Ahmad,

On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> 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 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. The key
>    material stays within the kernel only, but can optionally be user-set
>    instead of coming from RNG. 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.

Do have this series also in a git repo to pull from?
I'd like to give it a test on various systems.

> Looking forward to your feedback.

Thanks for working on this! David and I will have a closer look these days.

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
  2021-03-16 19:22   ` Jarkko Sakkinen
@ 2021-03-16 23:14   ` Richard Weinberger
  2021-03-17  7:39     ` Sumit Garg
  2021-03-17 14:02     ` Ahmad Fatoum
  2021-03-21 20:48   ` Horia Geantă
  2021-03-31 18:35   ` Richard Weinberger
  3 siblings, 2 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-03-16 23:14 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	Linux Crypto Mailing List, linux-doc, linux-integrity, LKML, LSM

Ahmad,

On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> +#include <keys/trusted_caam.h>
> +#include <keys/trusted-type.h>
> +#include <linux/build_bug.h>
> +#include <linux/key-type.h>
> +#include <soc/fsl/caam-blob.h>
> +
> +struct caam_blob_priv *blobifier;

Who is using this pointer too?
Otherwise I'd suggest marking it static.

>  module_param_named(source, trusted_key_source, charp, 0);
> -MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
> +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");

I didn't closely follow the previous discussions, but is a module
parameter really the right approach?
Is there also a way to set it via something like device tree?

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 23:14   ` Richard Weinberger
@ 2021-03-17  7:39     ` Sumit Garg
  2021-03-17  8:07       ` Richard Weinberger
  2021-03-17 14:02     ` Ahmad Fatoum
  1 sibling, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-03-17  7:39 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Ahmad Fatoum, Jonathan Corbet, David Howells, Jarkko Sakkinen,
	James Bottomley, Mimi Zohar, kernel, James Morris,
	Serge E. Hallyn, Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND,
	open list:ASYMMETRIC KEYS, Linux Crypto Mailing List,
	Linux Doc Mailing List, linux-integrity, LKML, LSM

Hi Richard,

On Wed, 17 Mar 2021 at 04:45, Richard Weinberger
<richard.weinberger@gmail.com> wrote:
>
> Ahmad,
>
> On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > +#include <keys/trusted_caam.h>
> > +#include <keys/trusted-type.h>
> > +#include <linux/build_bug.h>
> > +#include <linux/key-type.h>
> > +#include <soc/fsl/caam-blob.h>
> > +
> > +struct caam_blob_priv *blobifier;
>
> Who is using this pointer too?
> Otherwise I'd suggest marking it static.
>
> >  module_param_named(source, trusted_key_source, charp, 0);
> > -MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
> > +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
>
> I didn't closely follow the previous discussions, but is a module
> parameter really the right approach?
> Is there also a way to set it via something like device tree?
>

It's there to support a platform which possesses multiple trusted keys
backends. So that a user is able to select during boot which one to
use as a backend.

-Sumit

> --
> Thanks,
> //richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-17  7:39     ` Sumit Garg
@ 2021-03-17  8:07       ` Richard Weinberger
  0 siblings, 0 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-03-17  8:07 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Ahmad Fatoum, Jonathan Corbet, David Howells, Jarkko Sakkinen,
	James Bottomley, Mimi Zohar, kernel, James Morris,
	Serge E. Hallyn, horia geanta, aymen sghaier, Herbert Xu, davem,
	Udit Agarwal, david, Franck Lenormand, open list:ASYMMETRIC KEYS,
	Linux Crypto Mailing List, Linux Doc Mailing List,
	linux-integrity, linux-kernel, LSM, jlu

Sumit,

----- Ursprüngliche Mail -----
>> >  module_param_named(source, trusted_key_source, charp, 0);
>> > -MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>> > +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
>>
>> I didn't closely follow the previous discussions, but is a module
>> parameter really the right approach?
>> Is there also a way to set it via something like device tree?
>>
> 
> It's there to support a platform which possesses multiple trusted keys
> backends. So that a user is able to select during boot which one to
> use as a backend.

I understand the use case, my question was whether it makes actually sense to
have a module parameter for it, or additionally another way to define the
preferred backend.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 19:22   ` Jarkko Sakkinen
@ 2021-03-17 13:58     ` Ahmad Fatoum
  0 siblings, 0 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-17 13:58 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Jonathan Corbet, David Howells, James Bottomley, Mimi Zohar,
	kernel, James Morris, Serge E. Hallyn, Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

Hello Jarkko,

On 16.03.21 20:22, Jarkko Sakkinen wrote:
> On Tue, Mar 16, 2021 at 06:01:18PM +0100, Ahmad Fatoum wrote:
>> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
>> built into many newer i.MX and QorIQ SoCs by NXP.
>>
>> The CAAM does crypto acceleration, hardware number generation and
>> has a blob mechanism for encapsulation/decapsulation of sensitive material.
>>
>> This blob mechanism depends on a device specific random 256-bit One Time
>> Programmable Master Key that is fused in each SoC at manufacturing
>> time. This key is unreadable and can only be used by the CAAM for AES
>> encryption/decryption of user data.
>>
>> This makes it a suitable backend (source) for kernel trusted keys.
>>
>> Previous commits generalized trusted keys to support multiple backends
>> and added an API to access the CAAM blob mechanism. Based on these,
>> provide the necessary glue to use the CAAM for trusted keys.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> Too early to ack, as I've not included the TEE thing to any PR yet.

No problem. I'd be happy to incorporate the feedback I receive in the meantime.

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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 23:14   ` Richard Weinberger
  2021-03-17  7:39     ` Sumit Garg
@ 2021-03-17 14:02     ` Ahmad Fatoum
  2021-03-30 21:28       ` Richard Weinberger
  1 sibling, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-17 14:02 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	Linux Crypto Mailing List, linux-doc, linux-integrity, LKML, LSM

Hello Richard,

On 17.03.21 00:14, Richard Weinberger wrote:
> Ahmad,
> 
> On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> +#include <keys/trusted_caam.h>
>> +#include <keys/trusted-type.h>
>> +#include <linux/build_bug.h>
>> +#include <linux/key-type.h>
>> +#include <soc/fsl/caam-blob.h>
>> +
>> +struct caam_blob_priv *blobifier;
> 
> Who is using this pointer too?
> Otherwise I'd suggest marking it static.

You're right. Will do in v2.

>>  module_param_named(source, trusted_key_source, charp, 0);
>> -MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>> +MODULE_PARM_DESC(source, "Select trusted keys source (tpm, tee or caam)");
> 
> I didn't closely follow the previous discussions, but is a module
> parameter really the right approach?
> Is there also a way to set it via something like device tree?

Compiled-on sources are considered in the order: tpm, tee then caam.
Module parameters are the only override currently available.

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 23:10 ` [PATCH v1 0/3] " Richard Weinberger
@ 2021-03-17 14:08   ` Ahmad Fatoum
  2021-03-30 21:50     ` Richard Weinberger
                       ` (2 more replies)
  0 siblings, 3 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-17 14:08 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, Sumit Garg, linux-integrity,
	keyrings, Linux Crypto Mailing List, LKML, LSM

Hello Richard,

On 17.03.21 00:10, Richard Weinberger wrote:
> On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> This series has been tested with dmcrypt[5] on an i.MX6DL.
> 
> Do have this series also in a git repo to pull from?
> I'd like to give it a test on various systems.

Yes, please pull git://git.pengutronix.de/afa/linux
Branch v5.12/topic/trusted-source-caam

It includes these three patches on top of Jarkko's linux-tpmdd/master.

>> Looking forward to your feedback.
> 
> Thanks for working on this! David and I will have a closer look these days.

Great. Here is a simple testing regiment that could help you getting started:

# First boot
    DEV=/dev/loop0
    ALGO=aes-cbc-essiv:sha256
    KEYNAME=kmk
    BLOCKS=20

    mount -o remount,rw /
    fallocate -l $((BLOCKS*512)) ~/loop0.img
    losetup -P $DEV ~/loop0.img
    KEY="$(keyctl add trusted $KEYNAME 'new 32' @s)"
    keyctl pipe $KEY >~/kmk.blob

    TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
    echo $TABLE | dmsetup create mydev
    echo $TABLE | dmsetup load mydev
    dd if=/dev/zero of=/dev/mapper/mydev || true
    echo "It works!" 1<> /dev/mapper/mydev
    cryptsetup close mydev

# Second boot
    DEV=/dev/loop0
    ALGO=aes-cbc-essiv:sha256
    KEYNAME=kmk
    BLOCKS=20

    losetup -P $DEV ~/loop0.img
    keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
    TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
    echo $TABLE | dmsetup create mydev
    echo $TABLE | dmsetup load mydev

# Should print that It works!
    hexdump -C /dev/mapper/mydev

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2021-03-16 23:10 ` [PATCH v1 0/3] " Richard Weinberger
@ 2021-03-21 20:01 ` Horia Geantă
  2021-03-23 16:34   ` Ahmad Fatoum
  2021-03-23 16:37   ` Ahmad Fatoum
  4 siblings, 2 replies; 71+ messages in thread
From: Horia Geantă @ 2021-03-21 20:01 UTC (permalink / raw)
  To: Ahmad Fatoum, Jarkko Sakkinen, Mimi Zohar, Aymen Sghaier,
	Herbert Xu, David S. Miller, James Bottomley
  Cc: kernel, David Howells, James Morris, Serge E. Hallyn,
	Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, linux-integrity, keyrings,
	linux-crypto, linux-kernel, linux-security-module

On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> 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 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. The key
>    material stays within the kernel only, but can optionally be user-set
>    instead of coming from RNG. 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.
> 
The whole point was to use caam "black blobs", with the main advantage of
keys being kept encrypted in memory after "unsealing" the blobs.
(Keys in blobs are encrypted with a persistent BKEK - blob KEK, derived from
fuse-based OTPMK. OTOH black keys are keys encrypted with an ephemeral, random
KEK that is stored in an internal caam register. When a black blob is unsealed,
the key is practically rekeyed, the random key replacing the BKEK; key is never
exposed in plaintext, rekeying happens in caam).

Current implementation uses "red blobs", which means keys are left unprotected
in memory after blobs are unsealed.

>  - 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.
> 
Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
be updated to reflect the availability of multiple backends?

Thanks,
Horia

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator
  2021-03-16 17:01 ` [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator Ahmad Fatoum
@ 2021-03-21 20:46   ` Horia Geantă
  2021-03-23 16:41     ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Horia Geantă @ 2021-03-21 20:46 UTC (permalink / raw)
  To: Ahmad Fatoum, Aymen Sghaier, Herbert Xu, David S. Miller
  Cc: kernel, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
	David Howells, James Morris, Serge E. Hallyn, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck Lenormand, Sumit Garg,
	linux-integrity, keyrings, linux-crypto, linux-kernel,
	linux-security-module

On 3/16/2021 7:01 PM, Ahmad Fatoum wrote:
> +int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
> +		    void *input, void *output, size_t length)
> +{
> +	u32 *desc;
> +	struct device *jrdev = &priv->jrdev;
> +	dma_addr_t dma_in, dma_out;
> +	struct caam_blob_job_result testres;
> +	size_t keymod_len = strlen(keymod);
> +	int ret;
> +
> +	if (length <= CAAM_BLOB_OVERHEAD)
> +		return -EINVAL;
> +
> +	desc = caam_blob_alloc_desc(keymod_len);
> +	if (!desc) {
> +		dev_err(jrdev, "unable to allocate desc\n");
> +		return -ENOMEM;
> +	}
> +
> +	dma_in = dma_map_single(jrdev, input, length - CAAM_BLOB_OVERHEAD, DMA_TO_DEVICE);
> +	if (dma_mapping_error(jrdev, dma_in)) {
> +		dev_err(jrdev, "unable to map input DMA buffer\n");
> +		ret = -ENOMEM;
> +		goto out_free;
> +	}
> +
> +	dma_out = dma_map_single(jrdev, output, length,	DMA_FROM_DEVICE);
> +	if (dma_mapping_error(jrdev, dma_out)) {
> +		dev_err(jrdev, "unable to map output DMA buffer\n");
> +		ret = -ENOMEM;
> +		goto out_unmap_in;
> +	}
> +
> +	/*
> +	 * 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
> +	 * initial counter (Ctr0) are generated automatically and stored in
> +	 * Class 1 Context DWords 0+1+2+3. The random BK is stored in the
> +	 * Class 1 Key Register. Operation Mode is set to AES-CCM.
> +	 */
> +
> +	init_job_desc(desc, 0);
> +	append_key_as_imm(desc, keymod, keymod_len, keymod_len,
> +			  CLASS_2 | KEY_DEST_CLASS_REG);
> +	append_seq_in_ptr(desc, dma_in, length - CAAM_BLOB_OVERHEAD, 0);
> +	append_seq_out_ptr(desc, dma_out, length, 0);
In case length is known to be < 2^16, it's recommended to use instead
append_seq_in_ptr_intlen, append_seq_out_ptr_intlen.

> +	append_operation(desc, OP_TYPE_ENCAP_PROTOCOL | OP_PCLID_BLOB);
> +
> +	print_hex_dump_debug("data@"__stringify(__LINE__)": ",
> +			     DUMP_PREFIX_ADDRESS, 16, 1, input,
> +			     length - CAAM_BLOB_OVERHEAD, false);
> +	print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
> +			     DUMP_PREFIX_ADDRESS, 16, 1, desc,
> +			     desc_bytes(desc), false);
> +
> +	testres.err = 0;
> +	init_completion(&testres.completion);
> +
> +	ret = caam_jr_enqueue(jrdev, desc, caam_blob_job_done, &testres);
> +	if (ret == -EINPROGRESS) {
> +		wait_for_completion(&testres.completion);
> +		ret = testres.err;
> +		print_hex_dump_debug("output@"__stringify(__LINE__)": ",
> +				     DUMP_PREFIX_ADDRESS, 16, 1, output,
> +				     length, false);
> +	}
> +
> +	dma_unmap_single(jrdev, dma_out, length, DMA_FROM_DEVICE);
> +out_unmap_in:
> +	dma_unmap_single(jrdev, dma_in, length - CAAM_BLOB_OVERHEAD, DMA_TO_DEVICE);
> +out_free:
> +	kfree(desc);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(caam_encap_blob);
> +
[...]
> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
> new file mode 100644
> index 000000000000..7eea0f543832
> --- /dev/null
> +++ b/include/soc/fsl/caam-blob.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
> + */
> +
> +#ifndef __CAAM_BLOB_GEN
> +#define __CAAM_BLOB_GEN
> +
> +#include <linux/types.h>
> +
> +#define CAAM_BLOB_KEYMOD_LENGTH		16
The define isn't used here or on patch 3/3.

> +#define CAAM_BLOB_OVERHEAD		(32 + 16)
> +#define CAAM_BLOB_MAX_LEN		4096
> +
> +struct caam_blob_priv;
> +
> +/** caam_blob_gen_init - initialize blob generation
> + *
> + * returns either pointer to new caam_blob_priv instance
> + * or error pointer
> + */
> +struct caam_blob_priv *caam_blob_gen_init(void);
> +
> +/** caam_blob_gen_init - free blob generation resources
> + *
> + * @priv: instance returned by caam_blob_gen_init
> + */
> +void caam_blob_gen_exit(struct caam_blob_priv *priv);
> +
> +/** caam_encap_blob - encapsulate blob
> + *
> + * @priv:   instance returned by caam_blob_gen_init
> + * @keymod: string to use as key modifier for blob encapsulation
> + * @input:  buffer which CAAM will DMA from
> + * @output: buffer which CAAM will DMA to
Is it guaranteed that input, output can be DMA-mapped?

Horia

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
  2021-03-16 19:22   ` Jarkko Sakkinen
  2021-03-16 23:14   ` Richard Weinberger
@ 2021-03-21 20:48   ` Horia Geantă
  2021-03-23 16:35     ` Ahmad Fatoum
  2021-03-31 18:35   ` Richard Weinberger
  3 siblings, 1 reply; 71+ messages in thread
From: Horia Geantă @ 2021-03-21 20:48 UTC (permalink / raw)
  To: Ahmad Fatoum, Jonathan Corbet, David Howells, Jarkko Sakkinen,
	James Bottomley, Mimi Zohar
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
[...]
> +struct trusted_key_ops caam_trusted_key_ops = {
> +	.migratable = 0, /* non-migratable */
> +	.init = trusted_caam_init,
> +	.seal = trusted_caam_seal,
> +	.unseal = trusted_caam_unseal,
> +	.exit = trusted_caam_exit,
> +};
caam has random number generation capabilities, so it's worth using that
by implementing .get_random.

Horia

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-21 20:01 ` Horia Geantă
@ 2021-03-23 16:34   ` Ahmad Fatoum
  2021-03-24  6:23     ` Sumit Garg
  2021-03-23 16:37   ` Ahmad Fatoum
  1 sibling, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-23 16:34 UTC (permalink / raw)
  To: Horia Geantă,
	Jarkko Sakkinen, Mimi Zohar, Aymen Sghaier, Herbert Xu,
	David S. Miller, James Bottomley
  Cc: kernel, David Howells, James Morris, Serge E. Hallyn,
	Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, linux-integrity, keyrings,
	linux-crypto, linux-kernel, linux-security-module

Hello Horia,

On 21.03.21 21:01, Horia Geantă wrote:
> On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
>> This patch series builds on top of Sumit's rework to have the CAAM as yet another
>> trusted key backend.
>>
> Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> be updated to reflect the availability of multiple backends?

This is indeed no longer correct. It also depends on TCG_TPM, which AFAIU
is not really needed for the new TEE backend.

@Sumit, can you confirm?

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-21 20:48   ` Horia Geantă
@ 2021-03-23 16:35     ` Ahmad Fatoum
  2021-03-23 18:07       ` Mimi Zohar
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-23 16:35 UTC (permalink / raw)
  To: Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

Hello Horia,

On 21.03.21 21:48, Horia Geantă wrote:
> On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> [...]
>> +struct trusted_key_ops caam_trusted_key_ops = {
>> +	.migratable = 0, /* non-migratable */
>> +	.init = trusted_caam_init,
>> +	.seal = trusted_caam_seal,
>> +	.unseal = trusted_caam_unseal,
>> +	.exit = trusted_caam_exit,
>> +};
> caam has random number generation capabilities, so it's worth using that
> by implementing .get_random.

If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?

Makes for less code duplication IMO.

> 
> Horia
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-21 20:01 ` Horia Geantă
  2021-03-23 16:34   ` Ahmad Fatoum
@ 2021-03-23 16:37   ` Ahmad Fatoum
  1 sibling, 0 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-23 16:37 UTC (permalink / raw)
  To: Horia Geantă,
	Jarkko Sakkinen, Mimi Zohar, Aymen Sghaier, Herbert Xu,
	David S. Miller, James Bottomley
  Cc: kernel, David Howells, James Morris, Serge E. Hallyn,
	Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, linux-integrity, keyrings,
	linux-crypto, linux-kernel, linux-security-module

Hello Horia,

On 21.03.21 21:01, Horia Geantă wrote:
>>  - [RFC] drivers: crypto: caam: key: Add caam_tk key type
>>    Franck added[3] a new "caam_tk" key type based on Udit's work. The key
>>    material stays within the kernel only, but can optionally be user-set
>>    instead of coming from RNG. 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.
>>
> The whole point was to use caam "black blobs", with the main advantage of
> keys being kept encrypted in memory after "unsealing" the blobs.
> (Keys in blobs are encrypted with a persistent BKEK - blob KEK, derived from
> fuse-based OTPMK. OTOH black keys are keys encrypted with an ephemeral, random
> KEK that is stored in an internal caam register. When a black blob is unsealed,
> the key is practically rekeyed, the random key replacing the BKEK; key is never
> exposed in plaintext, rekeying happens in caam).
> 
> Current implementation uses "red blobs", which means keys are left unprotected
> in memory after blobs are unsealed.

Oh. I will reread the series when sending the v2 cover letter. Thanks for spotting.

(Sorry for the noise, missed this question first time)

>>  - 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.
>>
> Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> be updated to reflect the availability of multiple backends?
> 
> Thanks,
> Horia
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator
  2021-03-21 20:46   ` Horia Geantă
@ 2021-03-23 16:41     ` Ahmad Fatoum
  0 siblings, 0 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-23 16:41 UTC (permalink / raw)
  To: Horia Geantă, Aymen Sghaier, Herbert Xu, David S. Miller
  Cc: kernel, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
	David Howells, James Morris, Serge E. Hallyn, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck Lenormand, Sumit Garg,
	linux-integrity, keyrings, linux-crypto, linux-kernel,
	linux-security-module

Hello Horia,

On 21.03.21 21:46, Horia Geantă wrote:
> On 3/16/2021 7:01 PM, Ahmad Fatoum wrote:
>> +	init_job_desc(desc, 0);
>> +	append_key_as_imm(desc, keymod, keymod_len, keymod_len,
>> +			  CLASS_2 | KEY_DEST_CLASS_REG);
>> +	append_seq_in_ptr(desc, dma_in, length - CAAM_BLOB_OVERHEAD, 0);
>> +	append_seq_out_ptr(desc, dma_out, length, 0);
> In case length is known to be < 2^16, it's recommended to use instead
> append_seq_in_ptr_intlen, append_seq_out_ptr_intlen.

Didn't know about this one. Will look into using it for v2.

>> +#define CAAM_BLOB_KEYMOD_LENGTH		16
> The define isn't used here or on patch 3/3.

Will drop and adjust the function docs to note the maximum modifier length.


>> +#define CAAM_BLOB_OVERHEAD		(32 + 16)
>> +#define CAAM_BLOB_MAX_LEN		4096
>> +
>> +struct caam_blob_priv;
>> +
>> +/** caam_blob_gen_init - initialize blob generation
>> + *
>> + * returns either pointer to new caam_blob_priv instance
>> + * or error pointer
>> + */
>> +struct caam_blob_priv *caam_blob_gen_init(void);
>> +
>> +/** caam_blob_gen_init - free blob generation resources
>> + *
>> + * @priv: instance returned by caam_blob_gen_init
>> + */
>> +void caam_blob_gen_exit(struct caam_blob_priv *priv);
>> +
>> +/** caam_encap_blob - encapsulate blob
>> + *
>> + * @priv:   instance returned by caam_blob_gen_init
>> + * @keymod: string to use as key modifier for blob encapsulation
>> + * @input:  buffer which CAAM will DMA from
>> + * @output: buffer which CAAM will DMA to
> Is it guaranteed that input, output can be DMA-mapped?

It's expected callers ensure that this is the case. Trusted key buffers
are allocated with kmalloc and can be DMA-mapped.

Thanks for the review,
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-23 16:35     ` Ahmad Fatoum
@ 2021-03-23 18:07       ` Mimi Zohar
  2021-03-24  9:26         ` Ahmad Fatoum
  2021-03-24 16:14         ` James Bottomley
  0 siblings, 2 replies; 71+ messages in thread
From: Mimi Zohar @ 2021-03-23 18:07 UTC (permalink / raw)
  To: Ahmad Fatoum, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> Hello Horia,
> 
> On 21.03.21 21:48, Horia Geantă wrote:
> > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> > [...]
> >> +struct trusted_key_ops caam_trusted_key_ops = {
> >> +	.migratable = 0, /* non-migratable */
> >> +	.init = trusted_caam_init,
> >> +	.seal = trusted_caam_seal,
> >> +	.unseal = trusted_caam_unseal,
> >> +	.exit = trusted_caam_exit,
> >> +};
> > caam has random number generation capabilities, so it's worth using that
> > by implementing .get_random.
> 
> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> 
> Makes for less code duplication IMO.

Using kernel RNG, in general, for trusted keys has been discussed
before.   Please refer to Dave Safford's detailed explanation for not
using it [1].

thanks,

Mimi

[1] 
https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
 


^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-23 16:34   ` Ahmad Fatoum
@ 2021-03-24  6:23     ` Sumit Garg
  0 siblings, 0 replies; 71+ messages in thread
From: Sumit Garg @ 2021-03-24  6:23 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Horia Geantă,
	Jarkko Sakkinen, Mimi Zohar, Aymen Sghaier, Herbert Xu,
	David S. Miller, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck Lenormand, linux-integrity,
	keyrings, linux-crypto, linux-kernel, linux-security-module

On Tue, 23 Mar 2021 at 22:04, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Horia,
>
> On 21.03.21 21:01, Horia Geantă wrote:
> > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> >> This patch series builds on top of Sumit's rework to have the CAAM as yet another
> >> trusted key backend.
> >>
> > Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> > be updated to reflect the availability of multiple backends?
>
> This is indeed no longer correct. It also depends on TCG_TPM, which AFAIU
> is not really needed for the new TEE backend.
>
> @Sumit, can you confirm?
>

Yes, that's correct. Let me share a separate patch to fix that.

-Sumit

> --
> 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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-23 18:07       ` Mimi Zohar
@ 2021-03-24  9:26         ` Ahmad Fatoum
  2021-03-24 10:47           ` Sumit Garg
  2021-03-24 16:14         ` James Bottomley
  1 sibling, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-24  9:26 UTC (permalink / raw)
  To: Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

Hello Mimi,

On 23.03.21 19:07, Mimi Zohar wrote:
> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
>> On 21.03.21 21:48, Horia Geantă wrote:
>>> caam has random number generation capabilities, so it's worth using that
>>> by implementing .get_random.
>>
>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
>>
>> Makes for less code duplication IMO.
> 
> Using kernel RNG, in general, for trusted keys has been discussed
> before.   Please refer to Dave Safford's detailed explanation for not
> using it [1].

The argument seems to boil down to:

 - TPM RNG are known to be of good quality
 - Trusted keys always used it so far

Both are fine by me for TPMs, but the CAAM backend is new code and neither point
really applies.

get_random_bytes_wait is already used for generating key material elsewhere.
Why shouldn't new trusted key backends be able to do the same thing?

Cheers,
Ahmad

> 
> thanks,
> 
> Mimi
> 
> [1] 
> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
>  
> 
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24  9:26         ` Ahmad Fatoum
@ 2021-03-24 10:47           ` Sumit Garg
  2021-03-24 14:07             ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-03-24 10:47 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Mimi,
>
> On 23.03.21 19:07, Mimi Zohar wrote:
> > On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> >> On 21.03.21 21:48, Horia Geantă wrote:
> >>> caam has random number generation capabilities, so it's worth using that
> >>> by implementing .get_random.
> >>
> >> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> >>
> >> Makes for less code duplication IMO.
> >
> > Using kernel RNG, in general, for trusted keys has been discussed
> > before.   Please refer to Dave Safford's detailed explanation for not
> > using it [1].
>
> The argument seems to boil down to:
>
>  - TPM RNG are known to be of good quality
>  - Trusted keys always used it so far
>
> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
> really applies.
>
> get_random_bytes_wait is already used for generating key material elsewhere.
> Why shouldn't new trusted key backends be able to do the same thing?
>

Please refer to documented trusted keys behaviour here [1]. New
trusted key backends should align to this behaviour and in your case
CAAM offers HWRNG so we should be better using that.

Also, do update documentation corresponding to CAAM as a trusted keys backend.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/tree/Documentation/security/keys/trusted-encrypted.rst#n87

-Sumit

> Cheers,
> Ahmad
>
> >
> > thanks,
> >
> > Mimi
> >
> > [1]
> > https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
> >
> >
> >
>
> --
> 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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24 10:47           ` Sumit Garg
@ 2021-03-24 14:07             ` Ahmad Fatoum
  2021-03-25  5:26               ` Sumit Garg
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-24 14:07 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

Hello Sumit,

On 24.03.21 11:47, Sumit Garg wrote:
> On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> Hello Mimi,
>>
>> On 23.03.21 19:07, Mimi Zohar wrote:
>>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
>>>> On 21.03.21 21:48, Horia Geantă wrote:
>>>>> caam has random number generation capabilities, so it's worth using that
>>>>> by implementing .get_random.
>>>>
>>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
>>>>
>>>> Makes for less code duplication IMO.
>>>
>>> Using kernel RNG, in general, for trusted keys has been discussed
>>> before.   Please refer to Dave Safford's detailed explanation for not
>>> using it [1].
>>
>> The argument seems to boil down to:
>>
>>  - TPM RNG are known to be of good quality
>>  - Trusted keys always used it so far
>>
>> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
>> really applies.
>>
>> get_random_bytes_wait is already used for generating key material elsewhere.
>> Why shouldn't new trusted key backends be able to do the same thing?
>>
> 
> Please refer to documented trusted keys behaviour here [1]. New
> trusted key backends should align to this behaviour and in your case
> CAAM offers HWRNG so we should be better using that.

Why is it better?

Can you explain what benefit a CAAM user would have if the trusted key
randomness comes directly out of the CAAM instead of indirectly from
the kernel entropy pool that is seeded by it?

> Also, do update documentation corresponding to CAAM as a trusted keys backend.

Yes. The documentation should be updated for CAAM and it should describe
how the key material is derived. Will do so for v2.

Cheers,
Ahmad

> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/tree/Documentation/security/keys/trusted-encrypted.rst#n87
> 
> -Sumit
> 
>> Cheers,
>> Ahmad
>>
>>>
>>> thanks,
>>>
>>> Mimi
>>>
>>> [1]
>>> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
>>>
>>>
>>>
>>
>> --
>> 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 |
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-23 18:07       ` Mimi Zohar
  2021-03-24  9:26         ` Ahmad Fatoum
@ 2021-03-24 16:14         ` James Bottomley
  2021-03-24 20:49           ` Mimi Zohar
  2021-04-02  1:49           ` Serge E. Hallyn
  1 sibling, 2 replies; 71+ messages in thread
From: James Bottomley @ 2021-03-24 16:14 UTC (permalink / raw)
  To: Mimi Zohar, Ahmad Fatoum, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On Tue, 2021-03-23 at 14:07 -0400, Mimi Zohar wrote:
> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > Hello Horia,
> > 
> > On 21.03.21 21:48, Horia Geantă wrote:
> > > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> > > [...]
> > > > +struct trusted_key_ops caam_trusted_key_ops = {
> > > > +	.migratable = 0, /* non-migratable */
> > > > +	.init = trusted_caam_init,
> > > > +	.seal = trusted_caam_seal,
> > > > +	.unseal = trusted_caam_unseal,
> > > > +	.exit = trusted_caam_exit,
> > > > +};
> > > caam has random number generation capabilities, so it's worth
> > > using that
> > > by implementing .get_random.
> > 
> > If the CAAM HWRNG is already seeding the kernel RNG, why not use
> > the kernel's?
> > 
> > Makes for less code duplication IMO.
> 
> Using kernel RNG, in general, for trusted keys has been discussed
> before.   Please refer to Dave Safford's detailed explanation for not
> using it [1].
> 
> thanks,
> 
> Mimi
> 
> [1] 
> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/

I still don't think relying on one source of randomness to be
cryptographically secure is a good idea.  The fear of bugs in the
kernel entropy pool is reasonable, but since it's widely used they're
unlikely to persist very long.  Studies have shown that some TPMs
(notably the chinese manufactured ones) have suspicious failures in
their RNGs:

https://www.researchgate.net/publication/45934562_Benchmarking_the_True_Random_Number_Generator_of_TPM_Chips

And most cryptograhpers recommend using a TPM for entropy mixing rather
than directly:

https://blog.cryptographyengineering.com/category/rngs/

The TPMFail paper also shows that in spite of NIST certification
things can go wrong with a TPM:

https://tpm.fail/

James



^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24 16:14         ` James Bottomley
@ 2021-03-24 20:49           ` Mimi Zohar
  2021-03-24 21:58             ` James Bottomley
  2021-04-02  1:49           ` Serge E. Hallyn
  1 sibling, 1 reply; 71+ messages in thread
From: Mimi Zohar @ 2021-03-24 20:49 UTC (permalink / raw)
  To: jejb, Ahmad Fatoum, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module,
	Pascal Van Leeuwen

On Wed, 2021-03-24 at 09:14 -0700, James Bottomley wrote:
> On Tue, 2021-03-23 at 14:07 -0400, Mimi Zohar wrote:
> > On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > > Hello Horia,
> > > 
> > > On 21.03.21 21:48, Horia Geantă wrote:
> > > > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> > > > [...]
> > > > > +struct trusted_key_ops caam_trusted_key_ops = {
> > > > > +	.migratable = 0, /* non-migratable */
> > > > > +	.init = trusted_caam_init,
> > > > > +	.seal = trusted_caam_seal,
> > > > > +	.unseal = trusted_caam_unseal,
> > > > > +	.exit = trusted_caam_exit,
> > > > > +};
> > > > caam has random number generation capabilities, so it's worth
> > > > using that
> > > > by implementing .get_random.
> > > 
> > > If the CAAM HWRNG is already seeding the kernel RNG, why not use
> > > the kernel's?
> > > 
> > > Makes for less code duplication IMO.
> > 
> > Using kernel RNG, in general, for trusted keys has been discussed
> > before.   Please refer to Dave Safford's detailed explanation for not
> > using it [1].
> > 
> > [1] 
> > https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
> 
> I still don't think relying on one source of randomness to be
> cryptographically secure is a good idea.  The fear of bugs in the
> kernel entropy pool is reasonable, but since it's widely used they're
> unlikely to persist very long.  Studies have shown that some TPMs
> (notably the chinese manufactured ones) have suspicious failures in
> their RNGs:
> 
> https://www.researchgate.net/publication/45934562_Benchmarking_the_True_Random_Number_Generator_of_TPM_Chips
> 
> And most cryptograhpers recommend using a TPM for entropy mixing rather
> than directly:
> 
> https://blog.cryptographyengineering.com/category/rngs/
> 
> The TPMFail paper also shows that in spite of NIST certification
> things can go wrong with a TPM:
> 
> https://tpm.fail/

We already had a lengthy discussion on replacing the TPM RNG with the
kernel RNG for trusted keys, when TEE was being introduced [2,3].  I'm
not interested in re-hashing that discussion here.   The only
difference now is that CAAM is a new trust source.  I suspect the same
concerns/issues persist, but at least in this case using the kernel RNG
would not be a regression.

[2] Pascal Van Leeuwen on mixing different sources of entropy and certification -
 https://lore.kernel.org/linux-integrity/MN2PR20MB29732A856A40131A671F949FCA950@MN2PR20MB2973.namprd20.prod.outlook.com/
[3] Jarrko on "regression" and tpm_asym.c - 
https://lore.kernel.org/linux-integrity/20191014190033.GA15552@linux.intel.com/ 

Mimi


^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24 20:49           ` Mimi Zohar
@ 2021-03-24 21:58             ` James Bottomley
  0 siblings, 0 replies; 71+ messages in thread
From: James Bottomley @ 2021-03-24 21:58 UTC (permalink / raw)
  To: Mimi Zohar, Ahmad Fatoum, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen
  Cc: kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module,
	Pascal Van Leeuwen

On Wed, 2021-03-24 at 16:49 -0400, Mimi Zohar wrote:
> On Wed, 2021-03-24 at 09:14 -0700, James Bottomley wrote:
> > On Tue, 2021-03-23 at 14:07 -0400, Mimi Zohar wrote:
> > > On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > > > Hello Horia,
> > > > 
> > > > On 21.03.21 21:48, Horia Geantă wrote:
> > > > > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> > > > > [...]
> > > > > > +struct trusted_key_ops caam_trusted_key_ops = {
> > > > > > +	.migratable = 0, /* non-migratable */
> > > > > > +	.init = trusted_caam_init,
> > > > > > +	.seal = trusted_caam_seal,
> > > > > > +	.unseal = trusted_caam_unseal,
> > > > > > +	.exit = trusted_caam_exit,
> > > > > > +};
> > > > > caam has random number generation capabilities, so it's worth
> > > > > using that
> > > > > by implementing .get_random.
> > > > 
> > > > If the CAAM HWRNG is already seeding the kernel RNG, why not
> > > > use
> > > > the kernel's?
> > > > 
> > > > Makes for less code duplication IMO.
> > > 
> > > Using kernel RNG, in general, for trusted keys has been discussed
> > > before.   Please refer to Dave Safford's detailed explanation for
> > > not
> > > using it [1].
> > > 
> > > [1] 
> > > https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
> > 
> > I still don't think relying on one source of randomness to be
> > cryptographically secure is a good idea.  The fear of bugs in the
> > kernel entropy pool is reasonable, but since it's widely used
> > they're
> > unlikely to persist very long.  Studies have shown that some TPMs
> > (notably the chinese manufactured ones) have suspicious failures in
> > their RNGs:
> > 
> > https://www.researchgate.net/publication/45934562_Benchmarking_the_True_Random_Number_Generator_of_TPM_Chips
> > 
> > And most cryptograhpers recommend using a TPM for entropy mixing
> > rather
> > than directly:
> > 
> > https://blog.cryptographyengineering.com/category/rngs/
> > 
> > The TPMFail paper also shows that in spite of NIST certification
> > things can go wrong with a TPM:
> > 
> > https://tpm.fail/
> 
> We already had a lengthy discussion on replacing the TPM RNG with the
> kernel RNG for trusted keys, when TEE was being introduced
> [2,3].  I'm not interested in re-hashing that discussion here.   The
> only difference now is that CAAM is a new trust source.  I suspect
> the same concerns/issues persist, but at least in this case using the
> kernel RNG would not be a regression.

Upstreaming the ASN.1 parser gives us a way to create trusted keys
outside the kernel and so choose any RNG that suits the user, so I
don't think there's any need to rehash for TPM based keys either.

However CaaM doesn't have the ability to create keys outside the kernel
yet, so they do need to consider the problem.

James


> [2] Pascal Van Leeuwen on mixing different sources of entropy and
> certification -
>  
> https://lore.kernel.org/linux-integrity/MN2PR20MB29732A856A40131A671F949FCA950@MN2PR20MB2973.namprd20.prod.outlook.com/
> [3] Jarrko on "regression" and tpm_asym.c - 
> https://lore.kernel.org/linux-integrity/20191014190033.GA15552@linux.intel.com/ 
> 
> Mimi
> 



^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24 14:07             ` Ahmad Fatoum
@ 2021-03-25  5:26               ` Sumit Garg
  2021-03-27 12:41                 ` David Gstir
  0 siblings, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-03-25  5:26 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On Wed, 24 Mar 2021 at 19:37, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Sumit,
>
> On 24.03.21 11:47, Sumit Garg wrote:
> > On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>
> >> Hello Mimi,
> >>
> >> On 23.03.21 19:07, Mimi Zohar wrote:
> >>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> >>>> On 21.03.21 21:48, Horia Geantă wrote:
> >>>>> caam has random number generation capabilities, so it's worth using that
> >>>>> by implementing .get_random.
> >>>>
> >>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> >>>>
> >>>> Makes for less code duplication IMO.
> >>>
> >>> Using kernel RNG, in general, for trusted keys has been discussed
> >>> before.   Please refer to Dave Safford's detailed explanation for not
> >>> using it [1].
> >>
> >> The argument seems to boil down to:
> >>
> >>  - TPM RNG are known to be of good quality
> >>  - Trusted keys always used it so far
> >>
> >> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
> >> really applies.
> >>
> >> get_random_bytes_wait is already used for generating key material elsewhere.
> >> Why shouldn't new trusted key backends be able to do the same thing?
> >>
> >
> > Please refer to documented trusted keys behaviour here [1]. New
> > trusted key backends should align to this behaviour and in your case
> > CAAM offers HWRNG so we should be better using that.
>
> Why is it better?
>
> Can you explain what benefit a CAAM user would have if the trusted key
> randomness comes directly out of the CAAM instead of indirectly from
> the kernel entropy pool that is seeded by it?

IMO, user trust in case of trusted keys comes from trusted keys
backend which is CAAM here. If a user doesn't trust that CAAM would
act as a reliable source for RNG then CAAM shouldn't be used as a
trust source in the first place.

And I think building user's trust for kernel RNG implementation with
multiple entropy contributions is pretty difficult when compared with
CAAM HWRNG implementation.

-Sumit

>
> > Also, do update documentation corresponding to CAAM as a trusted keys backend.
>
> Yes. The documentation should be updated for CAAM and it should describe
> how the key material is derived. Will do so for v2.
>
> Cheers,
> Ahmad
>
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/tree/Documentation/security/keys/trusted-encrypted.rst#n87
> >
> > -Sumit
> >
> >> Cheers,
> >> Ahmad
> >>
> >>>
> >>> thanks,
> >>>
> >>> Mimi
> >>>
> >>> [1]
> >>> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
> >>>
> >>>
> >>>
> >>
> >> --
> >> 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 |
> >
>
> --
> 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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-25  5:26               ` Sumit Garg
@ 2021-03-27 12:41                 ` David Gstir
  2021-03-28 20:37                   ` Jarkko Sakkinen
  0 siblings, 1 reply; 71+ messages in thread
From: David Gstir @ 2021-03-27 12:41 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Ahmad Fatoum, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	kernel, James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

Hi!

> On 25.03.2021, at 06:26, Sumit Garg <sumit.garg@linaro.org> wrote:
> 
> On Wed, 24 Mar 2021 at 19:37, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> 
>> Hello Sumit,
>> 
>> On 24.03.21 11:47, Sumit Garg wrote:
>>> On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>>> 
>>>> Hello Mimi,
>>>> 
>>>> On 23.03.21 19:07, Mimi Zohar wrote:
>>>>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
>>>>>> On 21.03.21 21:48, Horia Geantă wrote:
>>>>>>> caam has random number generation capabilities, so it's worth using that
>>>>>>> by implementing .get_random.
>>>>>> 
>>>>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
>>>>>> 
>>>>>> Makes for less code duplication IMO.
>>>>> 
>>>>> Using kernel RNG, in general, for trusted keys has been discussed
>>>>> before.   Please refer to Dave Safford's detailed explanation for not
>>>>> using it [1].
>>>> 
>>>> The argument seems to boil down to:
>>>> 
>>>> - TPM RNG are known to be of good quality
>>>> - Trusted keys always used it so far
>>>> 
>>>> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
>>>> really applies.
>>>> 
>>>> get_random_bytes_wait is already used for generating key material elsewhere.
>>>> Why shouldn't new trusted key backends be able to do the same thing?
>>>> 
>>> 
>>> Please refer to documented trusted keys behaviour here [1]. New
>>> trusted key backends should align to this behaviour and in your case
>>> CAAM offers HWRNG so we should be better using that.
>> 
>> Why is it better?
>> 
>> Can you explain what benefit a CAAM user would have if the trusted key
>> randomness comes directly out of the CAAM instead of indirectly from
>> the kernel entropy pool that is seeded by it?
> 
> IMO, user trust in case of trusted keys comes from trusted keys
> backend which is CAAM here. If a user doesn't trust that CAAM would
> act as a reliable source for RNG then CAAM shouldn't be used as a
> trust source in the first place.
> 
> And I think building user's trust for kernel RNG implementation with
> multiple entropy contributions is pretty difficult when compared with
> CAAM HWRNG implementation.

Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
other features are two separate things. However, reading through the CAAM
key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
content) are generated using its internal RNG. So I’d save if the CAAM RNG
is insecure, so are generated key blobs. Maybe somebody with more insight
into the CAAM internals can verify that, but I don’t see any point in using
the kernel’s RNG as long as we let CAAM generate the key blob keys for us.

Cheers,
dave


^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-27 12:41                 ` David Gstir
@ 2021-03-28 20:37                   ` Jarkko Sakkinen
  2021-03-29 10:11                     ` Ahmad Fatoum
                                       ` (2 more replies)
  0 siblings, 3 replies; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-03-28 20:37 UTC (permalink / raw)
  To: David Gstir
  Cc: Sumit Garg, Ahmad Fatoum, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Sat, Mar 27, 2021 at 01:41:24PM +0100, David Gstir wrote:
> Hi!
> 
> > On 25.03.2021, at 06:26, Sumit Garg <sumit.garg@linaro.org> wrote:
> > 
> > On Wed, 24 Mar 2021 at 19:37, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >> 
> >> Hello Sumit,
> >> 
> >> On 24.03.21 11:47, Sumit Garg wrote:
> >>> On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >>>> 
> >>>> Hello Mimi,
> >>>> 
> >>>> On 23.03.21 19:07, Mimi Zohar wrote:
> >>>>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> >>>>>> On 21.03.21 21:48, Horia Geantă wrote:
> >>>>>>> caam has random number generation capabilities, so it's worth using that
> >>>>>>> by implementing .get_random.
> >>>>>> 
> >>>>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> >>>>>> 
> >>>>>> Makes for less code duplication IMO.
> >>>>> 
> >>>>> Using kernel RNG, in general, for trusted keys has been discussed
> >>>>> before.   Please refer to Dave Safford's detailed explanation for not
> >>>>> using it [1].
> >>>> 
> >>>> The argument seems to boil down to:
> >>>> 
> >>>> - TPM RNG are known to be of good quality
> >>>> - Trusted keys always used it so far
> >>>> 
> >>>> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
> >>>> really applies.
> >>>> 
> >>>> get_random_bytes_wait is already used for generating key material elsewhere.
> >>>> Why shouldn't new trusted key backends be able to do the same thing?
> >>>> 
> >>> 
> >>> Please refer to documented trusted keys behaviour here [1]. New
> >>> trusted key backends should align to this behaviour and in your case
> >>> CAAM offers HWRNG so we should be better using that.
> >> 
> >> Why is it better?
> >> 
> >> Can you explain what benefit a CAAM user would have if the trusted key
> >> randomness comes directly out of the CAAM instead of indirectly from
> >> the kernel entropy pool that is seeded by it?
> > 
> > IMO, user trust in case of trusted keys comes from trusted keys
> > backend which is CAAM here. If a user doesn't trust that CAAM would
> > act as a reliable source for RNG then CAAM shouldn't be used as a
> > trust source in the first place.
> > 
> > And I think building user's trust for kernel RNG implementation with
> > multiple entropy contributions is pretty difficult when compared with
> > CAAM HWRNG implementation.
> 
> Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
> other features are two separate things. However, reading through the CAAM
> key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
> content) are generated using its internal RNG. So I’d save if the CAAM RNG
> is insecure, so are generated key blobs. Maybe somebody with more insight
> into the CAAM internals can verify that, but I don’t see any point in using
> the kernel’s RNG as long as we let CAAM generate the key blob keys for us.

Here's my long'ish analysis. Please read it to the end if by ever means
possible, and apologies, I usually try to keep usually my comms short, but
this requires some more meat than the usual.

The Bad News
============

Now that we add multiple hardware trust sources for trusted keys, will
there ever be a scenario where a trusted key is originally sealed with a
backing hardware A, unsealed, and resealed with hardware B?

The hardware and vendor neutral way to generate the key material would be
unconditionally always just the kernel RNG.

CAAM is actually worse than TCG because it's not even a standards body, if
I got it right. Not a lot but at least a tiny fraction.

This brings an open item in TEE patches: trusted_tee_get_random() is an
issue in generating kernel material. I would rather replace that with
kernel RNG *for now*, because the same open question applies also to ARM
TEE. It's also a single company controlled backing technology.

By all practical means, I do trust ARM TEE in my personal life but this is
not important.

CAAM *and* TEE backends break the golden rule of putting as little trust as
possible to anything, even not anything weird is clear at sight, as
security is essentially a game of known unknowns and unknown unknowns.

Unfortunately, TPM trusted keys started this bad security practice, and
obviously it cannot be fixed without breaking uapi backwards compatibility.

This leaves me exactly two rational options:

A. Add a patch to remove trusted_tee_get_random() and use kernel RNG
   instead.
B. Drop the whole TEE patch set up until I have good reasons to believe
   that it's the best possible idea ever to use TEE RNG.

Doing does (A) does not disclude of doing (B) later on, if someone some
day sends a patch with sound reasoning.

It's also good to understand that when some day a vendor D, other than TCG,
CAAM or ARM, comes up, we need to go again this lenghty and messy
discussion. Now this already puts an already accepted patch set into a
risk, because by being a responsible maintainer I would have legit reasons
just simply to drop it.

OK, but....

The GOOD News
=============

So there's actually option (C) that also fixes the TPM trustd keys issue:

Add a new kernel patch, which:

1. Adds the use of kernel RNG as a boot option.
2. If this boot option is not active, the subsystem will print a warning
   to klog denoting this.
3. Default is of course vendor RNG given the bad design issue in the TPM
   trusted keys, but the warning in klog will help to address it at least
   a bit.
4. Document all this to Documentation/security/keys/trusted-encrypted.rst.

I'd prefer the choice between A, B and C be concluded rather sooner than
later.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-28 20:37                   ` Jarkko Sakkinen
@ 2021-03-29 10:11                     ` Ahmad Fatoum
  2021-03-31 23:29                       ` Jarkko Sakkinen
  2021-03-30  7:26                     ` Sumit Garg
  2021-03-30 21:47                     ` Eric Biggers
  2 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-03-29 10:11 UTC (permalink / raw)
  To: Jarkko Sakkinen, David Gstir
  Cc: Sumit Garg, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

Hello Jarkko,

On 28.03.21 22:37, Jarkko Sakkinen wrote:
> On Sat, Mar 27, 2021 at 01:41:24PM +0100, David Gstir wrote:
>> Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
>> other features are two separate things. However, reading through the CAAM
>> key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
>> content) are generated using its internal RNG. So I’d save if the CAAM RNG
>> is insecure, so are generated key blobs. Maybe somebody with more insight
>> into the CAAM internals can verify that, but I don’t see any point in using
>> the kernel’s RNG as long as we let CAAM generate the key blob keys for us.
> 
> Here's my long'ish analysis. Please read it to the end if by ever means
> possible, and apologies, I usually try to keep usually my comms short, but
> this requires some more meat than the usual.

Thanks for the write-up!

> The Bad News
> ============
> 
> Now that we add multiple hardware trust sources for trusted keys, will
> there ever be a scenario where a trusted key is originally sealed with a
> backing hardware A, unsealed, and resealed with hardware B?
> 
> The hardware and vendor neutral way to generate the key material would be
> unconditionally always just the kernel RNG.
> 
> CAAM is actually worse than TCG because it's not even a standards body, if
> I got it right. Not a lot but at least a tiny fraction.

CAAM is how NXP calls the crypto accelerator built into some of its SoCs.

> This brings an open item in TEE patches: trusted_tee_get_random() is an
> issue in generating kernel material. I would rather replace that with
> kernel RNG *for now*, because the same open question applies also to ARM
> TEE. It's also a single company controlled backing technology.
> 
> By all practical means, I do trust ARM TEE in my personal life but this is
> not important.
> 
> CAAM *and* TEE backends break the golden rule of putting as little trust as
> possible to anything, even not anything weird is clear at sight, as
> security is essentially a game of known unknowns and unknown unknowns.

Agreed.

> The GOOD News
> =============
> 
> So there's actually option (C) that also fixes the TPM trustd keys issue:
> 
> Add a new kernel patch, which:
> 
> 1. Adds the use of kernel RNG as a boot option.
> 2. If this boot option is not active, the subsystem will print a warning
>    to klog denoting this.
> 3. Default is of course vendor RNG given the bad design issue in the TPM
>    trusted keys, but the warning in klog will help to address it at least
>    a bit.

Why should the TPM backend's choice influence later backends? We could add
a new option for key creation time, e.g.:

   keyctl add trusted kmk "new keylen rng=kernel" @s

The default would be rng=vendor if available with a fallback to rng=kernel,
which should always be available.

> 4. Document all this to Documentation/security/keys/trusted-encrypted.rst.

Yes, backends would then document whether they support a rng=vendor or not.

> I'd prefer the choice between A, B and C be concluded rather sooner than
> later.

FWIW, my vote is for option C, with the change described above.

Cheers,
Ahmad

> 
> /Jarkko
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-28 20:37                   ` Jarkko Sakkinen
  2021-03-29 10:11                     ` Ahmad Fatoum
@ 2021-03-30  7:26                     ` Sumit Garg
  2021-03-31 23:30                       ` Jarkko Sakkinen
  2021-03-30 21:47                     ` Eric Biggers
  2 siblings, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-03-30  7:26 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: David Gstir, Ahmad Fatoum, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Mon, 29 Mar 2021 at 01:07, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Sat, Mar 27, 2021 at 01:41:24PM +0100, David Gstir wrote:
> > Hi!
> >
> > > On 25.03.2021, at 06:26, Sumit Garg <sumit.garg@linaro.org> wrote:
> > >
> > > On Wed, 24 Mar 2021 at 19:37, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > >>
> > >> Hello Sumit,
> > >>
> > >> On 24.03.21 11:47, Sumit Garg wrote:
> > >>> On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > >>>>
> > >>>> Hello Mimi,
> > >>>>
> > >>>> On 23.03.21 19:07, Mimi Zohar wrote:
> > >>>>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > >>>>>> On 21.03.21 21:48, Horia Geantă wrote:
> > >>>>>>> caam has random number generation capabilities, so it's worth using that
> > >>>>>>> by implementing .get_random.
> > >>>>>>
> > >>>>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> > >>>>>>
> > >>>>>> Makes for less code duplication IMO.
> > >>>>>
> > >>>>> Using kernel RNG, in general, for trusted keys has been discussed
> > >>>>> before.   Please refer to Dave Safford's detailed explanation for not
> > >>>>> using it [1].
> > >>>>
> > >>>> The argument seems to boil down to:
> > >>>>
> > >>>> - TPM RNG are known to be of good quality
> > >>>> - Trusted keys always used it so far
> > >>>>
> > >>>> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
> > >>>> really applies.
> > >>>>
> > >>>> get_random_bytes_wait is already used for generating key material elsewhere.
> > >>>> Why shouldn't new trusted key backends be able to do the same thing?
> > >>>>
> > >>>
> > >>> Please refer to documented trusted keys behaviour here [1]. New
> > >>> trusted key backends should align to this behaviour and in your case
> > >>> CAAM offers HWRNG so we should be better using that.
> > >>
> > >> Why is it better?
> > >>
> > >> Can you explain what benefit a CAAM user would have if the trusted key
> > >> randomness comes directly out of the CAAM instead of indirectly from
> > >> the kernel entropy pool that is seeded by it?
> > >
> > > IMO, user trust in case of trusted keys comes from trusted keys
> > > backend which is CAAM here. If a user doesn't trust that CAAM would
> > > act as a reliable source for RNG then CAAM shouldn't be used as a
> > > trust source in the first place.
> > >
> > > And I think building user's trust for kernel RNG implementation with
> > > multiple entropy contributions is pretty difficult when compared with
> > > CAAM HWRNG implementation.
> >
> > Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
> > other features are two separate things. However, reading through the CAAM
> > key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
> > content) are generated using its internal RNG. So I’d save if the CAAM RNG
> > is insecure, so are generated key blobs. Maybe somebody with more insight
> > into the CAAM internals can verify that, but I don’t see any point in using
> > the kernel’s RNG as long as we let CAAM generate the key blob keys for us.
>
> Here's my long'ish analysis. Please read it to the end if by ever means
> possible, and apologies, I usually try to keep usually my comms short, but
> this requires some more meat than the usual.
>
> The Bad News
> ============
>
> Now that we add multiple hardware trust sources for trusted keys, will
> there ever be a scenario where a trusted key is originally sealed with a
> backing hardware A, unsealed, and resealed with hardware B?
>
> The hardware and vendor neutral way to generate the key material would be
> unconditionally always just the kernel RNG.
>
> CAAM is actually worse than TCG because it's not even a standards body, if
> I got it right. Not a lot but at least a tiny fraction.
>
> This brings an open item in TEE patches: trusted_tee_get_random() is an
> issue in generating kernel material. I would rather replace that with
> kernel RNG *for now*, because the same open question applies also to ARM
> TEE. It's also a single company controlled backing technology.
>
> By all practical means, I do trust ARM TEE in my personal life but this is
> not important.
>
> CAAM *and* TEE backends break the golden rule of putting as little trust as
> possible to anything, even not anything weird is clear at sight, as
> security is essentially a game of known unknowns and unknown unknowns.
>
> Unfortunately, TPM trusted keys started this bad security practice, and
> obviously it cannot be fixed without breaking uapi backwards compatibility.
>
> This leaves me exactly two rational options:
>
> A. Add a patch to remove trusted_tee_get_random() and use kernel RNG
>    instead.
> B. Drop the whole TEE patch set up until I have good reasons to believe
>    that it's the best possible idea ever to use TEE RNG.
>
> Doing does (A) does not disclude of doing (B) later on, if someone some
> day sends a patch with sound reasoning.
>
> It's also good to understand that when some day a vendor D, other than TCG,
> CAAM or ARM, comes up, we need to go again this lenghty and messy
> discussion. Now this already puts an already accepted patch set into a
> risk, because by being a responsible maintainer I would have legit reasons
> just simply to drop it.
>
> OK, but....
>
> The GOOD News
> =============
>
> So there's actually option (C) that also fixes the TPM trustd keys issue:
>
> Add a new kernel patch, which:
>
> 1. Adds the use of kernel RNG as a boot option.
> 2. If this boot option is not active, the subsystem will print a warning
>    to klog denoting this.
> 3. Default is of course vendor RNG given the bad design issue in the TPM
>    trusted keys, but the warning in klog will help to address it at least
>    a bit.
> 4. Document all this to Documentation/security/keys/trusted-encrypted.rst.
>
> I'd prefer the choice between A, B and C be concluded rather sooner than
> later.

Option (C) sounds reasonable to me but I would rather prefer an info
message rather than warning as otherwise it would reflect that we are
enforcing kernel RNG choice for a user to trust upon.

-Sumit

>
> /Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-17 14:02     ` Ahmad Fatoum
@ 2021-03-30 21:28       ` Richard Weinberger
  0 siblings, 0 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-03-30 21:28 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	Linux Crypto Mailing List, linux-doc, linux-integrity, LKML, LSM

Ahmad,

On Wed, Mar 17, 2021 at 3:03 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> > I didn't closely follow the previous discussions, but is a module
> > parameter really the right approach?
> > Is there also a way to set it via something like device tree?
>
> Compiled-on sources are considered in the order: tpm, tee then caam.
> Module parameters are the only override currently available.

Okay. So in the ideal case only one of these backends is compiled in,
but the list can get long.

I'm asking because David and I currently port another caam-like
mechanism to the most recent
kernel which will also hook in there.
Out driver adds trusted keys support (with caam alike blobs) for i.mx
SoCs that come with DCP
instead of CAAM.
Patches will hopefully materialize soon.

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-28 20:37                   ` Jarkko Sakkinen
  2021-03-29 10:11                     ` Ahmad Fatoum
  2021-03-30  7:26                     ` Sumit Garg
@ 2021-03-30 21:47                     ` Eric Biggers
  2021-03-31 23:31                       ` Jarkko Sakkinen
  2 siblings, 1 reply; 71+ messages in thread
From: Eric Biggers @ 2021-03-30 21:47 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Sun, Mar 28, 2021 at 11:37:23PM +0300, Jarkko Sakkinen wrote:
> 
> Unfortunately, TPM trusted keys started this bad security practice, and
> obviously it cannot be fixed without breaking uapi backwards compatibility.
> 

The whole point of a randomness source is that it is random.  So userspace can't
be depending on any particular output, and the randomness source can be changed
without breaking backwards compatibility.

So IMO, trusted keys should simply be fixed to use get_random_bytes().

- Eric

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-17 14:08   ` Ahmad Fatoum
@ 2021-03-30 21:50     ` Richard Weinberger
  2021-04-01 10:04       ` Ahmad Fatoum
  2021-03-30 22:04     ` Richard Weinberger
  2021-04-01 11:11     ` David Howells
  2 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-03-30 21:50 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, Sumit Garg, linux-integrity,
	keyrings, Linux Crypto Mailing List, LKML, LSM

Ahmad,

On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

>     TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
>     echo $TABLE | dmsetup create mydev
>     echo $TABLE | dmsetup load mydev

Do you also plan to add support for this to cryptsetup?

David and I have added (rough) support for our CAAM/DCP based keyrings
to cryptsetup:
https://github.com/sigma-star/cryptsetup/tree/rw/plain

I'm pretty sure with minimal changes it will work with your recent approach too.

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-17 14:08   ` Ahmad Fatoum
  2021-03-30 21:50     ` Richard Weinberger
@ 2021-03-30 22:04     ` Richard Weinberger
  2021-03-30 22:16       ` James Bottomley
  2021-04-01 12:55       ` Sumit Garg
  2021-04-01 11:11     ` David Howells
  2 siblings, 2 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-03-30 22:04 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, Sumit Garg, linux-integrity,
	keyrings, Linux Crypto Mailing List, LKML, LSM

Ahmad,

On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s

Is there a reason why we can't pass the desired backend name in the
trusted key parameters?
e.g.
keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30 22:04     ` Richard Weinberger
@ 2021-03-30 22:16       ` James Bottomley
  2021-03-31 18:36         ` Richard Weinberger
  2021-04-01 12:55       ` Sumit Garg
  1 sibling, 1 reply; 71+ messages in thread
From: James Bottomley @ 2021-03-30 22:16 UTC (permalink / raw)
  To: Richard Weinberger, Ahmad Fatoum
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller, kernel,
	David Howells, James Morris, Serge E. Hallyn, Steffen Trumtrar,
	Udit Agarwal, Jan Luebbe, David Gstir, Franck LENORMAND,
	Sumit Garg, linux-integrity, keyrings, Linux Crypto Mailing List,
	LKML, LSM

On Wed, 2021-03-31 at 00:04 +0200, Richard Weinberger wrote:
> Ahmad,
> 
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de
> > wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
> 
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)"
> @s

Why would you want to in the load?  The blob should be type specific,
so a TPM key shouldn't load as a CAAM key and vice versa ... and if
they're not they need to be made so before the patches go upstream.

I could possibly see that you might want to be type specific in the
create, but once you're simply loading an already created key, the
trusted key subsystem should be able to figure what to do on its own.

James



^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
                     ` (2 preceding siblings ...)
  2021-03-21 20:48   ` Horia Geantă
@ 2021-03-31 18:35   ` Richard Weinberger
  2021-04-01 10:15     ` Ahmad Fatoum
  3 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-03-31 18:35 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	Linux Crypto Mailing List, linux-doc, linux-integrity, LKML, LSM

Ahmad,

On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> +#define KEYMOD "kernel:trusted"

why is the CAAM key modifier hard coded?
I'd love to have way to pass my own modifier.

That way existing blobs can also be used with this implementation.
IIRC the NXP vendor tree uses "SECURE_KEY" as default modifier.

-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30 22:16       ` James Bottomley
@ 2021-03-31 18:36         ` Richard Weinberger
  2021-03-31 18:49           ` James Bottomley
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-03-31 18:36 UTC (permalink / raw)
  To: James Bottomley
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

James,

----- Ursprüngliche Mail -----
> Von: "James Bottomley" <jejb@linux.ibm.com>
>> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de
>> > wrote:
>> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>> 
>> Is there a reason why we can't pass the desired backend name in the
>> trusted key parameters?
>> e.g.
>> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)"
>> @s
> 
> Why would you want to in the load?  The blob should be type specific,
> so a TPM key shouldn't load as a CAAM key and vice versa ... and if
> they're not they need to be made so before the patches go upstream.

I fear right now there is no good way to detect whether a blob is desired
for CAAM or TPM.

> I could possibly see that you might want to be type specific in the
> create, but once you're simply loading an already created key, the
> trusted key subsystem should be able to figure what to do on its own.

So you have some kind of container format in mind which denotes the
type of the blob?

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 18:36         ` Richard Weinberger
@ 2021-03-31 18:49           ` James Bottomley
  2021-03-31 19:36             ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: James Bottomley @ 2021-03-31 18:49 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

On Wed, 2021-03-31 at 20:36 +0200, Richard Weinberger wrote:
> James,
> 
> ----- Ursprüngliche Mail -----
> > Von: "James Bottomley" <jejb@linux.ibm.com>
> > > On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <
> > > a.fatoum@pengutronix.de wrote:
> > > >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
> > > 
> > > Is there a reason why we can't pass the desired backend name in
> > > the
> > > trusted key parameters?
> > > e.g.
> > > keyctl add trusted $KEYNAME "backendtype caam load $(cat
> > > ~/kmk.blob)"
> > > @s
> > 
> > Why would you want to in the load?  The blob should be type
> > specific, so a TPM key shouldn't load as a CAAM key and vice versa
> > ... and if they're not they need to be made so before the patches
> > go upstream.
> 
> I fear right now there is no good way to detect whether a blob is
> desired for CAAM or TPM.

At least for the TPM the old format is two TPM2B structures, and the
new one is ASN.1 so either should be completely distinguishable over
what CAAM does.

> > I could possibly see that you might want to be type specific in the
> > create, but once you're simply loading an already created key, the
> > trusted key subsystem should be able to figure what to do on its
> > own.
> 
> So you have some kind of container format in mind which denotes the
> type of the blob?

Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h

and part of the design of the file is that it's distinguishable either
in DER or PEM (by the guards) format so any crypto application can know
it's dealing with a TPM key simply by inspecting the file.  I think you
need the same thing for CAAM and any other format.

We're encouraging new ASN.1 formats to be of the form

SEQUENCE {
    type   OBJECT IDENTIFIER
    ... key specific fields ...
}

Where you choose a defined OID to represent the key and that means
every key even in DER form begins with a unique binary signature.

James



^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 18:49           ` James Bottomley
@ 2021-03-31 19:36             ` Richard Weinberger
  2021-04-01 10:06               ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-03-31 19:36 UTC (permalink / raw)
  To: James Bottomley
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

James,

----- Ursprüngliche Mail -----
> Von: "James Bottomley" <jejb@linux.ibm.com>
> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> 
> and part of the design of the file is that it's distinguishable either
> in DER or PEM (by the guards) format so any crypto application can know
> it's dealing with a TPM key simply by inspecting the file.  I think you
> need the same thing for CAAM and any other format.
> 
> We're encouraging new ASN.1 formats to be of the form
> 
> SEQUENCE {
>    type   OBJECT IDENTIFIER
>    ... key specific fields ...
> }
> 
> Where you choose a defined OID to represent the key and that means
> every key even in DER form begins with a unique binary signature.

I like this idea.
Ahmad, what do you think?

That way we could also get rid off the kernel parameter and all the fall back logic,
given that we find a way to reliable detect TEE blobs too...

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-29 10:11                     ` Ahmad Fatoum
@ 2021-03-31 23:29                       ` Jarkko Sakkinen
  0 siblings, 0 replies; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-03-31 23:29 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: David Gstir, Sumit Garg, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Mon, Mar 29, 2021 at 12:11:24PM +0200, Ahmad Fatoum wrote:
> Hello Jarkko,
> 
> On 28.03.21 22:37, Jarkko Sakkinen wrote:
> > On Sat, Mar 27, 2021 at 01:41:24PM +0100, David Gstir wrote:
> >> Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
> >> other features are two separate things. However, reading through the CAAM
> >> key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
> >> content) are generated using its internal RNG. So I’d save if the CAAM RNG
> >> is insecure, so are generated key blobs. Maybe somebody with more insight
> >> into the CAAM internals can verify that, but I don’t see any point in using
> >> the kernel’s RNG as long as we let CAAM generate the key blob keys for us.
> > 
> > Here's my long'ish analysis. Please read it to the end if by ever means
> > possible, and apologies, I usually try to keep usually my comms short, but
> > this requires some more meat than the usual.
> 
> Thanks for the write-up!
> 
> > The Bad News
> > ============
> > 
> > Now that we add multiple hardware trust sources for trusted keys, will
> > there ever be a scenario where a trusted key is originally sealed with a
> > backing hardware A, unsealed, and resealed with hardware B?
> > 
> > The hardware and vendor neutral way to generate the key material would be
> > unconditionally always just the kernel RNG.
> > 
> > CAAM is actually worse than TCG because it's not even a standards body, if
> > I got it right. Not a lot but at least a tiny fraction.
> 
> CAAM is how NXP calls the crypto accelerator built into some of its SoCs.
> 
> > This brings an open item in TEE patches: trusted_tee_get_random() is an
> > issue in generating kernel material. I would rather replace that with
> > kernel RNG *for now*, because the same open question applies also to ARM
> > TEE. It's also a single company controlled backing technology.
> > 
> > By all practical means, I do trust ARM TEE in my personal life but this is
> > not important.
> > 
> > CAAM *and* TEE backends break the golden rule of putting as little trust as
> > possible to anything, even not anything weird is clear at sight, as
> > security is essentially a game of known unknowns and unknown unknowns.
> 
> Agreed.
> 
> > The GOOD News
> > =============
> > 
> > So there's actually option (C) that also fixes the TPM trustd keys issue:
> > 
> > Add a new kernel patch, which:
> > 
> > 1. Adds the use of kernel RNG as a boot option.
> > 2. If this boot option is not active, the subsystem will print a warning
> >    to klog denoting this.
> > 3. Default is of course vendor RNG given the bad design issue in the TPM
> >    trusted keys, but the warning in klog will help to address it at least
> >    a bit.
> 
> Why should the TPM backend's choice influence later backends? We could add
> a new option for key creation time, e.g.:
> 
>    keyctl add trusted kmk "new keylen rng=kernel" @s
> 
> The default would be rng=vendor if available with a fallback to rng=kernel,
> which should always be available.

It matters a lot because it is existing ABI - for better or worse.

I think a new option is a bad idea, because it cannot easily enforced.
Kernel command-line on the other hand can be even signed.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30  7:26                     ` Sumit Garg
@ 2021-03-31 23:30                       ` Jarkko Sakkinen
  2021-04-01  7:41                         ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-03-31 23:30 UTC (permalink / raw)
  To: Sumit Garg
  Cc: David Gstir, Ahmad Fatoum, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Tue, Mar 30, 2021 at 12:56:41PM +0530, Sumit Garg wrote:
> On Mon, 29 Mar 2021 at 01:07, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Sat, Mar 27, 2021 at 01:41:24PM +0100, David Gstir wrote:
> > > Hi!
> > >
> > > > On 25.03.2021, at 06:26, Sumit Garg <sumit.garg@linaro.org> wrote:
> > > >
> > > > On Wed, 24 Mar 2021 at 19:37, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > > >>
> > > >> Hello Sumit,
> > > >>
> > > >> On 24.03.21 11:47, Sumit Garg wrote:
> > > >>> On Wed, 24 Mar 2021 at 14:56, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > > >>>>
> > > >>>> Hello Mimi,
> > > >>>>
> > > >>>> On 23.03.21 19:07, Mimi Zohar wrote:
> > > >>>>> On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > > >>>>>> On 21.03.21 21:48, Horia Geantă wrote:
> > > >>>>>>> caam has random number generation capabilities, so it's worth using that
> > > >>>>>>> by implementing .get_random.
> > > >>>>>>
> > > >>>>>> If the CAAM HWRNG is already seeding the kernel RNG, why not use the kernel's?
> > > >>>>>>
> > > >>>>>> Makes for less code duplication IMO.
> > > >>>>>
> > > >>>>> Using kernel RNG, in general, for trusted keys has been discussed
> > > >>>>> before.   Please refer to Dave Safford's detailed explanation for not
> > > >>>>> using it [1].
> > > >>>>
> > > >>>> The argument seems to boil down to:
> > > >>>>
> > > >>>> - TPM RNG are known to be of good quality
> > > >>>> - Trusted keys always used it so far
> > > >>>>
> > > >>>> Both are fine by me for TPMs, but the CAAM backend is new code and neither point
> > > >>>> really applies.
> > > >>>>
> > > >>>> get_random_bytes_wait is already used for generating key material elsewhere.
> > > >>>> Why shouldn't new trusted key backends be able to do the same thing?
> > > >>>>
> > > >>>
> > > >>> Please refer to documented trusted keys behaviour here [1]. New
> > > >>> trusted key backends should align to this behaviour and in your case
> > > >>> CAAM offers HWRNG so we should be better using that.
> > > >>
> > > >> Why is it better?
> > > >>
> > > >> Can you explain what benefit a CAAM user would have if the trusted key
> > > >> randomness comes directly out of the CAAM instead of indirectly from
> > > >> the kernel entropy pool that is seeded by it?
> > > >
> > > > IMO, user trust in case of trusted keys comes from trusted keys
> > > > backend which is CAAM here. If a user doesn't trust that CAAM would
> > > > act as a reliable source for RNG then CAAM shouldn't be used as a
> > > > trust source in the first place.
> > > >
> > > > And I think building user's trust for kernel RNG implementation with
> > > > multiple entropy contributions is pretty difficult when compared with
> > > > CAAM HWRNG implementation.
> > >
> > > Generally speaking, I’d say trusting the CAAM RNG and trusting in it’s
> > > other features are two separate things. However, reading through the CAAM
> > > key blob spec I’ve got here, CAAM key blob keys (the keys that secure a blob’s
> > > content) are generated using its internal RNG. So I’d save if the CAAM RNG
> > > is insecure, so are generated key blobs. Maybe somebody with more insight
> > > into the CAAM internals can verify that, but I don’t see any point in using
> > > the kernel’s RNG as long as we let CAAM generate the key blob keys for us.
> >
> > Here's my long'ish analysis. Please read it to the end if by ever means
> > possible, and apologies, I usually try to keep usually my comms short, but
> > this requires some more meat than the usual.
> >
> > The Bad News
> > ============
> >
> > Now that we add multiple hardware trust sources for trusted keys, will
> > there ever be a scenario where a trusted key is originally sealed with a
> > backing hardware A, unsealed, and resealed with hardware B?
> >
> > The hardware and vendor neutral way to generate the key material would be
> > unconditionally always just the kernel RNG.
> >
> > CAAM is actually worse than TCG because it's not even a standards body, if
> > I got it right. Not a lot but at least a tiny fraction.
> >
> > This brings an open item in TEE patches: trusted_tee_get_random() is an
> > issue in generating kernel material. I would rather replace that with
> > kernel RNG *for now*, because the same open question applies also to ARM
> > TEE. It's also a single company controlled backing technology.
> >
> > By all practical means, I do trust ARM TEE in my personal life but this is
> > not important.
> >
> > CAAM *and* TEE backends break the golden rule of putting as little trust as
> > possible to anything, even not anything weird is clear at sight, as
> > security is essentially a game of known unknowns and unknown unknowns.
> >
> > Unfortunately, TPM trusted keys started this bad security practice, and
> > obviously it cannot be fixed without breaking uapi backwards compatibility.
> >
> > This leaves me exactly two rational options:
> >
> > A. Add a patch to remove trusted_tee_get_random() and use kernel RNG
> >    instead.
> > B. Drop the whole TEE patch set up until I have good reasons to believe
> >    that it's the best possible idea ever to use TEE RNG.
> >
> > Doing does (A) does not disclude of doing (B) later on, if someone some
> > day sends a patch with sound reasoning.
> >
> > It's also good to understand that when some day a vendor D, other than TCG,
> > CAAM or ARM, comes up, we need to go again this lenghty and messy
> > discussion. Now this already puts an already accepted patch set into a
> > risk, because by being a responsible maintainer I would have legit reasons
> > just simply to drop it.
> >
> > OK, but....
> >
> > The GOOD News
> > =============
> >
> > So there's actually option (C) that also fixes the TPM trustd keys issue:
> >
> > Add a new kernel patch, which:
> >
> > 1. Adds the use of kernel RNG as a boot option.
> > 2. If this boot option is not active, the subsystem will print a warning
> >    to klog denoting this.
> > 3. Default is of course vendor RNG given the bad design issue in the TPM
> >    trusted keys, but the warning in klog will help to address it at least
> >    a bit.
> > 4. Document all this to Documentation/security/keys/trusted-encrypted.rst.
> >
> > I'd prefer the choice between A, B and C be concluded rather sooner than
> > later.
> 
> Option (C) sounds reasonable to me but I would rather prefer an info
> message rather than warning as otherwise it would reflect that we are
> enforcing kernel RNG choice for a user to trust upon.

I gave some though on this.

I take TEE as it is but I'd expect the CAAM patch set sort out this option
with some patch.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30 21:47                     ` Eric Biggers
@ 2021-03-31 23:31                       ` Jarkko Sakkinen
  2021-03-31 23:34                         ` Eric Biggers
  0 siblings, 1 reply; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-03-31 23:31 UTC (permalink / raw)
  To: Eric Biggers
  Cc: David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Tue, Mar 30, 2021 at 02:47:18PM -0700, Eric Biggers wrote:
> On Sun, Mar 28, 2021 at 11:37:23PM +0300, Jarkko Sakkinen wrote:
> > 
> > Unfortunately, TPM trusted keys started this bad security practice, and
> > obviously it cannot be fixed without breaking uapi backwards compatibility.
> > 
> 
> The whole point of a randomness source is that it is random.  So userspace can't
> be depending on any particular output, and the randomness source can be changed
> without breaking backwards compatibility.
> 
> So IMO, trusted keys should simply be fixed to use get_random_bytes().
> 
> - Eric

It's a bummer but uapi is the god in the end. Since TPM does not do it
today, that behaviour must be supported forever. That's why a boot option
AND a warning would be the best compromise.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 23:31                       ` Jarkko Sakkinen
@ 2021-03-31 23:34                         ` Eric Biggers
  2021-04-01  1:11                           ` Herbert Xu
  2021-04-01  5:46                           ` Jarkko Sakkinen
  0 siblings, 2 replies; 71+ messages in thread
From: Eric Biggers @ 2021-03-31 23:34 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Thu, Apr 01, 2021 at 02:31:46AM +0300, Jarkko Sakkinen wrote:
> 
> It's a bummer but uapi is the god in the end. Since TPM does not do it
> today, that behaviour must be supported forever. That's why a boot option
> AND a warning would be the best compromise.
> 

It's not UAPI if there is no way for userspace to tell if it changed.

- Eric

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 23:34                         ` Eric Biggers
@ 2021-04-01  1:11                           ` Herbert Xu
  2021-04-01  5:50                             ` Jarkko Sakkinen
  2021-04-01  5:46                           ` Jarkko Sakkinen
  1 sibling, 1 reply; 71+ messages in thread
From: Herbert Xu @ 2021-04-01  1:11 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Jarkko Sakkinen, David Gstir, Sumit Garg, Ahmad Fatoum,
	Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, David S. Miller,
	Udit Agarwal, Jan Luebbe, Franck Lenormand, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Wed, Mar 31, 2021 at 04:34:29PM -0700, Eric Biggers wrote:
> On Thu, Apr 01, 2021 at 02:31:46AM +0300, Jarkko Sakkinen wrote:
> > 
> > It's a bummer but uapi is the god in the end. Since TPM does not do it
> > today, that behaviour must be supported forever. That's why a boot option
> > AND a warning would be the best compromise.
> 
> It's not UAPI if there is no way for userspace to tell if it changed.

Exactly.  UAPI is only an issue if something *breaks*.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 23:34                         ` Eric Biggers
  2021-04-01  1:11                           ` Herbert Xu
@ 2021-04-01  5:46                           ` Jarkko Sakkinen
  1 sibling, 0 replies; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-04-01  5:46 UTC (permalink / raw)
  To: Eric Biggers
  Cc: David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Wed, Mar 31, 2021 at 04:34:29PM -0700, Eric Biggers wrote:
> On Thu, Apr 01, 2021 at 02:31:46AM +0300, Jarkko Sakkinen wrote:
> > 
> > It's a bummer but uapi is the god in the end. Since TPM does not do it
> > today, that behaviour must be supported forever. That's why a boot option
> > AND a warning would be the best compromise.
> > 
> 
> It's not UAPI if there is no way for userspace to tell if it changed.
> 
> - Eric

It's enough uapi for me. People might assume that the entropy source is
TPM for this, since it has been so far.

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01  1:11                           ` Herbert Xu
@ 2021-04-01  5:50                             ` Jarkko Sakkinen
  2021-04-01  6:03                               ` Eric Biggers
  0 siblings, 1 reply; 71+ messages in thread
From: Jarkko Sakkinen @ 2021-04-01  5:50 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Eric Biggers, David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, David S. Miller,
	Udit Agarwal, Jan Luebbe, Franck Lenormand, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Thu, Apr 01, 2021 at 12:11:32PM +1100, Herbert Xu wrote:
> On Wed, Mar 31, 2021 at 04:34:29PM -0700, Eric Biggers wrote:
> > On Thu, Apr 01, 2021 at 02:31:46AM +0300, Jarkko Sakkinen wrote:
> > > 
> > > It's a bummer but uapi is the god in the end. Since TPM does not do it
> > > today, that behaviour must be supported forever. That's why a boot option
> > > AND a warning would be the best compromise.
> > 
> > It's not UAPI if there is no way for userspace to tell if it changed.
> 
> Exactly.  UAPI is only an issue if something *breaks*.

If there's even one user that comes shouting that he has a user space
configuration, where e.g. rng entropy is consumed constantly and the
code assumes that trusted keys does not add to that, then something
would break.

It would be a crap user space yes, but I don't want to go on reverting
because of that. I think there is small but still existing chance that
something could break.

Why not just add a boot parameter instead of making brutal enforcing
changes, indirectly visible to the user space?

/Jarkko

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01  5:50                             ` Jarkko Sakkinen
@ 2021-04-01  6:03                               ` Eric Biggers
  0 siblings, 0 replies; 71+ messages in thread
From: Eric Biggers @ 2021-04-01  6:03 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Herbert Xu, David Gstir, Sumit Garg, Ahmad Fatoum, Mimi Zohar,
	Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, David S. Miller,
	Udit Agarwal, Jan Luebbe, Franck Lenormand, keyrings,
	linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

On Thu, Apr 01, 2021 at 08:50:05AM +0300, Jarkko Sakkinen wrote:
> On Thu, Apr 01, 2021 at 12:11:32PM +1100, Herbert Xu wrote:
> > On Wed, Mar 31, 2021 at 04:34:29PM -0700, Eric Biggers wrote:
> > > On Thu, Apr 01, 2021 at 02:31:46AM +0300, Jarkko Sakkinen wrote:
> > > > 
> > > > It's a bummer but uapi is the god in the end. Since TPM does not do it
> > > > today, that behaviour must be supported forever. That's why a boot option
> > > > AND a warning would be the best compromise.
> > > 
> > > It's not UAPI if there is no way for userspace to tell if it changed.
> > 
> > Exactly.  UAPI is only an issue if something *breaks*.
> 
> If there's even one user that comes shouting that he has a user space
> configuration, where e.g. rng entropy is consumed constantly and the
> code assumes that trusted keys does not add to that, then something
> would break.
> 
> It would be a crap user space yes, but I don't want to go on reverting
> because of that. I think there is small but still existing chance that
> something could break.

random.c no longer provides any interfaces that subtract entropy credits, as
that was never something that made sense.  So "consuming" all the entropy from
random.c isn't a thing anymore.

> 
> Why not just add a boot parameter instead of making brutal enforcing
> changes, indirectly visible to the user space?

Why not just fix this bug instead of providing an option to fix it that everyone
will need to remember to provide?

- Eric

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 23:30                       ` Jarkko Sakkinen
@ 2021-04-01  7:41                         ` Ahmad Fatoum
  0 siblings, 0 replies; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01  7:41 UTC (permalink / raw)
  To: Jarkko Sakkinen, Sumit Garg
  Cc: David Gstir, Mimi Zohar, Horia Geantă,
	Jonathan Corbet, David Howells, James Bottomley, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, Franck Lenormand,
	keyrings, linux-crypto, linux-doc, linux-integrity, linux-kernel,
	linux-security-module

Hello Jarkko,

On 01.04.21 01:30, Jarkko Sakkinen wrote:
>> Option (C) sounds reasonable to me but I would rather prefer an info
>> message rather than warning as otherwise it would reflect that we are
>> enforcing kernel RNG choice for a user to trust upon.
> 
> I gave some though on this.
> 
> I take TEE as it is but I'd expect the CAAM patch set sort out this option
> with some patch.

Is it ok to warn if a user requests vendor RNG with CAAM and default
to the kernel RNG? 

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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30 21:50     ` Richard Weinberger
@ 2021-04-01 10:04       ` Ahmad Fatoum
  2021-04-01 10:20         ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 10:04 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, Sumit Garg, linux-integrity,
	keyrings, Linux Crypto Mailing List, LKML, LSM

Hello Richard,

On 30.03.21 23:50, Richard Weinberger wrote:
> Ahmad,
> 
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
>>     TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
>>     echo $TABLE | dmsetup create mydev
>>     echo $TABLE | dmsetup load mydev
> 
> Do you also plan to add support for this to cryptsetup?
> 
> David and I have added (rough) support for our CAAM/DCP based keyrings
> to cryptsetup:
> https://github.com/sigma-star/cryptsetup/tree/rw/plain
> 
> I'm pretty sure with minimal changes it will work with your recent approach too.

I am using dmsetup directly in my project. I am not familiar with cryptsetup
plain. What benefits do you see with this over direct dmsetup?

What I'd like to see eventually is support for this with LUKS.
There is a RFE on trusted keys and cryptsetup on the project's repository[1].

The behavior I'd want it that the LUKS header would point at the trusted key
blob to use and load it into the kernel. This of course means that
you won't be able to have multiple keys for the encrypted partition.

[1]: https://gitlab.com/cryptsetup/cryptsetup/-/issues/443

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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 19:36             ` Richard Weinberger
@ 2021-04-01 10:06               ` Ahmad Fatoum
  2021-04-01 13:20                 ` Sumit Garg
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 10:06 UTC (permalink / raw)
  To: Richard Weinberger, James Bottomley
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	david, Franck Lenormand, Sumit Garg, linux-integrity, open list,
	ASYMMETRIC KEYS, Linux Crypto Mailing List, linux-kernel, LSM

Hello Richard,

On 31.03.21 21:36, Richard Weinberger wrote:
> James,
> 
> ----- Ursprüngliche Mail -----
>> Von: "James Bottomley" <jejb@linux.ibm.com>
>> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
>>
>> and part of the design of the file is that it's distinguishable either
>> in DER or PEM (by the guards) format so any crypto application can know
>> it's dealing with a TPM key simply by inspecting the file.  I think you
>> need the same thing for CAAM and any other format.
>>
>> We're encouraging new ASN.1 formats to be of the form
>>
>> SEQUENCE {
>>    type   OBJECT IDENTIFIER
>>    ... key specific fields ...
>> }
>>
>> Where you choose a defined OID to represent the key and that means
>> every key even in DER form begins with a unique binary signature.
> 
> I like this idea.
> Ahmad, what do you think?
> 
> That way we could also get rid off the kernel parameter and all the fall back logic,
> given that we find a way to reliable detect TEE blobs too...

Sounds good to me. Sumit, your thoughts on doing this for TEE as well?

> 
> Thanks,
> //richard
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-31 18:35   ` Richard Weinberger
@ 2021-04-01 10:15     ` Ahmad Fatoum
  2021-04-01 10:23       ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 10:15 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn,
	Horia Geantă,
	Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
	Jan Luebbe, David Gstir, Franck LENORMAND, Sumit Garg, keyrings,
	Linux Crypto Mailing List, linux-doc, linux-integrity, LKML, LSM

Hello Richard,

On 31.03.21 20:35, Richard Weinberger wrote:
> Ahmad,
> 
> On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> +#define KEYMOD "kernel:trusted"
> 
> why is the CAAM key modifier hard coded?
> I'd love to have way to pass my own modifier.
> 
> That way existing blobs can also be used with this implementation.
> IIRC the NXP vendor tree uses "SECURE_KEY" as default modifier.

Being binary compatible with other implementations is not an objective
for this patch set. If you need to migrate I'd suggest to get out a
clear text password and side-load it into the trusted key framework.

Jan and Mimi discussed this some weeks back:

https://lore.kernel.org/linux-integrity/e8f149cddce55a4e4615396108e4c900cbec75a8.camel@pengutronix.de/

There's no code to implement this yet though.

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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:04       ` Ahmad Fatoum
@ 2021-04-01 10:20         ` Richard Weinberger
  2021-04-01 10:28           ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 10:20 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> I'm pretty sure with minimal changes it will work with your recent approach too.
> 
> I am using dmsetup directly in my project. I am not familiar with cryptsetup
> plain. What benefits do you see with this over direct dmsetup?

cryptsetup is the de-facto standard to setup encrypted block devices.
There is a lot of existing tooling around cryptsetup already (systemd, etc..),
so being able to use CAAM keys for dm-crypt with cryptsetup seems natural to me.
Plain mode allows setting up dm-crypt without LUKS.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:15     ` Ahmad Fatoum
@ 2021-04-01 10:23       ` Richard Weinberger
  0 siblings, 0 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 10:23 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jonathan Corbet, David Howells, Jarkko Sakkinen, James Bottomley,
	Mimi Zohar, kernel, James Morris, Serge E. Hallyn, horia geanta,
	aymen sghaier, Herbert Xu, davem, Udit Agarwal, Jan Luebbe,
	david, Franck Lenormand, Sumit Garg, open list, ASYMMETRIC KEYS,
	Linux Crypto Mailing List, Linux Doc Mailing List,
	linux-integrity, linux-kernel, LSM

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> That way existing blobs can also be used with this implementation.
>> IIRC the NXP vendor tree uses "SECURE_KEY" as default modifier.
> 
> Being binary compatible with other implementations is not an objective
> for this patch set. If you need to migrate I'd suggest to get out a
> clear text password and side-load it into the trusted key framework.

Compatibility is only one argument, IMHO the much stronger argument is that there are
people out there that want to salt the CAAM blob with a key modifier of their
own choice.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:20         ` Richard Weinberger
@ 2021-04-01 10:28           ` Ahmad Fatoum
  2021-04-01 10:53             ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 10:28 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Hello,

On 01.04.21 12:20, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>>> I'm pretty sure with minimal changes it will work with your recent approach too.
>>
>> I am using dmsetup directly in my project. I am not familiar with cryptsetup
>> plain. What benefits do you see with this over direct dmsetup?
> 
> cryptsetup is the de-facto standard to setup encrypted block devices.
> There is a lot of existing tooling around cryptsetup already (systemd, etc..),

Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
won't work.

> so being able to use CAAM keys for dm-crypt with cryptsetup seems natural to me.
> Plain mode allows setting up dm-crypt without LUKS.

I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
to do the same I did with dmsetup, but with cryptsetup plain instead could
help me to see the benefits?

Cheers,
Ahmad

> 
> Thanks,
> //richard
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:28           ` Ahmad Fatoum
@ 2021-04-01 10:53             ` Richard Weinberger
  2021-04-01 10:57               ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 10:53 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Ahmad,

----- Ursprüngliche Mail -----
> Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
> the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
> won't work.

Nah, I meant existing scripts/service Files.

> I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
> to do the same I did with dmsetup, but with cryptsetup plain instead could
> help me to see the benefits?

My reasoning is simple, why do I need a different tool when there is already one
that could do the task too?
Usually the systems I get my hands on use already dm-crypt with cryptsetup in some way.
So I have the tooling already in my initramfs, etc.. and need to adopt the callers of cryptsetup a little.

If I need all of a sudden different/additional tooling, it means more work, more docs to write,
more hassle with crypto/system reviewers, etc...

I don't want you to force to use cryptsetup.
The only goal was pointing out that it can be done with cryptsetup and that there
is already code such that no work is done twice.
One the kernel side it does not matter.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:53             ` Richard Weinberger
@ 2021-04-01 10:57               ` Ahmad Fatoum
  2021-04-01 11:05                 ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 10:57 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Hello Richard,

On 01.04.21 12:53, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
>> the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
>> won't work.
> 
> Nah, I meant existing scripts/service Files.
> 
>> I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
>> to do the same I did with dmsetup, but with cryptsetup plain instead could
>> help me to see the benefits?
> 
> My reasoning is simple, why do I need a different tool when there is already one
> that could do the task too?
> Usually the systems I get my hands on use already dm-crypt with cryptsetup in some way.
> So I have the tooling already in my initramfs, etc.. and need to adopt the callers of cryptsetup a little.
> 
> If I need all of a sudden different/additional tooling, it means more work, more docs to write,
> more hassle with crypto/system reviewers, etc...
> 
> I don't want you to force to use cryptsetup.

I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take
a look and see if cryptsetup plain maybe a suitable stop-gap solution for us.

> The only goal was pointing out that it can be done with cryptsetup and that there
> is already code such that no work is done twice.
> One the kernel side it does not matter.

Thanks for the pointer,
Ahmad

> 
> Thanks,
> //richard
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:57               ` Ahmad Fatoum
@ 2021-04-01 11:05                 ` Richard Weinberger
  2021-04-01 11:13                   ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 11:05 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> I don't want you to force to use cryptsetup.
> 
> I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take

But using LUKS would mean that cryptsetup has access to the plain disc encryption key material?
This would be a no-go for many systems out there, key material must not accessible to userspace.
I know, distrusting userspace root is not easy, but doable. :)

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-17 14:08   ` Ahmad Fatoum
  2021-03-30 21:50     ` Richard Weinberger
  2021-03-30 22:04     ` Richard Weinberger
@ 2021-04-01 11:11     ` David Howells
  2 siblings, 0 replies; 71+ messages in thread
From: David Howells @ 2021-04-01 11:11 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: dhowells, Ahmad Fatoum, Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, James Morris, Serge E. Hallyn,
	Steffen Trumtrar, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck LENORMAND, Sumit Garg, linux-integrity, keyrings,
	Linux Crypto Mailing List, LKML, LSM

Richard Weinberger <richard.weinberger@gmail.com> wrote:

> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s

I wonder...  Does it make sense to add a new variant of the add_key() and
keyctl_instantiate() syscalls that takes an additional parameter string,
separate from the payload blob?

       key_serial_t add_key2(const char *type, const char *description,
			     const char *params,
                             const void *payload, size_t plen,
                             key_serial_t keyring);

which could then by used, say:

	keyctl add --payload=~/kmk.blob trusted $KEYNAME "backendtype caam load" @s

This would then appear in

	struct key_preparsed_payload {
		const char	*orig_description;
		char		*description;
		char		*params;	<---
		union key_payload payload;
		const void	*data;
		size_t		datalen;
		size_t		quotalen;
		time64_t	expiry;
	};

params would then be NULL for add_key().

If add_key2() is not available, the --payload param gets concatenated to the
parameters string.

Might be too complicated, I guess.  Though it might make sense just to do the
concatenation inside the keyctl program.

David


^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 11:05                 ` Richard Weinberger
@ 2021-04-01 11:13                   ` Ahmad Fatoum
  2021-04-01 11:16                     ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 11:13 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Hello Richard,

On 01.04.21 13:05, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>>> I don't want you to force to use cryptsetup.
>>
>> I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take
> 
> But using LUKS would mean that cryptsetup has access to the plain disc encryption key material?
> This would be a no-go for many systems out there, key material must not accessible to userspace.
> I know, distrusting userspace root is not easy, but doable. :)

The LUKS2 format supports tokens. I see no reason why the encrypted blob
couldn't be stored there along with the usual metadata. cryptsetup would
then load it as kernel trusted key and use it for dmcrypt decryption.

This will mean we have to part ways with features such as having multiple
keys, but I think it's worth it to have a plug and play solution for
trusted keys.

Of course, someone needs to implement this first ^^.

Cheers,
Ahmad

> 
> Thanks,
> //richard
> 

-- 
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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 11:13                   ` Ahmad Fatoum
@ 2021-04-01 11:16                     ` Richard Weinberger
  0 siblings, 0 replies; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 11:16 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, Sumit Garg, linux-integrity,
	open list, ASYMMETRIC KEYS, Linux Crypto Mailing List,
	linux-kernel, LSM

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> But using LUKS would mean that cryptsetup has access to the plain disc
>> encryption key material?
>> This would be a no-go for many systems out there, key material must not
>> accessible to userspace.
>> I know, distrusting userspace root is not easy, but doable. :)
> 
> The LUKS2 format supports tokens. I see no reason why the encrypted blob
> couldn't be stored there along with the usual metadata. cryptsetup would
> then load it as kernel trusted key and use it for dmcrypt decryption.
> 
> This will mean we have to part ways with features such as having multiple
> keys, but I think it's worth it to have a plug and play solution for
> trusted keys.

Ah, now I can follow your thoughts!
Yes, that would be nice to have. :)

I kind of assumed you want to use LUKS with passphrases and CAAM blobs.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-30 22:04     ` Richard Weinberger
  2021-03-30 22:16       ` James Bottomley
@ 2021-04-01 12:55       ` Sumit Garg
  2021-04-01 13:17         ` Richard Weinberger
  1 sibling, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-04-01 12:55 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Ahmad Fatoum, Jarkko Sakkinen, Horia Geantă,
	Mimi Zohar, Aymen Sghaier, Herbert Xu, David S. Miller,
	James Bottomley, kernel, David Howells, James Morris,
	Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal, Jan Luebbe,
	David Gstir, Franck LENORMAND, linux-integrity,
	open list:ASYMMETRIC KEYS, Linux Crypto Mailing List, LKML, LSM

Hi Richard,

On Wed, 31 Mar 2021 at 03:34, Richard Weinberger
<richard.weinberger@gmail.com> wrote:
>
> Ahmad,
>
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s
>

IIUC, this would require support for multiple trusted keys backends at
runtime but currently the trusted keys subsystem only supports a
single backend which is selected via kernel module parameter during
boot.

So the trusted keys framework needs to evolve to support multiple
trust sources at runtime but I would like to understand the use-cases
first. IMO, selecting the best trust source available on a platform
for trusted keys should be a one time operation, so why do we need to
have other backends available at runtime as well?

-Sumit

> --
> Thanks,
> //richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 12:55       ` Sumit Garg
@ 2021-04-01 13:17         ` Richard Weinberger
  2021-04-01 13:30           ` Ahmad Fatoum
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 13:17 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, James Bottomley, kernel,
	David Howells, James Morris, Serge E. Hallyn, Steffen Trumtrar,
	Udit Agarwal, Jan Luebbe, david, Franck Lenormand,
	linux-integrity, open list, ASYMMETRIC KEYS,
	Linux Crypto Mailing List, linux-kernel, LSM

Sumit,

----- Ursprüngliche Mail -----
> Von: "Sumit Garg" <sumit.garg@linaro.org>
> IIUC, this would require support for multiple trusted keys backends at
> runtime but currently the trusted keys subsystem only supports a
> single backend which is selected via kernel module parameter during
> boot.
> 
> So the trusted keys framework needs to evolve to support multiple
> trust sources at runtime but I would like to understand the use-cases
> first. IMO, selecting the best trust source available on a platform
> for trusted keys should be a one time operation, so why do we need to
> have other backends available at runtime as well?

I thought about devices with a TPM-Chip and CAAM.
IMHO allowing only one backend at the same time is a little over simplified. 

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 10:06               ` Ahmad Fatoum
@ 2021-04-01 13:20                 ` Sumit Garg
  2021-04-01 18:26                   ` James Bottomley
  0 siblings, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-04-01 13:20 UTC (permalink / raw)
  To: Ahmad Fatoum, James Bottomley
  Cc: Richard Weinberger, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, linux-integrity, open list,
	ASYMMETRIC KEYS, Linux Crypto Mailing List, linux-kernel, LSM

On Thu, 1 Apr 2021 at 15:36, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Richard,
>
> On 31.03.21 21:36, Richard Weinberger wrote:
> > James,
> >
> > ----- Ursprüngliche Mail -----
> >> Von: "James Bottomley" <jejb@linux.ibm.com>
> >> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> >>
> >> and part of the design of the file is that it's distinguishable either
> >> in DER or PEM (by the guards) format so any crypto application can know
> >> it's dealing with a TPM key simply by inspecting the file.  I think you
> >> need the same thing for CAAM and any other format.
> >>
> >> We're encouraging new ASN.1 formats to be of the form
> >>
> >> SEQUENCE {
> >>    type   OBJECT IDENTIFIER
> >>    ... key specific fields ...
> >> }
> >>
> >> Where you choose a defined OID to represent the key and that means
> >> every key even in DER form begins with a unique binary signature.
> >
> > I like this idea.
> > Ahmad, what do you think?
> >
> > That way we could also get rid off the kernel parameter and all the fall back logic,
> > given that we find a way to reliable detect TEE blobs too...
>
> Sounds good to me. Sumit, your thoughts on doing this for TEE as well?
>

AFAIU, ASN.1 formating should be independent of trusted keys backends
which could be abstracted to trusted keys core layer so that every
backend could be plugged in seamlessly.

James,

Would it be possible to achieve this?

-Sumit

> >
> > Thanks,
> > //richard
> >
>
> --
> 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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 13:17         ` Richard Weinberger
@ 2021-04-01 13:30           ` Ahmad Fatoum
  2021-04-01 13:52             ` Sumit Garg
  0 siblings, 1 reply; 71+ messages in thread
From: Ahmad Fatoum @ 2021-04-01 13:30 UTC (permalink / raw)
  To: Richard Weinberger, Sumit Garg
  Cc: Jarkko Sakkinen, horia geanta, Mimi Zohar, aymen sghaier,
	Herbert Xu, davem, James Bottomley, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, linux-integrity, open list,
	ASYMMETRIC KEYS, Linux Crypto Mailing List, linux-kernel, LSM

Hello Richard, Sumit,

On 01.04.21 15:17, Richard Weinberger wrote:
> Sumit,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Sumit Garg" <sumit.garg@linaro.org>
>> IIUC, this would require support for multiple trusted keys backends at
>> runtime but currently the trusted keys subsystem only supports a
>> single backend which is selected via kernel module parameter during
>> boot.
>>
>> So the trusted keys framework needs to evolve to support multiple
>> trust sources at runtime but I would like to understand the use-cases
>> first. IMO, selecting the best trust source available on a platform
>> for trusted keys should be a one time operation, so why do we need to
>> have other backends available at runtime as well?
> 
> I thought about devices with a TPM-Chip and CAAM.
> IMHO allowing only one backend at the same time is a little over simplified. 

It is, but I'd rather leave this until it's actually needed.
What can be done now is adopting a format for the exported keys that would
make this extension seamless in future.

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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 13:30           ` Ahmad Fatoum
@ 2021-04-01 13:52             ` Sumit Garg
  2021-04-01 13:59               ` Richard Weinberger
  0 siblings, 1 reply; 71+ messages in thread
From: Sumit Garg @ 2021-04-01 13:52 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Richard Weinberger, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, James Bottomley, kernel,
	David Howells, James Morris, Serge E. Hallyn, Steffen Trumtrar,
	Udit Agarwal, Jan Luebbe, david, Franck Lenormand,
	linux-integrity, open list, ASYMMETRIC KEYS,
	Linux Crypto Mailing List, linux-kernel, LSM

On Thu, 1 Apr 2021 at 19:00, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Richard, Sumit,
>
> On 01.04.21 15:17, Richard Weinberger wrote:
> > Sumit,
> >
> > ----- Ursprüngliche Mail -----
> >> Von: "Sumit Garg" <sumit.garg@linaro.org>
> >> IIUC, this would require support for multiple trusted keys backends at
> >> runtime but currently the trusted keys subsystem only supports a
> >> single backend which is selected via kernel module parameter during
> >> boot.
> >>
> >> So the trusted keys framework needs to evolve to support multiple
> >> trust sources at runtime but I would like to understand the use-cases
> >> first. IMO, selecting the best trust source available on a platform
> >> for trusted keys should be a one time operation, so why do we need to
> >> have other backends available at runtime as well?
> >
> > I thought about devices with a TPM-Chip and CAAM.

In this case why would one prefer to use CAAM when you have standards
compliant TPM-Chip which additionally offers sealing to specific PCR
(integrity measurement) values.

> > IMHO allowing only one backend at the same time is a little over simplified.
>
> It is, but I'd rather leave this until it's actually needed.
> What can be done now is adopting a format for the exported keys that would
> make this extension seamless in future.
>

+1

-Sumit

> 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 |

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 13:52             ` Sumit Garg
@ 2021-04-01 13:59               ` Richard Weinberger
  2021-04-01 14:12                 ` Sumit Garg
  0 siblings, 1 reply; 71+ messages in thread
From: Richard Weinberger @ 2021-04-01 13:59 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, James Bottomley, kernel,
	David Howells, James Morris, Serge E. Hallyn, Steffen Trumtrar,
	Udit Agarwal, Jan Luebbe, david, Franck Lenormand,
	linux-integrity, open list, ASYMMETRIC KEYS,
	Linux Crypto Mailing List, linux-kernel, LSM

Sumit,

----- Ursprüngliche Mail -----
> Von: "Sumit Garg" <sumit.garg@linaro.org>
> In this case why would one prefer to use CAAM when you have standards
> compliant TPM-Chip which additionally offers sealing to specific PCR
> (integrity measurement) values.

I don't think we can dictate what good/sane solutions are and which are not.
Both CAAM and TPM have pros and cons, I don't see why supporting both is a bad idea.

>> > IMHO allowing only one backend at the same time is a little over simplified.
>>
>> It is, but I'd rather leave this until it's actually needed.
>> What can be done now is adopting a format for the exported keys that would
>> make this extension seamless in future.
>>
> 
> +1

As long we don't make multiple backends at runtime impossible I'm
fine and will happily add support for it when needed. :-)

Thanks,
//richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 13:59               ` Richard Weinberger
@ 2021-04-01 14:12                 ` Sumit Garg
  0 siblings, 0 replies; 71+ messages in thread
From: Sumit Garg @ 2021-04-01 14:12 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Ahmad Fatoum, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, James Bottomley, kernel,
	David Howells, James Morris, Serge E. Hallyn, Steffen Trumtrar,
	Udit Agarwal, Jan Luebbe, david, Franck Lenormand,
	linux-integrity, open list, ASYMMETRIC KEYS,
	Linux Crypto Mailing List, linux-kernel, LSM

On Thu, 1 Apr 2021 at 19:29, Richard Weinberger <richard@nod.at> wrote:
>
> Sumit,
>
> ----- Ursprüngliche Mail -----
> > Von: "Sumit Garg" <sumit.garg@linaro.org>
> > In this case why would one prefer to use CAAM when you have standards
> > compliant TPM-Chip which additionally offers sealing to specific PCR
> > (integrity measurement) values.
>
> I don't think we can dictate what good/sane solutions are and which are not.
> Both CAAM and TPM have pros and cons, I don't see why supporting both is a bad idea.

I didn't mean to say that supporting both is a bad idea but rather I
was looking for use-cases where one time selection of the best trust
source (whether it be a TPM or TEE or CAAM etc.) for a platform
wouldn't suffice for user needs.

>
> >> > IMHO allowing only one backend at the same time is a little over simplified.
> >>
> >> It is, but I'd rather leave this until it's actually needed.
> >> What can be done now is adopting a format for the exported keys that would
> >> make this extension seamless in future.
> >>
> >
> > +1
>
> As long we don't make multiple backends at runtime impossible I'm
> fine and will happily add support for it when needed. :-)
>

You are most welcome to add such support. I will be happy to review it.

-Sumit

> Thanks,
> //richard

^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-04-01 13:20                 ` Sumit Garg
@ 2021-04-01 18:26                   ` James Bottomley
  0 siblings, 0 replies; 71+ messages in thread
From: James Bottomley @ 2021-04-01 18:26 UTC (permalink / raw)
  To: Sumit Garg, Ahmad Fatoum
  Cc: Richard Weinberger, Jarkko Sakkinen, horia geanta, Mimi Zohar,
	aymen sghaier, Herbert Xu, davem, kernel, David Howells,
	James Morris, Serge E. Hallyn, Steffen Trumtrar, Udit Agarwal,
	Jan Luebbe, david, Franck Lenormand, linux-integrity, open list,
	ASYMMETRIC KEYS, Linux Crypto Mailing List, linux-kernel, LSM

On Thu, 2021-04-01 at 18:50 +0530, Sumit Garg wrote:
> On Thu, 1 Apr 2021 at 15:36, Ahmad Fatoum <a.fatoum@pengutronix.de>
> wrote:
> > Hello Richard,
> > 
> > On 31.03.21 21:36, Richard Weinberger wrote:
> > > James,
> > > 
> > > ----- Ursprüngliche Mail -----
> > > > Von: "James Bottomley" <jejb@linux.ibm.com>
> > > > Well, yes.  For the TPM, there's a defined ASN.1 format for the
> > > > keys:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> > > > 
> > > > and part of the design of the file is that it's distinguishable
> > > > either
> > > > in DER or PEM (by the guards) format so any crypto application
> > > > can know
> > > > it's dealing with a TPM key simply by inspecting the file.  I
> > > > think you
> > > > need the same thing for CAAM and any other format.
> > > > 
> > > > We're encouraging new ASN.1 formats to be of the form
> > > > 
> > > > SEQUENCE {
> > > >    type   OBJECT IDENTIFIER
> > > >    ... key specific fields ...
> > > > }
> > > > 
> > > > Where you choose a defined OID to represent the key and that
> > > > means
> > > > every key even in DER form begins with a unique binary
> > > > signature.
> > > 
> > > I like this idea.
> > > Ahmad, what do you think?
> > > 
> > > That way we could also get rid off the kernel parameter and all
> > > the fall back logic,
> > > given that we find a way to reliable detect TEE blobs too...
> > 
> > Sounds good to me. Sumit, your thoughts on doing this for TEE as
> > well?
> > 
> 
> AFAIU, ASN.1 formating should be independent of trusted keys backends
> which could be abstracted to trusted keys core layer so that every
> backend could be plugged in seamlessly.
> 
> James,
> 
> Would it be possible to achieve this?

I'm not quite sure what you're asking.  The saved format of the keys is
particular to the underlying hardware.  The ASN.1 wraps this so we have
an identifier, some useful information to help load the key (like the
policy systemements) and then the blobs the hardware expects.

This makes the ASN.1 specific to the backend but having a
distinguishing OID that allows anyone to tell which backend it needs
from the file.

So you can call the ASN.1 format that begins with the type OID, the
"universal" format, but if you mean there should be a standard ASN.1
format for all keys that defines all the fields then that's not
possible because the fields after type are very specific to the
underlying hardware.

James



^ permalink raw reply	[flat|nested] 71+ messages in thread

* Re: [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
  2021-03-24 16:14         ` James Bottomley
  2021-03-24 20:49           ` Mimi Zohar
@ 2021-04-02  1:49           ` Serge E. Hallyn
  1 sibling, 0 replies; 71+ messages in thread
From: Serge E. Hallyn @ 2021-04-02  1:49 UTC (permalink / raw)
  To: James Bottomley
  Cc: Mimi Zohar, Ahmad Fatoum, Horia Geantă,
	Jonathan Corbet, David Howells, Jarkko Sakkinen, kernel,
	James Morris, Serge E. Hallyn, Aymen Sghaier, Herbert Xu,
	David S. Miller, Udit Agarwal, Jan Luebbe, David Gstir,
	Franck Lenormand, Sumit Garg, keyrings, linux-crypto, linux-doc,
	linux-integrity, linux-kernel, linux-security-module

On Wed, Mar 24, 2021 at 09:14:02AM -0700, James Bottomley wrote:
> On Tue, 2021-03-23 at 14:07 -0400, Mimi Zohar wrote:
> > On Tue, 2021-03-23 at 17:35 +0100, Ahmad Fatoum wrote:
> > > Hello Horia,
> > > 
> > > On 21.03.21 21:48, Horia Geantă wrote:
> > > > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> > > > [...]
> > > > > +struct trusted_key_ops caam_trusted_key_ops = {
> > > > > +	.migratable = 0, /* non-migratable */
> > > > > +	.init = trusted_caam_init,
> > > > > +	.seal = trusted_caam_seal,
> > > > > +	.unseal = trusted_caam_unseal,
> > > > > +	.exit = trusted_caam_exit,
> > > > > +};
> > > > caam has random number generation capabilities, so it's worth
> > > > using that
> > > > by implementing .get_random.
> > > 
> > > If the CAAM HWRNG is already seeding the kernel RNG, why not use
> > > the kernel's?
> > > 
> > > Makes for less code duplication IMO.
> > 
> > Using kernel RNG, in general, for trusted keys has been discussed
> > before.   Please refer to Dave Safford's detailed explanation for not
> > using it [1].
> > 
> > thanks,
> > 
> > Mimi
> > 
> > [1] 
> > https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035F2A38B@ALPMBAPA12.e2k.ad.ge.com/
> 
> I still don't think relying on one source of randomness to be
> cryptographically secure is a good idea.  The fear of bugs in the
> kernel entropy pool is reasonable, but since it's widely used they're
> unlikely to persist very long.

I'm not sure I agree - remember
https://www.schneier.com/blog/archives/2008/05/random_number_b.html ?  You'd
surely expect that to have been found quickly.

>   Studies have shown that some TPMs
> (notably the chinese manufactured ones) have suspicious failures in
> their RNGs:
> 
> https://www.researchgate.net/publication/45934562_Benchmarking_the_True_Random_Number_Generator_of_TPM_Chips
> 
> And most cryptograhpers recommend using a TPM for entropy mixing rather
> than directly:
> 
> https://blog.cryptographyengineering.com/category/rngs/
> 
> The TPMFail paper also shows that in spite of NIST certification
> things can go wrong with a TPM:
> 
> https://tpm.fail/

In this thread I've seen argument over "which is better" and "which is user api",
but noone's mentioned fips.  Unfortunately, so long as kernel rng refuses to be
fips-friendly (cf https://lkml.org/lkml/2020/9/21/157), making CAAM based trusted
keys depend on kernel rng would make them impossible to use in fips certified
applications without a forked kernel.

So I definitely am in favor of a config or kernel command line option to drive
which rng to use.

^ permalink raw reply	[flat|nested] 71+ messages in thread

end of thread, other threads:[~2021-04-02  1:58 UTC | newest]

Thread overview: 71+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 17:01 [PATCH v1 0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
2021-03-16 17:01 ` [PATCH v1 1/3] crypto: caam - add in-kernel interface for blob generator Ahmad Fatoum
2021-03-21 20:46   ` Horia Geantă
2021-03-23 16:41     ` Ahmad Fatoum
2021-03-16 17:01 ` [PATCH v1 2/3] KEYS: trusted: implement fallback to kernel RNG Ahmad Fatoum
2021-03-16 17:01 ` [PATCH v1 3/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys Ahmad Fatoum
2021-03-16 19:22   ` Jarkko Sakkinen
2021-03-17 13:58     ` Ahmad Fatoum
2021-03-16 23:14   ` Richard Weinberger
2021-03-17  7:39     ` Sumit Garg
2021-03-17  8:07       ` Richard Weinberger
2021-03-17 14:02     ` Ahmad Fatoum
2021-03-30 21:28       ` Richard Weinberger
2021-03-21 20:48   ` Horia Geantă
2021-03-23 16:35     ` Ahmad Fatoum
2021-03-23 18:07       ` Mimi Zohar
2021-03-24  9:26         ` Ahmad Fatoum
2021-03-24 10:47           ` Sumit Garg
2021-03-24 14:07             ` Ahmad Fatoum
2021-03-25  5:26               ` Sumit Garg
2021-03-27 12:41                 ` David Gstir
2021-03-28 20:37                   ` Jarkko Sakkinen
2021-03-29 10:11                     ` Ahmad Fatoum
2021-03-31 23:29                       ` Jarkko Sakkinen
2021-03-30  7:26                     ` Sumit Garg
2021-03-31 23:30                       ` Jarkko Sakkinen
2021-04-01  7:41                         ` Ahmad Fatoum
2021-03-30 21:47                     ` Eric Biggers
2021-03-31 23:31                       ` Jarkko Sakkinen
2021-03-31 23:34                         ` Eric Biggers
2021-04-01  1:11                           ` Herbert Xu
2021-04-01  5:50                             ` Jarkko Sakkinen
2021-04-01  6:03                               ` Eric Biggers
2021-04-01  5:46                           ` Jarkko Sakkinen
2021-03-24 16:14         ` James Bottomley
2021-03-24 20:49           ` Mimi Zohar
2021-03-24 21:58             ` James Bottomley
2021-04-02  1:49           ` Serge E. Hallyn
2021-03-31 18:35   ` Richard Weinberger
2021-04-01 10:15     ` Ahmad Fatoum
2021-04-01 10:23       ` Richard Weinberger
2021-03-16 23:10 ` [PATCH v1 0/3] " Richard Weinberger
2021-03-17 14:08   ` Ahmad Fatoum
2021-03-30 21:50     ` Richard Weinberger
2021-04-01 10:04       ` Ahmad Fatoum
2021-04-01 10:20         ` Richard Weinberger
2021-04-01 10:28           ` Ahmad Fatoum
2021-04-01 10:53             ` Richard Weinberger
2021-04-01 10:57               ` Ahmad Fatoum
2021-04-01 11:05                 ` Richard Weinberger
2021-04-01 11:13                   ` Ahmad Fatoum
2021-04-01 11:16                     ` Richard Weinberger
2021-03-30 22:04     ` Richard Weinberger
2021-03-30 22:16       ` James Bottomley
2021-03-31 18:36         ` Richard Weinberger
2021-03-31 18:49           ` James Bottomley
2021-03-31 19:36             ` Richard Weinberger
2021-04-01 10:06               ` Ahmad Fatoum
2021-04-01 13:20                 ` Sumit Garg
2021-04-01 18:26                   ` James Bottomley
2021-04-01 12:55       ` Sumit Garg
2021-04-01 13:17         ` Richard Weinberger
2021-04-01 13:30           ` Ahmad Fatoum
2021-04-01 13:52             ` Sumit Garg
2021-04-01 13:59               ` Richard Weinberger
2021-04-01 14:12                 ` Sumit Garg
2021-04-01 11:11     ` David Howells
2021-03-21 20:01 ` Horia Geantă
2021-03-23 16:34   ` Ahmad Fatoum
2021-03-24  6:23     ` Sumit Garg
2021-03-23 16:37   ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).