[PATCH 0/3] DCP as trusted keys backend

[PATCH 0/3] DCP as trusted keys backend

[Richard Weinberger]

DCP is an IP core found on NXP SoCs such as i.mx6ull.
While its bigger brother, CAAM, can directly wrap and unwrap blobs
in hardware[0], DCP offers only the bare minimum and the blob
mechanism needs aid from software.

This series adds support for a new trusted keys backend that makes use
of DCP's feature to use hardware keys which can never be read out.

[0] https://lore.kernel.org/lkml/cover.56fff82362af6228372ea82e6bd7e586e23f0966.1615914058.git-series.a.fatoum@pengutronix.de/

---
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>

David Gstir (1):
  doc: trusted-encrypted: add DCP as new trust source

Richard Weinberger (2):
  crypto: mxs-dcp: Add support for hardware provided keys
  KEYS: trusted: Introduce support for NXP DCP-based trusted keys

 .../admin-guide/kernel-parameters.txt         |   1 +
 .../security/keys/trusted-encrypted.rst       |  84 ++++-
 MAINTAINERS                                   |   9 +
 drivers/crypto/mxs-dcp.c                      | 110 +++++-
 include/keys/trusted_dcp.h                    |  13 +
 include/linux/mxs-dcp.h                       |  19 +
 security/keys/trusted-keys/Makefile           |   1 +
 security/keys/trusted-keys/trusted_core.c     |   6 +-
 security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
 9 files changed, 554 insertions(+), 14 deletions(-)
 create mode 100644 include/keys/trusted_dcp.h
 create mode 100644 include/linux/mxs-dcp.h
 create mode 100644 security/keys/trusted-keys/trusted_dcp.c

-- 
2.26.2

[PATCH 0/3] DCP as trusted keys backend

[Richard Weinberger]

DCP is an IP core found on NXP SoCs such as i.mx6ull.
While its bigger brother, CAAM, can directly wrap and unwrap blobs
in hardware[0], DCP offers only the bare minimum and the blob
mechanism needs aid from software.

This series adds support for a new trusted keys backend that makes use
of DCP's feature to use hardware keys which can never be read out.

[0] https://lore.kernel.org/lkml/cover.56fff82362af6228372ea82e6bd7e586e23f0966.1615914058.git-series.a.fatoum@pengutronix.de/

---
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>

David Gstir (1):
  doc: trusted-encrypted: add DCP as new trust source

Richard Weinberger (2):
  crypto: mxs-dcp: Add support for hardware provided keys
  KEYS: trusted: Introduce support for NXP DCP-based trusted keys

 .../admin-guide/kernel-parameters.txt         |   1 +
 .../security/keys/trusted-encrypted.rst       |  84 ++++-
 MAINTAINERS                                   |   9 +
 drivers/crypto/mxs-dcp.c                      | 110 +++++-
 include/keys/trusted_dcp.h                    |  13 +
 include/linux/mxs-dcp.h                       |  19 +
 security/keys/trusted-keys/Makefile           |   1 +
 security/keys/trusted-keys/trusted_core.c     |   6 +-
 security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
 9 files changed, 554 insertions(+), 14 deletions(-)
 create mode 100644 include/keys/trusted_dcp.h
 create mode 100644 include/linux/mxs-dcp.h
 create mode 100644 security/keys/trusted-keys/trusted_dcp.c

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

DCP is capable to performing AES with hardware-bound keys.
These keys are not stored in main memory and are therefore not directly
accessible by the operating system.

So instead of feeding the key into DCP, we need to place a
reference to such a key before initiating the crypto operation.
Keys are referenced by a one byte identifiers.

DCP supports 6 different keys: 4 slots in the secure memory area,
a one time programmable key which can be burnt via on-chip fuses
and an unique device key.

Using these keys is restricted to in-kernel users that use them as building
block for other crypto tools such as trusted keys. Allowing userspace
(e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
risk, because there is no access control mechanism.

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: David Gstir <david@sigma-star.at>
Signed-off-by: David Gstir <david@sigma-star.at>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
 include/linux/mxs-dcp.h  |  19 +++++++
 2 files changed, 117 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/mxs-dcp.h

diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index d6a7784d2988..c3e0c0ccbc20 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/stmp_device.h>
 #include <linux/clk.h>
+#include <linux/mxs-dcp.h>
 
 #include <crypto/aes.h>
 #include <crypto/sha1.h>
@@ -101,6 +102,7 @@ struct dcp_async_ctx {
 	struct crypto_skcipher		*fallback;
 	unsigned int			key_len;
 	uint8_t				key[AES_KEYSIZE_128];
+	bool				refkey;
 };
 
 struct dcp_aes_req_ctx {
@@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
 #define MXS_DCP_CONTROL0_HASH_TERM		(1 << 13)
 #define MXS_DCP_CONTROL0_HASH_INIT		(1 << 12)
 #define MXS_DCP_CONTROL0_PAYLOAD_KEY		(1 << 11)
+#define MXS_DCP_CONTROL0_OTP_KEY		(1 << 10)
 #define MXS_DCP_CONTROL0_CIPHER_ENCRYPT		(1 << 8)
 #define MXS_DCP_CONTROL0_CIPHER_INIT		(1 << 9)
 #define MXS_DCP_CONTROL0_ENABLE_HASH		(1 << 6)
@@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
 #define MXS_DCP_CONTROL1_CIPHER_MODE_ECB	(0 << 4)
 #define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128	(0 << 0)
 
+#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT	8
+
 static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
 {
 	struct dcp *sdcp = global_sdcp;
@@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	struct dcp *sdcp = global_sdcp;
 	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
 	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
+	dma_addr_t src_phys, dst_phys, key_phys = {0};
+	bool key_referenced = actx->refkey;
 	int ret;
 
-	dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
-					     2 * AES_KEYSIZE_128,
-					     DMA_TO_DEVICE);
-	dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
-					     DCP_BUF_SZ, DMA_TO_DEVICE);
-	dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf,
-					     DCP_BUF_SZ, DMA_FROM_DEVICE);
+	if (!key_referenced) {
+		key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
+					  2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
+	}
+	src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, DCP_BUF_SZ,
+				  DMA_TO_DEVICE);
+	dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, DCP_BUF_SZ,
+				  DMA_FROM_DEVICE);
 
 	if (actx->fill % AES_BLOCK_SIZE) {
 		dev_err(sdcp->dev, "Invalid block size!\n");
@@ -240,8 +248,13 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 		    MXS_DCP_CONTROL0_INTERRUPT |
 		    MXS_DCP_CONTROL0_ENABLE_CIPHER;
 
-	/* Payload contains the key. */
-	desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
+	if (key_referenced) {
+		/* Set OTP key bit to select the key via KEY_SELECT. */
+		desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
+	} else {
+		/* Payload contains the key. */
+		desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
+	}
 
 	if (rctx->enc)
 		desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
@@ -255,6 +268,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	else
 		desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
 
+	if (key_referenced)
+		desc->control1 |= sdcp->coh->aes_key[0] << MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
+
 	desc->next_cmd_addr = 0;
 	desc->source = src_phys;
 	desc->destination = dst_phys;
@@ -265,8 +281,10 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	ret = mxs_dcp_start_dma(actx);
 
 aes_done_run:
-	dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
-			 DMA_TO_DEVICE);
+	if (!key_referenced) {
+		dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
+				 DMA_TO_DEVICE);
+	}
 	dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE);
 	dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE);
 
@@ -454,7 +472,7 @@ static int mxs_dcp_aes_enqueue(struct skcipher_request *req, int enc, int ecb)
 	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
 	int ret;
 
-	if (unlikely(actx->key_len != AES_KEYSIZE_128))
+	if (unlikely(actx->key_len != AES_KEYSIZE_128 && !actx->refkey))
 		return mxs_dcp_block_fallback(req, enc);
 
 	rctx->enc = enc;
@@ -501,6 +519,7 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	 * there can still be an operation in progress.
 	 */
 	actx->key_len = len;
+	actx->refkey = false;
 	if (len == AES_KEYSIZE_128) {
 		memcpy(actx->key, key, len);
 		return 0;
@@ -517,6 +536,33 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	return crypto_skcipher_setkey(actx->fallback, key, len);
 }
 
+static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
+				 unsigned int len)
+{
+	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
+	int ret = -EINVAL;
+
+	if (len != DCP_PAES_KEYSIZE)
+		goto out;
+
+	actx->key_len = len;
+	actx->refkey = true;
+
+	switch (key[0]) {
+	case DCP_PAES_KEY_SLOT0:
+	case DCP_PAES_KEY_SLOT1:
+	case DCP_PAES_KEY_SLOT2:
+	case DCP_PAES_KEY_SLOT3:
+	case DCP_PAES_KEY_UNIQUE:
+	case DCP_PAES_KEY_OTP:
+		memcpy(actx->key, key, len);
+		ret = 0;
+	}
+
+out:
+	return ret;
+}
+
 static int mxs_dcp_aes_fallback_init_tfm(struct crypto_skcipher *tfm)
 {
 	const char *name = crypto_tfm_alg_name(crypto_skcipher_tfm(tfm));
@@ -540,6 +586,13 @@ static void mxs_dcp_aes_fallback_exit_tfm(struct crypto_skcipher *tfm)
 	crypto_free_skcipher(actx->fallback);
 }
 
+static int mxs_dcp_paes_init_tfm(struct crypto_skcipher *tfm)
+{
+	crypto_skcipher_set_reqsize(tfm, sizeof(struct dcp_aes_req_ctx));
+
+	return 0;
+}
+
 /*
  * Hashing (SHA1/SHA256)
  */
@@ -882,6 +935,39 @@ static struct skcipher_alg dcp_aes_algs[] = {
 		.ivsize			= AES_BLOCK_SIZE,
 		.init			= mxs_dcp_aes_fallback_init_tfm,
 		.exit			= mxs_dcp_aes_fallback_exit_tfm,
+	}, {
+		.base.cra_name		= "ecb(paes)",
+		.base.cra_driver_name	= "ecb-paes-dcp",
+		.base.cra_priority	= 401,
+		.base.cra_alignmask	= 15,
+		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
+		.base.cra_blocksize	= AES_BLOCK_SIZE,
+		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
+		.base.cra_module	= THIS_MODULE,
+
+		.min_keysize		= DCP_PAES_KEYSIZE,
+		.max_keysize		= DCP_PAES_KEYSIZE,
+		.setkey			= mxs_dcp_aes_setrefkey,
+		.encrypt		= mxs_dcp_aes_ecb_encrypt,
+		.decrypt		= mxs_dcp_aes_ecb_decrypt,
+		.init			= mxs_dcp_paes_init_tfm,
+	}, {
+		.base.cra_name		= "cbc(paes)",
+		.base.cra_driver_name	= "cbc-paes-dcp",
+		.base.cra_priority	= 401,
+		.base.cra_alignmask	= 15,
+		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
+		.base.cra_blocksize	= AES_BLOCK_SIZE,
+		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
+		.base.cra_module	= THIS_MODULE,
+
+		.min_keysize		= DCP_PAES_KEYSIZE,
+		.max_keysize		= DCP_PAES_KEYSIZE,
+		.setkey			= mxs_dcp_aes_setrefkey,
+		.encrypt		= mxs_dcp_aes_cbc_encrypt,
+		.decrypt		= mxs_dcp_aes_cbc_decrypt,
+		.ivsize			= AES_BLOCK_SIZE,
+		.init			= mxs_dcp_paes_init_tfm,
 	},
 };
 
diff --git a/include/linux/mxs-dcp.h b/include/linux/mxs-dcp.h
new file mode 100644
index 000000000000..df6678ee10a1
--- /dev/null
+++ b/include/linux/mxs-dcp.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#ifndef MXS_DCP_H
+#define MXS_DCP_H
+
+#define DCP_PAES_KEYSIZE 1
+#define DCP_PAES_KEY_SLOT0 0x00
+#define DCP_PAES_KEY_SLOT1 0x01
+#define DCP_PAES_KEY_SLOT2 0x02
+#define DCP_PAES_KEY_SLOT3 0x03
+#define DCP_PAES_KEY_UNIQUE 0xfe
+#define DCP_PAES_KEY_OTP 0xff
+
+#endif /* MXS_DCP_H */
-- 
2.26.2

[PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

DCP is capable to performing AES with hardware-bound keys.
These keys are not stored in main memory and are therefore not directly
accessible by the operating system.

So instead of feeding the key into DCP, we need to place a
reference to such a key before initiating the crypto operation.
Keys are referenced by a one byte identifiers.

DCP supports 6 different keys: 4 slots in the secure memory area,
a one time programmable key which can be burnt via on-chip fuses
and an unique device key.

Using these keys is restricted to in-kernel users that use them as building
block for other crypto tools such as trusted keys. Allowing userspace
(e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
risk, because there is no access control mechanism.

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: David Gstir <david@sigma-star.at>
Signed-off-by: David Gstir <david@sigma-star.at>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
 include/linux/mxs-dcp.h  |  19 +++++++
 2 files changed, 117 insertions(+), 12 deletions(-)
 create mode 100644 include/linux/mxs-dcp.h

diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
index d6a7784d2988..c3e0c0ccbc20 100644
--- a/drivers/crypto/mxs-dcp.c
+++ b/drivers/crypto/mxs-dcp.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/stmp_device.h>
 #include <linux/clk.h>
+#include <linux/mxs-dcp.h>
 
 #include <crypto/aes.h>
 #include <crypto/sha1.h>
@@ -101,6 +102,7 @@ struct dcp_async_ctx {
 	struct crypto_skcipher		*fallback;
 	unsigned int			key_len;
 	uint8_t				key[AES_KEYSIZE_128];
+	bool				refkey;
 };
 
 struct dcp_aes_req_ctx {
@@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
 #define MXS_DCP_CONTROL0_HASH_TERM		(1 << 13)
 #define MXS_DCP_CONTROL0_HASH_INIT		(1 << 12)
 #define MXS_DCP_CONTROL0_PAYLOAD_KEY		(1 << 11)
+#define MXS_DCP_CONTROL0_OTP_KEY		(1 << 10)
 #define MXS_DCP_CONTROL0_CIPHER_ENCRYPT		(1 << 8)
 #define MXS_DCP_CONTROL0_CIPHER_INIT		(1 << 9)
 #define MXS_DCP_CONTROL0_ENABLE_HASH		(1 << 6)
@@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
 #define MXS_DCP_CONTROL1_CIPHER_MODE_ECB	(0 << 4)
 #define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128	(0 << 0)
 
+#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT	8
+
 static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
 {
 	struct dcp *sdcp = global_sdcp;
@@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	struct dcp *sdcp = global_sdcp;
 	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
 	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
+	dma_addr_t src_phys, dst_phys, key_phys = {0};
+	bool key_referenced = actx->refkey;
 	int ret;
 
-	dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
-					     2 * AES_KEYSIZE_128,
-					     DMA_TO_DEVICE);
-	dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
-					     DCP_BUF_SZ, DMA_TO_DEVICE);
-	dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf,
-					     DCP_BUF_SZ, DMA_FROM_DEVICE);
+	if (!key_referenced) {
+		key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
+					  2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
+	}
+	src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, DCP_BUF_SZ,
+				  DMA_TO_DEVICE);
+	dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, DCP_BUF_SZ,
+				  DMA_FROM_DEVICE);
 
 	if (actx->fill % AES_BLOCK_SIZE) {
 		dev_err(sdcp->dev, "Invalid block size!\n");
@@ -240,8 +248,13 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 		    MXS_DCP_CONTROL0_INTERRUPT |
 		    MXS_DCP_CONTROL0_ENABLE_CIPHER;
 
-	/* Payload contains the key. */
-	desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
+	if (key_referenced) {
+		/* Set OTP key bit to select the key via KEY_SELECT. */
+		desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
+	} else {
+		/* Payload contains the key. */
+		desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
+	}
 
 	if (rctx->enc)
 		desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
@@ -255,6 +268,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	else
 		desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
 
+	if (key_referenced)
+		desc->control1 |= sdcp->coh->aes_key[0] << MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
+
 	desc->next_cmd_addr = 0;
 	desc->source = src_phys;
 	desc->destination = dst_phys;
@@ -265,8 +281,10 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
 	ret = mxs_dcp_start_dma(actx);
 
 aes_done_run:
-	dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
-			 DMA_TO_DEVICE);
+	if (!key_referenced) {
+		dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
+				 DMA_TO_DEVICE);
+	}
 	dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE);
 	dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE);
 
@@ -454,7 +472,7 @@ static int mxs_dcp_aes_enqueue(struct skcipher_request *req, int enc, int ecb)
 	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
 	int ret;
 
-	if (unlikely(actx->key_len != AES_KEYSIZE_128))
+	if (unlikely(actx->key_len != AES_KEYSIZE_128 && !actx->refkey))
 		return mxs_dcp_block_fallback(req, enc);
 
 	rctx->enc = enc;
@@ -501,6 +519,7 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	 * there can still be an operation in progress.
 	 */
 	actx->key_len = len;
+	actx->refkey = false;
 	if (len == AES_KEYSIZE_128) {
 		memcpy(actx->key, key, len);
 		return 0;
@@ -517,6 +536,33 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
 	return crypto_skcipher_setkey(actx->fallback, key, len);
 }
 
+static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
+				 unsigned int len)
+{
+	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
+	int ret = -EINVAL;
+
+	if (len != DCP_PAES_KEYSIZE)
+		goto out;
+
+	actx->key_len = len;
+	actx->refkey = true;
+
+	switch (key[0]) {
+	case DCP_PAES_KEY_SLOT0:
+	case DCP_PAES_KEY_SLOT1:
+	case DCP_PAES_KEY_SLOT2:
+	case DCP_PAES_KEY_SLOT3:
+	case DCP_PAES_KEY_UNIQUE:
+	case DCP_PAES_KEY_OTP:
+		memcpy(actx->key, key, len);
+		ret = 0;
+	}
+
+out:
+	return ret;
+}
+
 static int mxs_dcp_aes_fallback_init_tfm(struct crypto_skcipher *tfm)
 {
 	const char *name = crypto_tfm_alg_name(crypto_skcipher_tfm(tfm));
@@ -540,6 +586,13 @@ static void mxs_dcp_aes_fallback_exit_tfm(struct crypto_skcipher *tfm)
 	crypto_free_skcipher(actx->fallback);
 }
 
+static int mxs_dcp_paes_init_tfm(struct crypto_skcipher *tfm)
+{
+	crypto_skcipher_set_reqsize(tfm, sizeof(struct dcp_aes_req_ctx));
+
+	return 0;
+}
+
 /*
  * Hashing (SHA1/SHA256)
  */
