> On 21 Jul 2020, at 22:00, David Steele <da...@pgmasters.net> wrote: > > On 7/21/20 3:44 PM, Daniel Gustafsson wrote: >>> On 21 Jul 2020, at 17:31, David Steele <da...@pgmasters.net> wrote: >>> On 7/21/20 8:13 AM, Daniel Gustafsson wrote: >>>> 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. >> There is that, but I'm not convinced that relying on specific timing for >> anything RNG or similarly cryptographic-related is especially sane. > > I wasn't thinking specific timing -- just that the caller might be expecting > it to give up quickly if it doesn't work. That's what the code is trying to > do and I wonder if there is a reason for it.
I think the original intention was to handle older OpenSSL versions where multiple successful RAND_poll calls were required for RAND_status to succeed, the check working as an optimization since a failing RAND_poll would render all efforts useless anyway. I'm not sure this is true for the OpenSSL versions we support in HEAD, and/or for modern platforms, but without proof of it not being useful I would opt for keeping it. cheers ./daniel
