[PATCH 0/4] crypto/fsl: add RNG support

[PATCH 0/4] crypto/fsl: add RNG support

[Michael Walle]

First, improve the compatibility on newer Era CAAMs. These introduced new
version registers. Secondly, add RNG support for the CAAM. This way we get
random number generator support for EFI for free and KASLR will work with
ARM64 kernels booted with bootefi.

Michael Walle (4):
  crypto/fsl: make SEC%u status line consistent
  crypto/fsl: export caam_get_era()
  crypto/fsl: support newer SEC modules
  crypto/fsl: add RNG support

 drivers/crypto/fsl/Kconfig   | 11 +++++
 drivers/crypto/fsl/Makefile  |  1 +
 drivers/crypto/fsl/jobdesc.c |  9 ++++
 drivers/crypto/fsl/jobdesc.h |  3 ++
 drivers/crypto/fsl/jr.c      | 23 ++++++++--
 drivers/crypto/fsl/rng.c     | 87 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/fsl/sec.c     |  2 +-
 include/fsl_sec.h            | 57 +++++++++++++++++++----
 8 files changed, 180 insertions(+), 13 deletions(-)
 create mode 100644 drivers/crypto/fsl/rng.c

-- 
2.20.1

[PATCH 1/4] crypto/fsl: make SEC%u status line consistent

[Michael Walle]

Align the status line with all the other output in U-Boot.

Before the change:
DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
SEC0: RNG instantiated
WDT:   Started with servicing (60s timeout)

After the change:
DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
SEC0:  RNG instantiated
WDT:   Started with servicing (60s timeout)

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/jr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index e2d9216cfc..612e86818b 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -657,7 +657,7 @@ int sec_init_idx(uint8_t sec_idx)
 			printf("SEC%u: RNG instantiation failed\n", sec_idx);
 			return -1;
 		}
-		printf("SEC%u: RNG instantiated\n", sec_idx);
+		printf("SEC%u:  RNG instantiated\n", sec_idx);
 	}
 #endif
 	return ret;
-- 
2.20.1

[PATCH 2/4] crypto/fsl: export caam_get_era()

[Michael Walle]

We need the era in other modules, too. For example, to get the RNG
version.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/sec.c | 2 +-
 include/fsl_sec.h        | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/fsl/sec.c b/drivers/crypto/fsl/sec.c
index a2c0bfaf44..a2fe5b1cc9 100644
--- a/drivers/crypto/fsl/sec.c
+++ b/drivers/crypto/fsl/sec.c
@@ -98,7 +98,7 @@ void fdt_fixup_crypto_node(void *blob, int sec_rev)
 		       fdt_strerror(err));
 }
 #elif CONFIG_SYS_FSL_SEC_COMPAT >= 4  /* SEC4 */
