prng change feedback

[Max]

On 06.10.2017 17:43, ringsignature at riseup.net wrote:
> Is there any reason that it isn't just called a single time on system
> startup? It should never again block after that point in time.

Small clarification (more like not to myself reading this later on): that's the case
for default urandom source (used in current patch), not the GRND_RANDOM (discussed
elsewhere in the ML thread).

>  A measurement that might be worth considering is if it blocks, ever, in
> practice?

We'll sort of make that in the end: even in GRND_NONBLOCK mode the errno is set for
the cases where it would have been blocked otherwise. It's propagated to application
by osmo_get_rand_id() and the application will log it.

> Might it also be possible to call the
> wrapper for getrandom() at library init time as well as later when the
> random bytes are needed?

Sure, but it seems more complex.

What do we gain by using it that way? The underlying getrandom() call might still
fail so we have to handle/propagate errors anyway.
What do we loose by using it that way? Seems like nothing - see my note about error
propagation above.

> Isn't it more error prone to handle errors and unfilled buffers than to
> block a single time?

We don't have to deal with unfilled buffers in application code: either call to
osmo_get_rand_id() succeeds or not.

Also it depends on what kind of error handling we choose. To me single "if (rc < 0) {
log("doh!"); exit(1); }" looks less error-prone than potentially blocking code.

>  Seems tricky, though I agree that consistent
> behavior might be worth the trade off. If GRND_NONBLOCK ensures that no
> buffer is ever underfilled, that might be the middle ground that makes
> the most sense.

GRND_NONBLOCK doesn't guarantee that, using OSMO_MAX_RAND_ID_LEN = 16 and default urandom source does.

I think there might be some confusion so let me stress the following point: GRND_*
flags are independent. There are 2 flags currently available. So we have 4 flag
combinations.

GRND_RANDOM let us select randomness source.
GRND_NONBLOCK let us control blocking behavior.

So in the absence of explicit guarantees (provided by _NONBLOCK) we can implicitly
control blocking (to block at most once) by selecting random source and buffer size.

I prefer explicit control unless there are strong arguments against it.

> That is my understanding as well. The key difference is that you were
> spot on about the pool being drained very seriously - to the point of
> underflowing, thus potentially encountering serious error states. Those
> error states might be a 512 byte buffer with only one random byte, for
> example.

There're 2 possible causes for underfilled buffer after getrandom() according to man:
1) insufficient entropy

2) signal interrupt

Both are impossible in the current implementation under review in
https://gerrit.osmocom.org/#/c/1526/

1) Impossible because we use default urandom source

2) Impossible because we always request less than 256 bytes

I still check for it out of paranoia.

However, from application PoV it should not matter anyway: if call to some function
might fail than we should handle it. There are basically 2 things we can do after
logging the error:

- terminate the application

- fallback to insecure random numbers

So far we used the latter. If understood the summary of ongoing discussion right,
than we should opt for former. Shall I make it configurable via application
vty/config (OsmoBSC/OsmoMSC/OsmoSGSN)?

-- 
Max Suraev <msuraev at sysmocom.de> http://www.sysmocom.de/
======================================================================= 
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93 
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B 
* Geschaeftsfuehrer / Managing Director: Harald Welte