On 06.04.24 19:47, Daniel Gustafsson wrote:
> In bumping we want to move to 1.1.1 since that's the first version with the
> rewritten RNG which is fork-safe by design, something PostgreSQL clearly
> benefits from.
I think it might be better to separate this into two steps:
1. Move to 1.1.0. This is an API update. Change OPENSSL_API_COMPAT,
and remove a bunch of code that no longer needs to be conditional. We
could check for a representative function like OPENSSL_init_ssl() in
configure/meson, or we could just let the compilation fail with older
versions.
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.
I don't know how LibreSSL interacts with either of these two points.
That's something that could be clearer.
Some more detailed review on the v6 patch:
* doc/src/sgml/libpq.sgml
This small documentation patch could be committed forthwith.
* src/backend/libpq/be-secure-openssl.c
+#include <openssl/bn.h>
This patch doesn't appear to add anything, so why does it need a new
include?
Could the additions of SSL_OP_NO_CLIENT_RENEGOTIATION and
SSL_R_VERSION_TOO_LOW be separate patches?
* src/common/hmac_openssl.c
There appears to be some unrelated refactoring happening here?
* src/include/common/openssl.h
Is the comment no longer applicable to OpenSSL, only to LibreSSL?
* 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?