-static u8 caam_get_era(void)
+u8 caam_get_era(void)
 {
 	static const struct {
 		u16 ip_id;
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index c0d2c7e866..2ebb75c9b2 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -316,6 +316,8 @@ int blob_dek(const u8 *src, u8 *dst, u8 len);
 int sec_init_idx(uint8_t);
 #endif
 int sec_init(void);
+
+u8 caam_get_era(void);
 #endif
 
 #endif /* __FSL_SEC_H */
-- 
2.20.1

[PATCH 3/4] crypto/fsl: support newer SEC modules

[Michael Walle]

Since Era 10, the version registers changed. Add the version registers
and use them on newer modules.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/jr.c | 12 ++++++++--
 include/fsl_sec.h       | 51 +++++++++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 612e86818b..9f3da9c474 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -498,9 +498,17 @@ static int instantiate_rng(uint8_t sec_idx)
 static u8 get_rng_vid(uint8_t sec_idx)
 {
 	ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
-	u32 cha_vid = sec_in32(&sec->chavid_ls);
+	u8 vid;
 
-	return (cha_vid & SEC_CHAVID_RNG_LS_MASK) >> SEC_CHAVID_LS_RNG_SHIFT;
+	if (caam_get_era() < 10) {
+		vid = (sec_in32(&sec->chavid_ls) & SEC_CHAVID_RNG_LS_MASK)
+		       >> SEC_CHAVID_LS_RNG_SHIFT;
+	} else {
+		vid = (sec_in32(&sec->vreg.rng) & CHA_VER_VID_MASK)
+		       >> CHA_VER_VID_SHIFT;
+	}
+
+	return vid;
 }
 
 /*
diff --git a/include/fsl_sec.h b/include/fsl_sec.h
index 2ebb75c9b2..8dce0bbb1b 100644
--- a/include/fsl_sec.h
+++ b/include/fsl_sec.h
@@ -73,6 +73,41 @@ struct rng4tst {
 	u32 rsvd2[15];
 };
 
+/* Version registers (Era 10+) */
+struct version_regs {
+	u32 crca;	/* CRCA_VERSION */
+	u32 afha;	/* AFHA_VERSION */
+	u32 kfha;	/* KFHA_VERSION */
+	u32 pkha;	/* PKHA_VERSION */
+	u32 aesa;	/* AESA_VERSION */
+	u32 mdha;	/* MDHA_VERSION */
+	u32 desa;	/* DESA_VERSION */
+	u32 snw8a;	/* SNW8A_VERSION */
+	u32 snw9a;	/* SNW9A_VERSION */
+	u32 zuce;	/* ZUCE_VERSION */
+	u32 zuca;	/* ZUCA_VERSION */
+	u32 ccha;	/* CCHA_VERSION */
+	u32 ptha;	/* PTHA_VERSION */
+	u32 rng;	/* RNG_VERSION */
+	u32 trng;	/* TRNG_VERSION */
+	u32 aaha;	/* AAHA_VERSION */
+	u32 rsvd[10];
+	u32 sr;		/* SR_VERSION */
+	u32 dma;	/* DMA_VERSION */
+	u32 ai;		/* AI_VERSION */
+	u32 qi;		/* QI_VERSION */
+	u32 jr;		/* JR_VERSION */
+	u32 deco;	/* DECO_VERSION */
+};
+
+#define CHA_VER_NUM_MASK	0x000000ff
+#define CHA_VER_MISC_SHIFT	8
+#define CHA_VER_MISC_MASK	0x0000ff00
+#define CHA_VER_REV_SHIFT	16
+#define CHA_VER_REV_MASK	0x00ff0000
+#define CHA_VER_VID_SHIFT	24
+#define CHA_VER_VID_MASK	0xff000000
+
 typedef struct ccsr_sec {
 	u32	res0;
 	u32	mcfgr;		/* Master CFG Register */
@@ -98,17 +133,19 @@ typedef struct ccsr_sec {
 	u32	drr;		/* DECO Reset Register */
 	u8	res5[0x4d8];
 	struct rng4tst rng;	/* RNG Registers */
-	u8	res6[0x8a0];
+	u8	res6[0x780];
+	struct version_regs vreg; /* version registers since era 10 */
+	u8	res7[0xa0];
 	u32	crnr_ms;	/* CHA Revision Number Register, MS */
 	u32	crnr_ls;	/* CHA Revision Number Register, LS */
 	u32	ctpr_ms;	/* Compile Time Parameters Register, MS */
 	u32	ctpr_ls;	/* Compile Time Parameters Register, LS */
-	u8	res7[0x10];
+	u8	res8[0x10];
 	u32	far_ms;		/* Fault Address Register, MS */
 	u32	far_ls;		/* Fault Address Register, LS */
 	u32	falr;		/* Fault Address LIODN Register */
 	u32	fadr;		/* Fault Address Detail Register */
-	u8	res8[0x4];
+	u8	res9[0x4];
 	u32	csta;		/* CAAM Status Register */
 	u32	smpart;		/* Secure Memory Partition Parameters */
 	u32	smvid;		/* Secure Memory Version ID */
@@ -121,16 +158,16 @@ typedef struct ccsr_sec {
 	u32	secvid_ms;	/* SEC Version ID Register, MS */
 	u32	secvid_ls;	/* SEC Version ID Register, LS */
 #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
-	u8	res9[0x6f020];
+	u8	res10[0x6f020];
 #else
-	u8	res9[0x6020];
+	u8	res10[0x6020];
 #endif
 	u32	qilcr_ms;	/* Queue Interface LIODN CFG Register, MS */
 	u32	qilcr_ls;	/* Queue Interface LIODN CFG Register, LS */
 #if defined(CONFIG_FSL_LSCH2) || defined(CONFIG_FSL_LSCH3)
-	u8	res10[0x8ffd8];
+	u8	res11[0x8ffd8];
 #else
-	u8	res10[0x8fd8];
+	u8	res11[0x8fd8];
 #endif
 } ccsr_sec_t;
 
-- 
2.20.1

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Register the random number generator with the rng subsystem in u-boot.
This way it can be used by EFI as well as for the 'rng' command.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/fsl/Kconfig   | 11 +++++
 drivers/crypto/fsl/Makefile  |  1 +
 drivers/crypto/fsl/jobdesc.c |  9 ++++
 drivers/crypto/fsl/jobdesc.h |  3 ++
 drivers/crypto/fsl/jr.c      |  9 ++++
 drivers/crypto/fsl/rng.c     | 84 ++++++++++++++++++++++++++++++++++++
 6 files changed, 117 insertions(+)
 create mode 100644 drivers/crypto/fsl/rng.c

diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
index 181a1e5e99..5936b77494 100644
--- a/drivers/crypto/fsl/Kconfig
+++ b/drivers/crypto/fsl/Kconfig
@@ -45,3 +45,14 @@ config SYS_FSL_SEC_COMPAT
 
 config SYS_FSL_SEC_LE
 	bool "Little-endian access to Freescale Secure Boot"
+
+if FSL_CAAM
+
+config FSL_CAAM_RNG
+	bool "Enable Random Number Generator support"
+	depends on DM_RNG
+	default y
+	help
+	  Enable support for the random number generator module of the CAAM.
+
+endif
diff --git a/drivers/crypto/fsl/Makefile b/drivers/crypto/fsl/Makefile
index cfb36f3bb9..a5e8d38e38 100644
--- a/drivers/crypto/fsl/Makefile
+++ b/drivers/crypto/fsl/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_FSL_CAAM) += jr.o fsl_hash.o jobdesc.o error.o
 obj-$(CONFIG_CMD_BLOB) += fsl_blob.o
 obj-$(CONFIG_CMD_DEKBLOB) += fsl_blob.o
 obj-$(CONFIG_RSA_FREESCALE_EXP) += fsl_rsa.o
+obj-$(CONFIG_FSL_CAAM_RNG) += rng.o
diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
index 2f35e0c90b..5602a3d93c 100644
--- a/drivers/crypto/fsl/jobdesc.c
+++ b/drivers/crypto/fsl/jobdesc.c
@@ -286,6 +286,15 @@ void inline_cnstr_jobdesc_rng_instantiation(uint32_t *desc, int handle)
 	}
 }
 
+void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size)
+{
+	dma_addr_t dma_data_out = virt_to_phys(data_out);
+
+	init_job_desc(desc, 0);
+	append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG);
+	append_fifo_store(desc, dma_data_out, size, FIFOST_TYPE_RNGSTORE);
+}
+
 /* Change key size to bytes form bits in calling function*/
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
diff --git a/drivers/crypto/fsl/jobdesc.h b/drivers/crypto/fsl/jobdesc.h
index d782c46b9d..35075ff434 100644
--- a/drivers/crypto/fsl/jobdesc.h
+++ b/drivers/crypto/fsl/jobdesc.h
@@ -41,7 +41,10 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
 
 void inline_cnstr_jobdesc_rng_instantiation(uint32_t *desc, int handle);
 
+void inline_cnstr_jobdesc_rng(u32 *desc, void *data_out, u32 size);
+
 void inline_cnstr_jobdesc_pkha_rsaexp(uint32_t *desc,
 				      struct pk_in_params *pkin, uint8_t *out,
 				      uint32_t out_siz);
+
 #endif
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
index 9f3da9c474..8ecc0f05b0 100644
--- a/drivers/crypto/fsl/jr.c
+++ b/drivers/crypto/fsl/jr.c
@@ -19,6 +19,7 @@
 #include <asm/cache.h>
 #include <asm/fsl_pamu.h>
 #endif
+#include <dm/lists.h>
 
 #define CIRC_CNT(head, tail, size)	(((head) - (tail)) & (size - 1))
 #define CIRC_SPACE(head, tail, size)	CIRC_CNT((tail), (head) + 1, (size))
@@ -665,6 +666,14 @@ int sec_init_idx(uint8_t sec_idx)
 			printf("SEC%u: RNG instantiation failed\n", sec_idx);
 			return -1;
 		}
+#ifdef CONFIG_DM_RNG
+		if (IS_ENABLED(CONFIG_DM_RNG)) {
+			ret = device_bind_driver(NULL, "caam-rng", "caam-rng",
+						 NULL);
+			if (ret)
+				printf("Couldn't bind rng driver (%d)\n", ret);
+		}
+#endif
 		printf("SEC%u:  RNG instantiated\n", sec_idx);
 	}
 #endif
