public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Eisentraut <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Postgres hackers <[email protected]>
Cc: [email protected]
Cc: Tom Lane <[email protected]>
Subject: Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
Date: Wed, 10 Apr 2024 09:31:16 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+hUKGJrfU7dhxwJEG2wPDx0LFS2svB0DWmDCKzYuX2XS5f1hw@mail.gmail.com>
	<[email protected]>
	<CAOYmi+mXbMVhHNm-GAzQaEopfsquqm1-wvCA73k4xnxa2Ln6yA@mail.gmail.com>
	<[email protected]>
	<CAOYmi+nh=o=tGjpDYsqV8tR6f2Qba4KNE5nvxWZw1ErW5izoZw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAOYmi+=KOxuj3z91ijwya6ycdBhhc7LDBathdFVy3y8rf=Qi_Q@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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?

view thread (60+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Cutting support for OpenSSL 1.0.1 and 1.0.2 in 17~?
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox