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
signature.asc
Description: PGP signature