Re: Move OpenSSL random under USE_OPENSSL_RANDOM - Mailing list pgsql-hackers

> On 16 Nov 2020, at 01:20, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sun, Nov 15, 2020 at 12:16:56PM -0500, Tom Lane wrote:
>> The obvious problem with this is that if !USE_OPENSSL, we will not have
>> pulled in openssl's headers.
>
> FWIW, I argued upthread against including this part because it is
> useless: if not building with OpenSSL, we'll never have the base to be
> able to use RAND_poll().

How do you mean?  The OpenSSL PRNG can be used without setting up a context
etc, the code in pg_strong_random is all we need to use it without USE_OPENSSL
(whether we'd like to is another story) or am I missing something?

>> However ... all these machines are pointing at line 96, which is not
>> that one but the one under "#if defined(USE_OPENSSL)".  So I'm not sure
>> what to make of that, except that a bit more finesse seems required.
>
> The build scripts of src/tools/msvc/ choose to not use OpenSSL as
> strong random source even if building with OpenSSL.  The top of the
> file only includes openssl/rand.h if using USE_OPENSSL_RANDOM.

The fallout here is precisely the reason why I argued for belts and suspenders
such that PRNG init is performed for (USE_OPENSSL || USE_OPENSSL_RANDOM).  I
don't trust the assumption that if one is there other will always be there as
well as long as they are disjoint.  Since we expose this PRNG to users, there
is a vector for spooling the rand state via UUID generation in case the PRNG is
faulty and have predictability, so failing to protect the after-fork case can
be expensive.  Granted, such vulnerabilities are rare but not inconcievable.

Now, this patch didn't get the header inclusion right which is why thise broke.

> Thinking about that afresh, I think that we got that wrong here on
> three points:
> - If attempting to use OpenSSL on Windows, let's just bite the bullet
> and use OpenSSL as random source, using Windows as source only when
> not building with OpenSSL.
> - Instead of using a call to RAND_poll() that we know will never work,
> let's just issue a compilation failure if attempting to use
> USE_OPENSSL_RANDOM without USE_OPENSSL.

Taking a step back, what is the usecase of USE_OPENSSL_RANDOM if we force it to
be equal to USE_OPENSSL?  Shouldn't we in that case just have USE_OPENSSL,
adjust the logic and remove the below comment from configure.ac which isn't
really telling the truth? 

  # Select random number source
  #
  # You can override this logic by setting the appropriate USE_*RANDOM flag to 1
  # in the template or configure command line.

I might be thick but I'm struggling to see the use for complications when we
don't support any pluggability.  Having said that, it might be the sane way in
the end to forcibly use the TLS library as a randomness source should there be
one (FIPS compliance comes to mind), but then we might as well spell that out.

> - rand.h needs to be included under USE_OPENSSL.


It needs to be included for both USE_OPENSSL and USE_OPENSSL_RANDOM unless we
combine them as per the above.