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