On Tue, Nov 3, 2020 at 1:00 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>
> > On 3 Nov 2020, at 11:35, Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Tue, Nov 03, 2020 at 10:15:48AM +0100, Magnus Hagander wrote:
> >> On Wed, Aug 26, 2020 at 2:19 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> >>> That's certainly true. The intention though is to make the code easier to
> >>> follow (more explicit/discoverable) for anyone trying to implement support for
> >>
> >> Is it really a reasonable usecase to use RAND_bytes() outside of both
> >> pg_stroing_random() *and' outside of the openssl-specific files (like
> >> be-secure-openssl.c)? Because it would only be those cases that would
> >> have this case, right?
> >
> > It does not sound that strange to me to assume if some out-of-core
> > code makes use of that to fetch a random set of bytes. Now I don't
> > know of any code doing that. Who knows.
>
> Even if there are, I doubt such codepaths will be stumped by using
> USE_OPENSSL_RANDOM for pg_strong_random code as opposed to USE_OPENSSL.
>
> >> If anything, perhaps the call to RAND_poll() in fork_process.c should
> >> actually be a call to a strong_random_initialize() or something which
> >> would have an implementation in pg_strong_random.c, thereby isolating
> >> the openssl specific code in there? (And with a void implementation
> >> without openssl)
> >
> > I don't think that we have any need to go to such extent just for this
> > case, as RAND_poll() after forking a process is irrelevant in 1.1.1.
> > We are still many years away from removing its support though.
>
> Agreed, I doubt we'll be able to retire our <1.1.1 suppport any time soon.
>
> > No idea if other SSL implementations would require such a thing.
> > Daniel, what about NSS?
>
> PK11_GenerateRandom in NSS requires an NSSContext to be set up after fork,
> which could be performed in such an _initialize function. The PRNG in NSPR has
> a similar requirement (which may be the one the NSS patch end up using, not
> sure yet).
>
> I kind of like the idea of continuing to abstract this functionality, not
> pulling in OpenSSL headers in fork_process.c is a neat bonus. I'd say it's
> worth implementing to see what it would imply, and am happy to do unless
> someone beats me to it.
Yeah, if it's likely to be usable in the other implementations, then I
think we should definitely explore exactly what that kind of an
abstraction would imply. Anything isolating the dependency on OpenSSL
would likely have to be done at that time anyway in that case, so
better have it ready.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/