diff --git a/drivers/crypto/fsl/rng.c b/drivers/crypto/fsl/rng.c
new file mode 100644
index 0000000000..3da318d767
--- /dev/null
+++ b/drivers/crypto/fsl/rng.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Michael Walle <michael@walle.cc>
+ *
+ * Driver for Freescale Cryptographic Accelerator and Assurance
+ * Module (CAAM) hardware random number generator.
+ */
+
+#include <asm/cache.h>
+#include <common.h>
+#include <cpu_func.h>
+#include <dm.h>
+#include <rng.h>
+#include "desc_constr.h"
+#include "jobdesc.h"
+#include "jr.h"
+
+#define CAAM_RNG_MAX_FIFO_STORE_SIZE 16
+#define CAAM_RNG_DESC_LEN (3 * CAAM_CMD_SZ + CAAM_PTR_SZ)
+
+struct caam_rng_platdata {
+	u32 desc[CAAM_RNG_DESC_LEN / 4];
+	u8 data[CAAM_RNG_MAX_FIFO_STORE_SIZE] __aligned(ARCH_DMA_MINALIGN);
+};
+
+static int caam_rng_read_one(struct caam_rng_platdata *pdata)
+{
+	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
+	int ret;
+
+	ret = run_descriptor_jr(pdata->desc);
+	if (ret < 0)
+		return -EIO;
+
+	invalidate_dcache_range((unsigned long)pdata->data,
+				(unsigned long)pdata->data + size);
+
+	return 0;
+}
+
+static int caam_rng_read(struct udevice *dev, void *data, size_t len)
+{
+	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
+	u8 *buffer = data;
+	size_t size;
+	int ret;
+
+	while (len) {
+		ret = caam_rng_read_one(pdata);
+		if (ret)
+			return ret;
+
+		size = min(len, (size_t)CAAM_RNG_MAX_FIFO_STORE_SIZE);
+
+		memcpy(buffer, pdata->data, len);
+		buffer += size;
+		len -= size;
+	}
+
+	return 0;
+}
+
+static int caam_rng_probe(struct udevice *dev)
+{
+	struct caam_rng_platdata *pdata = dev_get_platdata(dev);
+
+	inline_cnstr_jobdesc_rng(pdata->desc, pdata->data,
+				 CAAM_RNG_MAX_FIFO_STORE_SIZE);
+
+	return 0;
+}
+
+static const struct dm_rng_ops caam_rng_ops = {
+	.read = caam_rng_read,
+};
+
+U_BOOT_DRIVER(caam_rng) = {
+	.name = "caam-rng",
+	.id = UCLASS_RNG,
+	.ops = &caam_rng_ops,
+	.probe = caam_rng_probe,
+	.platdata_auto_alloc_size = sizeof(struct caam_rng_platdata),
+	.flags = DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.20.1

[PATCH 4/4] crypto/fsl: add RNG support

[Horia Geantă]

On 6/3/2020 1:06 AM, Michael Walle wrote:
> Register the random number generator with the rng subsystem in u-boot.
> This way it can be used by EFI as well as for the 'rng' command.
> 
I am trying to understand what's expected from UCLASS_RNG...

UEFI spec mentions:
<<The ?raw? algorithm, when supported, is intended to provide entropy
directly from the source, without it going through
some deterministic random bit generator.>>

lib/efi_loader/efi_rng.c uses EFI_RNG_ALGORITHM_RAW and is happy
with whatever UCLASS_RNG implementation it gets.

From above I'd conclude all UCLASS_RNG implementations are expected to
provide "full" entropy directly from a TRNG source, without any DRBG
connected in the middle (b/w TRNG and user).

Is this correct?
If so, note that using CAAM's job ring interface without prediction resistance
to extract randomness does not fit the bill, since TRNG output
is connected to a DRBG (DRBG_Hash, SP800-90A compliant).

For CAAM prediction resistance (PR) details I suggest looking in the kernel:
https://lore.kernel.org/linux-crypto/VI1PR0402MB3485EF10976A4A69F90E5B0F98580 at VI1PR0402MB3485.eurprd04.prod.outlook.com
358ba762d9f1 crypto: caam - enable prediction resistance in HRWNG
ea53756d831a crypto: caam - limit single JD RNG output to maximum of 16 bytes

Steps required to add PR support:
-initialize ("instantiate") the RNG state handles (the DRBGs)
with PR support; if already instantiated (by ROM, OP-TEE etc.),
they must be re-instantiated if PR support was not enabled
-use the "PR" option when extracting randomness from the DRBG;
this forces a re-seed of the DRBG
-limit the data size drawn from the DRBG; this is already done
(see CAAM_RNG_MAX_FIFO_STORE_SIZE)

> @@ -665,6 +666,14 @@ int sec_init_idx(uint8_t sec_idx)
>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);
>  			return -1;
>  		}
> +#ifdef CONFIG_DM_RNG
> +		if (IS_ENABLED(CONFIG_DM_RNG)) {
Shouldn't one or the other (#ifdef / IS_ENABLED) suffice?

> +static int caam_rng_read_one(struct caam_rng_platdata *pdata)
> +{
> +	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
> +	int ret;
> +
> +	ret = run_descriptor_jr(pdata->desc);
> +	if (ret < 0)
> +		return -EIO;
> +
> +	invalidate_dcache_range((unsigned long)pdata->data,
> +				(unsigned long)pdata->data + size);
Side note: this is not required on Layerscape SoCs, since CAAM is HW coherent.
Most surely this needs to be handled in a separate patch set,
throughout drivers/crypto/fsl/*.

Horia

[PATCH 1/4] crypto/fsl: make SEC%u status line consistent

[Horia Geantă]

On 6/3/2020 1:06 AM, Michael Walle wrote:
> @@ -657,7 +657,7 @@ int sec_init_idx(uint8_t sec_idx)
>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);
>  			return -1;
>  		}
> -		printf("SEC%u: RNG instantiated\n", sec_idx);
> +		printf("SEC%u:  RNG instantiated\n", sec_idx);
Shouldn't also the string printed in case of failure be updated?

Horia

[PATCH 2/4] crypto/fsl: export caam_get_era()

[Horia Geantă]

On 6/3/2020 1:06 AM, Michael Walle wrote:
> We need the era in other modules, too. For example, to get the RNG
> version.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>

Thanks,
Horia

[PATCH 1/4] crypto/fsl: make SEC%u status line consistent

[Heinrich Schuchardt]

On 6/3/20 12:05 AM, Michael Walle wrote:
> Align the status line with all the other output in U-Boot.
>
> Before the change:
> DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
> SEC0: RNG instantiated
> WDT:   Started with servicing (60s timeout)
>
> After the change:
> DDR    3.9 GiB (DDR3, 32-bit, CL=11, ECC on)
> SEC0:  RNG instantiated
> WDT:   Started with servicing (60s timeout)
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/jr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index e2d9216cfc..612e86818b 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -657,7 +657,7 @@ int sec_init_idx(uint8_t sec_idx)
>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);

And how about this line?

>  			return -1;
>  		}
> -		printf("SEC%u: RNG instantiated\n", sec_idx);
> +		printf("SEC%u:  RNG instantiated\n", sec_idx);
>  	}
>  #endif
>  	return ret;
>

[PATCH 2/4] crypto/fsl: export caam_get_era()

[Heinrich Schuchardt]

On 6/3/20 12:05 AM, Michael Walle wrote:
> We need the era in other modules, too. For example, to get the RNG
> version.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/sec.c | 2 +-
>  include/fsl_sec.h        | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/fsl/sec.c b/drivers/crypto/fsl/sec.c
> index a2c0bfaf44..a2fe5b1cc9 100644
> --- a/drivers/crypto/fsl/sec.c
> +++ b/drivers/crypto/fsl/sec.c
> @@ -98,7 +98,7 @@ void fdt_fixup_crypto_node(void *blob, int sec_rev)
>  		       fdt_strerror(err));
>  }
>  #elif CONFIG_SYS_FSL_SEC_COMPAT >= 4  /* SEC4 */
> -static u8 caam_get_era(void)
> +u8 caam_get_era(void)
>  {
>  	static const struct {
>  		u16 ip_id;
> diff --git a/include/fsl_sec.h b/include/fsl_sec.h
> index c0d2c7e866..2ebb75c9b2 100644
> --- a/include/fsl_sec.h
> +++ b/include/fsl_sec.h
> @@ -316,6 +316,8 @@ int blob_dek(const u8 *src, u8 *dst, u8 len);
>  int sec_init_idx(uint8_t);
>  #endif
>  int sec_init(void);
> +

Exported functions should be documented. See
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation

> +u8 caam_get_era(void);
>  #endif
>
>  #endif /* __FSL_SEC_H */
>

[PATCH 4/4] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 6/3/20 6:50 PM, Horia Geant? wrote:
> On 6/3/2020 1:06 AM, Michael Walle wrote:
>> Register the random number generator with the rng subsystem in u-boot.
>> This way it can be used by EFI as well as for the 'rng' command.
>>
> I am trying to understand what's expected from UCLASS_RNG...
>
> UEFI spec mentions:
> <<The ?raw? algorithm, when supported, is intended to provide entropy
> directly from the source, without it going through
> some deterministic random bit generator.>>
>
> lib/efi_loader/efi_rng.c uses EFI_RNG_ALGORITHM_RAW and is happy
> with whatever UCLASS_RNG implementation it gets.
>
>>From above I'd conclude all UCLASS_RNG implementations are expected to
> provide "full" entropy directly from a TRNG source, without any DRBG
> connected in the middle (b/w TRNG and user).
>
> Is this correct?
> If so, note that using CAAM's job ring interface without prediction resistance
> to extract randomness does not fit the bill, since TRNG output
> is connected to a DRBG (DRBG_Hash, SP800-90A compliant).

I would assume that all hardware RNGs use some algorithms to even out
the distribution of the random bits coming from noise source. My
understanding of the UEFI spec is that EFI_RNG_ALGORITHM_RAW simply does
not provide any guarantee for the distribution quality while a PRNG
reseeded via a hardware ring provides some guarantees.

Should you be aware that the FSL hardware RNG does not provide a well
distributed entropy it would be preferable to pass the stream through a
PRNG even if provided as EFI_RNG_ALGORITHM_RAW.

Best regards

Heinrich

>
> For CAAM prediction resistance (PR) details I suggest looking in the kernel:
> https://lore.kernel.org/linux-crypto/VI1PR0402MB3485EF10976A4A69F90E5B0F98580 at VI1PR0402MB3485.eurprd04.prod.outlook.com
> 358ba762d9f1 crypto: caam - enable prediction resistance in HRWNG
> ea53756d831a crypto: caam - limit single JD RNG output to maximum of 16 bytes
>
> Steps required to add PR support:
> -initialize ("instantiate") the RNG state handles (the DRBGs)
> with PR support; if already instantiated (by ROM, OP-TEE etc.),
> they must be re-instantiated if PR support was not enabled
> -use the "PR" option when extracting randomness from the DRBG;
> this forces a re-seed of the DRBG
> -limit the data size drawn from the DRBG; this is already done
> (see CAAM_RNG_MAX_FIFO_STORE_SIZE)
>
>> @@ -665,6 +666,14 @@ int sec_init_idx(uint8_t sec_idx)
>>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);
>>  			return -1;
>>  		}
>> +#ifdef CONFIG_DM_RNG
>> +		if (IS_ENABLED(CONFIG_DM_RNG)) {
> Shouldn't one or the other (#ifdef / IS_ENABLED) suffice?
>
>> +static int caam_rng_read_one(struct caam_rng_platdata *pdata)
>> +{
>> +	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
>> +	int ret;
>> +
>> +	ret = run_descriptor_jr(pdata->desc);
>> +	if (ret < 0)
>> +		return -EIO;
>> +
>> +	invalidate_dcache_range((unsigned long)pdata->data,
>> +				(unsigned long)pdata->data + size);
> Side note: this is not required on Layerscape SoCs, since CAAM is HW coherent.
> Most surely this needs to be handled in a separate patch set,
> throughout drivers/crypto/fsl/*.
>
> Horia
>

[PATCH 3/4] crypto/fsl: support newer SEC modules

[Horia Geantă]

On 6/3/2020 1:06 AM, Michael Walle wrote:
> Since Era 10, the version registers changed. Add the version registers
> and use them on newer modules.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Horia Geant? <horia.geanta@nxp.com>

Thanks,
Horia

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Hi Horia, Hi Heinrich,

thanks for reviewing.

Am 2020-06-03 19:35, schrieb Heinrich Schuchardt:
> On 6/3/20 6:50 PM, Horia Geant? wrote:
>> On 6/3/2020 1:06 AM, Michael Walle wrote:
>>> Register the random number generator with the rng subsystem in 
>>> u-boot.
>>> This way it can be used by EFI as well as for the 'rng' command.
>>> 
>> I am trying to understand what's expected from UCLASS_RNG...
>> 
>> UEFI spec mentions:
>> <<The ?raw? algorithm, when supported, is intended to provide entropy
>> directly from the source, without it going through
>> some deterministic random bit generator.>>
>> 
>> lib/efi_loader/efi_rng.c uses EFI_RNG_ALGORITHM_RAW and is happy
>> with whatever UCLASS_RNG implementation it gets.
>> 
>>> From above I'd conclude all UCLASS_RNG implementations are expected 
>>> to
>> provide "full" entropy directly from a TRNG source, without any DRBG
>> connected in the middle (b/w TRNG and user).
>> 
>> Is this correct?
>> If so, note that using CAAM's job ring interface without prediction 
>> resistance
>> to extract randomness does not fit the bill, since TRNG output
>> is connected to a DRBG (DRBG_Hash, SP800-90A compliant).
> 
> I would assume that all hardware RNGs use some algorithms to even out
> the distribution of the random bits coming from noise source. My
> understanding of the UEFI spec is that EFI_RNG_ALGORITHM_RAW simply 
> does
> not provide any guarantee for the distribution quality while a PRNG
> reseeded via a hardware ring provides some guarantees.
> 
> Should you be aware that the FSL hardware RNG does not provide a well
> distributed entropy it would be preferable to pass the stream through a
> PRNG even if provided as EFI_RNG_ALGORITHM_RAW.
>> 
>> For CAAM prediction resistance (PR) details I suggest looking in the 
>> kernel:
>> https://lore.kernel.org/linux-crypto/VI1PR0402MB3485EF10976A4A69F90E5B0F98580 at VI1PR0402MB3485.eurprd04.prod.outlook.com
>> 358ba762d9f1 crypto: caam - enable prediction resistance in HRWNG
>> ea53756d831a crypto: caam - limit single JD RNG output to maximum of 
>> 16 bytes

I noticed that PR bit, but I wanted to keep things simple. Does all SEC 
modules and
versions support this, i.e. the kernel checks versions of some 
management controller
in QorIQ SoCs.

>> Steps required to add PR support:
>> -initialize ("instantiate") the RNG state handles (the DRBGs)
>> with PR support; if already instantiated (by ROM, OP-TEE etc.),
>> they must be re-instantiated if PR support was not enabled
>> -use the "PR" option when extracting randomness from the DRBG;
>> this forces a re-seed of the DRBG
>> -limit the data size drawn from the DRBG; this is already done
>> (see CAAM_RNG_MAX_FIFO_STORE_SIZE)
>> 
>>> @@ -665,6 +666,14 @@ int sec_init_idx(uint8_t sec_idx)
>>>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);
>>>  			return -1;
>>>  		}
>>> +#ifdef CONFIG_DM_RNG
>>> +		if (IS_ENABLED(CONFIG_DM_RNG)) {
>> Shouldn't one or the other (#ifdef / IS_ENABLED) suffice?

Yes, and it should have only been the if (IS_ENABLED(..)).

>>> +static int caam_rng_read_one(struct caam_rng_platdata *pdata)
>>> +{
>>> +	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
>>> +	int ret;
>>> +
>>> +	ret = run_descriptor_jr(pdata->desc);
>>> +	if (ret < 0)
>>> +		return -EIO;
>>> +
>>> +	invalidate_dcache_range((unsigned long)pdata->data,
>>> +				(unsigned long)pdata->data + size);
>> Side note: this is not required on Layerscape SoCs, since CAAM is HW 
>> coherent.
>> Most surely this needs to be handled in a separate patch set,
>> throughout drivers/crypto/fsl/*.

Is this also true for IMX and the PPC QorIQ SoCs?

-michael

[PATCH 4/4] crypto/fsl: add RNG support

[Horia Geantă]

On 6/3/2020 9:25 PM, Michael Walle wrote:
> Hi Horia, Hi Heinrich,
> 
> thanks for reviewing.
> 
> Am 2020-06-03 19:35, schrieb Heinrich Schuchardt:
>> On 6/3/20 6:50 PM, Horia Geant? wrote:
>>> On 6/3/2020 1:06 AM, Michael Walle wrote:
>>>> Register the random number generator with the rng subsystem in 
>>>> u-boot.
>>>> This way it can be used by EFI as well as for the 'rng' command.
>>>>
>>> I am trying to understand what's expected from UCLASS_RNG...
>>>
>>> UEFI spec mentions:
>>> <<The ?raw? algorithm, when supported, is intended to provide entropy
>>> directly from the source, without it going through
>>> some deterministic random bit generator.>>
>>>
>>> lib/efi_loader/efi_rng.c uses EFI_RNG_ALGORITHM_RAW and is happy
>>> with whatever UCLASS_RNG implementation it gets.
>>>
>>>> From above I'd conclude all UCLASS_RNG implementations are expected 
>>>> to
>>> provide "full" entropy directly from a TRNG source, without any DRBG
>>> connected in the middle (b/w TRNG and user).
>>>
>>> Is this correct?
>>> If so, note that using CAAM's job ring interface without prediction 
>>> resistance
>>> to extract randomness does not fit the bill, since TRNG output
>>> is connected to a DRBG (DRBG_Hash, SP800-90A compliant).
>>
>> I would assume that all hardware RNGs use some algorithms to even out
>> the distribution of the random bits coming from noise source. My
>> understanding of the UEFI spec is that EFI_RNG_ALGORITHM_RAW simply 
>> does
>> not provide any guarantee for the distribution quality while a PRNG
>> reseeded via a hardware ring provides some guarantees.
>>
>> Should you be aware that the FSL hardware RNG does not provide a well
>> distributed entropy it would be preferable to pass the stream through a
>> PRNG even if provided as EFI_RNG_ALGORITHM_RAW.
>>>
>>> For CAAM prediction resistance (PR) details I suggest looking in the 
>>> kernel:
>>> https://lore.kernel.org/linux-crypto/VI1PR0402MB3485EF10976A4A69F90E5B0F98580 at VI1PR0402MB3485.eurprd04.prod.outlook.com
>>> 358ba762d9f1 crypto: caam - enable prediction resistance in HRWNG
>>> ea53756d831a crypto: caam - limit single JD RNG output to maximum of 
>>> 16 bytes
> 
> I noticed that PR bit, but I wanted to keep things simple. Does all SEC 
If UCLASS_RNG does not mandate for a TRNG,
i.e. providing randomness from a PRNG / DRBG is acceptable,
then adding PR support is not needed.

> modules and
> versions support this, i.e. the kernel checks versions of some 
> management controller
> in QorIQ SoCs.
> 
All SEC / CAAM having RNG4 block support the PR bit.

I assume the version checking you are referring to is for
the Management Complex (MC) firmware.
MC block is available on SoCs with DPAA2 architecture:
LS1088A, LS2088A, LX2160A
The reason the check was added is that up to a certain point
the f/w did not configure the RNG with PR support, and kernel would have to
re-initialize the RNG only for those legacy f/w versions.

>>>> +static int caam_rng_read_one(struct caam_rng_platdata *pdata)
>>>> +{
>>>> +	int size = CAAM_RNG_MAX_FIFO_STORE_SIZE;
>>>> +	int ret;
>>>> +
>>>> +	ret = run_descriptor_jr(pdata->desc);
>>>> +	if (ret < 0)
>>>> +		return -EIO;
>>>> +
>>>> +	invalidate_dcache_range((unsigned long)pdata->data,
>>>> +				(unsigned long)pdata->data + size);
>>> Side note: this is not required on Layerscape SoCs, since CAAM is HW 
>>> coherent.
>>> Most surely this needs to be handled in a separate patch set,
>>> throughout drivers/crypto/fsl/*.
> 
> Is this also true for IMX and the PPC QorIQ SoCs?
> 
CAAM is _NOT_ HW coherent on i.MX SoCs,
while on the other PPC / ARM SoCs (QorIQ, Layerscape) it is.

Horia

[PATCH 4/4] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 6/3/20 12:05 AM, Michael Walle wrote:
> Register the random number generator with the rng subsystem in u-boot.
> This way it can be used by EFI as well as for the 'rng' command.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  drivers/crypto/fsl/Kconfig   | 11 +++++
>  drivers/crypto/fsl/Makefile  |  1 +
>  drivers/crypto/fsl/jobdesc.c |  9 ++++
>  drivers/crypto/fsl/jobdesc.h |  3 ++
>  drivers/crypto/fsl/jr.c      |  9 ++++
>  drivers/crypto/fsl/rng.c     | 84 ++++++++++++++++++++++++++++++++++++
>  6 files changed, 117 insertions(+)
>  create mode 100644 drivers/crypto/fsl/rng.c
>
> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
> index 181a1e5e99..5936b77494 100644
> --- a/drivers/crypto/fsl/Kconfig
> +++ b/drivers/crypto/fsl/Kconfig
> @@ -45,3 +45,14 @@ config SYS_FSL_SEC_COMPAT
>
>  config SYS_FSL_SEC_LE
>  	bool "Little-endian access to Freescale Secure Boot"
> +
> +if FSL_CAAM
> +
> +config FSL_CAAM_RNG
> +	bool "Enable Random Number Generator support"
> +	depends on DM_RNG
> +	default y
> +	help
> +	  Enable support for the random number generator module of the CAAM.

Hello Michael,

when typing CAAM into Google I got a lot of answers but "Cryptographic
Accelerator and Assurance Module" was not under the first 50 hits.

If this is a hardware RNG I think we should put this into the text.

So how about:

"Enable support the hardware random number generator of Freescale SOCs
using the Cryptographic Accelerator and Assurance Module (CAAM)."

Best regards

Heinrich

[PATCH 4/4] crypto/fsl: add RNG support

[Horia Geantă]

On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
> On 6/3/20 12:05 AM, Michael Walle wrote:
>> Register the random number generator with the rng subsystem in u-boot.
>> This way it can be used by EFI as well as for the 'rng' command.
>>
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>>  drivers/crypto/fsl/Kconfig   | 11 +++++
>>  drivers/crypto/fsl/Makefile  |  1 +
>>  drivers/crypto/fsl/jobdesc.c |  9 ++++
>>  drivers/crypto/fsl/jobdesc.h |  3 ++
>>  drivers/crypto/fsl/jr.c      |  9 ++++
>>  drivers/crypto/fsl/rng.c     | 84 ++++++++++++++++++++++++++++++++++++
>>  6 files changed, 117 insertions(+)
>>  create mode 100644 drivers/crypto/fsl/rng.c
>>
>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>> index 181a1e5e99..5936b77494 100644
>> --- a/drivers/crypto/fsl/Kconfig
>> +++ b/drivers/crypto/fsl/Kconfig
>> @@ -45,3 +45,14 @@ config SYS_FSL_SEC_COMPAT
>>
>>  config SYS_FSL_SEC_LE
>>  	bool "Little-endian access to Freescale Secure Boot"
>> +
>> +if FSL_CAAM
>> +
>> +config FSL_CAAM_RNG
>> +	bool "Enable Random Number Generator support"
>> +	depends on DM_RNG
>> +	default y
>> +	help
>> +	  Enable support for the random number generator module of the CAAM.
> 
> Hello Michael,
> 
> when typing CAAM into Google I got a lot of answers but "Cryptographic
> Accelerator and Assurance Module" was not under the first 50 hits.
> 
> If this is a hardware RNG I think we should put this into the text.
> 
Totally agree.

Besides other cryptographic services, CAAM offers:
-a hardware RNG / TRNG
-a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
from the TRNG

Both are accessible by SW, so clarifying what the driver does
would be useful (unless DM_RNG / UCLASS_RNG already implies
one or the other).

From what I see, driver added by Michael is using the PRNG / DRBG
and not the TRNG. Is this acceptable?

Conceptually this is similar to choosing between
RDSEED vs. RDRDAND x86 instructions:
https://software.intel.com/content/www/us/en/develop/blogs/the-difference-between-rdrand-and-rdseed.html

> So how about:
> 
> "Enable support the hardware random number generator of Freescale SOCs
> using the Cryptographic Accelerator and Assurance Module (CAAM)."
> 
The CAAM acronym is expanded at the top of the same file,
under FSL_CAAM's help:
<<Enables the Freescale's Cryptographic Accelerator and Assurance
Module (CAAM), also known as the SEC version 4 (SEC4). The driver uses
Job Ring as interface to communicate with CAAM.>>

Horia

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Hi Horia, Hi Heinrich,

Am 2020-06-04 10:05, schrieb Horia Geant?:
> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>> On 6/3/20 12:05 AM, Michael Walle wrote:
>>> Register the random number generator with the rng subsystem in 
>>> u-boot.
>>> This way it can be used by EFI as well as for the 'rng' command.
>>> 
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  drivers/crypto/fsl/Kconfig   | 11 +++++
>>>  drivers/crypto/fsl/Makefile  |  1 +
>>>  drivers/crypto/fsl/jobdesc.c |  9 ++++
>>>  drivers/crypto/fsl/jobdesc.h |  3 ++
>>>  drivers/crypto/fsl/jr.c      |  9 ++++
>>>  drivers/crypto/fsl/rng.c     | 84 
>>> ++++++++++++++++++++++++++++++++++++
>>>  6 files changed, 117 insertions(+)
>>>  create mode 100644 drivers/crypto/fsl/rng.c
>>> 
>>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>>> index 181a1e5e99..5936b77494 100644
>>> --- a/drivers/crypto/fsl/Kconfig
>>> +++ b/drivers/crypto/fsl/Kconfig
>>> @@ -45,3 +45,14 @@ config SYS_FSL_SEC_COMPAT
>>> 
>>>  config SYS_FSL_SEC_LE
>>>  	bool "Little-endian access to Freescale Secure Boot"
>>> +
>>> +if FSL_CAAM
>>> +
>>> +config FSL_CAAM_RNG
>>> +	bool "Enable Random Number Generator support"
>>> +	depends on DM_RNG
>>> +	default y
>>> +	help
>>> +	  Enable support for the random number generator module of the 
>>> CAAM.
>> 
>> Hello Michael,
>> 
>> when typing CAAM into Google I got a lot of answers but "Cryptographic
>> Accelerator and Assurance Module" was not under the first 50 hits.
>> 
>> If this is a hardware RNG I think we should put this into the text.
>> 
> Totally agree.

Well I was under the impression that UCLASS_RNG is just for hardware
RNGs.

config DM_RNG
         bool "Driver support for Random Number Generator devices"

Whatever "device" means in that context. But I can certainly add
that this is a h/w rng.

> Besides other cryptographic services, CAAM offers:
> -a hardware RNG / TRNG
> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
> from the TRNG

Together with that.

> Both are accessible by SW, so clarifying what the driver does
> would be useful (unless DM_RNG / UCLASS_RNG already implies
> one or the other).
> 
> From what I see, driver added by Michael is using the PRNG / DRBG
> and not the TRNG. Is this acceptable?

Well there is no, expectation from UCLASS_RNG. EFI "blindly" uses
the first RNG device.. so it is just a "better than nothing".

RNG is also used for the BLOB protocol. Will it interfere this if
I instantiate the RNG with PR?

> Conceptually this is similar to choosing between
> RDSEED vs. RDRDAND x86 instructions:
> https://software.intel.com/content/www/us/en/develop/blogs/the-difference-between-rdrand-and-rdseed.html
> 
>> So how about:
>> 
>> "Enable support the hardware random number generator of Freescale SOCs
>> using the Cryptographic Accelerator and Assurance Module (CAAM)."
>> 
> The CAAM acronym is expanded at the top of the same file,
> under FSL_CAAM's help:
> <<Enables the Freescale's Cryptographic Accelerator and Assurance
> Module (CAAM), also known as the SEC version 4 (SEC4). The driver uses
> Job Ring as interface to communicate with CAAM.>>

This isn't apparent from the patch. But please note that the new kconfig
option is "if FSL_CAAM", where CAAM is explained.

-michael

[PATCH 4/4] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 04.06.20 10:05, Horia Geant? wrote:
> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>> On 6/3/20 12:05 AM, Michael Walle wrote:
>>> Register the random number generator with the rng subsystem in u-boot.
>>> This way it can be used by EFI as well as for the 'rng' command.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>  drivers/crypto/fsl/Kconfig   | 11 +++++
>>>  drivers/crypto/fsl/Makefile  |  1 +
>>>  drivers/crypto/fsl/jobdesc.c |  9 ++++
>>>  drivers/crypto/fsl/jobdesc.h |  3 ++
>>>  drivers/crypto/fsl/jr.c      |  9 ++++
>>>  drivers/crypto/fsl/rng.c     | 84 ++++++++++++++++++++++++++++++++++++
>>>  6 files changed, 117 insertions(+)
>>>  create mode 100644 drivers/crypto/fsl/rng.c
>>>
>>> diff --git a/drivers/crypto/fsl/Kconfig b/drivers/crypto/fsl/Kconfig
>>> index 181a1e5e99..5936b77494 100644
>>> --- a/drivers/crypto/fsl/Kconfig
>>> +++ b/drivers/crypto/fsl/Kconfig
>>> @@ -45,3 +45,14 @@ config SYS_FSL_SEC_COMPAT
>>>
>>>  config SYS_FSL_SEC_LE
>>>  	bool "Little-endian access to Freescale Secure Boot"
>>> +
>>> +if FSL_CAAM
>>> +
>>> +config FSL_CAAM_RNG
>>> +	bool "Enable Random Number Generator support"
>>> +	depends on DM_RNG
>>> +	default y
>>> +	help
>>> +	  Enable support for the random number generator module of the CAAM.
>>
>> Hello Michael,
>>
>> when typing CAAM into Google I got a lot of answers but "Cryptographic
>> Accelerator and Assurance Module" was not under the first 50 hits.
>>
>> If this is a hardware RNG I think we should put this into the text.
>>
> Totally agree.
>
> Besides other cryptographic services, CAAM offers:
> -a hardware RNG / TRNG
> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
> from the TRNG
>
> Both are accessible by SW, so clarifying what the driver does
> would be useful (unless DM_RNG / UCLASS_RNG already implies
> one or the other).

The idea of DM_RNG is to collect entropy from hardware RNGs needed for
implementing the EFI_RNG_PROTOCOL. Our implementation of the
EFI_RNG_PROTOCOL up to now can only supply raw entropy. The UEFI
specification allows to additionally implement certain PRNG algorithms
seeded by the raw entropy which the caller can choose. So in a later
embodiment it may make sense to use what exists as hardware acceleration
for these.

Cf. UEFI Specification Version 2.8 (Errata A) (released February 2020)
https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf

>
>>From what I see, driver added by Michael is using the PRNG / DRBG
> and not the TRNG. Is this acceptable?
>

If it is only PRNG, this is not what we look for. If a PRNG/DRBG is used
to ameliorate the raw entropy stream like Linux does for the /dev/random
device this is fine. We need something non-deterministic.

Isn't this what Linux' drivers/crypto/caam/caamrng.c does?

Best regards

Heinrich

> Conceptually this is similar to choosing between
> RDSEED vs. RDRDAND x86 instructions:
> https://software.intel.com/content/www/us/en/develop/blogs/the-difference-between-rdrand-and-rdseed.html
>
>> So how about:
>>
>> "Enable support the hardware random number generator of Freescale SOCs
>> using the Cryptographic Accelerator and Assurance Module (CAAM)."
>>
> The CAAM acronym is expanded at the top of the same file,
> under FSL_CAAM's help:
> <<Enables the Freescale's Cryptographic Accelerator and Assurance
> Module (CAAM), also known as the SEC version 4 (SEC4). The driver uses
> Job Ring as interface to communicate with CAAM.>>
>
> Horia
>

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-04 14:26, schrieb Heinrich Schuchardt:
> On 04.06.20 10:05, Horia Geant? wrote:
>> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:

>> From what I see, driver added by Michael is using the PRNG / DRBG
>> and not the TRNG. Is this acceptable?
>> 
> 
> If it is only PRNG, this is not what we look for. If a PRNG/DRBG is 
> used
> to ameliorate the raw entropy stream like Linux does for the 
> /dev/random
> device this is fine. We need something non-deterministic.

What do you mean by "only PRNG"?

>> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
>> from the TRNG

So while it is a PRNG, it is non-deterministic because its seeded
from the TRNG.

-michael

[PATCH 4/4] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 04.06.20 14:52, Michael Walle wrote:
> Am 2020-06-04 14:26, schrieb Heinrich Schuchardt:
>> On 04.06.20 10:05, Horia Geant? wrote:
>>> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>
>>> From what I see, driver added by Michael is using the PRNG / DRBG
>>> and not the TRNG. Is this acceptable?
>>>
>>
>> If it is only PRNG, this is not what we look for. If a PRNG/DRBG is used
>> to ameliorate the raw entropy stream like Linux does for the /dev/random
>> device this is fine. We need something non-deterministic.
>
> What do you mean by "only PRNG"?
>
>>> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
>>> from the TRNG
>
> So while it is a PRNG, it is non-deterministic because its seeded
> from the TRNG.

If for every byte that your DM_RNG driver outputs at least one byte from
the TRNG is consumed, it is fine. Otherwise it is not what we are
looking for.

Best regards

Heinrich

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-04 14:58, schrieb Heinrich Schuchardt:
> On 04.06.20 14:52, Michael Walle wrote:
>> Am 2020-06-04 14:26, schrieb Heinrich Schuchardt:
>>> On 04.06.20 10:05, Horia Geant? wrote:
>>>> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>> 
>>>> From what I see, driver added by Michael is using the PRNG / DRBG
>>>> and not the TRNG. Is this acceptable?
>>>> 
>>> 
>>> If it is only PRNG, this is not what we look for. If a PRNG/DRBG is 
>>> used
>>> to ameliorate the raw entropy stream like Linux does for the 
>>> /dev/random
>>> device this is fine. We need something non-deterministic.
>> 
>> What do you mean by "only PRNG"?
>> 
>>>> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
>>>> from the TRNG
>> 
>> So while it is a PRNG, it is non-deterministic because its seeded
>> from the TRNG.
> 
> If for every byte that your DM_RNG driver outputs at least one byte 
> from
> the TRNG is consumed, it is fine. Otherwise it is not what we are
> looking for.

And why is that? This should really be documented somewhere.

-michael

[PATCH 4/4] crypto/fsl: add RNG support

[Heinrich Schuchardt]

On 04.06.20 15:20, Michael Walle wrote:
> Am 2020-06-04 14:58, schrieb Heinrich Schuchardt:
>> On 04.06.20 14:52, Michael Walle wrote:
>>> Am 2020-06-04 14:26, schrieb Heinrich Schuchardt:
>>>> On 04.06.20 10:05, Horia Geant? wrote:
>>>>> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>>>
>>>>> From what I see, driver added by Michael is using the PRNG / DRBG
>>>>> and not the TRNG. Is this acceptable?
>>>>>
>>>>
>>>> If it is only PRNG, this is not what we look for. If a PRNG/DRBG is
>>>> used
>>>> to ameliorate the raw entropy stream like Linux does for the
>>>> /dev/random
>>>> device this is fine. We need something non-deterministic.
>>>
>>> What do you mean by "only PRNG"?
>>>
>>>>> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
>>>>> from the TRNG
>>>
>>> So while it is a PRNG, it is non-deterministic because its seeded
>>> from the TRNG.
>>
>> If for every byte that your DM_RNG driver outputs at least one byte from
>> the TRNG is consumed, it is fine. Otherwise it is not what we are
>> looking for.
>
> And why is that? This should really be documented somewhere.

We want to provide raw entropy in the EFI_RNG_PROTOCOL. So this cannot
be a deterministic sequence of bytes where you only have to know the
current state of a PRNG to find the next byte.

As mentioned above you have a TRNG available. What is problematic about
providing its output?

Best regards

Heinrich

[PATCH 4/4] crypto/fsl: add RNG support

[Michael Walle]

Am 2020-06-04 17:45, schrieb Heinrich Schuchardt:
> On 04.06.20 15:20, Michael Walle wrote:
>> Am 2020-06-04 14:58, schrieb Heinrich Schuchardt:
>>> On 04.06.20 14:52, Michael Walle wrote:
>>>> Am 2020-06-04 14:26, schrieb Heinrich Schuchardt:
>>>>> On 04.06.20 10:05, Horia Geant? wrote:
>>>>>> On 6/4/2020 5:31 AM, Heinrich Schuchardt wrote:
>>>> 
>>>>>> From what I see, driver added by Michael is using the PRNG / DRBG
>>>>>> and not the TRNG. Is this acceptable?
>>>>>> 
>>>>> 
>>>>> If it is only PRNG, this is not what we look for. If a PRNG/DRBG is
>>>>> used
>>>>> to ameliorate the raw entropy stream like Linux does for the
>>>>> /dev/random
>>>>> device this is fine. We need something non-deterministic.
>>>> 
>>>> What do you mean by "only PRNG"?
>>>> 
>>>>>> -a PRNG / DRBG (SP800-90A compliant DRBG_Hash) - which is seeded
>>>>>> from the TRNG
>>>> 
>>>> So while it is a PRNG, it is non-deterministic because its seeded
>>>> from the TRNG.
>>> 
>>> If for every byte that your DM_RNG driver outputs at least one byte 
>>> from
>>> the TRNG is consumed, it is fine. Otherwise it is not what we are
>>> looking for.
>> 
>> And why is that? This should really be documented somewhere.
> 
> We want to provide raw entropy in the EFI_RNG_PROTOCOL. So this cannot
> be a deterministic sequence of bytes where you only have to know the
> current state of a PRNG to find the next byte.

I wasn't aware of the fact that UCLASS_RNG was solely for
EFI_RNG_PROTOCOL. And there are no requirements for the UCLASS_RNG,
are there?

TBH I find this somewhat overkill for just having a random seed for
KASLR. Everyone is complaining about the size of the bootloader steadily
increasing, but then we throw in more and more for what use? Even the 
UEFI
spec states:

   When a Deterministic Random Bit Generator (DRBG) is used on the output
   of a (raw) entropy source, its security level must be at least 256 
bits.

Why does linux use ALGORITHM_RAW? What happens if that is not supported?

> As mentioned above you have a TRNG available. What is problematic about
> providing its output?

See v2, it should be now be the TRNG output, or at least it it reseeded
on every read and the read is limited to 16 bytes, like Horia said in
its very first reply. So I conclude the PRNG is at least seeded with
16 bytes.

-michael