@@ -882,6 +935,39 @@ static struct skcipher_alg dcp_aes_algs[] = {
 		.ivsize			= AES_BLOCK_SIZE,
 		.init			= mxs_dcp_aes_fallback_init_tfm,
 		.exit			= mxs_dcp_aes_fallback_exit_tfm,
+	}, {
+		.base.cra_name		= "ecb(paes)",
+		.base.cra_driver_name	= "ecb-paes-dcp",
+		.base.cra_priority	= 401,
+		.base.cra_alignmask	= 15,
+		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
+		.base.cra_blocksize	= AES_BLOCK_SIZE,
+		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
+		.base.cra_module	= THIS_MODULE,
+
+		.min_keysize		= DCP_PAES_KEYSIZE,
+		.max_keysize		= DCP_PAES_KEYSIZE,
+		.setkey			= mxs_dcp_aes_setrefkey,
+		.encrypt		= mxs_dcp_aes_ecb_encrypt,
+		.decrypt		= mxs_dcp_aes_ecb_decrypt,
+		.init			= mxs_dcp_paes_init_tfm,
+	}, {
+		.base.cra_name		= "cbc(paes)",
+		.base.cra_driver_name	= "cbc-paes-dcp",
+		.base.cra_priority	= 401,
+		.base.cra_alignmask	= 15,
+		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
+		.base.cra_blocksize	= AES_BLOCK_SIZE,
+		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
+		.base.cra_module	= THIS_MODULE,
+
+		.min_keysize		= DCP_PAES_KEYSIZE,
+		.max_keysize		= DCP_PAES_KEYSIZE,
+		.setkey			= mxs_dcp_aes_setrefkey,
+		.encrypt		= mxs_dcp_aes_cbc_encrypt,
+		.decrypt		= mxs_dcp_aes_cbc_decrypt,
+		.ivsize			= AES_BLOCK_SIZE,
+		.init			= mxs_dcp_paes_init_tfm,
 	},
 };
 
diff --git a/include/linux/mxs-dcp.h b/include/linux/mxs-dcp.h
new file mode 100644
index 000000000000..df6678ee10a1
--- /dev/null
+++ b/include/linux/mxs-dcp.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#ifndef MXS_DCP_H
+#define MXS_DCP_H
+
+#define DCP_PAES_KEYSIZE 1
+#define DCP_PAES_KEY_SLOT0 0x00
+#define DCP_PAES_KEY_SLOT1 0x01
+#define DCP_PAES_KEY_SLOT2 0x02
+#define DCP_PAES_KEY_SLOT3 0x03
+#define DCP_PAES_KEY_UNIQUE 0xfe
+#define DCP_PAES_KEY_OTP 0xff
+
+#endif /* MXS_DCP_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Herbert,

On Mon, Jun 14, 2021 at 10:18 PM Richard Weinberger <richard@nod.at> wrote:
>
> DCP is capable to performing AES with hardware-bound keys.
> These keys are not stored in main memory and are therefore not directly
> accessible by the operating system.
>
> So instead of feeding the key into DCP, we need to place a
> reference to such a key before initiating the crypto operation.
> Keys are referenced by a one byte identifiers.
>
> DCP supports 6 different keys: 4 slots in the secure memory area,
> a one time programmable key which can be burnt via on-chip fuses
> and an unique device key.
>
> Using these keys is restricted to in-kernel users that use them as building
> block for other crypto tools such as trusted keys. Allowing userspace
> (e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
> risk, because there is no access control mechanism.
>
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
>  include/linux/mxs-dcp.h  |  19 +++++++
>  2 files changed, 117 insertions(+), 12 deletions(-)
>  create mode 100644 include/linux/mxs-dcp.h

This patch was judged as not applicable in your patchwork.
Is something missing? How can we proceed?

-- 
Thanks,
//richard

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Herbert,

On Mon, Jun 14, 2021 at 10:18 PM Richard Weinberger <richard@nod.at> wrote:
>
> DCP is capable to performing AES with hardware-bound keys.
> These keys are not stored in main memory and are therefore not directly
> accessible by the operating system.
>
> So instead of feeding the key into DCP, we need to place a
> reference to such a key before initiating the crypto operation.
> Keys are referenced by a one byte identifiers.
>
> DCP supports 6 different keys: 4 slots in the secure memory area,
> a one time programmable key which can be burnt via on-chip fuses
> and an unique device key.
>
> Using these keys is restricted to in-kernel users that use them as building
> block for other crypto tools such as trusted keys. Allowing userspace
> (e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
> risk, because there is no access control mechanism.
>
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
>  include/linux/mxs-dcp.h  |  19 +++++++
>  2 files changed, 117 insertions(+), 12 deletions(-)
>  create mode 100644 include/linux/mxs-dcp.h

This patch was judged as not applicable in your patchwork.
Is something missing? How can we proceed?

-- 
Thanks,
//richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Herbert Xu]

On Fri, Jun 25, 2021 at 02:21:16PM +0200, Richard Weinberger wrote:
>
> This patch was judged as not applicable in your patchwork.
> Is something missing? How can we proceed?

I'm happy to take this patch.  I marked it as not applicable
mainly because the other two patches didn't have acks and I'm
not sure if they were meant for the crypto tree or not.

Would you like me to take just the first patch?

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

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Herbert Xu]

On Fri, Jun 25, 2021 at 02:21:16PM +0200, Richard Weinberger wrote:
>
> This patch was judged as not applicable in your patchwork.
> Is something missing? How can we proceed?

I'm happy to take this patch.  I marked it as not applicable
mainly because the other two patches didn't have acks and I'm
not sure if they were meant for the crypto tree or not.

Would you like me to take just the first patch?

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Herbert,

On Fri, Jun 25, 2021 at 2:29 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > This patch was judged as not applicable in your patchwork.
> > Is something missing? How can we proceed?
>
> I'm happy to take this patch.  I marked it as not applicable
> mainly because the other two patches didn't have acks and I'm
> not sure if they were meant for the crypto tree or not.

Maybe we have a chicken/egg situation and integrity folks wait for you. ;-)

> Would you like me to take just the first patch?

IMHO all three patches should go through the integrity tree.
Given that you're fine with the first patch, can you please ack it?

-- 
Thanks,
//richard

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Herbert,

On Fri, Jun 25, 2021 at 2:29 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > This patch was judged as not applicable in your patchwork.
> > Is something missing? How can we proceed?
>
> I'm happy to take this patch.  I marked it as not applicable
> mainly because the other two patches didn't have acks and I'm
> not sure if they were meant for the crypto tree or not.

Maybe we have a chicken/egg situation and integrity folks wait for you. ;-)

> Would you like me to take just the first patch?

IMHO all three patches should go through the integrity tree.
Given that you're fine with the first patch, can you please ack it?

-- 
Thanks,
//richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Ahmad Fatoum]

Hi Richard,

Let's trade reviews to get the ball rolling?

On 14.06.21 22:16, Richard Weinberger wrote:
> DCP is capable to performing AES with hardware-bound keys.
> These keys are not stored in main memory and are therefore not directly
> accessible by the operating system.
> 
> So instead of feeding the key into DCP, we need to place a
> reference to such a key before initiating the crypto operation.
> Keys are referenced by a one byte identifiers.
> 
> DCP supports 6 different keys: 4 slots in the secure memory area,
> a one time programmable key which can be burnt via on-chip fuses
> and an unique device key.
> 
> Using these keys is restricted to in-kernel users that use them as building
> block for other crypto tools such as trusted keys. Allowing userspace
> (e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
> risk, because there is no access control mechanism.
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
>  include/linux/mxs-dcp.h  |  19 +++++++
>  2 files changed, 117 insertions(+), 12 deletions(-)
>  create mode 100644 include/linux/mxs-dcp.h
> 
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index d6a7784d2988..c3e0c0ccbc20 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/stmp_device.h>
>  #include <linux/clk.h>
> +#include <linux/mxs-dcp.h>

The CAAM specific headers are in <soc/fsl/*.h>.
Should this be done likewise here as well?

>  
>  #include <crypto/aes.h>
>  #include <crypto/sha1.h>
> @@ -101,6 +102,7 @@ struct dcp_async_ctx {
>  	struct crypto_skcipher		*fallback;
>  	unsigned int			key_len;
>  	uint8_t				key[AES_KEYSIZE_128];
> +	bool				refkey;
>  };
>  
>  struct dcp_aes_req_ctx {
> @@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL0_HASH_TERM		(1 << 13)
>  #define MXS_DCP_CONTROL0_HASH_INIT		(1 << 12)
>  #define MXS_DCP_CONTROL0_PAYLOAD_KEY		(1 << 11)
> +#define MXS_DCP_CONTROL0_OTP_KEY		(1 << 10)
>  #define MXS_DCP_CONTROL0_CIPHER_ENCRYPT		(1 << 8)
>  #define MXS_DCP_CONTROL0_CIPHER_INIT		(1 << 9)
>  #define MXS_DCP_CONTROL0_ENABLE_HASH		(1 << 6)
> @@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL1_CIPHER_MODE_ECB	(0 << 4)
>  #define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128	(0 << 0)
>  
> +#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT	8
> +
>  static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
>  {
>  	struct dcp *sdcp = global_sdcp;
> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	struct dcp *sdcp = global_sdcp;
>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
> +	dma_addr_t src_phys, dst_phys, key_phys = {0};

Why = {0}; ? dma_addr_t is a scalar type and the value is always
written here before access.

> +	bool key_referenced = actx->refkey;
>  	int ret;
>  
> -	dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> -					     2 * AES_KEYSIZE_128,
> -					     DMA_TO_DEVICE);
> -	dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
> -					     DCP_BUF_SZ, DMA_TO_DEVICE);
> -	dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf,
> -					     DCP_BUF_SZ, DMA_FROM_DEVICE);
> +	if (!key_referenced) {
> +		key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> +					  2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
> +	}
> +	src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, DCP_BUF_SZ,
> +				  DMA_TO_DEVICE);
> +	dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, DCP_BUF_SZ,
> +				  DMA_FROM_DEVICE);
>  
>  	if (actx->fill % AES_BLOCK_SIZE) {
>  		dev_err(sdcp->dev, "Invalid block size!\n");
> @@ -240,8 +248,13 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  		    MXS_DCP_CONTROL0_INTERRUPT |
>  		    MXS_DCP_CONTROL0_ENABLE_CIPHER;
>  
> -	/* Payload contains the key. */
> -	desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
> +	if (key_referenced) {
> +		/* Set OTP key bit to select the key via KEY_SELECT. */
> +		desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
> +	} else {
> +		/* Payload contains the key. */
> +		desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
> +	}
>  
>  	if (rctx->enc)
>  		desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
> @@ -255,6 +268,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	else
>  		desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
>  
> +	if (key_referenced)
> +		desc->control1 |= sdcp->coh->aes_key[0] << MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
> +
>  	desc->next_cmd_addr = 0;
>  	desc->source = src_phys;
>  	desc->destination = dst_phys;
> @@ -265,8 +281,10 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	ret = mxs_dcp_start_dma(actx);
>  
>  aes_done_run:
> -	dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
> -			 DMA_TO_DEVICE);
> +	if (!key_referenced) {
> +		dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
> +				 DMA_TO_DEVICE);
> +	}
>  	dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE);
>  	dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE);
>  
> @@ -454,7 +472,7 @@ static int mxs_dcp_aes_enqueue(struct skcipher_request *req, int enc, int ecb)
>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>  	int ret;
>  
> -	if (unlikely(actx->key_len != AES_KEYSIZE_128))
> +	if (unlikely(actx->key_len != AES_KEYSIZE_128 && !actx->refkey))
>  		return mxs_dcp_block_fallback(req, enc);
>  
>  	rctx->enc = enc;
> @@ -501,6 +519,7 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
>  	 * there can still be an operation in progress.
>  	 */
>  	actx->key_len = len;
> +	actx->refkey = false;
>  	if (len == AES_KEYSIZE_128) {
>  		memcpy(actx->key, key, len);
>  		return 0;
> @@ -517,6 +536,33 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
>  	return crypto_skcipher_setkey(actx->fallback, key, len);
>  }
>  
> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
> +				 unsigned int len)
> +{
> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
> +	int ret = -EINVAL;
> +
> +	if (len != DCP_PAES_KEYSIZE)
> +		goto out;

Nitpick: there is no cleanup, so why not return -EINVAL here
and unconditionally return 0 below?

> +
> +	actx->key_len = len;
> +	actx->refkey = true;
> +
> +	switch (key[0]) {
> +	case DCP_PAES_KEY_SLOT0:
> +	case DCP_PAES_KEY_SLOT1:
> +	case DCP_PAES_KEY_SLOT2:
> +	case DCP_PAES_KEY_SLOT3:
> +	case DCP_PAES_KEY_UNIQUE:
> +	case DCP_PAES_KEY_OTP:
> +		memcpy(actx->key, key, len);
> +		ret = 0;
> +	}

In the error case you return -EINVAL below, but you still write
into actx. Is that intentional?

> +
> +out:
> +	return ret;
> +}
> +
>  static int mxs_dcp_aes_fallback_init_tfm(struct crypto_skcipher *tfm)
>  {
>  	const char *name = crypto_tfm_alg_name(crypto_skcipher_tfm(tfm));
> @@ -540,6 +586,13 @@ static void mxs_dcp_aes_fallback_exit_tfm(struct crypto_skcipher *tfm)
>  	crypto_free_skcipher(actx->fallback);
>  }
>  
> +static int mxs_dcp_paes_init_tfm(struct crypto_skcipher *tfm)
> +{
> +	crypto_skcipher_set_reqsize(tfm, sizeof(struct dcp_aes_req_ctx));
> +
> +	return 0;
> +}
> +
>  /*
>   * Hashing (SHA1/SHA256)
>   */
> @@ -882,6 +935,39 @@ static struct skcipher_alg dcp_aes_algs[] = {
>  		.ivsize			= AES_BLOCK_SIZE,
>  		.init			= mxs_dcp_aes_fallback_init_tfm,
>  		.exit			= mxs_dcp_aes_fallback_exit_tfm,
> +	}, {
> +		.base.cra_name		= "ecb(paes)",
> +		.base.cra_driver_name	= "ecb-paes-dcp",
> +		.base.cra_priority	= 401,
> +		.base.cra_alignmask	= 15,
> +		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
> +		.base.cra_blocksize	= AES_BLOCK_SIZE,
> +		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
> +		.base.cra_module	= THIS_MODULE,
> +
> +		.min_keysize		= DCP_PAES_KEYSIZE,
> +		.max_keysize		= DCP_PAES_KEYSIZE,
> +		.setkey			= mxs_dcp_aes_setrefkey,
> +		.encrypt		= mxs_dcp_aes_ecb_encrypt,
> +		.decrypt		= mxs_dcp_aes_ecb_decrypt,
> +		.init			= mxs_dcp_paes_init_tfm,
> +	}, {
> +		.base.cra_name		= "cbc(paes)",
> +		.base.cra_driver_name	= "cbc-paes-dcp",
> +		.base.cra_priority	= 401,
> +		.base.cra_alignmask	= 15,
> +		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
> +		.base.cra_blocksize	= AES_BLOCK_SIZE,
> +		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
> +		.base.cra_module	= THIS_MODULE,
> +
> +		.min_keysize		= DCP_PAES_KEYSIZE,
> +		.max_keysize		= DCP_PAES_KEYSIZE,
> +		.setkey			= mxs_dcp_aes_setrefkey,
> +		.encrypt		= mxs_dcp_aes_cbc_encrypt,
> +		.decrypt		= mxs_dcp_aes_cbc_decrypt,
> +		.ivsize			= AES_BLOCK_SIZE,
> +		.init			= mxs_dcp_paes_init_tfm,
>  	},
>  };
>  
> diff --git a/include/linux/mxs-dcp.h b/include/linux/mxs-dcp.h
> new file mode 100644
> index 000000000000..df6678ee10a1
> --- /dev/null
> +++ b/include/linux/mxs-dcp.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#ifndef MXS_DCP_H
> +#define MXS_DCP_H
> +
> +#define DCP_PAES_KEYSIZE 1
> +#define DCP_PAES_KEY_SLOT0 0x00
> +#define DCP_PAES_KEY_SLOT1 0x01
> +#define DCP_PAES_KEY_SLOT2 0x02
> +#define DCP_PAES_KEY_SLOT3 0x03
> +#define DCP_PAES_KEY_UNIQUE 0xfe
> +#define DCP_PAES_KEY_OTP 0xff
> +
> +#endif /* MXS_DCP_H */

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 |

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Ahmad Fatoum]

Hi Richard,

Let's trade reviews to get the ball rolling?

On 14.06.21 22:16, Richard Weinberger wrote:
> DCP is capable to performing AES with hardware-bound keys.
> These keys are not stored in main memory and are therefore not directly
> accessible by the operating system.
> 
> So instead of feeding the key into DCP, we need to place a
> reference to such a key before initiating the crypto operation.
> Keys are referenced by a one byte identifiers.
> 
> DCP supports 6 different keys: 4 slots in the secure memory area,
> a one time programmable key which can be burnt via on-chip fuses
> and an unique device key.
> 
> Using these keys is restricted to in-kernel users that use them as building
> block for other crypto tools such as trusted keys. Allowing userspace
> (e.g. via AF_ALG) to use these keys to crypt or decrypt data is a security
> risk, because there is no access control mechanism.
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/crypto/mxs-dcp.c | 110 ++++++++++++++++++++++++++++++++++-----
>  include/linux/mxs-dcp.h  |  19 +++++++
>  2 files changed, 117 insertions(+), 12 deletions(-)
>  create mode 100644 include/linux/mxs-dcp.h
> 
> diff --git a/drivers/crypto/mxs-dcp.c b/drivers/crypto/mxs-dcp.c
> index d6a7784d2988..c3e0c0ccbc20 100644
> --- a/drivers/crypto/mxs-dcp.c
> +++ b/drivers/crypto/mxs-dcp.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/stmp_device.h>
>  #include <linux/clk.h>
> +#include <linux/mxs-dcp.h>

