Обсуждение: Poor buildfarm coverage of strong-random alternatives

I just noticed that, since I retired pademelon in August, we have
exactly no buildfarm coverage of --disable-strong-random code paths.
What's more, because the vast majority of the buildfarm enables
--with-openssl, we're mostly just testing the punt-to-OpenSSL
variant of pg_strong_random.  Checking the buildfarm database,
the last builds that did anything different from that are

 protosciurus  | 2018-08-15 13:37:08 | checking which random number source to use... /dev/urandom
 pademelon     | 2018-08-16 18:47:07 | checking which random number source to use... weak builtin PRNG
 castoroides   | 2018-09-26 12:03:07 | checking which random number source to use... /dev/urandom
 locust        | 2018-12-14 01:14:35 | checking which random number source to use... /dev/urandom
 frogfish      | 2018-12-22 18:39:35 | checking which random number source to use... /dev/urandom
 anole         | 2018-12-27 10:30:33 | checking which random number source to use... /dev/urandom
 gharial       | 2018-12-27 13:30:46 | checking which random number source to use... /dev/urandom
 jacana        | 2018-12-27 13:45:14 | checking which random number source to use... Windows native

Do we need more coverage of the "Windows native" alternative?

More urgently, what about the lack of --disable-strong-random
coverage?  I feel like we should either have a buildfarm
critter or two testing that code, or decide that it's no longer
interesting and rip it out.  backend_random.c, to name just
one place, has a complex enough !HAVE_STRONG_RANDOM code path
that I don't feel comfortable letting it go totally untested.

There's certainly a reasonable argument to be made that everybody
should have /dev/urandom these days, or else be willing to
install OpenSSL and let it figure out what to do.  (Even my hoary
old HPUX 10.20 box does have OpenSSL and a working entropy daemon
to feed it; I was just intentionally not using that in the
pademelon configuration.)

            regards, tom lane

On Thu, Dec 27, 2018 at 03:56:52PM -0500, Tom Lane wrote:
> More urgently, what about the lack of --disable-strong-random
> coverage?  I feel like we should either have a buildfarm
> critter or two testing that code, or decide that it's no longer
> interesting and rip it out.  backend_random.c, to name just
> one place, has a complex enough !HAVE_STRONG_RANDOM code path
> that I don't feel comfortable letting it go totally untested.

If that proves to not be useful, just dropping the switch sounds like
a good option to me.  I would be curious to hear Heikki on the matter
as he has introduced the switch in the v10 time-frame.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Dec 27, 2018 at 03:56:52PM -0500, Tom Lane wrote:
>> More urgently, what about the lack of --disable-strong-random
>> coverage?  I feel like we should either have a buildfarm
>> critter or two testing that code, or decide that it's no longer
>> interesting and rip it out.  backend_random.c, to name just
>> one place, has a complex enough !HAVE_STRONG_RANDOM code path
>> that I don't feel comfortable letting it go totally untested.

> If that proves to not be useful, just dropping the switch sounds like
> a good option to me.  I would be curious to hear Heikki on the matter
> as he has introduced the switch in the v10 time-frame.

I might be misremembering, but I think it was me that pressed to have
that switch in the first place :-).  The reason my feelings have changed
on the matter is mainly that we recently moved the compiler goalposts
to C99.  That reduces to about nil the chances of people being able to
build PG on pre-turn-of-the-century platforms, at least without a lot
of add-on software.  So the idea that we should be able to cope with
platforms lacking /dev/urandom has correspondingly dropped in value.
Judging by our buildfarm sample, nothing released in this century
lacks /dev/urandom.

            regards, tom lane

