Am Dienstag, 7. Mai 2019, 15:10:38 CEST schrieb Yann Droneaud:
Hi Yann,
> > + if (IS_ENABLED(CONFIG_CRYPTO_FIPS)) {
>
> if (!IS_ENABLED(CONFIG_CRYPTO_FIPS))
> return 0;
Changed
>
> > + unsigned short entropylen = drbg_sec_strength(drbg->core-
>flags);
> > + int ret = 0;
> > +
> > + /* skip test if we test the overall system */
> > + if (list_empty(&drbg->test_data.list))
> > + return 0;
> > + /* only perform test in FIPS mode */
> > + if (!fips_enabled)
> > + return 0;
> > +
> > + if (!drbg->fips_primed) {
> > + /* Priming of FIPS test */
> > + memcpy(drbg->prev, entropy, entropylen);
> > + drbg->fips_primed = true;
> > + /* priming: another round is needed */
> > + return -EAGAIN;
> > + }
> > + ret = memcmp(drbg->prev, entropy, entropylen);
> > + if (!ret)
> > + panic("DRBG continuous self test failed\n");
>
> Previous version from commit b3614763059b ("crypto: drbg - remove FIPS
> 140-2 continuous test") already has it, and so does the "self test" in
> drivers/char/random.c, but do we really want to panic() in the
> unlikely, but still possible, event of a duplicated output from the
> PRNG ? The longer the system is up, the likelier this can happen ... if
> one can wait for the end of the universe :)
Yes, we want that in FIPS mode.
The requirement is that the "crypto module" (i.e. the kernel crypto API) must
become unavailable if that issue happens.
Commonly this can only be achieved in two ways: either we have a kind of
global flag that effectively deactivates the kernel crypto API or we terminate
the kernel.
>
> > + memcpy(drbg->prev, entropy, entropylen);
> > + /* the test shall pass when the two values are not equal */
> > + return (ret != 0) ? 0 : -EFAULT;
>
> Here, it's not possible to have ret == 0, since that would panic(), so
> -EFAULT cannot be returned.
Agreed.
>
> > + } else {
> > + return 0;
> > + }
> > +}
> > +
> >
> > /*
> >
> > * Convert an integer into a byte representation of this integer.
> > * The byte representation is big-endian
> >
> > @@ -1006,16 +1057,23 @@ static void drbg_async_seed(struct work_struct
> > *work)>
> > seed_work);
> >
> > unsigned int entropylen = drbg_sec_strength(drbg->core->flags);
> > unsigned char entropy[32];
> >
> > + int ret;
> >
> > BUG_ON(!entropylen);
> > BUG_ON(entropylen > sizeof(entropy));
> >
> > - get_random_bytes(entropy, entropylen);
> >
> > drbg_string_fill(&data, entropy, entropylen);
> > list_add_tail(&data.list, &seedlist);
> >
> > mutex_lock(&drbg->drbg_mutex);
> >
> > + do {
> > + get_random_bytes(entropy, entropylen);
> > + ret = drbg_fips_continuous_test(drbg, entropy);
> > + if (ret && ret != -EAGAIN)
> > + goto unlock;
> > + } while (ret);
> > +
>
> A function doing get_random_bytes() and continous_test() would be
> useful to both sync and async seed function.
Changed
Thanks.
Ciao
Stephan
