On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:
>> On 10 Apr 2024, at 09:31, Peter Eisentraut <pe...@eisentraut.org> wrote:
>> 2. Move to 1.1.1.  I understand this has to do with the fork-safety of 
>> pg_strong_random(), and it's not an API change but a behavior change. Let's 
>> make this association clearer in the code.  For example, add a version check 
>> or assertion about this into pg_strong_random() itself.

> 0005 moves the fork safety init inline with calling pg_strong_random, and
> removes it for everyone else.  This allows 1.1.0 to be supported as we
> effectively are at the 1.1.0 API level, at the cost of calls for strong random
> being slower on 1.1.0.  An unscientific guess based on packaged OpenSSL
> versions and the EOL and ELS/LTS status of 1.1.0, is that the number of
> production installs of postgres 17 using OpenSSL 1.1.0 is close to zero.

It is only necessary to call RAND_poll once after forking.  Wouldn't
it be OK to use a static flag and use the initialization once?

>> * src/port/pg_strong_random.c
>> 
>> I would prefer to remove pg_strong_random_init() if it's no longer
>> useful.  I mean, if we leave it as is, and we are not removing any
>> callers, then we are effectively continuing to support OpenSSL
>> <1.1.1, right?
> 
> The attached removes pg_strong_random_init and instead calls it explicitly for
> 1.1.0 users by checking the OpenSSL version.
> 
> Is the attached split in line with how you were thinking about it?

If I may, 0001 looks sensible here.  The bits from 0003 and 0004 could
be applied before 0002, as well.

--- a/src/backend/postmaster/fork_process.c
+++ b/src/backend/postmaster/fork_process.c
@@ -110,9 +110,6 @@ fork_process(void)
                                close(fd);
                        }
                }
-
-               /* do post-fork initialization for random number generation */
-               pg_strong_random_init();

Perhaps you intented this diff to be in 0005 rather than in 0002?
With 0002 applied, only support for 1.0.2 is removed, not 1.1.0 yet.

 pg_strong_random(void *buf, size_t len)
 {
        int                     i;
 
+#if (OPENSSL_VERSION_NUMBER <= 0x10100000L)
+       /*
+        * Make sure processes do not share OpenSSL randomness state.  This is 
not
+        * requred on LibreSSL and no longer required in OpenSSL 1.1.1 and later
+        * versions.
+        */
+       RAND_poll();
+#endif

s/requred/required/.  Rather than calling always RAND_poll(), this
could use a static flag to call it only once when pg_strong_random is
called for the first time.  I would not mind seeing this part entirely
gone with the removal of support for 1.1.0.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to