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(at)eisentraut(dot)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