| Message ID | e4800de3138d3987d9f3c68310fcd9f3abc7a366.camel@sipsolutions.net (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | jitterentropy vs. simulation | expand |
[I guess we should keep the CCs so other see it] > Looking at the stuck check it will be bogus in simulations. True. > You might as well ifdef that instead. > > If a simulation is running insert the entropy regardless and do not compute the derivatives used in the check. Actually you mostly don't want anything inserted in that case, so it's not bad to skip it. I was mostly thinking this might be better than adding a completely unrelated ifdef. Also I guess in real systems with a bad implementation of random_get_entropy(), the second/third derivates might be constant/zero for quite a while, so may be better to abort? In any case, I couldn't figure out any way to not configure this into the kernel when any kind of crypto is also in ... johannes
On Fri, 2023-12-01 at 19:35 +0100, Johannes Berg wrote: > [I guess we should keep the CCs so other see it] > > > Looking at the stuck check it will be bogus in simulations. > > True. > > > You might as well ifdef that instead. > > > > If a simulation is running insert the entropy regardless and do not compute the derivatives used in the check. > > Actually you mostly don't want anything inserted in that case, so it's > not bad to skip it. > > I was mostly thinking this might be better than adding a completely > unrelated ifdef. Also I guess in real systems with a bad implementation > of random_get_entropy(), the second/third derivates might be > constant/zero for quite a while, so may be better to abort? > > In any case, I couldn't figure out any way to not configure this into > the kernel when any kind of crypto is also in ... Doesn't this imply the simulation is not complete and you need to add clock jitter for the simulation to be more useful? You can use the host rng to add random jitter to the simulation clock. Simo.
On Fri, 2023-12-01 at 14:25 -0500, Simo Sorce wrote: > > Doesn't this imply the simulation is not complete Kind of? > and you need to add > clock jitter for the simulation to be more useful? > No, it's more _intentionally_ incomplete. This works fine in normal ARCH=um, but with time-travel variants we have more of a discrete event- based simulation, to integrates well with other things (e.g. SystemC or similar based device simulations). So this is quite intentional, and yes, it breaks in a few spots such as this one. johannes
Am Freitag, 1. Dezember 2023, 19:35:11 CET schrieb Johannes Berg: Hi Johannes, > [I guess we should keep the CCs so other see it] > > > Looking at the stuck check it will be bogus in simulations. > > True. > > > You might as well ifdef that instead. > > > > If a simulation is running insert the entropy regardless and do not > > compute the derivatives used in the check. > Actually you mostly don't want anything inserted in that case, so it's > not bad to skip it. > > I was mostly thinking this might be better than adding a completely > unrelated ifdef. Also I guess in real systems with a bad implementation > of random_get_entropy(), the second/third derivates might be > constant/zero for quite a while, so may be better to abort? > > In any case, I couldn't figure out any way to not configure this into > the kernel when any kind of crypto is also in ... The reason for the Jitter RNG to be dragged in is the Kconfig select in CRYPTO_DRBG. Why does the DRBG require it? The DRBG seeds from get_random_bytes || jitter rng output. It pulls an equal amount of data from each source where each source alone is able to provide all entropy that the DRBG needs. That said, the Jitter RNG can be designated as optional, because the code already can handle the situation where this Jitter RNG is not available. However, in FIPS mode, we must have the Jitter RNG source. That said, I could fathom to change CRYPTO_DRBG to remove the select CRYPTO_JITTERENTROPY. But instead, add the select to CRYPTO_FIPS. This change would entail a new log entry when a DRBG instance initializes: pr_info("DRBG: Continuing without Jitter RNG\n"); I would assume that this change could help you to deselect the Jitter RNG in your environment. Side note: do you have an idea how that Jitter RNG perhaps could still be selected by default when the DRBG is enabled, but allows it being deselected following the aforementioned suggestion? Ciao Stephan
Hi Stephan, > The reason for the Jitter RNG to be dragged in is the Kconfig select in > CRYPTO_DRBG. Yes, but you can't get rid of DRBG either. That'd require getting rid of CRYPTO_RNG_DEFAULT, which means you cannot even have CRYPTO_GENIV? Hmm. Maybe it _is_ possible to get rid of all these. > Why does the DRBG require it? > > The DRBG seeds from get_random_bytes || jitter rng output. It pulls an equal > amount of data from each source where each source alone is able to provide all > entropy that the DRBG needs. That said, the Jitter RNG can be designated as > optional, because the code already can handle the situation where this Jitter > RNG is not available. However, in FIPS mode, we must have the Jitter RNG > source. > > That said, I could fathom to change CRYPTO_DRBG to remove the select > CRYPTO_JITTERENTROPY. But instead, add the select to CRYPTO_FIPS. Well, CRYPTO_FIPS also would break our testing, since we still have to support WEP/TKIP (RC4-based) ... > This change would entail a new log entry when a DRBG instance initializes: > > pr_info("DRBG: Continuing without Jitter RNG\n"); > > I would assume that this change could help you to deselect the Jitter RNG in > your environment. > > Side note: do you have an idea how that Jitter RNG perhaps could still be > selected by default when the DRBG is enabled, but allows it being deselected > following the aforementioned suggestion? Well it _looks_ like it might be possible to get rid of it (see above), however, what I was really thinking is that jitter RNG might detect that it doesn't actually get any reasonable data and eventually give up, rather than looping indefinitely? It seems that might be useful more generally too? johannes
Am Montag, 4. Dezember 2023, 11:24:25 CET schrieb Johannes Berg: Hi Johannes, > > Well it _looks_ like it might be possible to get rid of it (see above), > however, what I was really thinking is that jitter RNG might detect that > it doesn't actually get any reasonable data and eventually give up, > rather than looping indefinitely? It seems that might be useful more > generally too? Agreed. Let me have a close look at the patch then. Thanks. Ciao Stephan
Am 01.12.2023 um 19:35 schrieb Johannes Berg: > [I guess we should keep the CCs so other see it] > >> Looking at the stuck check it will be bogus in simulations. > > True. > >> You might as well ifdef that instead. >> >> If a simulation is running insert the entropy regardless and do not compute the derivatives used in the check. > > Actually you mostly don't want anything inserted in that case, so it's > not bad to skip it. > > I was mostly thinking this might be better than adding a completely > unrelated ifdef. Also I guess in real systems with a bad implementation > of random_get_entropy(), the second/third derivates might be > constant/zero for quite a while, so may be better to abort? Maybe dump question: could we simply implement a timex.h for UM which delegates in non-timetravel mode to the x86 variant and otherwise pull some randomness from the host or from a file/pipe configurable from the UML commandline for random_get_entropy()? I would say, if the random jitter is truly deterministic for a simulation, that seems to be good enough. Said that, it would be nice to be able to configure all random sources to pull entropy from some file that we are able to configure from the command line, but that is a different topic. > > In any case, I couldn't figure out any way to not configure this into > the kernel when any kind of crypto is also in ... > > johannes > >
On 04/12/2023 12:06, Benjamin Beichler wrote: > Am 01.12.2023 um 19:35 schrieb Johannes Berg: >> [I guess we should keep the CCs so other see it] >> >>> Looking at the stuck check it will be bogus in simulations. >> >> True. >> >>> You might as well ifdef that instead. >>> >>> If a simulation is running insert the entropy regardless and do not compute the derivatives used in the check. >> >> Actually you mostly don't want anything inserted in that case, so it's >> not bad to skip it. >> >> I was mostly thinking this might be better than adding a completely >> unrelated ifdef. Also I guess in real systems with a bad implementation >> of random_get_entropy(), the second/third derivates might be >> constant/zero for quite a while, so may be better to abort? > Maybe dump question: could we simply implement a timex.h for UM which delegates in non-timetravel mode to the x86 variant Sounds reasonable. > and otherwise pull some randomness from the host or from a file/pipe configurable from the UML commandline for random_get_entropy()? Second one. We can run haveged in pipe mode and read from the pipe. Additionally, this will allow deterministic simulation if need be. You can record the haveged output and use it for more than one simulation. > > I would say, if the random jitter is truly deterministic for a simulation, that seems to be good enough. > > Said that, it would be nice to be able to configure all random sources to pull entropy from some file that we are able to configure from the command line, but that is a different topic. > >> >> In any case, I couldn't figure out any way to not configure this into >> the kernel when any kind of crypto is also in ... >> >> johannes >> >> > > > > >
--- a/crypto/jitterentropy.c +++ b/crypto/jitterentropy.c @@ -552,10 +552,13 @@ static int jent_measure_jitter(struct rand_data *ec, __u64 *ret_current_delta) * Function fills rand_data->hash_state * * @ec [in] Reference to entropy collector + * + * Return: 0 if entropy reading failed (was stuck), 1 otherwise */ -static void jent_gen_entropy(struct rand_data *ec) +static int jent_gen_entropy(struct rand_data *ec) { unsigned int k = 0, safety_factor = 0; + unsigned int stuck_counter = 0; if (fips_enabled) safety_factor = JENT_ENTROPY_SAFETY_FACTOR; @@ -565,8 +568,13 @@ static void jent_gen_entropy(struct rand_data *ec) while (!jent_health_failure(ec)) { /* If a stuck measurement is received, repeat measurement */ - if (jent_measure_jitter(ec, NULL)) + if (jent_measure_jitter(ec, NULL)) { + if (stuck_counter++ > 100) + return 0; continue; + } + + stuck_counter = 0; /* * We multiply the loop value with ->osr to obtain the @@ -575,6 +583,8 @@ static void jent_gen_entropy(struct rand_data *ec) if (++k >= ((DATA_SIZE_BITS + safety_factor) * ec->osr)) break; } + + return 1; } /* @@ -611,7 +621,8 @@ int jent_read_entropy(struct rand_data *ec, unsigned char *data, while (len > 0) { unsigned int tocopy, health_test_result; - jent_gen_entropy(ec); + if (!jent_gen_entropy(ec)) + return -3; health_test_result = jent_health_failure(ec); if (health_test_result > JENT_PERMANENT_FAILURE_SHIFT) {