2019-11-26 09:14:56

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH v4 2/6] crypto: caam - enable prediction resistance in HRWNG

On 11/21/2019 5:56 PM, Andrey Smirnov wrote:
> Instantiate CAAM RNG with prediction resistance enabled to improve its
> quality.
>
It's worth noting there are two RNG operations being changed:
-instantiation
-generation

Generation with prediction resistance (PR) is only supported on
RNG state handles instantiated with PR option.

Using PR when generating randomness effectively forces a reseed
of the DRBG / PRNG - that's how quality is improved.

> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
> index e8baacaabe07..6dde8ae3cd9b 100644
> --- a/drivers/crypto/caam/caamrng.c
> +++ b/drivers/crypto/caam/caamrng.c
> @@ -202,7 +202,8 @@ static inline int rng_create_sh_desc(struct caam_rng_ctx *ctx)
> init_sh_desc(desc, HDR_SHARE_SERIAL);
>
> /* Generate random bytes */
> - append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG);
> + append_operation(desc, OP_ALG_ALGSEL_RNG | OP_TYPE_CLASS1_ALG |
> + OP_ALG_PR_ON);
>
> /* Store bytes */
> append_seq_fifo_store(desc, RN_BUF_SIZE, FIFOST_TYPE_RNGSTORE);
> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
> index df4db10e9fca..a1c879820286 100644
> --- a/drivers/crypto/caam/ctrl.c
> +++ b/drivers/crypto/caam/ctrl.c
> @@ -36,7 +36,8 @@ static void build_instantiation_desc(u32 *desc, int handle, int do_sk)
> init_job_desc(desc, 0);
>
> op_flags = OP_TYPE_CLASS1_ALG | OP_ALG_ALGSEL_RNG |
> - (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT;
> + (handle << OP_ALG_AAI_SHIFT) | OP_ALG_AS_INIT |
> + OP_ALG_PR_ON;
>
> /* INIT RNG in non-test mode */
> append_operation(desc, op_flags);
> @@ -275,11 +276,12 @@ static int instantiate_rng(struct device *ctrldev, int state_handle_mask,
> return -ENOMEM;
>
> for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
> + const u32 rdsta_mask = (RDSTA_PR0 | RDSTA_IF0) << sh_idx;
> /*
> * If the corresponding bit is set, this state handle
> * was initialized by somebody else, so it's left alone.
> */
> - if ((1 << sh_idx) & state_handle_mask)
> + if (rdsta_mask & state_handle_mask)
> continue;
>
If a state handle was previously instantiated (e.g. by U-boot),
but without prediction resistance support, it won't be re-instantiated
("continue" / skip above).

The result is using a state handle without PR support.
Due to this, when extracting / generating randomness (in caamrng.c) with
the PR bit set, job descriptor will generate a prediction resistance error.

IMO the proper thing to do in case a state handle was instantiated
without PR support is to re-instantiate it.

There's an assumption here though: kernel handles RNG initialization only
in some cases, when it's effectively "controlling" it.
In cases when it's not, like when Management Complex (MC) f/w is present
(or OP-TEE OS, SECO etc.), kernel skips RNG init and only uses the job ring
interface for random generation.
==> MC, OP-TEE, SECO etc. have to be updated to initialize
RNG state handles with PR support

Horia