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

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

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;

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

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;

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

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

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-do...
+u8 caam_get_era(void); #endif
#endif /* __FSL_SEC_H */

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;

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

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, +};

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/VI1PR0402MB3485EF10976A4A69F90E5B0F9858... 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

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/VI1PR0402MB3485EF10976A4A69F90E5B0F9858... 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

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/VI1PR0402MB3485EF10976A4A69F90E5B0F9858... 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

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/VI1PR0402MB3485EF10976A4A69F90E5B0F9858... 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

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

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

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

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

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

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

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

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

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
participants (3)
-
Heinrich Schuchardt
-
Horia Geantă
-
Michael Walle