The CAAM specific headers are in <soc/fsl/*.h>.
Should this be done likewise here as well?

>  
>  #include <crypto/aes.h>
>  #include <crypto/sha1.h>
> @@ -101,6 +102,7 @@ struct dcp_async_ctx {
>  	struct crypto_skcipher		*fallback;
>  	unsigned int			key_len;
>  	uint8_t				key[AES_KEYSIZE_128];
> +	bool				refkey;
>  };
>  
>  struct dcp_aes_req_ctx {
> @@ -155,6 +157,7 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL0_HASH_TERM		(1 << 13)
>  #define MXS_DCP_CONTROL0_HASH_INIT		(1 << 12)
>  #define MXS_DCP_CONTROL0_PAYLOAD_KEY		(1 << 11)
> +#define MXS_DCP_CONTROL0_OTP_KEY		(1 << 10)
>  #define MXS_DCP_CONTROL0_CIPHER_ENCRYPT		(1 << 8)
>  #define MXS_DCP_CONTROL0_CIPHER_INIT		(1 << 9)
>  #define MXS_DCP_CONTROL0_ENABLE_HASH		(1 << 6)
> @@ -168,6 +171,8 @@ static struct dcp *global_sdcp;
>  #define MXS_DCP_CONTROL1_CIPHER_MODE_ECB	(0 << 4)
>  #define MXS_DCP_CONTROL1_CIPHER_SELECT_AES128	(0 << 0)
>  
> +#define MXS_DCP_CONTROL1_KEY_SELECT_SHIFT	8
> +
>  static int mxs_dcp_start_dma(struct dcp_async_ctx *actx)
>  {
>  	struct dcp *sdcp = global_sdcp;
> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	struct dcp *sdcp = global_sdcp;
>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
> +	dma_addr_t src_phys, dst_phys, key_phys = {0};

Why = {0}; ? dma_addr_t is a scalar type and the value is always
written here before access.

> +	bool key_referenced = actx->refkey;
>  	int ret;
>  
> -	dma_addr_t key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> -					     2 * AES_KEYSIZE_128,
> -					     DMA_TO_DEVICE);
> -	dma_addr_t src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf,
> -					     DCP_BUF_SZ, DMA_TO_DEVICE);
> -	dma_addr_t dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf,
> -					     DCP_BUF_SZ, DMA_FROM_DEVICE);
> +	if (!key_referenced) {
> +		key_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_key,
> +					  2 * AES_KEYSIZE_128, DMA_TO_DEVICE);
> +	}
> +	src_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_in_buf, DCP_BUF_SZ,
> +				  DMA_TO_DEVICE);
> +	dst_phys = dma_map_single(sdcp->dev, sdcp->coh->aes_out_buf, DCP_BUF_SZ,
> +				  DMA_FROM_DEVICE);
>  
>  	if (actx->fill % AES_BLOCK_SIZE) {
>  		dev_err(sdcp->dev, "Invalid block size!\n");
> @@ -240,8 +248,13 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  		    MXS_DCP_CONTROL0_INTERRUPT |
>  		    MXS_DCP_CONTROL0_ENABLE_CIPHER;
>  
> -	/* Payload contains the key. */
> -	desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
> +	if (key_referenced) {
> +		/* Set OTP key bit to select the key via KEY_SELECT. */
> +		desc->control0 |= MXS_DCP_CONTROL0_OTP_KEY;
> +	} else {
> +		/* Payload contains the key. */
> +		desc->control0 |= MXS_DCP_CONTROL0_PAYLOAD_KEY;
> +	}
>  
>  	if (rctx->enc)
>  		desc->control0 |= MXS_DCP_CONTROL0_CIPHER_ENCRYPT;
> @@ -255,6 +268,9 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	else
>  		desc->control1 |= MXS_DCP_CONTROL1_CIPHER_MODE_CBC;
>  
> +	if (key_referenced)
> +		desc->control1 |= sdcp->coh->aes_key[0] << MXS_DCP_CONTROL1_KEY_SELECT_SHIFT;
> +
>  	desc->next_cmd_addr = 0;
>  	desc->source = src_phys;
>  	desc->destination = dst_phys;
> @@ -265,8 +281,10 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>  	ret = mxs_dcp_start_dma(actx);
>  
>  aes_done_run:
> -	dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
> -			 DMA_TO_DEVICE);
> +	if (!key_referenced) {
> +		dma_unmap_single(sdcp->dev, key_phys, 2 * AES_KEYSIZE_128,
> +				 DMA_TO_DEVICE);
> +	}
>  	dma_unmap_single(sdcp->dev, src_phys, DCP_BUF_SZ, DMA_TO_DEVICE);
>  	dma_unmap_single(sdcp->dev, dst_phys, DCP_BUF_SZ, DMA_FROM_DEVICE);
>  
> @@ -454,7 +472,7 @@ static int mxs_dcp_aes_enqueue(struct skcipher_request *req, int enc, int ecb)
>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>  	int ret;
>  
> -	if (unlikely(actx->key_len != AES_KEYSIZE_128))
> +	if (unlikely(actx->key_len != AES_KEYSIZE_128 && !actx->refkey))
>  		return mxs_dcp_block_fallback(req, enc);
>  
>  	rctx->enc = enc;
> @@ -501,6 +519,7 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
>  	 * there can still be an operation in progress.
>  	 */
>  	actx->key_len = len;
> +	actx->refkey = false;
>  	if (len == AES_KEYSIZE_128) {
>  		memcpy(actx->key, key, len);
>  		return 0;
> @@ -517,6 +536,33 @@ static int mxs_dcp_aes_setkey(struct crypto_skcipher *tfm, const u8 *key,
>  	return crypto_skcipher_setkey(actx->fallback, key, len);
>  }
>  
> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
> +				 unsigned int len)
> +{
> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
> +	int ret = -EINVAL;
> +
> +	if (len != DCP_PAES_KEYSIZE)
> +		goto out;

Nitpick: there is no cleanup, so why not return -EINVAL here
and unconditionally return 0 below?

> +
> +	actx->key_len = len;
> +	actx->refkey = true;
> +
> +	switch (key[0]) {
> +	case DCP_PAES_KEY_SLOT0:
> +	case DCP_PAES_KEY_SLOT1:
> +	case DCP_PAES_KEY_SLOT2:
> +	case DCP_PAES_KEY_SLOT3:
> +	case DCP_PAES_KEY_UNIQUE:
> +	case DCP_PAES_KEY_OTP:
> +		memcpy(actx->key, key, len);
> +		ret = 0;
> +	}

In the error case you return -EINVAL below, but you still write
into actx. Is that intentional?

> +
> +out:
> +	return ret;
> +}
> +
>  static int mxs_dcp_aes_fallback_init_tfm(struct crypto_skcipher *tfm)
>  {
>  	const char *name = crypto_tfm_alg_name(crypto_skcipher_tfm(tfm));
> @@ -540,6 +586,13 @@ static void mxs_dcp_aes_fallback_exit_tfm(struct crypto_skcipher *tfm)
>  	crypto_free_skcipher(actx->fallback);
>  }
>  
> +static int mxs_dcp_paes_init_tfm(struct crypto_skcipher *tfm)
> +{
> +	crypto_skcipher_set_reqsize(tfm, sizeof(struct dcp_aes_req_ctx));
> +
> +	return 0;
> +}
> +
>  /*
>   * Hashing (SHA1/SHA256)
>   */
> @@ -882,6 +935,39 @@ static struct skcipher_alg dcp_aes_algs[] = {
>  		.ivsize			= AES_BLOCK_SIZE,
>  		.init			= mxs_dcp_aes_fallback_init_tfm,
>  		.exit			= mxs_dcp_aes_fallback_exit_tfm,
> +	}, {
> +		.base.cra_name		= "ecb(paes)",
> +		.base.cra_driver_name	= "ecb-paes-dcp",
> +		.base.cra_priority	= 401,
> +		.base.cra_alignmask	= 15,
> +		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
> +		.base.cra_blocksize	= AES_BLOCK_SIZE,
> +		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
> +		.base.cra_module	= THIS_MODULE,
> +
> +		.min_keysize		= DCP_PAES_KEYSIZE,
> +		.max_keysize		= DCP_PAES_KEYSIZE,
> +		.setkey			= mxs_dcp_aes_setrefkey,
> +		.encrypt		= mxs_dcp_aes_ecb_encrypt,
> +		.decrypt		= mxs_dcp_aes_ecb_decrypt,
> +		.init			= mxs_dcp_paes_init_tfm,
> +	}, {
> +		.base.cra_name		= "cbc(paes)",
> +		.base.cra_driver_name	= "cbc-paes-dcp",
> +		.base.cra_priority	= 401,
> +		.base.cra_alignmask	= 15,
> +		.base.cra_flags		= CRYPTO_ALG_ASYNC | CRYPTO_ALG_INTERNAL,
> +		.base.cra_blocksize	= AES_BLOCK_SIZE,
> +		.base.cra_ctxsize	= sizeof(struct dcp_async_ctx),
> +		.base.cra_module	= THIS_MODULE,
> +
> +		.min_keysize		= DCP_PAES_KEYSIZE,
> +		.max_keysize		= DCP_PAES_KEYSIZE,
> +		.setkey			= mxs_dcp_aes_setrefkey,
> +		.encrypt		= mxs_dcp_aes_cbc_encrypt,
> +		.decrypt		= mxs_dcp_aes_cbc_decrypt,
> +		.ivsize			= AES_BLOCK_SIZE,
> +		.init			= mxs_dcp_paes_init_tfm,
>  	},
>  };
>  
> diff --git a/include/linux/mxs-dcp.h b/include/linux/mxs-dcp.h
> new file mode 100644
> index 000000000000..df6678ee10a1
> --- /dev/null
> +++ b/include/linux/mxs-dcp.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#ifndef MXS_DCP_H
> +#define MXS_DCP_H
> +
> +#define DCP_PAES_KEYSIZE 1
> +#define DCP_PAES_KEY_SLOT0 0x00
> +#define DCP_PAES_KEY_SLOT1 0x01
> +#define DCP_PAES_KEY_SLOT2 0x02
> +#define DCP_PAES_KEY_SLOT3 0x03
> +#define DCP_PAES_KEY_UNIQUE 0xfe
> +#define DCP_PAES_KEY_OTP 0xff
> +
> +#endif /* MXS_DCP_H */

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 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
> Let's trade reviews to get the ball rolling?

Sounds like a fair deal. :-)

[...]

>> --- a/drivers/crypto/mxs-dcp.c
>> +++ b/drivers/crypto/mxs-dcp.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/stmp_device.h>
>>  #include <linux/clk.h>
>> +#include <linux/mxs-dcp.h>
> 
> The CAAM specific headers are in <soc/fsl/*.h>.
> Should this be done likewise here as well?

I have no preferences. If soc/fsl/ is the way to go, fine by me.

[...]

>> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>>  	struct dcp *sdcp = global_sdcp;
>>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>> +	dma_addr_t src_phys, dst_phys, key_phys = {0};
> 
> Why = {0}; ? dma_addr_t is a scalar type and the value is always
> written here before access.

Initializing a scalar with {} is allowed in C, the braces are optional.
I like the braces because it works even when the underlaying type changes.
But that's just a matter of taste.

key_phys is initialized because it triggered a false positive gcc warning
on one of my targets. Let me re-run again to be sure, the code saw a lot of
refactoring since that.
 
[...]
  
>> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
>> +				 unsigned int len)
>> +{
>> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
>> +	int ret = -EINVAL;
>> +
>> +	if (len != DCP_PAES_KEYSIZE)
>> +		goto out;
> 
> Nitpick: there is no cleanup, so why not return -EINVAL here
> and unconditionally return 0 below?

What is the benefit?
Usually I try to use goto to have a single exit point of a function
but I don't have a strong preference...

>> +
>> +	actx->key_len = len;
>> +	actx->refkey = true;
>> +
>> +	switch (key[0]) {
>> +	case DCP_PAES_KEY_SLOT0:
>> +	case DCP_PAES_KEY_SLOT1:
>> +	case DCP_PAES_KEY_SLOT2:
>> +	case DCP_PAES_KEY_SLOT3:
>> +	case DCP_PAES_KEY_UNIQUE:
>> +	case DCP_PAES_KEY_OTP:
>> +		memcpy(actx->key, key, len);
>> +		ret = 0;
>> +	}
> 
> In the error case you return -EINVAL below, but you still write
> into actx. Is that intentional?

You mean acts->key_len and actk->refkey?
Is this a problem?

Thanks,
//richard

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Richard Weinberger]

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
> Let's trade reviews to get the ball rolling?

Sounds like a fair deal. :-)

[...]

>> --- a/drivers/crypto/mxs-dcp.c
>> +++ b/drivers/crypto/mxs-dcp.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/stmp_device.h>
>>  #include <linux/clk.h>
>> +#include <linux/mxs-dcp.h>
> 
> The CAAM specific headers are in <soc/fsl/*.h>.
> Should this be done likewise here as well?

I have no preferences. If soc/fsl/ is the way to go, fine by me.

[...]

>> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>>  	struct dcp *sdcp = global_sdcp;
>>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>> +	dma_addr_t src_phys, dst_phys, key_phys = {0};
> 
> Why = {0}; ? dma_addr_t is a scalar type and the value is always
> written here before access.

Initializing a scalar with {} is allowed in C, the braces are optional.
I like the braces because it works even when the underlaying type changes.
But that's just a matter of taste.

key_phys is initialized because it triggered a false positive gcc warning
on one of my targets. Let me re-run again to be sure, the code saw a lot of
refactoring since that.
 
[...]
  
>> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
>> +				 unsigned int len)
>> +{
>> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
>> +	int ret = -EINVAL;
>> +
>> +	if (len != DCP_PAES_KEYSIZE)
>> +		goto out;
> 
> Nitpick: there is no cleanup, so why not return -EINVAL here
> and unconditionally return 0 below?

What is the benefit?
Usually I try to use goto to have a single exit point of a function
but I don't have a strong preference...

>> +
>> +	actx->key_len = len;
>> +	actx->refkey = true;
>> +
>> +	switch (key[0]) {
>> +	case DCP_PAES_KEY_SLOT0:
>> +	case DCP_PAES_KEY_SLOT1:
>> +	case DCP_PAES_KEY_SLOT2:
>> +	case DCP_PAES_KEY_SLOT3:
>> +	case DCP_PAES_KEY_UNIQUE:
>> +	case DCP_PAES_KEY_OTP:
>> +		memcpy(actx->key, key, len);
>> +		ret = 0;
>> +	}
> 
> In the error case you return -EINVAL below, but you still write
> into actx. Is that intentional?

You mean acts->key_len and actk->refkey?
Is this a problem?

Thanks,
//richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Ahmad Fatoum]

Hi,

On 14.07.21 12:39, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> Let's trade reviews to get the ball rolling?
> 
> Sounds like a fair deal. :-)

:)

> [...]
> 
>>> --- a/drivers/crypto/mxs-dcp.c
>>> +++ b/drivers/crypto/mxs-dcp.c
>>> @@ -15,6 +15,7 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/stmp_device.h>
>>>  #include <linux/clk.h>
>>> +#include <linux/mxs-dcp.h>
>>
>> The CAAM specific headers are in <soc/fsl/*.h>.
>> Should this be done likewise here as well?
> 
> I have no preferences. If soc/fsl/ is the way to go, fine by me.

I think it's the more appropriate place, but if the maintainers
are fine with <linux/mxs-dcp.h>, I don't mind.

> 
> [...]
> 
>>> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>>>  	struct dcp *sdcp = global_sdcp;
>>>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>>>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>>> +	dma_addr_t src_phys, dst_phys, key_phys = {0};
>>
>> Why = {0}; ? dma_addr_t is a scalar type and the value is always
>> written here before access.
> 
> Initializing a scalar with {} is allowed in C, the braces are optional.
> I like the braces because it works even when the underlaying type changes.
> But that's just a matter of taste.
> 
> key_phys is initialized because it triggered a false positive gcc warning
> on one of my targets. Let me re-run again to be sure, the code saw a lot of
> refactoring since that.
>  
> [...]
>   
>>> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
>>> +				 unsigned int len)
>>> +{
>>> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
>>> +	int ret = -EINVAL;
>>> +
>>> +	if (len != DCP_PAES_KEYSIZE)
>>> +		goto out;
>>
>> Nitpick: there is no cleanup, so why not return -EINVAL here
>> and unconditionally return 0 below?
> 
> What is the benefit?

Similar to why you wouldn't write: 

  if (len == DCP_PAES_KEYSIZE) { 
  	/* longer code block */
  }

  return ret;

Code is easier to scan through with early-exits.

> Usually I try to use goto to have a single exit point of a function
> but I don't have a strong preference...

It's just a nitpick. I am fine with it either way.

>>> +
>>> +	actx->key_len = len;
>>> +	actx->refkey = true;
>>> +
>>> +	switch (key[0]) {
>>> +	case DCP_PAES_KEY_SLOT0:
>>> +	case DCP_PAES_KEY_SLOT1:
>>> +	case DCP_PAES_KEY_SLOT2:
>>> +	case DCP_PAES_KEY_SLOT3:
>>> +	case DCP_PAES_KEY_UNIQUE:
>>> +	case DCP_PAES_KEY_OTP:
>>> +		memcpy(actx->key, key, len);
>>> +		ret = 0;
>>> +	}
>>
>> In the error case you return -EINVAL below, but you still write
>> into actx. Is that intentional?
> 
> You mean acts->key_len and actk->refkey?
> Is this a problem?

It's easier to reason about code when it doesn't leave objects
it operates on in invalid states on failure. Changing key_len,
but leaving actx->key uninitialized is surprising IMO.

I can't judge whether this is a problem in practice, but less
surprises are a worthwhile goal.

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 |

Re: [PATCH 1/3] crypto: mxs-dcp: Add support for hardware provided keys

[Ahmad Fatoum]

Hi,

On 14.07.21 12:39, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> Let's trade reviews to get the ball rolling?
> 
> Sounds like a fair deal. :-)

:)

> [...]
> 
>>> --- a/drivers/crypto/mxs-dcp.c
>>> +++ b/drivers/crypto/mxs-dcp.c
>>> @@ -15,6 +15,7 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/stmp_device.h>
>>>  #include <linux/clk.h>
>>> +#include <linux/mxs-dcp.h>
>>
>> The CAAM specific headers are in <soc/fsl/*.h>.
>> Should this be done likewise here as well?
> 
> I have no preferences. If soc/fsl/ is the way to go, fine by me.

I think it's the more appropriate place, but if the maintainers
are fine with <linux/mxs-dcp.h>, I don't mind.

> 
> [...]
> 
>>> @@ -219,15 +224,18 @@ static int mxs_dcp_run_aes(struct dcp_async_ctx *actx,
>>>  	struct dcp *sdcp = global_sdcp;
>>>  	struct dcp_dma_desc *desc = &sdcp->coh->desc[actx->chan];
>>>  	struct dcp_aes_req_ctx *rctx = skcipher_request_ctx(req);
>>> +	dma_addr_t src_phys, dst_phys, key_phys = {0};
>>
>> Why = {0}; ? dma_addr_t is a scalar type and the value is always
>> written here before access.
> 
> Initializing a scalar with {} is allowed in C, the braces are optional.
> I like the braces because it works even when the underlaying type changes.
> But that's just a matter of taste.
> 
> key_phys is initialized because it triggered a false positive gcc warning
> on one of my targets. Let me re-run again to be sure, the code saw a lot of
> refactoring since that.
>  
> [...]
>   
>>> +static int mxs_dcp_aes_setrefkey(struct crypto_skcipher *tfm, const u8 *key,
>>> +				 unsigned int len)
>>> +{
>>> +	struct dcp_async_ctx *actx = crypto_skcipher_ctx(tfm);
>>> +	int ret = -EINVAL;
>>> +
>>> +	if (len != DCP_PAES_KEYSIZE)
>>> +		goto out;
>>
>> Nitpick: there is no cleanup, so why not return -EINVAL here
>> and unconditionally return 0 below?
> 
> What is the benefit?

Similar to why you wouldn't write: 

  if (len == DCP_PAES_KEYSIZE) { 
  	/* longer code block */
  }

  return ret;

Code is easier to scan through with early-exits.

> Usually I try to use goto to have a single exit point of a function
> but I don't have a strong preference...

It's just a nitpick. I am fine with it either way.

>>> +
>>> +	actx->key_len = len;
>>> +	actx->refkey = true;
>>> +
>>> +	switch (key[0]) {
>>> +	case DCP_PAES_KEY_SLOT0:
>>> +	case DCP_PAES_KEY_SLOT1:
>>> +	case DCP_PAES_KEY_SLOT2:
>>> +	case DCP_PAES_KEY_SLOT3:
>>> +	case DCP_PAES_KEY_UNIQUE:
>>> +	case DCP_PAES_KEY_OTP:
>>> +		memcpy(actx->key, key, len);
>>> +		ret = 0;
>>> +	}
>>
>> In the error case you return -EINVAL below, but you still write
>> into actx. Is that intentional?
> 
> You mean acts->key_len and actk->refkey?
> Is this a problem?

It's easier to reason about code when it doesn't leave objects
it operates on in invalid states on failure. Changing key_len,
but leaving actx->key uninitialized is surprising IMO.

