> On 22 Jul 2020, at 07:00, Noah Misch <noah@leadboat.com> wrote:
>
> On Tue, Jul 21, 2020 at 02:13:32PM +0200, Daniel Gustafsson wrote:
>> The silver lining here is that while OpenSSL nooped RAND_cleanup, they also
>> changed what is mixed into seeding so we are still not sharing a sequence. To
>> fix this, changing the RAND_cleanup call to RAND_poll should be enough to
>> ensure re-seeding after forking across all supported OpenSSL versions. Patch
>> 0001 implements this along with a comment referencing when it can be removed
>> (which most likely won't be for quite some time).
>>
>> Another thing that stood out when reviewing this code is that we optimize for
>> RAND_poll failing in pg_strong_random, when we already have RAND_status
>> checking for a sufficiently seeded RNG for us. ISTM that we can simplify the
>> code by letting RAND_status do the work as per 0002, and also (while unlikely)
>> survive any transient failures in RAND_poll by allowing all the retries we've
>> defined for the loop.
>
>> Thoughts on these?
>
> These look good. I'll push them on Saturday or later.
Thanks for picking it up!
> I wondered whether to
> do both RAND_cleanup() and RAND_poll(), to purge all traces of the old seed on
> versions supporting both. Since that would strictly (albeit negligibly)
> increase seed predictability, I like your decision here.
That's a good question. I believe that if one actually do use RAND_cleanup as
a re-seeding mechanism then that can break FIPS enabled OpenSSL installations
as RAND_cleanup resets the RNG method from the FIPS RNG to the built-in one. I
would be inclined to follow the upstream recommendations of using RAND_poll
exclusively, but I'm far from an expert here.
> Do you happen to know how OpenSSL 1.1.1 changed its reaction to forks in order
> to make the RAND_poll() superfluous? (No need to research it if you don't.)
I'm not entirely sure, but I do believe that 1.1.1 ported over the RNG from the
FIPS module which re-seeds itself with fork() protection. There was however a
bug, fixed in 1.1.1d or thereabouts as CVE-2019-1549, where the fork protection
wasn't activated by default.. so there is that. Since that bug was found,
there has been tests introduced to catch any regression on that which is
comforting.
cheers ./daniel
