[PATCH libXdmcp] Use getrandom() syscall if available

[Mark Kettenis]

> From: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> Date: Tue, 4 Apr 2017 11:05:47 +0200
> 
> On Mon, Apr 3, 2017 at 9:17 PM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
> > OpenBSD does not have a getrandom(3) function in libc.  The OpenBSD
> > postion is that the arc4random(3) family of functions is always the
> > preferred way to get proper random numbers as all the other interfaces
> > have nasty corner cases that people shouldn't have to deal with.  In
> > your case for example, getrandom(2) may block and therefore could
> > return EINTR, a case which your patch doesn't handle.
> 
> I see. So if arc4random_buf(3) is available, it should always be
> preferred. When it is not available, I guess I should go for
> getrandom/getentropy and probably don't care about the syscall at all.

Right.  You probably shouldn't bother with getentropy() though, unless
you build a complete proper PRNG on top of it.

> > So I'd argue that from a Linux distro point of view, making sure that
> > a proper arc4random_buf(3) implementation is available would be the
> > best way to fix this vulnerability.  Either by improving the version
> > provided in libbsd (feel free to take the code from libressl) or by
> > lobbying for its inclusion in glibc.
> 
> TBH, the issue with the libbsd implementation is that it's not shipped
> with RHEL (where I try to provide a fix). For various reasons we do
> not ship libbsd and adding it there only for one PRNG is something
> that will be costly in term of maintenance.

More costly than pushing/maintaining patches for several other
projects?

> I don't think we ship libressl either, but the problem is not that
> much with RHEL (we can ship custom fixes if required), but more the
> fact that there is no true fix besides "use libbsd".
> 
> Which is why I wanted to provide an alternative for Linux that we can
> point customers at.
> 
> Regarding adding arc4random_buf(3) in glibc, I am not sure I
> personally want to go this path. It took a little bit more than 2
> years to have getrandom() in glibc, when it was pushed by far more
> knowledgeable people than me :(

I sympathize.

> >  Adding more #ifdef spaghetti
> > everywhere isn't going to help.
> 
> True. But the current situation where the code might be vulnerable or
> not is not a good situation either. Having a CVE out there without a
> fix is not good. Users will want it to be fixed and we can't guarantee
> that the code they are running is safe. I am not pointing fingers at
> anybody (I was not in the security discussion), I am just trying to
> justify the fact that we still need a patch that we can point people
> at.

The CVE is a bit of a joke though.  The original commit to use
arc4random_buf() in libICE was made in 2013, exactly to avoid the use
of poor quality random numbers in a security-sensitive bit of code
that the CVE is about.  A bit sad to see that four years there still
isn't a one-stop solution for this on Linux....

> How about (in pseudo-c):
> 
> ---
> #ifndef HAVE_ARC4RANDOM_BUF
> void arc4random_buf(char *auth, int len)
> {
> #if HAVE_GET_ENTROPY
>     getentropy(auth, len);
>     if (failed)
> #endif
>     {
>       /* use the older, unsafe code */
>     }
> }
> #endif
> 
> void
> XdmcpGenerateKey (XdmAuthKeyPtr key)
> {
>     arc4random_buf(key->data, 8)
> }
> 
> ---
> 
> This should make the code cleaner, remove a little bit of spaghetti,
> and will allow for future libc to implement arc4random_buf in the
> missing architectures.

Agreed.  That would be the best way to move forward.