I can't judge whether this is a problem in practice, but less
surprises are a worthwhile goal.

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 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Richard Weinberger]

DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.

Beside of accelerated crypto operations, it also offers support for
hardware-bound keys. Using this feature it is possible to implement a blob
mechanism just like CAAM offers. Unlike on CAAM, constructing and
parsing the blob has to happen in software.

We chose the following format for the blob:
/*
 * struct dcp_blob_fmt - DCP BLOB format.
 *
 * @fmt_version: Format version, currently being %1
 * @blob_key: Random AES 128 key which is used to encrypt @payload,
 *            @blob_key itself is encrypted with OTP or UNIQUE device key in
 *            AES-128-ECB mode by DCP.
 * @nonce: Random nonce used for @payload encryption.
 * @payload_len: Length of the plain text @payload.
 * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
 *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
 *
 * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
 * AES_BLOCK_SIZE.
 */
struct dcp_blob_fmt {
	__u8 fmt_version;
	__u8 blob_key[AES_KEYSIZE_128];
	__u8 nonce[AES_KEYSIZE_128];
	__le32 payload_len;
	__u8 payload[0];
} __packed;

@payload is the key provided by trusted_key_ops->seal().

By default the UNIQUE device key is used, it is also possible to use
the OTP key. While the UNIQUE device key should be unique it is not
entirely clear whether this is the case due to unclear documentation.
If someone wants to be sure they can burn their own unique key
into the OTP fuse and set the use_otp_key module parameter.

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: David Gstir <david@sigma-star.at>
Signed-off-by: David Gstir <david@sigma-star.at>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 .../admin-guide/kernel-parameters.txt         |   1 +
 MAINTAINERS                                   |   9 +
 include/keys/trusted_dcp.h                    |  13 +
 security/keys/trusted-keys/Makefile           |   1 +
 security/keys/trusted-keys/trusted_core.c     |   6 +-
 security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
 6 files changed, 354 insertions(+), 1 deletion(-)
 create mode 100644 include/keys/trusted_dcp.h
 create mode 100644 security/keys/trusted-keys/trusted_dcp.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb89dbdedc46..d8b2aa94a566 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5639,6 +5639,7 @@
 			sources:
 			- "tpm"
 			- "tee"
+			- "dcp"
 			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/MAINTAINERS b/MAINTAINERS
index b706dd20ff2b..779cac00827c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10099,6 +10099,15 @@ F:	include/keys/trusted-type.h
 F:	include/keys/trusted_tpm.h
 F:	security/keys/trusted-keys/
 
+KEYS-TRUSTED-DCP
+M:	David Gstir <david@sigma-star.at>
+M:	Richard Weinberger <richard@nod.at>
+L:	linux-integrity@vger.kernel.org
+L:	keyrings@vger.kernel.org
+S:	Supported
+F:	include/keys/trusted_dcp.h
+F:	security/keys/trusted-keys/trusted_dcp.c
+
 KEYS-TRUSTED-TEE
 M:	Sumit Garg <sumit.garg@linaro.org>
 L:	linux-integrity@vger.kernel.org
diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
new file mode 100644
index 000000000000..7b2a1275c527
--- /dev/null
+++ b/include/keys/trusted_dcp.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#ifndef TRUSTED_DCP_H
+#define TRUSTED_DCP_H
+
+extern struct trusted_key_ops dcp_trusted_key_ops;
+
+#endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index feb8b6c3cc79..992b591692dc 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_MXS_DCP) += trusted_dcp.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..66b631ffe876 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -8,6 +8,7 @@
 
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
+#include <keys/trusted_dcp.h>
 #include <keys/trusted_tee.h>
 #include <keys/trusted_tpm.h>
 #include <linux/capability.h>
@@ -24,7 +25,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 dcp)");
 
 static const struct trusted_key_source trusted_key_sources[] = {
 #if defined(CONFIG_TCG_TPM)
@@ -33,6 +34,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_MXS_DCP)
+	{ "dcp", &dcp_trusted_key_ops },
+#endif
 };
 
 DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
new file mode 100644
index 000000000000..02db5dc261c7
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_dcp.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#include <crypto/aead.h>
+#include <crypto/aes.h>
+#include <crypto/algapi.h>
+#include <crypto/gcm.h>
+#include <crypto/skcipher.h>
+#include <keys/trusted-type.h>
+#include <linux/key-type.h>
+#include <linux/module.h>
+#include <linux/mxs-dcp.h>
+#include <linux/printk.h>
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+
+#define DCP_BLOB_VERSION 1
+#define DCP_BLOB_AUTHLEN 16
+
+/**
+ * struct dcp_blob_fmt - DCP BLOB format.
+ *
+ * @fmt_version: Format version, currently being %1.
+ * @blob_key: Random AES 128 key which is used to encrypt @payload,
+ *            @blob_key itself is encrypted with OTP or UNIQUE device key in
+ *            AES-128-ECB mode by DCP.
+ * @nonce: Random nonce used for @payload encryption.
+ * @payload_len: Length of the plain text @payload.
+ * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
+ *           GCM auth tag of size DCP_BLOB_AUTHLEN is attached at the end of it.
+ *
+ * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
+ * DCP_BLOB_AUTHLEN.
+ */
+struct dcp_blob_fmt {
+	__u8 fmt_version;
+	__u8 blob_key[AES_KEYSIZE_128];
+	__u8 nonce[AES_KEYSIZE_128];
+	__le32 payload_len;
+	__u8 payload[0];
+} __packed;
+
+static bool use_otp_key;
+module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
+MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
+
+static bool skip_zk_test;
+module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
+MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are zero'ed");
+
+static unsigned int calc_blob_len(unsigned int payload_len)
+{
+	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
+}
+
+static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
+{
+	int res = 0;
+	struct skcipher_request *req = NULL;
+	DECLARE_CRYPTO_WAIT(wait);
+	struct scatterlist src_sg, dst_sg;
+	struct crypto_skcipher *tfm;
+	u8 paes_key[DCP_PAES_KEYSIZE];
+
+	if (!use_otp_key)
+		paes_key[0] = DCP_PAES_KEY_UNIQUE;
+	else
+		paes_key[0] = DCP_PAES_KEY_OTP;
+
+	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
+				    CRYPTO_ALG_INTERNAL);
+	if (IS_ERR(tfm)) {
+		res = PTR_ERR(tfm);
+		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
+		tfm = NULL;
+		goto out;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_NOFS);
+	if (!req) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				      CRYPTO_TFM_REQ_MAY_SLEEP,
+				      crypto_req_done, &wait);
+	res = crypto_skcipher_setkey(tfm, paes_key, sizeof(paes_key));
+	if (res < 0)
+		goto out;
+
+	sg_init_one(&src_sg, in, AES_KEYSIZE_128);
+	sg_init_one(&dst_sg, out, AES_KEYSIZE_128);
+	skcipher_request_set_crypt(req, &src_sg, &dst_sg, AES_KEYSIZE_128,
+				   NULL);
+
+	if (is_encrypt)
+		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+	else
+		res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
+
+out:
+	skcipher_request_free(req);
+	crypto_free_skcipher(tfm);
+
+	return res;
+}
+
+static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
+			  bool is_encrypt)
+{
+	struct aead_request *aead_req = NULL;
+	struct scatterlist src_sg, dst_sg;
+	struct crypto_aead *aead;
+	int ret;
+
+	aead = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(aead)) {
+		ret = PTR_ERR(aead);
+		pr_err("Unable to request AES-GCM cipher: %i\n", ret);
+		goto out;
+	}
+
+	ret = crypto_aead_setauthsize(aead, DCP_BLOB_AUTHLEN);
+	if (ret < 0) {
+		pr_err("Can't set crypto auth tag len: %d\n", ret);
+		goto free_aead;
+	}
+
+	aead_req = aead_request_alloc(aead, GFP_KERNEL);
+	if (!aead_req) {
+		ret = -ENOMEM;
+		goto free_aead;
+	}
+
+	sg_init_one(&src_sg, in, len);
+	if (is_encrypt) {
+		/*
+		 * If we encrypt our buffer has extra space for the auth tag.
+		 */
+		sg_init_one(&dst_sg, out, len + DCP_BLOB_AUTHLEN);
+	} else {
+		sg_init_one(&dst_sg, out, len);
+	}
+
+	aead_request_set_crypt(aead_req, &src_sg, &dst_sg, len, nonce);
+	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL,
+				  NULL);
+	aead_request_set_ad(aead_req, 0);
+
+	if (crypto_aead_setkey(aead, key, AES_KEYSIZE_128)) {
+		pr_err("Can't set crypto AEAD key\n");
+		ret = -EINVAL;
+		goto free_req;
+	}
+
+	if (is_encrypt)
+		ret = crypto_aead_encrypt(aead_req);
+	else
+		ret = crypto_aead_decrypt(aead_req);
+
+free_req:
+	aead_request_free(aead_req);
+free_aead:
+	crypto_free_aead(aead);
+out:
+	return ret;
+}
+
+static int decrypt_blob_key(u8 *key)
+{
+	return do_dcp_crypto(key, key, false);
+}
+
+static int encrypt_blob_key(u8 *key)
+{
+	return do_dcp_crypto(key, key, true);
+}
+
+static int trusted_dcp_seal(struct trusted_key_payload *p, char *datablob)
+{
+	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
+	int blen, ret;
+
+	blen = calc_blob_len(p->key_len);
+	if (blen > MAX_BLOB_SIZE) {
+		ret = -E2BIG;
+		goto out;
+	}
+
+	b->fmt_version = DCP_BLOB_VERSION;
+	get_random_bytes(b->nonce, AES_KEYSIZE_128);
+	get_random_bytes(b->blob_key, AES_KEYSIZE_128);
+
+	ret = do_aead_crypto(p->key, b->payload, p->key_len, b->blob_key,
+			     b->nonce, true);
+	if (ret) {
+		pr_err("Unable to encrypt blob payload: %i\n", ret);
+		goto out;
+	}
+
+	ret = encrypt_blob_key(b->blob_key);
+	if (ret) {
+		pr_err("Unable to encrypt blob key: %i\n", ret);
+		goto out;
+	}
+
+	b->payload_len = cpu_to_le32(p->key_len);
+	p->blob_len = blen;
+	ret = 0;
+out:
+	return ret;
+}
+
+static int trusted_dcp_unseal(struct trusted_key_payload *p, char *datablob)
+{
+	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
+	int blen, ret;
+
+	if (b->fmt_version != DCP_BLOB_VERSION) {
+		pr_err("DCP blob has bad version: %i, expected %i\n",
+		       b->fmt_version, DCP_BLOB_VERSION);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	p->key_len = le32_to_cpu(b->payload_len);
+	blen = calc_blob_len(p->key_len);
+	if (blen != p->blob_len) {
+		pr_err("DCP blob has bad length: %i != %i\n", blen,
+		       p->blob_len);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = decrypt_blob_key(b->blob_key);
+	if (ret) {
+		pr_err("Unable to decrypt blob key: %i\n", ret);
+		goto out;
+	}
+
+	ret = do_aead_crypto(b->payload, p->key, p->key_len + DCP_BLOB_AUTHLEN,
+			     b->blob_key, b->nonce, false);
+	if (ret) {
+		pr_err("Unwrap of DCP payload failed: %i\n", ret);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	return ret;
+}
+
+static int trusted_dcp_get_random(unsigned char *key, size_t key_len)
+{
+	get_random_bytes(key, key_len);
+
+	return key_len;
+}
+
+static int test_for_zero_key(void)
+{
+	static const u8 bad[] = {0x9a, 0xda, 0xe0, 0x54, 0xf6, 0x3d, 0xfa, 0xff,
+				 0x5e, 0xa1, 0x8e, 0x45, 0xed, 0xf6, 0xea, 0x6f};
+	void *buf = NULL;
+	int ret = 0;
+
+	if (skip_zk_test)
+		goto out;
+
+	buf = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memset(buf, 0x55, AES_BLOCK_SIZE);
+
+	ret = do_dcp_crypto(buf, buf, true);
+	if (ret)
+		goto out;
+
+	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
+		pr_err("Device neither in secure nor trusted mode!\n");
+		ret = -EINVAL;
+	}
+out:
+	kfree(buf);
+	return ret;
+}
+
+static int trusted_dcp_init(void)
+{
+	int ret;
+
+	if (use_otp_key)
+		pr_info("Using DCP OTP key\n");
+
+	ret = test_for_zero_key();
+	if (ret) {
+		pr_err("Test for zero'ed keys failed: %i\n", ret);
+
+		return -EINVAL;
+	}
+
+	return register_key_type(&key_type_trusted);
+}
+
+static void trusted_dcp_exit(void)
+{
+	unregister_key_type(&key_type_trusted);
+}
+
+struct trusted_key_ops dcp_trusted_key_ops = {
+	.exit = trusted_dcp_exit,
+	.init = trusted_dcp_init,
+	.seal = trusted_dcp_seal,
+	.unseal = trusted_dcp_unseal,
+	.get_random = trusted_dcp_get_random,
+	.migratable = 0,
+};
-- 
2.26.2

[PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Richard Weinberger]

DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.

Beside of accelerated crypto operations, it also offers support for
hardware-bound keys. Using this feature it is possible to implement a blob
mechanism just like CAAM offers. Unlike on CAAM, constructing and
parsing the blob has to happen in software.

We chose the following format for the blob:
/*
 * struct dcp_blob_fmt - DCP BLOB format.
 *
 * @fmt_version: Format version, currently being %1
 * @blob_key: Random AES 128 key which is used to encrypt @payload,
 *            @blob_key itself is encrypted with OTP or UNIQUE device key in
 *            AES-128-ECB mode by DCP.
 * @nonce: Random nonce used for @payload encryption.
 * @payload_len: Length of the plain text @payload.
 * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
 *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
 *
 * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
 * AES_BLOCK_SIZE.
 */
struct dcp_blob_fmt {
	__u8 fmt_version;
	__u8 blob_key[AES_KEYSIZE_128];
	__u8 nonce[AES_KEYSIZE_128];
	__le32 payload_len;
	__u8 payload[0];
} __packed;

@payload is the key provided by trusted_key_ops->seal().

By default the UNIQUE device key is used, it is also possible to use
the OTP key. While the UNIQUE device key should be unique it is not
entirely clear whether this is the case due to unclear documentation.
If someone wants to be sure they can burn their own unique key
into the OTP fuse and set the use_otp_key module parameter.

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: David Gstir <david@sigma-star.at>
Signed-off-by: David Gstir <david@sigma-star.at>
Signed-off-by: Richard Weinberger <richard@nod.at>
---
 .../admin-guide/kernel-parameters.txt         |   1 +
 MAINTAINERS                                   |   9 +
 include/keys/trusted_dcp.h                    |  13 +
 security/keys/trusted-keys/Makefile           |   1 +
 security/keys/trusted-keys/trusted_core.c     |   6 +-
 security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
 6 files changed, 354 insertions(+), 1 deletion(-)
 create mode 100644 include/keys/trusted_dcp.h
 create mode 100644 security/keys/trusted-keys/trusted_dcp.c

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index cb89dbdedc46..d8b2aa94a566 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5639,6 +5639,7 @@
 			sources:
 			- "tpm"
 			- "tee"
+			- "dcp"
 			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/MAINTAINERS b/MAINTAINERS
index b706dd20ff2b..779cac00827c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10099,6 +10099,15 @@ F:	include/keys/trusted-type.h
 F:	include/keys/trusted_tpm.h
 F:	security/keys/trusted-keys/
 
+KEYS-TRUSTED-DCP
+M:	David Gstir <david@sigma-star.at>
+M:	Richard Weinberger <richard@nod.at>
+L:	linux-integrity@vger.kernel.org
+L:	keyrings@vger.kernel.org
+S:	Supported
+F:	include/keys/trusted_dcp.h
+F:	security/keys/trusted-keys/trusted_dcp.c
+
 KEYS-TRUSTED-TEE
 M:	Sumit Garg <sumit.garg@linaro.org>
 L:	linux-integrity@vger.kernel.org
diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
new file mode 100644
index 000000000000..7b2a1275c527
--- /dev/null
+++ b/include/keys/trusted_dcp.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#ifndef TRUSTED_DCP_H
+#define TRUSTED_DCP_H
+
+extern struct trusted_key_ops dcp_trusted_key_ops;
+
+#endif
diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
index feb8b6c3cc79..992b591692dc 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_MXS_DCP) += trusted_dcp.o
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..66b631ffe876 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -8,6 +8,7 @@
 
 #include <keys/user-type.h>
 #include <keys/trusted-type.h>
+#include <keys/trusted_dcp.h>
 #include <keys/trusted_tee.h>
 #include <keys/trusted_tpm.h>
 #include <linux/capability.h>
@@ -24,7 +25,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 dcp)");
 
 static const struct trusted_key_source trusted_key_sources[] = {
 #if defined(CONFIG_TCG_TPM)
@@ -33,6 +34,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_MXS_DCP)
+	{ "dcp", &dcp_trusted_key_ops },
+#endif
 };
 
 DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
new file mode 100644
index 000000000000..02db5dc261c7
--- /dev/null
+++ b/security/keys/trusted-keys/trusted_dcp.c
@@ -0,0 +1,325 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 sigma star gmbh
+ * Authors: David Gstir <david@sigma-star.at>
+ *          Richard Weinberger <richard@sigma-star.at>
+ */
+
+#include <crypto/aead.h>
+#include <crypto/aes.h>
+#include <crypto/algapi.h>
+#include <crypto/gcm.h>
+#include <crypto/skcipher.h>
+#include <keys/trusted-type.h>
+#include <linux/key-type.h>
+#include <linux/module.h>
+#include <linux/mxs-dcp.h>
+#include <linux/printk.h>
+#include <linux/random.h>
+#include <linux/scatterlist.h>
+
+#define DCP_BLOB_VERSION 1
+#define DCP_BLOB_AUTHLEN 16
+
+/**
+ * struct dcp_blob_fmt - DCP BLOB format.
+ *
+ * @fmt_version: Format version, currently being %1.
+ * @blob_key: Random AES 128 key which is used to encrypt @payload,
+ *            @blob_key itself is encrypted with OTP or UNIQUE device key in
+ *            AES-128-ECB mode by DCP.
+ * @nonce: Random nonce used for @payload encryption.
+ * @payload_len: Length of the plain text @payload.
+ * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
+ *           GCM auth tag of size DCP_BLOB_AUTHLEN is attached at the end of it.
+ *
+ * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
+ * DCP_BLOB_AUTHLEN.
+ */
+struct dcp_blob_fmt {
+	__u8 fmt_version;
+	__u8 blob_key[AES_KEYSIZE_128];
+	__u8 nonce[AES_KEYSIZE_128];
+	__le32 payload_len;
+	__u8 payload[0];
+} __packed;
+
+static bool use_otp_key;
+module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
+MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
+
+static bool skip_zk_test;
+module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
+MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are zero'ed");
+
+static unsigned int calc_blob_len(unsigned int payload_len)
+{
+	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
+}
+
+static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
+{
+	int res = 0;
+	struct skcipher_request *req = NULL;
+	DECLARE_CRYPTO_WAIT(wait);
+	struct scatterlist src_sg, dst_sg;
+	struct crypto_skcipher *tfm;
+	u8 paes_key[DCP_PAES_KEYSIZE];
+
+	if (!use_otp_key)
+		paes_key[0] = DCP_PAES_KEY_UNIQUE;
+	else
+		paes_key[0] = DCP_PAES_KEY_OTP;
+
+	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
+				    CRYPTO_ALG_INTERNAL);
+	if (IS_ERR(tfm)) {
+		res = PTR_ERR(tfm);
+		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
+		tfm = NULL;
+		goto out;
+	}
+
+	req = skcipher_request_alloc(tfm, GFP_NOFS);
+	if (!req) {
+		res = -ENOMEM;
+		goto out;
+	}
+
+	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+				      CRYPTO_TFM_REQ_MAY_SLEEP,
+				      crypto_req_done, &wait);
+	res = crypto_skcipher_setkey(tfm, paes_key, sizeof(paes_key));
+	if (res < 0)
+		goto out;
+
+	sg_init_one(&src_sg, in, AES_KEYSIZE_128);
+	sg_init_one(&dst_sg, out, AES_KEYSIZE_128);
+	skcipher_request_set_crypt(req, &src_sg, &dst_sg, AES_KEYSIZE_128,
+				   NULL);
+
+	if (is_encrypt)
+		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+	else
+		res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
+
+out:
+	skcipher_request_free(req);
+	crypto_free_skcipher(tfm);
+
+	return res;
+}
+
+static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
+			  bool is_encrypt)
+{
+	struct aead_request *aead_req = NULL;
+	struct scatterlist src_sg, dst_sg;
+	struct crypto_aead *aead;
+	int ret;
+
+	aead = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(aead)) {
+		ret = PTR_ERR(aead);
+		pr_err("Unable to request AES-GCM cipher: %i\n", ret);
+		goto out;
+	}
+
+	ret = crypto_aead_setauthsize(aead, DCP_BLOB_AUTHLEN);
+	if (ret < 0) {
+		pr_err("Can't set crypto auth tag len: %d\n", ret);
+		goto free_aead;
+	}
+
+	aead_req = aead_request_alloc(aead, GFP_KERNEL);
+	if (!aead_req) {
+		ret = -ENOMEM;
+		goto free_aead;
+	}
+
+	sg_init_one(&src_sg, in, len);
+	if (is_encrypt) {
+		/*
+		 * If we encrypt our buffer has extra space for the auth tag.
+		 */
+		sg_init_one(&dst_sg, out, len + DCP_BLOB_AUTHLEN);
+	} else {
+		sg_init_one(&dst_sg, out, len);
+	}
+
+	aead_request_set_crypt(aead_req, &src_sg, &dst_sg, len, nonce);
+	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL,
+				  NULL);
+	aead_request_set_ad(aead_req, 0);
+
+	if (crypto_aead_setkey(aead, key, AES_KEYSIZE_128)) {
+		pr_err("Can't set crypto AEAD key\n");
+		ret = -EINVAL;
+		goto free_req;
+	}
+
+	if (is_encrypt)
+		ret = crypto_aead_encrypt(aead_req);
+	else
+		ret = crypto_aead_decrypt(aead_req);
+
+free_req:
+	aead_request_free(aead_req);
+free_aead:
+	crypto_free_aead(aead);
+out:
+	return ret;
+}
+
+static int decrypt_blob_key(u8 *key)
+{
+	return do_dcp_crypto(key, key, false);
+}
+
+static int encrypt_blob_key(u8 *key)
+{
+	return do_dcp_crypto(key, key, true);
+}
+
+static int trusted_dcp_seal(struct trusted_key_payload *p, char *datablob)
+{
+	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
+	int blen, ret;
+
+	blen = calc_blob_len(p->key_len);
+	if (blen > MAX_BLOB_SIZE) {
+		ret = -E2BIG;
+		goto out;
+	}
+
+	b->fmt_version = DCP_BLOB_VERSION;
+	get_random_bytes(b->nonce, AES_KEYSIZE_128);
+	get_random_bytes(b->blob_key, AES_KEYSIZE_128);
+
+	ret = do_aead_crypto(p->key, b->payload, p->key_len, b->blob_key,
+			     b->nonce, true);
+	if (ret) {
+		pr_err("Unable to encrypt blob payload: %i\n", ret);
+		goto out;
+	}
+
+	ret = encrypt_blob_key(b->blob_key);
+	if (ret) {
+		pr_err("Unable to encrypt blob key: %i\n", ret);
+		goto out;
+	}
+
+	b->payload_len = cpu_to_le32(p->key_len);
+	p->blob_len = blen;
+	ret = 0;
+out:
+	return ret;
+}
+
+static int trusted_dcp_unseal(struct trusted_key_payload *p, char *datablob)
+{
+	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
+	int blen, ret;
+
+	if (b->fmt_version != DCP_BLOB_VERSION) {
+		pr_err("DCP blob has bad version: %i, expected %i\n",
+		       b->fmt_version, DCP_BLOB_VERSION);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	p->key_len = le32_to_cpu(b->payload_len);
+	blen = calc_blob_len(p->key_len);
+	if (blen != p->blob_len) {
+		pr_err("DCP blob has bad length: %i != %i\n", blen,
+		       p->blob_len);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = decrypt_blob_key(b->blob_key);
+	if (ret) {
+		pr_err("Unable to decrypt blob key: %i\n", ret);
+		goto out;
+	}
+
+	ret = do_aead_crypto(b->payload, p->key, p->key_len + DCP_BLOB_AUTHLEN,
+			     b->blob_key, b->nonce, false);
+	if (ret) {
+		pr_err("Unwrap of DCP payload failed: %i\n", ret);
+		goto out;
+	}
+
+	ret = 0;
+out:
+	return ret;
+}
+
+static int trusted_dcp_get_random(unsigned char *key, size_t key_len)
+{
+	get_random_bytes(key, key_len);
+
+	return key_len;
+}
+
+static int test_for_zero_key(void)
+{
+	static const u8 bad[] = {0x9a, 0xda, 0xe0, 0x54, 0xf6, 0x3d, 0xfa, 0xff,
+				 0x5e, 0xa1, 0x8e, 0x45, 0xed, 0xf6, 0xea, 0x6f};
+	void *buf = NULL;
+	int ret = 0;
+
+	if (skip_zk_test)
+		goto out;
+
+	buf = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	memset(buf, 0x55, AES_BLOCK_SIZE);
+
+	ret = do_dcp_crypto(buf, buf, true);
+	if (ret)
+		goto out;
+
+	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
+		pr_err("Device neither in secure nor trusted mode!\n");
+		ret = -EINVAL;
+	}
+out:
+	kfree(buf);
+	return ret;
+}
+
+static int trusted_dcp_init(void)
+{
+	int ret;
+
+	if (use_otp_key)
+		pr_info("Using DCP OTP key\n");
+
+	ret = test_for_zero_key();
+	if (ret) {
+		pr_err("Test for zero'ed keys failed: %i\n", ret);
+
+		return -EINVAL;
+	}
+
+	return register_key_type(&key_type_trusted);
+}
+
+static void trusted_dcp_exit(void)
+{
+	unregister_key_type(&key_type_trusted);
+}
+
+struct trusted_key_ops dcp_trusted_key_ops = {
+	.exit = trusted_dcp_exit,
+	.init = trusted_dcp_init,
+	.seal = trusted_dcp_seal,
+	.unseal = trusted_dcp_unseal,
+	.get_random = trusted_dcp_get_random,
+	.migratable = 0,
+};
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Ahmad Fatoum]

Hello Richard,

On 14.06.21 22:16, Richard Weinberger wrote:
> DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.
> 
> Beside of accelerated crypto operations, it also offers support for
> hardware-bound keys. Using this feature it is possible to implement a blob
> mechanism just like CAAM offers. Unlike on CAAM, constructing and
> parsing the blob has to happen in software.
> 
> We chose the following format for the blob:
> /*
>  * struct dcp_blob_fmt - DCP BLOB format.
>  *
>  * @fmt_version: Format version, currently being %1
>  * @blob_key: Random AES 128 key which is used to encrypt @payload,
>  *            @blob_key itself is encrypted with OTP or UNIQUE device key in
>  *            AES-128-ECB mode by DCP.
>  * @nonce: Random nonce used for @payload encryption.
>  * @payload_len: Length of the plain text @payload.
>  * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
>  *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
>  *
>  * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
>  * AES_BLOCK_SIZE.
>  */
> struct dcp_blob_fmt {
> 	__u8 fmt_version;
> 	__u8 blob_key[AES_KEYSIZE_128];
> 	__u8 nonce[AES_KEYSIZE_128];
> 	__le32 payload_len;
> 	__u8 payload[0];

There's been ongoing effort to replace the [0] GNU extension with C99
flexible array members. Please use [] here as well.

> } __packed;
> 
> @payload is the key provided by trusted_key_ops->seal().
> 
> By default the UNIQUE device key is used, it is also possible to use
> the OTP key. While the UNIQUE device key should be unique it is not
> entirely clear whether this is the case due to unclear documentation.
> If someone wants to be sure they can burn their own unique key
> into the OTP fuse and set the use_otp_key module parameter.
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  .../admin-guide/kernel-parameters.txt         |   1 +
>  MAINTAINERS                                   |   9 +
>  include/keys/trusted_dcp.h                    |  13 +
>  security/keys/trusted-keys/Makefile           |   1 +
>  security/keys/trusted-keys/trusted_core.c     |   6 +-
>  security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
>  6 files changed, 354 insertions(+), 1 deletion(-)
>  create mode 100644 include/keys/trusted_dcp.h
>  create mode 100644 security/keys/trusted-keys/trusted_dcp.c
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index cb89dbdedc46..d8b2aa94a566 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5639,6 +5639,7 @@
>  			sources:
>  			- "tpm"
>  			- "tee"
> +			- "dcp"
>  			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/MAINTAINERS b/MAINTAINERS
> index b706dd20ff2b..779cac00827c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10099,6 +10099,15 @@ F:	include/keys/trusted-type.h
>  F:	include/keys/trusted_tpm.h
>  F:	security/keys/trusted-keys/
>  
> +KEYS-TRUSTED-DCP
> +M:	David Gstir <david@sigma-star.at>
> +M:	Richard Weinberger <richard@nod.at>
> +L:	linux-integrity@vger.kernel.org
> +L:	keyrings@vger.kernel.org
> +S:	Supported
> +F:	include/keys/trusted_dcp.h
> +F:	security/keys/trusted-keys/trusted_dcp.c

Hmm, I didn't add a MAINTAINERS entry for CAAM trusted keys. Do you think I should?

> +
>  KEYS-TRUSTED-TEE
>  M:	Sumit Garg <sumit.garg@linaro.org>
>  L:	linux-integrity@vger.kernel.org
> diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
> new file mode 100644
> index 000000000000..7b2a1275c527
> --- /dev/null
> +++ b/include/keys/trusted_dcp.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#ifndef TRUSTED_DCP_H
> +#define TRUSTED_DCP_H
> +
> +extern struct trusted_key_ops dcp_trusted_key_ops;
> +
> +#endif
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index feb8b6c3cc79..992b591692dc 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_MXS_DCP) += trusted_dcp.o
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..66b631ffe876 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -8,6 +8,7 @@
>  
>  #include <keys/user-type.h>
>  #include <keys/trusted-type.h>
> +#include <keys/trusted_dcp.h>
>  #include <keys/trusted_tee.h>
>  #include <keys/trusted_tpm.h>
>  #include <linux/capability.h>
> @@ -24,7 +25,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 dcp)");
>  
>  static const struct trusted_key_source trusted_key_sources[] = {
>  #if defined(CONFIG_TCG_TPM)
> @@ -33,6 +34,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_MXS_DCP)
> +	{ "dcp", &dcp_trusted_key_ops },
> +#endif
>  };
>  
>  DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
> diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
> new file mode 100644
> index 000000000000..02db5dc261c7
> --- /dev/null
> +++ b/security/keys/trusted-keys/trusted_dcp.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#include <crypto/aead.h>
> +#include <crypto/aes.h>
> +#include <crypto/algapi.h>
> +#include <crypto/gcm.h>
> +#include <crypto/skcipher.h>
> +#include <keys/trusted-type.h>
> +#include <linux/key-type.h>
> +#include <linux/module.h>
> +#include <linux/mxs-dcp.h>
> +#include <linux/printk.h>
> +#include <linux/random.h>
> +#include <linux/scatterlist.h>
> +
> +#define DCP_BLOB_VERSION 1
> +#define DCP_BLOB_AUTHLEN 16
> +
> +/**
> + * struct dcp_blob_fmt - DCP BLOB format.
> + *
> + * @fmt_version: Format version, currently being %1.
> + * @blob_key: Random AES 128 key which is used to encrypt @payload,
> + *            @blob_key itself is encrypted with OTP or UNIQUE device key in
> + *            AES-128-ECB mode by DCP.
> + * @nonce: Random nonce used for @payload encryption.
> + * @payload_len: Length of the plain text @payload.
> + * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
> + *           GCM auth tag of size DCP_BLOB_AUTHLEN is attached at the end of it.
> + *
> + * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
> + * DCP_BLOB_AUTHLEN.
> + */
> +struct dcp_blob_fmt {
> +	__u8 fmt_version;
> +	__u8 blob_key[AES_KEYSIZE_128];
> +	__u8 nonce[AES_KEYSIZE_128];
> +	__le32 payload_len;
> +	__u8 payload[0];

[], see above.

> +} __packed;
> +
> +static bool use_otp_key;
> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");

Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?

> +static bool skip_zk_test;
> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are zero'ed");

Does this need to be configurible? I'd assume this can only happen when using an
unfused OTP. In such a case, it's ok to always warn, so you don't need to make
this configurible.

> +
> +static unsigned int calc_blob_len(unsigned int payload_len)
> +{
> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
> +}
> +
> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)

I assume in can't be const because the use with sg APIs?

> +{
> +	int res = 0;
> +	struct skcipher_request *req = NULL;
> +	DECLARE_CRYPTO_WAIT(wait);
> +	struct scatterlist src_sg, dst_sg;
> +	struct crypto_skcipher *tfm;
> +	u8 paes_key[DCP_PAES_KEYSIZE];
> +
> +	if (!use_otp_key)

I'd invert this. Makes code easier to read.

> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
> +	else
> +		paes_key[0] = DCP_PAES_KEY_OTP;
> +
> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
> +				    CRYPTO_ALG_INTERNAL);
> +	if (IS_ERR(tfm)) {
> +		res = PTR_ERR(tfm);
> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);

Can you define pr_fmt above? There's also %pe now that can directly print out an
error pointer.

