diff mbox series

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

Message ID 20200602220554.22477-5-michael@walle.cc
State Superseded
Headers show
Series crypto/fsl: add RNG support | expand

Commit Message

Michael Walle June 2, 2020, 10:05 p.m. UTC
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 at 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

Comments

Horia Geanta June 3, 2020, 4:50 p.m. UTC | #1
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
Heinrich Schuchardt June 3, 2020, 5:35 p.m. UTC | #2
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
>
Michael Walle June 3, 2020, 6:25 p.m. UTC | #3
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
Horia Geanta June 3, 2020, 8:25 p.m. UTC | #4
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
Heinrich Schuchardt June 4, 2020, 2:31 a.m. UTC | #5
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 at 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
Horia Geanta June 4, 2020, 8:05 a.m. UTC | #6
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 at 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
Michael Walle June 4, 2020, 10:28 a.m. UTC | #7
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 at 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
Heinrich Schuchardt June 4, 2020, 12:26 p.m. UTC | #8
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 at 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
>
Michael Walle June 4, 2020, 12:52 p.m. UTC | #9
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
Heinrich Schuchardt June 4, 2020, 12:58 p.m. UTC | #10
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
Michael Walle June 4, 2020, 1:20 p.m. UTC | #11
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
Heinrich Schuchardt June 4, 2020, 3:45 p.m. UTC | #12
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
Michael Walle June 5, 2020, 12:15 p.m. UTC | #13
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
diff mbox series

Patch

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