On 7/21/20 8:13 AM, Daniel Gustafsson wrote:
> After forking we call RAND_cleanup in fork_process.c to force a re-seed to
> ensure that two backends cannot share sequence. OpenSSL 1.1.0 deprecated
> RAND_cleanup, and contrary to how they usually leave deprecated APIs working
> until removed, they decided to silently make this call a noop like below:
>
> # define RAND_cleanup() while(0) continue
>
> This leaves our defence against pool sharing seemingly useless, and also
> against the recommendations of OpenSSL for versions > 1.1.0 and < 1.1.1 where
> the RNG was rewritten:
>
> https://wiki.openssl.org/index.php/Random_fork-safety
>
> 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).
This looks reasonable to me based on your explanation and the OpenSSL wiki.
> 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.
I wonder how effective the retries are going to be if they happen
immediately. However, most of the code paths I followed ended in a hard
error when pg_strong_random() failed so it may not hurt to try. I just
worry that some caller is depending on a faster failure here.
Regards,
--
-David
david@pgmasters.net