Ondrej Zajicek <[email protected]> writes: > On Tue, Mar 10, 2020 at 04:58:26PM +0100, Toke Høiland-Jørgensen wrote: >> > I think that random_bytes() should not fail. >> >> Preferably not; but we don't really have any guarantees that the syscall >> will succeed, do we? I guess I can add some sanity checks on startup and >> bail out if (e.g.) /dev/urandom cannot be opened. It would still be >> possible for read() or getrandom() to fail later on, though, no? > > It is mostly whether we want error handling directly in random_bytes(), > or in caller code. If we could have some reasonable error handling code > in the caller (e.g. log error message and drop packet), then we can do > that, but otherwise (as there are no error handling code in Babel patch) > it seems better to just die() directly in random_bytes() code if > underlying syscalls fail. > > Definitely, we should not silently ignore these errors. > > It seems getrandom() and getentropy() should not fail for buflen <= 256, > so we may die() for unexpected errors from these syscalls. > > For read() from /dev/urandom, it might be good to handle EINTR, but we > may die() for other errors.
Right, sure. I think we can handle it from the caller (but did realise I didn't do that in this version of the patch). Will fix that, and also handle EINTR. >> > There are '#if defined(NATIVE_LITTLE_ENDIAN)' in the code, does >> > anybody define these? >> >> Hmm, probably not? The FreeBSD Blake implementation seems to have a >> #define based on __BYTE_ORDER, so guess we could just add something like >> that as well? > > We already have CPU_BIG_ENDIAN in BIRD, so perhaps just use that. ACK, will do. Thanks for your comments; I'll see if I can't get a revised version ready soonish (certainly sooner than the 1.5 years it took me to do this version ;)). -Toke
