Poor buildfarm coverage of strong-random alternatives

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