On 28/12/2018 01:14, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> On Thu, Dec 27, 2018 at 03:56:52PM -0500, Tom Lane wrote:
>>> More urgently, what about the lack of --disable-strong-random
>>> coverage?  I feel like we should either have a buildfarm
>>> critter or two testing that code, or decide that it's no longer
>>> interesting and rip it out.  backend_random.c, to name just
>>> one place, has a complex enough !HAVE_STRONG_RANDOM code path
>>> that I don't feel comfortable letting it go totally untested.
> 
>> If that proves to not be useful, just dropping the switch sounds like
>> a good option to me.  I would be curious to hear Heikki on the matter
>> as he has introduced the switch in the v10 time-frame.
> 
> I might be misremembering, but I think it was me that pressed to have
> that switch in the first place :-).  The reason my feelings have changed
> on the matter is mainly that we recently moved the compiler goalposts
> to C99.  That reduces to about nil the chances of people being able to
> build PG on pre-turn-of-the-century platforms, at least without a lot
> of add-on software.  So the idea that we should be able to cope with
> platforms lacking /dev/urandom has correspondingly dropped in value.
> Judging by our buildfarm sample, nothing released in this century
> lacks /dev/urandom.

Yeah, there probably isn't anyone needing --disable-strong-random in 
practice. The situation is slightly different between the frontend and 
backend, though. It's more likely that someone might need to build libpq 
on a very ancient system, but not the server. And libpq only needs 
pg_strong_random() for SCRAM support. It'd be kind of nice to still be 
able to build libpq without pg_strong_random(), with SCRAM disabled. But 
that's awkward to arrange with autoconf, there is no "--libpq-only" 
flag. Perhaps replace the backend !HAVE_STRONG_RANDOM code with #error.

+1 for just ripping it out, nevertheless. If someone needs libpq on an 
ancient system, they can build an older version of libpq as a last resort.

- Heikki

Heikki Linnakangas <hlinnaka@iki.fi> writes:
> Yeah, there probably isn't anyone needing --disable-strong-random in 
> practice. The situation is slightly different between the frontend and 
> backend, though. It's more likely that someone might need to build libpq 
> on a very ancient system, but not the server. And libpq only needs 
> pg_strong_random() for SCRAM support. It'd be kind of nice to still be 
> able to build libpq without pg_strong_random(), with SCRAM disabled. But 
> that's awkward to arrange with autoconf, there is no "--libpq-only" 
> flag. Perhaps replace the backend !HAVE_STRONG_RANDOM code with #error.

> +1 for just ripping it out, nevertheless. If someone needs libpq on an 
> ancient system, they can build an older version of libpq as a last resort.

The other workaround that remains available is to build --with-openssl.
So the arguments for keeping !HAVE_STRONG_RANDOM seem pretty weak from
here.

            regards, tom lane

Further to this ... I was just doing some measurements to see how much
it'd add to backend startup time if we start using pg_strong_random()
to set the initial random seed.  The answer, at least on my slightly
long-in-the-tooth RHEL6 box, is "about 25 usec using /dev/urandom,
or about 80 usec using OpenSSL".  So I'm wondering why configure is
coded to prefer OpenSSL.

I'm going to go do some timing checks on some other platforms, but
this result suggests that we may need to question that choice.

            regards, tom lane

I wrote:
> Further to this ... I was just doing some measurements to see how much
> it'd add to backend startup time if we start using pg_strong_random()
> to set the initial random seed.  The answer, at least on my slightly
> long-in-the-tooth RHEL6 box, is "about 25 usec using /dev/urandom,
> or about 80 usec using OpenSSL".  So I'm wondering why configure is
> coded to prefer OpenSSL.
> I'm going to go do some timing checks on some other platforms, but
> this result suggests that we may need to question that choice.

Further testing (on Fedora, macOS, FreeBSD, and NetBSD) has confirmed
that the OpenSSL code path is 2x to 3x slower than the /dev/urandom
code path for fetching half a dozen random bytes.  So I'm still
wondering why the current preference order.  My mental model of
this is that on platforms with /dev/*random, OpenSSL's RAND_bytes
isn't doing much more than wrapping /dev/*random --- so is it
really doing anything we need?

            regards, tom lane

On Fri, Dec 28, 2018 at 03:27:58PM +0200, Heikki Linnakangas wrote:
> +1 for just ripping it out, nevertheless. If someone needs libpq on
> an ancient system, they can build an older version of libpq as a
> last resort.

Okay, let's do the cleanup then.  I am just going to create a thread
on the matter.
--
Michael