Re: [PATCH 1/2] wrapper: add a helper to generate numbers from a CSPRNG

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Nov 16, 2021, 22:41 UTC
Message-ID: <YZQzqjWMzaWVkkfP@camp.crustytoothpaste.net>
In-Reply-To: <009d01d7db03$354ecae0$9fec60a0$@nexbridge.com>

On 2021-11-16 at 16:01:20, rsbecker@nexbridge.com wrote:

Show 30 quoted lines

> On November 16, 2021 10:31 AM, Jeff King wrote:
> > On Tue, Nov 16, 2021 at 03:35:41AM +0000, brian m. carlson wrote:
> > 
> > > 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'm really not excited about a fallback here, and I specifically did not include one for that reason. I'm happy to add an appropriate dependency on an OpenSSL or libgcrypt PRNG if you're linking against that already (e.g., for libcurl) or support for libbsd's arc4random or getentropy if that will work on your system. For example, how are you dealing with TLS connections over HTTPS? That library will almost certainly provide the required primitives in a straightforward and portable way.

I do fundamentally believe every operating system and language environment need to provide a readily available CSPRNG in 2021, especially because in the vast majority of cases, hash tables must be randomized to avoid hash DoS attacks on untrusted input. I'm planning to look into our hash tables in the future to see if they are vulnerable to that kind of attack, and if so, we'll need to have a CSPRNG for basic security reasons, and platforms that can't provide one would be subject to a CVE.

If we really can't find a solution, I won't object to a patch on top that adds an insecure fallback, but I don't want to put my name or sign-off on such a patch because I think it's a mistake. But I think we almost certainly can, though.

-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA