Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?

On Fri, Apr 12, 2024 at 02:42:57PM +0200, Daniel Gustafsson wrote:
>> On 10 Apr 2024, at 09:31, Peter Eisentraut <peter@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