> +		tfm = NULL;
> +		goto out;
> +	}
> +
> +	req = skcipher_request_alloc(tfm, GFP_NOFS);
> +	if (!req) {
> +		res = -ENOMEM;
> +		goto out;
> +	}
> +
> +	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
> +				      CRYPTO_TFM_REQ_MAY_SLEEP,
> +				      crypto_req_done, &wait);
> +	res = crypto_skcipher_setkey(tfm, paes_key, sizeof(paes_key));
> +	if (res < 0)
> +		goto out;
> +
> +	sg_init_one(&src_sg, in, AES_KEYSIZE_128);
> +	sg_init_one(&dst_sg, out, AES_KEYSIZE_128);
> +	skcipher_request_set_crypt(req, &src_sg, &dst_sg, AES_KEYSIZE_128,
> +				   NULL);
> +
> +	if (is_encrypt)
> +		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
> +	else
> +		res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
> +
> +out:
> +	skcipher_request_free(req);
> +	crypto_free_skcipher(tfm);
> +
> +	return res;
> +}
> +
> +static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
> +			  bool is_encrypt)
> +{
> +	struct aead_request *aead_req = NULL;
> +	struct scatterlist src_sg, dst_sg;
> +	struct crypto_aead *aead;
> +	int ret;
> +
> +	aead = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(aead)) {
> +		ret = PTR_ERR(aead);
> +		pr_err("Unable to request AES-GCM cipher: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = crypto_aead_setauthsize(aead, DCP_BLOB_AUTHLEN);
> +	if (ret < 0) {
> +		pr_err("Can't set crypto auth tag len: %d\n", ret);
> +		goto free_aead;
> +	}
> +
> +	aead_req = aead_request_alloc(aead, GFP_KERNEL);
> +	if (!aead_req) {
> +		ret = -ENOMEM;
> +		goto free_aead;
> +	}
> +
> +	sg_init_one(&src_sg, in, len);
> +	if (is_encrypt) {
> +		/*
> +		 * If we encrypt our buffer has extra space for the auth tag.
> +		 */
> +		sg_init_one(&dst_sg, out, len + DCP_BLOB_AUTHLEN);
> +	} else {
> +		sg_init_one(&dst_sg, out, len);
> +	}
> +
> +	aead_request_set_crypt(aead_req, &src_sg, &dst_sg, len, nonce);
> +	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL,
> +				  NULL);
> +	aead_request_set_ad(aead_req, 0);
> +
> +	if (crypto_aead_setkey(aead, key, AES_KEYSIZE_128)) {
> +		pr_err("Can't set crypto AEAD key\n");
> +		ret = -EINVAL;
> +		goto free_req;
> +	}
> +
> +	if (is_encrypt)
> +		ret = crypto_aead_encrypt(aead_req);
> +	else
> +		ret = crypto_aead_decrypt(aead_req);
> +
> +free_req:
> +	aead_request_free(aead_req);
> +free_aead:
> +	crypto_free_aead(aead);
> +out:
> +	return ret;
> +}
> +
> +static int decrypt_blob_key(u8 *key)
> +{
> +	return do_dcp_crypto(key, key, false);
> +}
> +
> +static int encrypt_blob_key(u8 *key)
> +{
> +	return do_dcp_crypto(key, key, true);
> +}
> +
> +static int trusted_dcp_seal(struct trusted_key_payload *p, char *datablob)
> +{
> +	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
> +	int blen, ret;
> +
> +	blen = calc_blob_len(p->key_len);
> +	if (blen > MAX_BLOB_SIZE) {
> +		ret = -E2BIG;
> +		goto out;

Nitpick, just return and drop the goto?

> +	}
> +
> +	b->fmt_version = DCP_BLOB_VERSION;
> +	get_random_bytes(b->nonce, AES_KEYSIZE_128);
> +	get_random_bytes(b->blob_key, AES_KEYSIZE_128);
> +
> +	ret = do_aead_crypto(p->key, b->payload, p->key_len, b->blob_key,
> +			     b->nonce, true);
> +	if (ret) {
> +		pr_err("Unable to encrypt blob payload: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = encrypt_blob_key(b->blob_key);
> +	if (ret) {
> +		pr_err("Unable to encrypt blob key: %i\n", ret);
> +		goto out;
> +	}
> +
> +	b->payload_len = cpu_to_le32(p->key_len);

 - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
   size checks. Couldn't you read beyond the buffer this way?

 - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.


> +	p->blob_len = blen;
> +	ret = 0;
> +out:
> +	return ret;
> +}
> +
> +static int trusted_dcp_unseal(struct trusted_key_payload *p, char *datablob)
> +{
> +	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
> +	int blen, ret;
> +
> +	if (b->fmt_version != DCP_BLOB_VERSION) {
> +		pr_err("DCP blob has bad version: %i, expected %i\n",
> +		       b->fmt_version, DCP_BLOB_VERSION);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	p->key_len = le32_to_cpu(b->payload_len);
> +	blen = calc_blob_len(p->key_len);
> +	if (blen != p->blob_len) {
> +		pr_err("DCP blob has bad length: %i != %i\n", blen,
> +		       p->blob_len);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = decrypt_blob_key(b->blob_key);
> +	if (ret) {
> +		pr_err("Unable to decrypt blob key: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = do_aead_crypto(b->payload, p->key, p->key_len + DCP_BLOB_AUTHLEN,
> +			     b->blob_key, b->nonce, false);
> +	if (ret) {
> +		pr_err("Unwrap of DCP payload failed: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = 0;
> +out:
> +	return ret;
> +}
> +
> +static int trusted_dcp_get_random(unsigned char *key, size_t key_len)
> +{
> +	get_random_bytes(key, key_len);
> +
> +	return key_len;
> +}

jfyi, in the prelude of my CAAM series, I made this the default
when .get_random == NULL.

> +
> +static int test_for_zero_key(void)
> +{
> +	static const u8 bad[] = {0x9a, 0xda, 0xe0, 0x54, 0xf6, 0x3d, 0xfa, 0xff,
> +				 0x5e, 0xa1, 0x8e, 0x45, 0xed, 0xf6, 0xea, 0x6f};
> +	void *buf = NULL;
> +	int ret = 0;
> +
> +	if (skip_zk_test)
> +		goto out;
> +
> +	buf = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL);
> +	if (!buf) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	memset(buf, 0x55, AES_BLOCK_SIZE);
> +
> +	ret = do_dcp_crypto(buf, buf, true);
> +	if (ret)
> +		goto out;
> +
> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
> +		pr_err("Device neither in secure nor trusted mode!\n");

What's the difference between secure and trusted? Can't this test be skipped
if use_otp_key == false?

> +		ret = -EINVAL;
> +	}
> +out:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static int trusted_dcp_init(void)
> +{
> +	int ret;
> +
> +	if (use_otp_key)
> +		pr_info("Using DCP OTP key\n");
> +
> +	ret = test_for_zero_key();
> +	if (ret) {
> +		pr_err("Test for zero'ed keys failed: %i\n", ret);
> +
> +		return -EINVAL;
> +	}
> +
> +	return register_key_type(&key_type_trusted);
> +}
> +
> +static void trusted_dcp_exit(void)
> +{
> +	unregister_key_type(&key_type_trusted);
> +}
> +
> +struct trusted_key_ops dcp_trusted_key_ops = {
> +	.exit = trusted_dcp_exit,
> +	.init = trusted_dcp_init,
> +	.seal = trusted_dcp_seal,
> +	.unseal = trusted_dcp_unseal,
> +	.get_random = trusted_dcp_get_random,
> +	.migratable = 0,
> +};

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 |

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Ahmad Fatoum]

Hello Richard,

On 14.06.21 22:16, Richard Weinberger wrote:
> DCP (Data Co-Processor) is the little brother of NXP's CAAM IP.
> 
> Beside of accelerated crypto operations, it also offers support for
> hardware-bound keys. Using this feature it is possible to implement a blob
> mechanism just like CAAM offers. Unlike on CAAM, constructing and
> parsing the blob has to happen in software.
> 
> We chose the following format for the blob:
> /*
>  * struct dcp_blob_fmt - DCP BLOB format.
>  *
>  * @fmt_version: Format version, currently being %1
>  * @blob_key: Random AES 128 key which is used to encrypt @payload,
>  *            @blob_key itself is encrypted with OTP or UNIQUE device key in
>  *            AES-128-ECB mode by DCP.
>  * @nonce: Random nonce used for @payload encryption.
>  * @payload_len: Length of the plain text @payload.
>  * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
>  *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
>  *
>  * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
>  * AES_BLOCK_SIZE.
>  */
> struct dcp_blob_fmt {
> 	__u8 fmt_version;
> 	__u8 blob_key[AES_KEYSIZE_128];
> 	__u8 nonce[AES_KEYSIZE_128];
> 	__le32 payload_len;
> 	__u8 payload[0];

There's been ongoing effort to replace the [0] GNU extension with C99
flexible array members. Please use [] here as well.

> } __packed;
> 
> @payload is the key provided by trusted_key_ops->seal().
> 
> By default the UNIQUE device key is used, it is also possible to use
> the OTP key. While the UNIQUE device key should be unique it is not
> entirely clear whether this is the case due to unclear documentation.
> If someone wants to be sure they can burn their own unique key
> into the OTP fuse and set the use_otp_key module parameter.
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: David Gstir <david@sigma-star.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  .../admin-guide/kernel-parameters.txt         |   1 +
>  MAINTAINERS                                   |   9 +
>  include/keys/trusted_dcp.h                    |  13 +
>  security/keys/trusted-keys/Makefile           |   1 +
>  security/keys/trusted-keys/trusted_core.c     |   6 +-
>  security/keys/trusted-keys/trusted_dcp.c      | 325 ++++++++++++++++++
>  6 files changed, 354 insertions(+), 1 deletion(-)
>  create mode 100644 include/keys/trusted_dcp.h
>  create mode 100644 security/keys/trusted-keys/trusted_dcp.c
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index cb89dbdedc46..d8b2aa94a566 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5639,6 +5639,7 @@
>  			sources:
>  			- "tpm"
>  			- "tee"
> +			- "dcp"
>  			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/MAINTAINERS b/MAINTAINERS
> index b706dd20ff2b..779cac00827c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10099,6 +10099,15 @@ F:	include/keys/trusted-type.h
>  F:	include/keys/trusted_tpm.h
>  F:	security/keys/trusted-keys/
>  
> +KEYS-TRUSTED-DCP
> +M:	David Gstir <david@sigma-star.at>
> +M:	Richard Weinberger <richard@nod.at>
> +L:	linux-integrity@vger.kernel.org
> +L:	keyrings@vger.kernel.org
> +S:	Supported
> +F:	include/keys/trusted_dcp.h
> +F:	security/keys/trusted-keys/trusted_dcp.c

Hmm, I didn't add a MAINTAINERS entry for CAAM trusted keys. Do you think I should?

> +
>  KEYS-TRUSTED-TEE
>  M:	Sumit Garg <sumit.garg@linaro.org>
>  L:	linux-integrity@vger.kernel.org
> diff --git a/include/keys/trusted_dcp.h b/include/keys/trusted_dcp.h
> new file mode 100644
> index 000000000000..7b2a1275c527
> --- /dev/null
> +++ b/include/keys/trusted_dcp.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#ifndef TRUSTED_DCP_H
> +#define TRUSTED_DCP_H
> +
> +extern struct trusted_key_ops dcp_trusted_key_ops;
> +
> +#endif
> diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile
> index feb8b6c3cc79..992b591692dc 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_MXS_DCP) += trusted_dcp.o
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..66b631ffe876 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -8,6 +8,7 @@
>  
>  #include <keys/user-type.h>
>  #include <keys/trusted-type.h>
> +#include <keys/trusted_dcp.h>
>  #include <keys/trusted_tee.h>
>  #include <keys/trusted_tpm.h>
>  #include <linux/capability.h>
> @@ -24,7 +25,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 dcp)");
>  
>  static const struct trusted_key_source trusted_key_sources[] = {
>  #if defined(CONFIG_TCG_TPM)
> @@ -33,6 +34,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_MXS_DCP)
> +	{ "dcp", &dcp_trusted_key_ops },
> +#endif
>  };
>  
>  DEFINE_STATIC_CALL_NULL(trusted_key_init, *trusted_key_sources[0].ops->init);
> diff --git a/security/keys/trusted-keys/trusted_dcp.c b/security/keys/trusted-keys/trusted_dcp.c
> new file mode 100644
> index 000000000000..02db5dc261c7
> --- /dev/null
> +++ b/security/keys/trusted-keys/trusted_dcp.c
> @@ -0,0 +1,325 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 sigma star gmbh
> + * Authors: David Gstir <david@sigma-star.at>
> + *          Richard Weinberger <richard@sigma-star.at>
> + */
> +
> +#include <crypto/aead.h>
> +#include <crypto/aes.h>
> +#include <crypto/algapi.h>
> +#include <crypto/gcm.h>
> +#include <crypto/skcipher.h>
> +#include <keys/trusted-type.h>
> +#include <linux/key-type.h>
> +#include <linux/module.h>
> +#include <linux/mxs-dcp.h>
> +#include <linux/printk.h>
> +#include <linux/random.h>
> +#include <linux/scatterlist.h>
> +
> +#define DCP_BLOB_VERSION 1
> +#define DCP_BLOB_AUTHLEN 16
> +
> +/**
> + * struct dcp_blob_fmt - DCP BLOB format.
> + *
> + * @fmt_version: Format version, currently being %1.
> + * @blob_key: Random AES 128 key which is used to encrypt @payload,
> + *            @blob_key itself is encrypted with OTP or UNIQUE device key in
> + *            AES-128-ECB mode by DCP.
> + * @nonce: Random nonce used for @payload encryption.
> + * @payload_len: Length of the plain text @payload.
> + * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
> + *           GCM auth tag of size DCP_BLOB_AUTHLEN is attached at the end of it.
> + *
> + * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
> + * DCP_BLOB_AUTHLEN.
> + */
> +struct dcp_blob_fmt {
> +	__u8 fmt_version;
> +	__u8 blob_key[AES_KEYSIZE_128];
> +	__u8 nonce[AES_KEYSIZE_128];
> +	__le32 payload_len;
> +	__u8 payload[0];

[], see above.

> +} __packed;
> +
> +static bool use_otp_key;
> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");

Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?

> +static bool skip_zk_test;
> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are zero'ed");

Does this need to be configurible? I'd assume this can only happen when using an
unfused OTP. In such a case, it's ok to always warn, so you don't need to make
this configurible.

> +
> +static unsigned int calc_blob_len(unsigned int payload_len)
> +{
> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
> +}
> +
> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)

I assume in can't be const because the use with sg APIs?

> +{
> +	int res = 0;
> +	struct skcipher_request *req = NULL;
> +	DECLARE_CRYPTO_WAIT(wait);
> +	struct scatterlist src_sg, dst_sg;
> +	struct crypto_skcipher *tfm;
> +	u8 paes_key[DCP_PAES_KEYSIZE];
> +
> +	if (!use_otp_key)

I'd invert this. Makes code easier to read.

> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
> +	else
> +		paes_key[0] = DCP_PAES_KEY_OTP;
> +
> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
> +				    CRYPTO_ALG_INTERNAL);
> +	if (IS_ERR(tfm)) {
> +		res = PTR_ERR(tfm);
> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);

Can you define pr_fmt above? There's also %pe now that can directly print out an
error pointer.

> +		tfm = NULL;
> +		goto out;
> +	}
> +
> +	req = skcipher_request_alloc(tfm, GFP_NOFS);
> +	if (!req) {
> +		res = -ENOMEM;
> +		goto out;
> +	}
> +
> +	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
> +				      CRYPTO_TFM_REQ_MAY_SLEEP,
> +				      crypto_req_done, &wait);
> +	res = crypto_skcipher_setkey(tfm, paes_key, sizeof(paes_key));
> +	if (res < 0)
> +		goto out;
> +
> +	sg_init_one(&src_sg, in, AES_KEYSIZE_128);
> +	sg_init_one(&dst_sg, out, AES_KEYSIZE_128);
> +	skcipher_request_set_crypt(req, &src_sg, &dst_sg, AES_KEYSIZE_128,
> +				   NULL);
> +
> +	if (is_encrypt)
> +		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
> +	else
> +		res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
> +
> +out:
> +	skcipher_request_free(req);
> +	crypto_free_skcipher(tfm);
> +
> +	return res;
> +}
> +
> +static int do_aead_crypto(u8 *in, u8 *out, size_t len, u8 *key, u8 *nonce,
> +			  bool is_encrypt)
> +{
> +	struct aead_request *aead_req = NULL;
> +	struct scatterlist src_sg, dst_sg;
> +	struct crypto_aead *aead;
> +	int ret;
> +
> +	aead = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
> +	if (IS_ERR(aead)) {
> +		ret = PTR_ERR(aead);
> +		pr_err("Unable to request AES-GCM cipher: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = crypto_aead_setauthsize(aead, DCP_BLOB_AUTHLEN);
> +	if (ret < 0) {
> +		pr_err("Can't set crypto auth tag len: %d\n", ret);
> +		goto free_aead;
> +	}
> +
> +	aead_req = aead_request_alloc(aead, GFP_KERNEL);
> +	if (!aead_req) {
> +		ret = -ENOMEM;
> +		goto free_aead;
> +	}
> +
> +	sg_init_one(&src_sg, in, len);
> +	if (is_encrypt) {
> +		/*
> +		 * If we encrypt our buffer has extra space for the auth tag.
> +		 */
> +		sg_init_one(&dst_sg, out, len + DCP_BLOB_AUTHLEN);
> +	} else {
> +		sg_init_one(&dst_sg, out, len);
> +	}
> +
> +	aead_request_set_crypt(aead_req, &src_sg, &dst_sg, len, nonce);
> +	aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL,
> +				  NULL);
> +	aead_request_set_ad(aead_req, 0);
> +
> +	if (crypto_aead_setkey(aead, key, AES_KEYSIZE_128)) {
> +		pr_err("Can't set crypto AEAD key\n");
> +		ret = -EINVAL;
> +		goto free_req;
> +	}
> +
> +	if (is_encrypt)
> +		ret = crypto_aead_encrypt(aead_req);
> +	else
> +		ret = crypto_aead_decrypt(aead_req);
> +
> +free_req:
> +	aead_request_free(aead_req);
> +free_aead:
> +	crypto_free_aead(aead);
> +out:
> +	return ret;
> +}
> +
> +static int decrypt_blob_key(u8 *key)
> +{
> +	return do_dcp_crypto(key, key, false);
> +}
> +
> +static int encrypt_blob_key(u8 *key)
> +{
> +	return do_dcp_crypto(key, key, true);
> +}
> +
> +static int trusted_dcp_seal(struct trusted_key_payload *p, char *datablob)
> +{
> +	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
> +	int blen, ret;
> +
> +	blen = calc_blob_len(p->key_len);
> +	if (blen > MAX_BLOB_SIZE) {
> +		ret = -E2BIG;
> +		goto out;

Nitpick, just return and drop the goto?

> +	}
> +
> +	b->fmt_version = DCP_BLOB_VERSION;
> +	get_random_bytes(b->nonce, AES_KEYSIZE_128);
> +	get_random_bytes(b->blob_key, AES_KEYSIZE_128);
> +
> +	ret = do_aead_crypto(p->key, b->payload, p->key_len, b->blob_key,
> +			     b->nonce, true);
> +	if (ret) {
> +		pr_err("Unable to encrypt blob payload: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = encrypt_blob_key(b->blob_key);
> +	if (ret) {
> +		pr_err("Unable to encrypt blob key: %i\n", ret);
> +		goto out;
> +	}
> +
> +	b->payload_len = cpu_to_le32(p->key_len);

 - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
   size checks. Couldn't you read beyond the buffer this way?

 - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.


> +	p->blob_len = blen;
> +	ret = 0;
> +out:
> +	return ret;
> +}
> +
> +static int trusted_dcp_unseal(struct trusted_key_payload *p, char *datablob)
> +{
> +	struct dcp_blob_fmt *b = (struct dcp_blob_fmt *)p->blob;
> +	int blen, ret;
> +
> +	if (b->fmt_version != DCP_BLOB_VERSION) {
> +		pr_err("DCP blob has bad version: %i, expected %i\n",
> +		       b->fmt_version, DCP_BLOB_VERSION);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	p->key_len = le32_to_cpu(b->payload_len);
> +	blen = calc_blob_len(p->key_len);
> +	if (blen != p->blob_len) {
> +		pr_err("DCP blob has bad length: %i != %i\n", blen,
> +		       p->blob_len);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = decrypt_blob_key(b->blob_key);
> +	if (ret) {
> +		pr_err("Unable to decrypt blob key: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = do_aead_crypto(b->payload, p->key, p->key_len + DCP_BLOB_AUTHLEN,
> +			     b->blob_key, b->nonce, false);
> +	if (ret) {
> +		pr_err("Unwrap of DCP payload failed: %i\n", ret);
> +		goto out;
> +	}
> +
> +	ret = 0;
> +out:
> +	return ret;
> +}
> +
> +static int trusted_dcp_get_random(unsigned char *key, size_t key_len)
> +{
> +	get_random_bytes(key, key_len);
> +
> +	return key_len;
> +}

jfyi, in the prelude of my CAAM series, I made this the default
when .get_random == NULL.

> +
> +static int test_for_zero_key(void)
> +{
> +	static const u8 bad[] = {0x9a, 0xda, 0xe0, 0x54, 0xf6, 0x3d, 0xfa, 0xff,
> +				 0x5e, 0xa1, 0x8e, 0x45, 0xed, 0xf6, 0xea, 0x6f};
> +	void *buf = NULL;
> +	int ret = 0;
> +
> +	if (skip_zk_test)
> +		goto out;
> +
> +	buf = kmalloc(AES_BLOCK_SIZE, GFP_KERNEL);
> +	if (!buf) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	memset(buf, 0x55, AES_BLOCK_SIZE);
> +
> +	ret = do_dcp_crypto(buf, buf, true);
> +	if (ret)
> +		goto out;
> +
> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
> +		pr_err("Device neither in secure nor trusted mode!\n");

What's the difference between secure and trusted? Can't this test be skipped
if use_otp_key == false?

> +		ret = -EINVAL;
> +	}
> +out:
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static int trusted_dcp_init(void)
> +{
> +	int ret;
> +
> +	if (use_otp_key)
> +		pr_info("Using DCP OTP key\n");
> +
> +	ret = test_for_zero_key();
> +	if (ret) {
> +		pr_err("Test for zero'ed keys failed: %i\n", ret);
> +
> +		return -EINVAL;
> +	}
> +
> +	return register_key_type(&key_type_trusted);
> +}
> +
> +static void trusted_dcp_exit(void)
> +{
> +	unregister_key_type(&key_type_trusted);
> +}
> +
> +struct trusted_key_ops dcp_trusted_key_ops = {
> +	.exit = trusted_dcp_exit,
> +	.init = trusted_dcp_init,
> +	.seal = trusted_dcp_seal,
> +	.unseal = trusted_dcp_unseal,
> +	.get_random = trusted_dcp_get_random,
> +	.migratable = 0,
> +};

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 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Richard Weinberger]

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>

[...]

>> /*
>>  * struct dcp_blob_fmt - DCP BLOB format.
>>  *
>>  * @fmt_version: Format version, currently being %1
>>  * @blob_key: Random AES 128 key which is used to encrypt @payload,
>>  *            @blob_key itself is encrypted with OTP or UNIQUE device key in
>>  *            AES-128-ECB mode by DCP.
>>  * @nonce: Random nonce used for @payload encryption.
>>  * @payload_len: Length of the plain text @payload.
>>  * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
>>  *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
>>  *
>>  * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
>>  * AES_BLOCK_SIZE.
>>  */
>> struct dcp_blob_fmt {
>> 	__u8 fmt_version;
>> 	__u8 blob_key[AES_KEYSIZE_128];
>> 	__u8 nonce[AES_KEYSIZE_128];
>> 	__le32 payload_len;
>> 	__u8 payload[0];
> 
> There's been ongoing effort to replace the [0] GNU extension with C99
> flexible array members. Please use [] here as well.

Makes sense!

[...]

>> +KEYS-TRUSTED-DCP
>> +M:	David Gstir <david@sigma-star.at>
>> +M:	Richard Weinberger <richard@nod.at>
>> +L:	linux-integrity@vger.kernel.org
>> +L:	keyrings@vger.kernel.org
>> +S:	Supported
>> +F:	include/keys/trusted_dcp.h
>> +F:	security/keys/trusted-keys/trusted_dcp.c
> 
> Hmm, I didn't add a MAINTAINERS entry for CAAM trusted keys. Do you think I
> should?

Sure, why not? It shows that you will also in future take care of it.

[...]

>> +} __packed;
>> +
>> +static bool use_otp_key;
>> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
>> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
> 
> Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?

Yes. Will do.

>> +static bool skip_zk_test;
>> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
>> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are
>> zero'ed");
> 
> Does this need to be configurible? I'd assume this can only happen when using an
> unfused OTP. In such a case, it's ok to always warn, so you don't need to make
> this configurible.

We found such a setting super useful while working with targets where the keys are
zero'ed for various reasons.
There are cases where you want to use/test trusted keys even when the master key
is void. Our detection logic does not only print a warning, it refuses to load
blobs. So IMHO the config knob makes sense.

>> +
>> +static unsigned int calc_blob_len(unsigned int payload_len) 
>> +{
>> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
>> +}
>> +
>> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
> 
> I assume in can't be const because the use with sg APIs?

I'm pretty sure this was the main reason, but I can check again.

>> +{
>> +	int res = 0;
>> +	struct skcipher_request *req = NULL;
>> +	DECLARE_CRYPTO_WAIT(wait);
>> +	struct scatterlist src_sg, dst_sg;
>> +	struct crypto_skcipher *tfm;
>> +	u8 paes_key[DCP_PAES_KEYSIZE];
>> +
>> +	if (!use_otp_key)
> 
> I'd invert this. Makes code easier to read.

Ok. :-)

>> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
>> +	else
>> +		paes_key[0] = DCP_PAES_KEY_OTP;
>> +
>> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
>> +				    CRYPTO_ALG_INTERNAL);
>> +	if (IS_ERR(tfm)) {
>> +		res = PTR_ERR(tfm);
>> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
> 
> Can you define pr_fmt above? There's also %pe now that can directly print out an
> error pointer.

pr_fmt is not defined on purpose. include/keys/trusted-type.h defines already one
and I assumed "trusted_key:" is the desired prefix for all kinds of trusted keys.

[...]

> - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
>   size checks. Couldn't you read beyond the buffer this way?

The key has a minimum size of MIN_KEY_SIZE, but p->blob (being struct trusted_key_payload->blob[MAX_BLOB_SIZE])
is much larger.
So the assumption is that a DCP blob will always be smaller than MAX_BLOB_SIZE.

> - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.

Oh yes. Makes sense!

[...]

> 
> jfyi, in the prelude of my CAAM series, I made this the default
> when .get_random == NULL.

Right. :-)

[...]

>> +	ret = do_dcp_crypto(buf, buf, true);
>> +	if (ret)
>> +		goto out;
>> +
>> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
>> +		pr_err("Device neither in secure nor trusted mode!\n");
> 
> What's the difference between secure and trusted? Can't this test be skipped
> if use_otp_key == false?

DCP has many modes of operation. Secure is one level above trusted.
For the gory details see "Security Reference Manual for the i.MX 6ULL Applications Processor".
I'm not sure whether all information my manual describes is publicly available so I
don't dare to copy&paste from it.

As David and I understood the logic, both OTP and UNIQUE keys can be zero'ed.
It is also possible that DCP has no support at all for these keys,
then you'll also get a zero key. That's why we have this check here.

Thanks,
//richard

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Richard Weinberger]

Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>

[...]

>> /*
>>  * struct dcp_blob_fmt - DCP BLOB format.
>>  *
>>  * @fmt_version: Format version, currently being %1
>>  * @blob_key: Random AES 128 key which is used to encrypt @payload,
>>  *            @blob_key itself is encrypted with OTP or UNIQUE device key in
>>  *            AES-128-ECB mode by DCP.
>>  * @nonce: Random nonce used for @payload encryption.
>>  * @payload_len: Length of the plain text @payload.
>>  * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
>>  *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
>>  *
>>  * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
>>  * AES_BLOCK_SIZE.
>>  */
>> struct dcp_blob_fmt {
>> 	__u8 fmt_version;
>> 	__u8 blob_key[AES_KEYSIZE_128];
>> 	__u8 nonce[AES_KEYSIZE_128];
>> 	__le32 payload_len;
>> 	__u8 payload[0];
> 
> There's been ongoing effort to replace the [0] GNU extension with C99
> flexible array members. Please use [] here as well.

Makes sense!

[...]

>> +KEYS-TRUSTED-DCP
>> +M:	David Gstir <david@sigma-star.at>
>> +M:	Richard Weinberger <richard@nod.at>
>> +L:	linux-integrity@vger.kernel.org
>> +L:	keyrings@vger.kernel.org
>> +S:	Supported
>> +F:	include/keys/trusted_dcp.h
>> +F:	security/keys/trusted-keys/trusted_dcp.c
> 
> Hmm, I didn't add a MAINTAINERS entry for CAAM trusted keys. Do you think I
> should?

Sure, why not? It shows that you will also in future take care of it.

[...]

>> +} __packed;
>> +
>> +static bool use_otp_key;
>> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
>> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
> 
> Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?

Yes. Will do.

>> +static bool skip_zk_test;
>> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
>> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are
>> zero'ed");
> 
> Does this need to be configurible? I'd assume this can only happen when using an
> unfused OTP. In such a case, it's ok to always warn, so you don't need to make
> this configurible.

We found such a setting super useful while working with targets where the keys are
zero'ed for various reasons.
There are cases where you want to use/test trusted keys even when the master key
is void. Our detection logic does not only print a warning, it refuses to load
blobs. So IMHO the config knob makes sense.

>> +
>> +static unsigned int calc_blob_len(unsigned int payload_len) 
>> +{
>> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
>> +}
>> +
>> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
> 
> I assume in can't be const because the use with sg APIs?

I'm pretty sure this was the main reason, but I can check again.

>> +{
>> +	int res = 0;
>> +	struct skcipher_request *req = NULL;
>> +	DECLARE_CRYPTO_WAIT(wait);
>> +	struct scatterlist src_sg, dst_sg;
>> +	struct crypto_skcipher *tfm;
>> +	u8 paes_key[DCP_PAES_KEYSIZE];
>> +
>> +	if (!use_otp_key)
> 
> I'd invert this. Makes code easier to read.

Ok. :-)

>> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
>> +	else
>> +		paes_key[0] = DCP_PAES_KEY_OTP;
>> +
>> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
>> +				    CRYPTO_ALG_INTERNAL);
>> +	if (IS_ERR(tfm)) {
>> +		res = PTR_ERR(tfm);
>> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
> 
> Can you define pr_fmt above? There's also %pe now that can directly print out an
> error pointer.

pr_fmt is not defined on purpose. include/keys/trusted-type.h defines already one
and I assumed "trusted_key:" is the desired prefix for all kinds of trusted keys.

[...]

> - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
>   size checks. Couldn't you read beyond the buffer this way?

The key has a minimum size of MIN_KEY_SIZE, but p->blob (being struct trusted_key_payload->blob[MAX_BLOB_SIZE])
is much larger.
So the assumption is that a DCP blob will always be smaller than MAX_BLOB_SIZE.

> - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.

Oh yes. Makes sense!

[...]

> 
> jfyi, in the prelude of my CAAM series, I made this the default
> when .get_random == NULL.

Right. :-)

[...]

>> +	ret = do_dcp_crypto(buf, buf, true);
>> +	if (ret)
>> +		goto out;
>> +
>> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
>> +		pr_err("Device neither in secure nor trusted mode!\n");
> 
> What's the difference between secure and trusted? Can't this test be skipped
> if use_otp_key == false?

DCP has many modes of operation. Secure is one level above trusted.
For the gory details see "Security Reference Manual for the i.MX 6ULL Applications Processor".
I'm not sure whether all information my manual describes is publicly available so I
don't dare to copy&paste from it.

As David and I understood the logic, both OTP and UNIQUE keys can be zero'ed.
It is also possible that DCP has no support at all for these keys,
then you'll also get a zero key. That's why we have this check here.

Thanks,
//richard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Ahmad Fatoum]

Hello Richard,

On 14.07.21 12:44, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
> 
> [...]
> 
> Sure, why not? It shows that you will also in future take care of it.

Good point. I did that for v3.

> 
> [...]
> 
>>> +} __packed;
>>> +
>>> +static bool use_otp_key;
>>> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
>>> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
>>
>> Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?
> 
> Yes. Will do.
> 
>>> +static bool skip_zk_test;
>>> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
>>> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are
>>> zero'ed");
>>
>> Does this need to be configurible? I'd assume this can only happen when using an
>> unfused OTP. In such a case, it's ok to always warn, so you don't need to make
>> this configurible.
> 
> We found such a setting super useful while working with targets where the keys are
> zero'ed for various reasons.
> There are cases where you want to use/test trusted keys even when the master key
> is void. Our detection logic does not only print a warning, it refuses to load
> blobs. So IMHO the config knob makes sense.

Ah, I missed that it refuses to continue in that case.

> 
>>> +
>>> +static unsigned int calc_blob_len(unsigned int payload_len) 
>>> +{
>>> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
>>> +}
>>> +
>>> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
>>
>> I assume in can't be const because the use with sg APIs?
> 
> I'm pretty sure this was the main reason, but I can check again.
> 
>>> +{
>>> +	int res = 0;
>>> +	struct skcipher_request *req = NULL;
>>> +	DECLARE_CRYPTO_WAIT(wait);
>>> +	struct scatterlist src_sg, dst_sg;
>>> +	struct crypto_skcipher *tfm;
>>> +	u8 paes_key[DCP_PAES_KEYSIZE];
>>> +
>>> +	if (!use_otp_key)
>>
>> I'd invert this. Makes code easier to read.
> 
> Ok. :-)
> 
>>> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
>>> +	else
>>> +		paes_key[0] = DCP_PAES_KEY_OTP;
>>> +
>>> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
>>> +				    CRYPTO_ALG_INTERNAL);
>>> +	if (IS_ERR(tfm)) {
>>> +		res = PTR_ERR(tfm);
>>> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
>>
>> Can you define pr_fmt above? There's also %pe now that can directly print out an
>> error pointer.
> 
> pr_fmt is not defined on purpose. include/keys/trusted-type.h defines already one
> and I assumed "trusted_key:" is the desired prefix for all kinds of trusted keys.

Ah, all good then. I didn't define it for CAAM either, but forgot why I didn't
along the way. May've been the same reason.

> [...]
> 
>> - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
>>   size checks. Couldn't you read beyond the buffer this way?
> 
> The key has a minimum size of MIN_KEY_SIZE, but p->blob (being struct trusted_key_payload->blob[MAX_BLOB_SIZE])
> is much larger.
> So the assumption is that a DCP blob will always be smaller than MAX_BLOB_SIZE.
> 
>> - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.
> 
> Oh yes. Makes sense!
> 
> [...]
> 
>>
>> jfyi, in the prelude of my CAAM series, I made this the default
>> when .get_random == NULL.
> 
> Right. :-)
> 
> [...]
> 
>>> +	ret = do_dcp_crypto(buf, buf, true);
>>> +	if (ret)
>>> +		goto out;
>>> +
>>> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
>>> +		pr_err("Device neither in secure nor trusted mode!\n");
>>
>> What's the difference between secure and trusted? Can't this test be skipped
>> if use_otp_key == false?
> 
> DCP has many modes of operation. Secure is one level above trusted.
> For the gory details see "Security Reference Manual for the i.MX 6ULL Applications Processor".
> I'm not sure whether all information my manual describes is publicly available so I
> don't dare to copy&paste from it.
> 
> As David and I understood the logic, both OTP and UNIQUE keys can be zero'ed.
> It is also possible that DCP has no support at all for these keys,
> then you'll also get a zero key. That's why we have this check here.

