Re: [PATCH 1/2] wrapper: add a helper to generate numbers from a CSPRNG
From: Taylor Blau <hidden>
Date: 2021-11-16 18:23:06
On Tue, Nov 16, 2021 at 11:01:20AM -0500, rsbecker@nexbridge.com wrote:
On November 16, 2021 10:31 AM, Jeff King wrote:quoted
On Tue, Nov 16, 2021 at 03:35:41AM +0000, brian m. carlson wrote:quoted
The order of options is also important here. On systems with arc4random, which is most of the BSDs, we use that, since, except on MirBSD, it uses ChaCha20, which is extremely fast, and sits entirely in userspace, avoiding a system call. We then prefer getrandom over getentropy, because the former has been available longer on Linux, and finally, if none of those are available, we use /dev/urandom, because most Unix-like operating systems provide that API. We prefer options that don't involve device files when possible because those work in some restricted environments where device files may not be available.I wonder if we'll need a low-quality fallback for older systems which don't even have /dev/urandom. Because it's going to be used in such a core part of the system (tempfiles), this basically becomes a hard requirement for using Git at all. I can't say I'm excited in general to be introducing a dependency like this, just because of the portability headaches. But it may be the least bad thing (especially if we can fall back to the existing behavior). One alternative would be to build on top of the system mkstemp(), which makes it libc's problem. I'm not sure if we'd run into problems there, though.None of /dev/urandom, /dev/random, or mkstemp are available on some platforms, including NonStop. This is not a good dependency to add. One variant PRNGD is used in ia64 OpenSSL, while the CPU random generator in hardware is used on x86. I cannot get behind this at all. Libc is also not used in or available to our port. I am very worried about this direction.
I share Peff's lack of enthusiasm about the dependency situation. But making Git depend on having /dev/urandom available is simply not feasible, as you point out. I wonder if the suitable fall-back should be the existing behavior of git_mkstemps_mode()? That leaves us in a somewhat-disappointing situation of not having fully resolved the DOS attack on all platforms. But it makes our dependency situation less complicated, and leaves things no worse off than the were before on platforms like NonStop. Thanks, Taylor