Thanks for the clarification.

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 |

Re: [PATCH 2/3] KEYS: trusted: Introduce support for NXP DCP-based trusted keys

[Ahmad Fatoum]

Hello Richard,

On 14.07.21 12:44, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
> 
> [...]
> 
> Sure, why not? It shows that you will also in future take care of it.

Good point. I did that for v3.

> 
> [...]
> 
>>> +} __packed;
>>> +
>>> +static bool use_otp_key;
>>> +module_param_named(dcp_use_otp_key, use_otp_key, bool, 0);
>>> +MODULE_PARM_DESC(dcp_use_otp_key, "Use OTP instead of UNIQUE key for sealing");
>>
>> Shouldn't these be documented in admin-guide/kernel-parameters.txt as well?
> 
> Yes. Will do.
> 
>>> +static bool skip_zk_test;
>>> +module_param_named(dcp_skip_zk_test, skip_zk_test, bool, 0);
>>> +MODULE_PARM_DESC(dcp_skip_zk_test, "Don't test whether device keys are
>>> zero'ed");
>>
>> Does this need to be configurible? I'd assume this can only happen when using an
>> unfused OTP. In such a case, it's ok to always warn, so you don't need to make
>> this configurible.
> 
> We found such a setting super useful while working with targets where the keys are
> zero'ed for various reasons.
> There are cases where you want to use/test trusted keys even when the master key
> is void. Our detection logic does not only print a warning, it refuses to load
> blobs. So IMHO the config knob makes sense.

Ah, I missed that it refuses to continue in that case.

> 
>>> +
>>> +static unsigned int calc_blob_len(unsigned int payload_len) 
>>> +{
>>> +	return sizeof(struct dcp_blob_fmt) + payload_len + DCP_BLOB_AUTHLEN;
>>> +}
>>> +
>>> +static int do_dcp_crypto(u8 *in, u8 *out, bool is_encrypt)
>>
>> I assume in can't be const because the use with sg APIs?
> 
> I'm pretty sure this was the main reason, but I can check again.
> 
>>> +{
>>> +	int res = 0;
>>> +	struct skcipher_request *req = NULL;
>>> +	DECLARE_CRYPTO_WAIT(wait);
>>> +	struct scatterlist src_sg, dst_sg;
>>> +	struct crypto_skcipher *tfm;
>>> +	u8 paes_key[DCP_PAES_KEYSIZE];
>>> +
>>> +	if (!use_otp_key)
>>
>> I'd invert this. Makes code easier to read.
> 
> Ok. :-)
> 
>>> +		paes_key[0] = DCP_PAES_KEY_UNIQUE;
>>> +	else
>>> +		paes_key[0] = DCP_PAES_KEY_OTP;
>>> +
>>> +	tfm = crypto_alloc_skcipher("ecb-paes-dcp", CRYPTO_ALG_INTERNAL,
>>> +				    CRYPTO_ALG_INTERNAL);
>>> +	if (IS_ERR(tfm)) {
>>> +		res = PTR_ERR(tfm);
>>> +		pr_err("Unable to request DCP pAES-ECB cipher: %i\n", res);
>>
>> Can you define pr_fmt above? There's also %pe now that can directly print out an
>> error pointer.
> 
> pr_fmt is not defined on purpose. include/keys/trusted-type.h defines already one
> and I assumed "trusted_key:" is the desired prefix for all kinds of trusted keys.

Ah, all good then. I didn't define it for CAAM either, but forgot why I didn't
along the way. May've been the same reason.

> [...]
> 
>> - payload_len is at offset 33, but MIN_KEY_SIZE == 32 and there are no minimum
>>   size checks. Couldn't you read beyond the buffer this way?
> 
> The key has a minimum size of MIN_KEY_SIZE, but p->blob (being struct trusted_key_payload->blob[MAX_BLOB_SIZE])
> is much larger.
> So the assumption is that a DCP blob will always be smaller than MAX_BLOB_SIZE.
> 
>> - offset 33 is unaligned for payload_len. Please use get_unaligned_le32 here.
> 
> Oh yes. Makes sense!
> 
> [...]
> 
>>
>> jfyi, in the prelude of my CAAM series, I made this the default
>> when .get_random == NULL.
> 
> Right. :-)
> 
> [...]
> 
>>> +	ret = do_dcp_crypto(buf, buf, true);
>>> +	if (ret)
>>> +		goto out;
>>> +
>>> +	if (memcmp(buf, bad, AES_BLOCK_SIZE) == 0) {
>>> +		pr_err("Device neither in secure nor trusted mode!\n");
>>
>> What's the difference between secure and trusted? Can't this test be skipped
>> if use_otp_key == false?
> 
> DCP has many modes of operation. Secure is one level above trusted.
> For the gory details see "Security Reference Manual for the i.MX 6ULL Applications Processor".
> I'm not sure whether all information my manual describes is publicly available so I
> don't dare to copy&paste from it.
> 
> As David and I understood the logic, both OTP and UNIQUE keys can be zero'ed.
> It is also possible that DCP has no support at all for these keys,
> then you'll also get a zero key. That's why we have this check here.

Thanks for the clarification.

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 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] doc: trusted-encrypted: add DCP as new trust source

[Richard Weinberger]

From: David Gstir <david@sigma-star.at>

Update the documentation for trusted and encrypted KEYS with DCP as new
trust source:

- Describe security properties of DCP trust source
- Describe key usage
- Document blob format

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: Richard Weinberger <richard@nod.at>
Signed-off-by: David Gstir <david@sigma-star.at>
---
 .../security/keys/trusted-encrypted.rst       | 84 ++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..e8413122e4bc 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -35,6 +35,11 @@ safe.
          Rooted to Hardware Unique Key (HUK) which is generally burnt in on-chip
          fuses and is accessible to TEE only.
 
+     (3) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
+
+         Rooted to a one-time programmable key (OTP) that is generally burnt in
+         the on-chip fuses and is accessbile to the DCP encryption engine only.
+
   *  Execution isolation
 
      (1) TPM
@@ -46,6 +51,12 @@ safe.
          Customizable set of operations running in isolated execution
          environment verified via Secure/Trusted boot process.
 
+     (3) DCP
+
+         Fixed set of cryptographic operations running in isolated execution
+         environment. Only basic blob key encryption is executed there.
+         The actual key sealing/unsealing is done on main processor/kernel space.
+
   * Optional binding to platform integrity state
 
      (1) TPM
@@ -63,6 +74,11 @@ safe.
          Relies on Secure/Trusted boot process for platform integrity. It can
          be extended with TEE based measured boot process.
 
+     (3) DCP
+
+         Relies on Secure/Trusted boot process (called HAB by vendor) for
+         platform integrity.
+
   *  Interfaces and APIs
 
      (1) TPM
@@ -74,10 +90,14 @@ safe.
          TEEs have well-documented, standardized client interface and APIs. For
          more details refer to ``Documentation/staging/tee.rst``.
 
+     (3) DCP
+
+         Vendor-specific API that is implemented as part of the DCP crypto driver in
+         ``drivers/crypto/mxs-dcp.c``.
 
   *  Threat model
 
-     The strength and appropriateness of a particular TPM or TEE for a given
+     The strength and appropriateness of a particular TPM, TEE or DCP for a given
      purpose must be assessed when using them to protect security-relevant data.
 
 
@@ -103,6 +123,14 @@ access control policy within the trust source.
      from platform specific hardware RNG or a software based Fortuna CSPRNG
      which can be seeded via multiple entropy sources.
 
+  * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
+
+     The DCP hardware device itself does not provide a dedicated RNG interface,
+     so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do have
+     a dedicated hardware RNG that is independent from DCP which can be enabled
+     to back the kernel RNG.
+
+
 Encrypted Keys
 --------------
 
@@ -188,6 +216,19 @@ Usage::
 specific to TEE device implementation.  The key length for new keys is always
 in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
 
+Trusted Keys usage: DCP
+-----------------------
+
+Usage::
+
+    keyctl add trusted name "new keylen" ring
+    keyctl add trusted name "load hex_blob" ring
+    keyctl print keyid
+
+"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
+specific to this DCP key-blob implementation.  The key length for new keys is
+always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+
 Encrypted Keys usage
 --------------------
 
@@ -370,3 +411,44 @@ string length.
 privkey is the binary representation of TPM2B_PUBLIC excluding the
 initial TPM2B header which can be reconstructed from the ASN.1 octed
 string length.
+
+DCP Blob Format
+---------------
+
+The Data Co-Processor (DCP) provides hardware-bound AES keys using its
+AES encryption engine only. It does not provide direct key sealing/unsealing.
+To make DCP hardware encryption keys usable as trust source, we define
+our own custom format that uses a hardware-bound key to secure the sealing
+key stored in the key blob.
+
+Whenever a new tusted key using DCP is generated, we generate a random 128-bit
+blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to
+encrypt the trusted key payload using AES-128-GCM.
+
+The BEK itself is encrypted using the hardware-bound key using the DCP's AES
+encryption engine with AES-128-ECB. The encrypted BEK, generated nonce,
+BEK-encrypted payload and authentication tag make up the blob format together
+with a version number, payload length and authentication tag::
+
+    /*
+     * struct dcp_blob_fmt - DCP BLOB format.
+     *
+     * @fmt_version: Format version, currently being %1
+     * @blob_key: Random AES 128 key which is used to encrypt @payload,
+     *            @blob_key itself is encrypted with OTP or UNIQUE device key in
+     *            AES-128-ECB mode by DCP.
+     * @nonce: Random nonce used for @payload encryption.
+     * @payload_len: Length of the plain text @payload.
+     * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
+     *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
+     *
+     * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
+     * AES_BLOCK_SIZE.
+     */
+    struct dcp_blob_fmt {
+            __u8 fmt_version;
+            __u8 blob_key[AES_KEYSIZE_128];
+            __u8 nonce[AES_KEYSIZE_128];
+            __le32 payload_len;
+            __u8 payload[0];
+    } __packed;
-- 
2.26.2

[PATCH 3/3] doc: trusted-encrypted: add DCP as new trust source

[Richard Weinberger]

From: David Gstir <david@sigma-star.at>

Update the documentation for trusted and encrypted KEYS with DCP as new
trust source:

- Describe security properties of DCP trust source
- Describe key usage
- Document blob format

Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: David Howells <dhowells@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: keyrings@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.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
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Weinberger <richard@nod.at>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Co-developed-by: Richard Weinberger <richard@nod.at>
Signed-off-by: David Gstir <david@sigma-star.at>
---
 .../security/keys/trusted-encrypted.rst       | 84 ++++++++++++++++++-
 1 file changed, 83 insertions(+), 1 deletion(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 80d5a5af62a1..e8413122e4bc 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -35,6 +35,11 @@ safe.
          Rooted to Hardware Unique Key (HUK) which is generally burnt in on-chip
          fuses and is accessible to TEE only.
 
+     (3) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
+
+         Rooted to a one-time programmable key (OTP) that is generally burnt in
+         the on-chip fuses and is accessbile to the DCP encryption engine only.
+
   *  Execution isolation
 
      (1) TPM
@@ -46,6 +51,12 @@ safe.
          Customizable set of operations running in isolated execution
          environment verified via Secure/Trusted boot process.
 
+     (3) DCP
+
+         Fixed set of cryptographic operations running in isolated execution
+         environment. Only basic blob key encryption is executed there.
+         The actual key sealing/unsealing is done on main processor/kernel space.
+
   * Optional binding to platform integrity state
 
      (1) TPM
@@ -63,6 +74,11 @@ safe.
          Relies on Secure/Trusted boot process for platform integrity. It can
          be extended with TEE based measured boot process.
 
+     (3) DCP
+
+         Relies on Secure/Trusted boot process (called HAB by vendor) for
+         platform integrity.
+
   *  Interfaces and APIs
 
      (1) TPM
@@ -74,10 +90,14 @@ safe.
          TEEs have well-documented, standardized client interface and APIs. For
          more details refer to ``Documentation/staging/tee.rst``.
 
+     (3) DCP
+
+         Vendor-specific API that is implemented as part of the DCP crypto driver in
+         ``drivers/crypto/mxs-dcp.c``.
 
   *  Threat model
 
-     The strength and appropriateness of a particular TPM or TEE for a given
+     The strength and appropriateness of a particular TPM, TEE or DCP for a given
      purpose must be assessed when using them to protect security-relevant data.
 
 
@@ -103,6 +123,14 @@ access control policy within the trust source.
      from platform specific hardware RNG or a software based Fortuna CSPRNG
      which can be seeded via multiple entropy sources.
 
+  * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
+
+     The DCP hardware device itself does not provide a dedicated RNG interface,
+     so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do have
+     a dedicated hardware RNG that is independent from DCP which can be enabled
+     to back the kernel RNG.
+
+
 Encrypted Keys
 --------------
 
@@ -188,6 +216,19 @@ Usage::
 specific to TEE device implementation.  The key length for new keys is always
 in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
 
+Trusted Keys usage: DCP
+-----------------------
+
+Usage::
+
+    keyctl add trusted name "new keylen" ring
+    keyctl add trusted name "load hex_blob" ring
+    keyctl print keyid
+
+"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
+specific to this DCP key-blob implementation.  The key length for new keys is
+always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+
 Encrypted Keys usage
 --------------------
 
@@ -370,3 +411,44 @@ string length.
 privkey is the binary representation of TPM2B_PUBLIC excluding the
 initial TPM2B header which can be reconstructed from the ASN.1 octed
 string length.
+
+DCP Blob Format
+---------------
+
+The Data Co-Processor (DCP) provides hardware-bound AES keys using its
+AES encryption engine only. It does not provide direct key sealing/unsealing.
+To make DCP hardware encryption keys usable as trust source, we define
+our own custom format that uses a hardware-bound key to secure the sealing
+key stored in the key blob.
+
+Whenever a new tusted key using DCP is generated, we generate a random 128-bit
+blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to
+encrypt the trusted key payload using AES-128-GCM.
+
+The BEK itself is encrypted using the hardware-bound key using the DCP's AES
+encryption engine with AES-128-ECB. The encrypted BEK, generated nonce,
+BEK-encrypted payload and authentication tag make up the blob format together
+with a version number, payload length and authentication tag::
+
+    /*
+     * struct dcp_blob_fmt - DCP BLOB format.
+     *
+     * @fmt_version: Format version, currently being %1
+     * @blob_key: Random AES 128 key which is used to encrypt @payload,
+     *            @blob_key itself is encrypted with OTP or UNIQUE device key in
+     *            AES-128-ECB mode by DCP.
+     * @nonce: Random nonce used for @payload encryption.
+     * @payload_len: Length of the plain text @payload.
+     * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
+     *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
+     *
+     * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
+     * AES_BLOCK_SIZE.
+     */
+    struct dcp_blob_fmt {
+            __u8 fmt_version;
+            __u8 blob_key[AES_KEYSIZE_128];
+            __u8 nonce[AES_KEYSIZE_128];
+            __le32 payload_len;
+            __u8 payload[0];
+    } __packed;
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] doc: trusted-encrypted: add DCP as new trust source

[Ahmad Fatoum]

Hello Richard,
Hello David,

On 14.06.21 22:16, Richard Weinberger wrote:
> From: David Gstir <david@sigma-star.at>
> 
> Update the documentation for trusted and encrypted KEYS with DCP as new
> trust source:
> 
> - Describe security properties of DCP trust source
> - Describe key usage
> - Document blob format
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: Richard Weinberger <richard@nod.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> ---
>  .../security/keys/trusted-encrypted.rst       | 84 ++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index 80d5a5af62a1..e8413122e4bc 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -35,6 +35,11 @@ safe.
>           Rooted to Hardware Unique Key (HUK) which is generally burnt in on-chip
>           fuses and is accessible to TEE only.
>  
> +     (3) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> +
> +         Rooted to a one-time programmable key (OTP) that is generally burnt in
> +         the on-chip fuses and is accessbile to the DCP encryption engine only.

s/accessbile/accessible/ . In the code you differentiate between UNIQUE and OTP.
Here you use OTP to mean both. Perhaps explicitly mention this?

> +
>    *  Execution isolation
>  
>       (1) TPM
> @@ -46,6 +51,12 @@ safe.
>           Customizable set of operations running in isolated execution
>           environment verified via Secure/Trusted boot process.
>  
> +     (3) DCP
> +
> +         Fixed set of cryptographic operations running in isolated execution
> +         environment. Only basic blob key encryption is executed there.
> +         The actual key sealing/unsealing is done on main processor/kernel space.
> +
>    * Optional binding to platform integrity state
>  
>       (1) TPM
> @@ -63,6 +74,11 @@ safe.
>           Relies on Secure/Trusted boot process for platform integrity. It can
>           be extended with TEE based measured boot process.
>  
> +     (3) DCP
> +
> +         Relies on Secure/Trusted boot process (called HAB by vendor) for
> +         platform integrity.
> +
>    *  Interfaces and APIs
>  
>       (1) TPM
> @@ -74,10 +90,14 @@ safe.
>           TEEs have well-documented, standardized client interface and APIs. For
>           more details refer to ``Documentation/staging/tee.rst``.
>  
> +     (3) DCP
> +
> +         Vendor-specific API that is implemented as part of the DCP crypto driver in
> +         ``drivers/crypto/mxs-dcp.c``.
>  
>    *  Threat model
>  
> -     The strength and appropriateness of a particular TPM or TEE for a given
> +     The strength and appropriateness of a particular TPM, TEE or DCP for a given
>       purpose must be assessed when using them to protect security-relevant data.
>  
>  
> @@ -103,6 +123,14 @@ access control policy within the trust source.
>       from platform specific hardware RNG or a software based Fortuna CSPRNG
>       which can be seeded via multiple entropy sources.
>  
> +  * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> +
> +     The DCP hardware device itself does not provide a dedicated RNG interface,
> +     so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do have
> +     a dedicated hardware RNG that is independent from DCP which can be enabled
> +     to back the kernel RNG.
> +
> +
>  Encrypted Keys
>  --------------
>  
> @@ -188,6 +216,19 @@ Usage::
>  specific to TEE device implementation.  The key length for new keys is always
>  in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
>  
> +Trusted Keys usage: DCP
> +-----------------------
> +
> +Usage::
> +
> +    keyctl add trusted name "new keylen" ring
> +    keyctl add trusted name "load hex_blob" ring
> +    keyctl print keyid
> +
> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
> +specific to this DCP key-blob implementation.  The key length for new keys is
> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> +
>  Encrypted Keys usage
>  --------------------
>  
> @@ -370,3 +411,44 @@ string length.
>  privkey is the binary representation of TPM2B_PUBLIC excluding the
>  initial TPM2B header which can be reconstructed from the ASN.1 octed
>  string length.
> +
> +DCP Blob Format
> +---------------
> +
> +The Data Co-Processor (DCP) provides hardware-bound AES keys using its
> +AES encryption engine only. It does not provide direct key sealing/unsealing.
> +To make DCP hardware encryption keys usable as trust source, we define
> +our own custom format that uses a hardware-bound key to secure the sealing
> +key stored in the key blob.
> +
> +Whenever a new tusted key using DCP is generated, we generate a random 128-bit

s/tusted/trusted/

> +blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to
> +encrypt the trusted key payload using AES-128-GCM.
> +
> +The BEK itself is encrypted using the hardware-bound key using the DCP's AES
> +encryption engine with AES-128-ECB. The encrypted BEK, generated nonce,
> +BEK-encrypted payload and authentication tag make up the blob format together
> +with a version number, payload length and authentication tag::
> +
> +    /*
> +     * struct dcp_blob_fmt - DCP BLOB format.
> +     *
> +     * @fmt_version: Format version, currently being %1
> +     * @blob_key: Random AES 128 key which is used to encrypt @payload,
> +     *            @blob_key itself is encrypted with OTP or UNIQUE device key in
> +     *            AES-128-ECB mode by DCP.
> +     * @nonce: Random nonce used for @payload encryption.
> +     * @payload_len: Length of the plain text @payload.
> +     * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
> +     *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
> +     *
> +     * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
> +     * AES_BLOCK_SIZE.
> +     */
> +    struct dcp_blob_fmt {
> +            __u8 fmt_version;
> +            __u8 blob_key[AES_KEYSIZE_128];
> +            __u8 nonce[AES_KEYSIZE_128];
> +            __le32 payload_len;
> +            __u8 payload[0];

[] ?

> +    } __packed;
> 

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 |

Re: [PATCH 3/3] doc: trusted-encrypted: add DCP as new trust source

[Ahmad Fatoum]

Hello Richard,
Hello David,

On 14.06.21 22:16, Richard Weinberger wrote:
> From: David Gstir <david@sigma-star.at>
> 
> Update the documentation for trusted and encrypted KEYS with DCP as new
> trust source:
> 
> - Describe security properties of DCP trust source
> - Describe key usage
> - Document blob format
> 
> Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: David Howells <dhowells@redhat.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: keyrings@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.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
> Cc: Mimi Zohar <zohar@linux.ibm.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Co-developed-by: Richard Weinberger <richard@nod.at>
> Signed-off-by: David Gstir <david@sigma-star.at>
> ---
>  .../security/keys/trusted-encrypted.rst       | 84 ++++++++++++++++++-
>  1 file changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index 80d5a5af62a1..e8413122e4bc 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -35,6 +35,11 @@ safe.
>           Rooted to Hardware Unique Key (HUK) which is generally burnt in on-chip
>           fuses and is accessible to TEE only.
>  
> +     (3) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> +
> +         Rooted to a one-time programmable key (OTP) that is generally burnt in
> +         the on-chip fuses and is accessbile to the DCP encryption engine only.

s/accessbile/accessible/ . In the code you differentiate between UNIQUE and OTP.
Here you use OTP to mean both. Perhaps explicitly mention this?

> +
>    *  Execution isolation
>  
>       (1) TPM
> @@ -46,6 +51,12 @@ safe.
>           Customizable set of operations running in isolated execution
>           environment verified via Secure/Trusted boot process.
>  
> +     (3) DCP
> +
> +         Fixed set of cryptographic operations running in isolated execution
> +         environment. Only basic blob key encryption is executed there.
> +         The actual key sealing/unsealing is done on main processor/kernel space.
> +
>    * Optional binding to platform integrity state
>  
>       (1) TPM
> @@ -63,6 +74,11 @@ safe.
>           Relies on Secure/Trusted boot process for platform integrity. It can
>           be extended with TEE based measured boot process.
>  
> +     (3) DCP
> +
> +         Relies on Secure/Trusted boot process (called HAB by vendor) for
> +         platform integrity.
> +
>    *  Interfaces and APIs
>  
>       (1) TPM
> @@ -74,10 +90,14 @@ safe.
>           TEEs have well-documented, standardized client interface and APIs. For
>           more details refer to ``Documentation/staging/tee.rst``.
>  
> +     (3) DCP
> +
> +         Vendor-specific API that is implemented as part of the DCP crypto driver in
> +         ``drivers/crypto/mxs-dcp.c``.
>  
>    *  Threat model
>  
> -     The strength and appropriateness of a particular TPM or TEE for a given
> +     The strength and appropriateness of a particular TPM, TEE or DCP for a given
>       purpose must be assessed when using them to protect security-relevant data.
>  
>  
> @@ -103,6 +123,14 @@ access control policy within the trust source.
>       from platform specific hardware RNG or a software based Fortuna CSPRNG
>       which can be seeded via multiple entropy sources.
>  
> +  * DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> +
> +     The DCP hardware device itself does not provide a dedicated RNG interface,
> +     so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do have
> +     a dedicated hardware RNG that is independent from DCP which can be enabled
> +     to back the kernel RNG.
> +
> +
>  Encrypted Keys
>  --------------
>  
> @@ -188,6 +216,19 @@ Usage::
>  specific to TEE device implementation.  The key length for new keys is always
>  in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
>  
> +Trusted Keys usage: DCP
> +-----------------------
> +
> +Usage::
> +
> +    keyctl add trusted name "new keylen" ring
> +    keyctl add trusted name "load hex_blob" ring
> +    keyctl print keyid
> +
> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
> +specific to this DCP key-blob implementation.  The key length for new keys is
> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> +
>  Encrypted Keys usage
>  --------------------
>  
> @@ -370,3 +411,44 @@ string length.
>  privkey is the binary representation of TPM2B_PUBLIC excluding the
>  initial TPM2B header which can be reconstructed from the ASN.1 octed
>  string length.
> +
> +DCP Blob Format
> +---------------
> +
> +The Data Co-Processor (DCP) provides hardware-bound AES keys using its
> +AES encryption engine only. It does not provide direct key sealing/unsealing.
> +To make DCP hardware encryption keys usable as trust source, we define
> +our own custom format that uses a hardware-bound key to secure the sealing
> +key stored in the key blob.
> +
> +Whenever a new tusted key using DCP is generated, we generate a random 128-bit

s/tusted/trusted/

> +blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to
> +encrypt the trusted key payload using AES-128-GCM.
> +
> +The BEK itself is encrypted using the hardware-bound key using the DCP's AES
> +encryption engine with AES-128-ECB. The encrypted BEK, generated nonce,
> +BEK-encrypted payload and authentication tag make up the blob format together
> +with a version number, payload length and authentication tag::
> +
> +    /*
> +     * struct dcp_blob_fmt - DCP BLOB format.
> +     *
> +     * @fmt_version: Format version, currently being %1
> +     * @blob_key: Random AES 128 key which is used to encrypt @payload,
> +     *            @blob_key itself is encrypted with OTP or UNIQUE device key in
> +     *            AES-128-ECB mode by DCP.
> +     * @nonce: Random nonce used for @payload encryption.
> +     * @payload_len: Length of the plain text @payload.
> +     * @payload: The payload itself, encrypted using AES-128-GCM and @blob_key,
> +     *           GCM auth tag of size AES_BLOCK_SIZE is attached at the end of it.
> +     *
> +     * The total size of a DCP BLOB is sizeof(struct dcp_blob_fmt) + @payload_len +
> +     * AES_BLOCK_SIZE.
> +     */
> +    struct dcp_blob_fmt {
> +            __u8 fmt_version;
> +            __u8 blob_key[AES_KEYSIZE_128];
> +            __u8 nonce[AES_KEYSIZE_128];
> +            __le32 payload_len;
> +            __u8 payload[0];

[] ?

> +    } __packed;
> 

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 |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel