OpenSSL 3.0.0 compatibility

On Fri, 2020-05-29 at 00:16 +0200, Daniel Gustafsson wrote:

Since OpenSSL is now releasing 3.0.0-alpha versions, I took a look at using it
with postgres to see what awaits us. As it is now shipping in releases (with
GA planned for Q4), users will probably soon start to test against it so I
wanted to be prepared.

Regarding the deprecations, we can either set preprocessor directives or use
compiler flags to silence the warning and do nothing (for now), or we could
update to the new API. We probably want to different things for master vs
back-branches, but as an illustration of what the latter could look like I've
implemented this in 0001.

An important question will be: if we convert to functions that are not deprecated,
what is the earliest OpenSSL version we can support?

Yours,
Laurenz Albe

On 29 May 2020, at 08:06, Laurenz Albe <laurenz.albe@cybertec.at> wrote:

Regarding the deprecations, we can either set preprocessor directives or use
compiler flags to silence the warning and do nothing (for now), or we could
update to the new API. We probably want to different things for master vs
back-branches, but as an illustration of what the latter could look like I've
implemented this in 0001.

An important question will be: if we convert to functions that are not deprecated,
what is the earliest OpenSSL version we can support?

The replacement functions for _locations calls are introduced together with the
deprecation in 3.0.0, so there is no overlap.

For pgcrypto, that remains to be seen once it attempted, but ideally all the
way down to 1.0.1.

cheers ./daniel

On 2020-05-29 00:16, Daniel Gustafsson wrote:

Regarding the deprecations, we can either set preprocessor directives or use
compiler flags to silence the warning and do nothing (for now), or we could
update to the new API. We probably want to different things for master vs
back-branches, but as an illustration of what the latter could look like I've
implemented this in 0001.

I think we should set OPENSSL_API_COMPAT=10001, and move that along with
whatever our oldest supported release is going forward. That declares
our intention, it will silence the deprecation warnings, and IIUC, if
the deprecated stuff actually gets removed, you get a clean compiler
error that your API level is too low.

OpenSSL also deprecates DES keys in 3.0.0, which cause our password callback
tests to fail with the cryptic error "fetch failed", as the test suite keys are
encrypted with DES. 0002 fixes this by changing to AES256 (randomly chosen
among the ciphers supported in 1.0.1+ and likely to be around), and could be
applied already today as there is nothing 3.0.0 specific about it.

It appears you can load a "legacy provider" to support these keys. What
if someone made a key using 0.9.* with an older PostgreSQL version and
keeps using the same key? I'm wondering about the implications in
practice here. Obviously moving off legacy crypto is good in general.

There is also the question of what to do with the test suites in the
back branches.

Maybe we will want some user-exposed option about which providers to
load, since that also affects whether the FIPS provider gets loaded.
What's the plan there?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 29 May 2020, at 13:34, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

On 2020-05-29 00:16, Daniel Gustafsson wrote:

Regarding the deprecations, we can either set preprocessor directives or use
compiler flags to silence the warning and do nothing (for now), or we could
update to the new API. We probably want to different things for master vs
back-branches, but as an illustration of what the latter could look like I've
implemented this in 0001.

I think we should set OPENSSL_API_COMPAT=10001, and move that along with whatever our oldest supported release is going forward. That declares our intention, it will silence the deprecation warnings, and IIUC, if the deprecated stuff actually gets removed, you get a clean compiler error that your API level is too low.

I think I know what you mean but just to clarify: I master, back-branches or
all of the above?

Considering how little effort it is to not use the deprecated API's I'm not
entirely convinced, but I don't have too strong opinions there.

OpenSSL also deprecates DES keys in 3.0.0, which cause our password callback
tests to fail with the cryptic error "fetch failed", as the test suite keys are
encrypted with DES. 0002 fixes this by changing to AES256 (randomly chosen
among the ciphers supported in 1.0.1+ and likely to be around), and could be
applied already today as there is nothing 3.0.0 specific about it.

It appears you can load a "legacy provider" to support these keys. What if someone made a key using 0.9.* with an older PostgreSQL version and keeps using the same key? I'm wondering about the implications in practice here. Obviously moving off legacy crypto is good in general.

If they do, then that key will stop working with any OpenSSL 3 enabled software
unless the legacy provider has been loaded. My understanding is that users can
load these in openssl.conf, so maybe it's mostly a documentation patch for us?

Iff key loading fails with an errormessage that indicates that the algorithm
couldn't be fetched (ie fetch failed), we could add an errhint on algorithm
use. Currently it's easy to believe that it's the key file that couldn't be
fetched, as the error message from OpenSSL is quite cryptic and expects the
user to understand OpenSSL terminology. This could happen already in pre-3
versions, so maybe it's worthwhile to do?

There is also the question of what to do with the test suites in the back branches.

If we don't want to change the testdata in the backbranches, we could add a
SKIP section for the password key tests iff OpenSSL is 3.0.0+?

Maybe we will want some user-exposed option about which providers to load, since that also affects whether the FIPS provider gets loaded. What's the plan there?

This again boils down to if we want to load providers, or if we want to punt
that to openssl.conf completely. Since we will support olders versions for a
long time still, maybe punting will render the cleanest code?

AFAICT, if care is taken with openssl.conf one could already load providers
such that postgres will operate in FIPS mode due to nothing non-FIPS being
available. For libpq I'm not sure if there is anything to do, as we don't
mandate any cipher use (SSL_CTX_set_cipher_list will simply fail IIUC). For
pgcrypto however it's another story, but there we'd need to rewrite it to not
use low-level APIs first since the use of those aren't FIPS compliant. Once
done, we could have an option for FIPS mode and pass the "fips=yes" property
when loading ciphers to make sure the right version is loaded if multiple ones
are available.

cheers ./daniel

On 2020-05-29 14:45, Daniel Gustafsson wrote:

I think we should set OPENSSL_API_COMPAT=10001, and move that along with whatever our oldest supported release is going forward. That declares our intention, it will silence the deprecation warnings, and IIUC, if the deprecated stuff actually gets removed, you get a clean compiler error that your API level is too low.

I think I know what you mean but just to clarify: I master, back-branches or
all of the above?

I'm not sure. I don't have a good sense of what OpenSSL versions we
claim to support in branches older than PG13. We made a conscious
decision for 1.0.1 in PG13, but I seem to recall that that discussion
also revealed that the version assumptions before that were quite
inconsistent. Code in PG12 and before makes references to OpenSSL as
old as 0.9.6. But OpenSSL 3.0.0 will reject a compat level older than
0.9.8.

My proposal would be to introduce OPENSSL_API_COMPAT=10001 into master
after the 13/14 branching, along with any other changes to make it
compile cleanly against OpenSSL 3.0.0. Once that has survived some
scrutiny from the buildfarm and also from folks building against
LibreSSL etc., it should probably be backpatched into PG13. In the
immediate future, I wouldn't bother about the older branches (<=PG12) at
all. As long as they still compile, users can just disable deprecation
warnings, and we may add some patches to that effect at some point, but
it's not like OpenSSL 3.0.0 will be adopted into production builds any
time soon.

Considering how little effort it is to not use the deprecated API's I'm not
entirely convinced, but I don't have too strong opinions there.

Well, in the case like X509_STORE_load_locations(), the solution is in
either case to write a wrapper. It doesn't matter if we write the
wrapper or OpenSSL writes the wrapper. Only OpenSSL has already written
the wrapper and has created a well-defined way to declare that you want
to use the wrapper, so I'd just take that.

In any case, using OPENSSL_API_COMPAT is also good just for our own
documentation, so we can keep track of what version we claim to support
in different branches.

If they do, then that key will stop working with any OpenSSL 3 enabled software
unless the legacy provider has been loaded. My understanding is that users can
load these in openssl.conf, so maybe it's mostly a documentation patch for us?

Yes, it looks like that should work, so no additional work required from us.

There is also the question of what to do with the test suites in the back branches.

If we don't want to change the testdata in the backbranches, we could add a
SKIP section for the password key tests iff OpenSSL is 3.0.0+?

I suggest to update the test data in PG13+, since we require OpenSSL
1.0.1 there. For the older branches, I would look into changing the
test driver setup so that it loads a custom openssl.cnf that loads the
legacy providers.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 5/28/20 6:16 PM, Daniel Gustafsson wrote:

OpenSSL also deprecates DES keys in 3.0.0, which cause our password callback
tests to fail with the cryptic error "fetch failed", as the test suite keys are
encrypted with DES. 0002 fixes this by changing to AES256 (randomly chosen
among the ciphers supported in 1.0.1+ and likely to be around), and could be
applied already today as there is nothing 3.0.0 specific about it.

+1 for applying this forthwith. The key in my recent commit 896fcdb230
is encrypted with AES256.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Sat, May 30, 2020 at 11:29:11AM +0200, Peter Eisentraut wrote:

I'm not sure. I don't have a good sense of what OpenSSL versions we claim
to support in branches older than PG13. We made a conscious decision for
1.0.1 in PG13, but I seem to recall that that discussion also revealed that
the version assumptions before that were quite inconsistent. Code in PG12
and before makes references to OpenSSL as old as 0.9.6. But OpenSSL 3.0.0
will reject a compat level older than 0.9.8.

593d4e4 claims that we only support OpenSSL >= 0.9.8, meaning that
down to PG 10 we have this requirement, and that PG 9.6 and 9.5 should
be able to work with OpenSSL 0.9.7 and 0.9.6, but little effort has
been put in testing these.

My proposal would be to introduce OPENSSL_API_COMPAT=10001 into master after
the 13/14 branching, along with any other changes to make it compile cleanly
against OpenSSL 3.0.0. Once that has survived some scrutiny from the
buildfarm and also from folks building against LibreSSL etc., it should
probably be backpatched into PG13. In the immediate future, I wouldn't
bother about the older branches (<=PG12) at all. As long as they still
compile, users can just disable deprecation warnings, and we may add some
patches to that effect at some point, but it's not like OpenSSL 3.0.0 will
be adopted into production builds any time soon.

Please note that I actually may have to bother about 12 and OpenSSL
3.0.0 as 1.0.2 deprecation is visibly accelerating a move to newer
versions at least in my close neighborhood. We are not there yet of
course, so doing this work with 14 and 13 sounds fine to me for now.
--
Michael

On 2020-05-30 14:34, Andrew Dunstan wrote:

On 5/28/20 6:16 PM, Daniel Gustafsson wrote:

OpenSSL also deprecates DES keys in 3.0.0, which cause our password callback
tests to fail with the cryptic error "fetch failed", as the test suite keys are
encrypted with DES. 0002 fixes this by changing to AES256 (randomly chosen
among the ciphers supported in 1.0.1+ and likely to be around), and could be
applied already today as there is nothing 3.0.0 specific about it.

+1 for applying this forthwith. The key in my recent commit 896fcdb230
is encrypted with AES256.

I don't see anything in that commit about how to regenerate those files,
such as a makefile rule. Is that missing?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2020-05-31 04:52, Michael Paquier wrote:

593d4e4 claims that we only support OpenSSL >= 0.9.8, meaning that
down to PG 10 we have this requirement, and that PG 9.6 and 9.5 should
be able to work with OpenSSL 0.9.7 and 0.9.6, but little effort has
been put in testing these.

Then we can stick a OPENSSL_API_COMPAT=908 into at least PG10, 11, and
12, and probably also into PG9.5 and 9.6 without harm.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 30 May 2020, at 11:29, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

On 2020-05-29 14:45, Daniel Gustafsson wrote:

I think we should set OPENSSL_API_COMPAT=10001, and move that along with whatever our oldest supported release is going forward. That declares our intention, it will silence the deprecation warnings, and IIUC, if the deprecated stuff actually gets removed, you get a clean compiler error that your API level is too low.

I think I know what you mean but just to clarify: I master, back-branches or
all of the above?

I'm not sure. I don't have a good sense of what OpenSSL versions we claim to support in branches older than PG13. We made a conscious decision for 1.0.1 in PG13, but I seem to recall that that discussion also revealed that the version assumptions before that were quite inconsistent. Code in PG12 and before makes references to OpenSSL as old as 0.9.6. But OpenSSL 3.0.0 will reject a compat level older than 0.9.8.

My proposal would be to introduce OPENSSL_API_COMPAT=10001 into master after the 13/14 branching, along with any other changes to make it compile cleanly against OpenSSL 3.0.0. Once that has survived some scrutiny from the buildfarm and also from folks building against LibreSSL etc., it should probably be backpatched into PG13. In the immediate future, I wouldn't bother about the older branches (<=PG12) at all. As long as they still compile, users can just disable deprecation warnings, and we may add some patches to that effect at some point, but it's not like OpenSSL 3.0.0 will be adopted into production builds any time soon.

Considering how little effort it is to not use the deprecated API's I'm not
entirely convinced, but I don't have too strong opinions there.

Well, in the case like X509_STORE_load_locations(), the solution is in either case to write a wrapper. It doesn't matter if we write the wrapper or OpenSSL writes the wrapper. Only OpenSSL has already written the wrapper and has created a well-defined way to declare that you want to use the wrapper, so I'd just take that.

I'll buy that argument.

In any case, using OPENSSL_API_COMPAT is also good just for our own documentation, so we can keep track of what version we claim to support in different branches.

Good point.

There is also the question of what to do with the test suites in the back branches.

If we don't want to change the testdata in the backbranches, we could add a
SKIP section for the password key tests iff OpenSSL is 3.0.0+?

I suggest to update the test data in PG13+, since we require OpenSSL 1.0.1 there. For the older branches, I would look into changing the test driver setup so that it loads a custom openssl.cnf that loads the legacy providers.

Ok, I'll roll a patch along these lines for master for ~ the 13/14 branch time
and then we'll see how we deal with PG13 once the dust has settled not only on
our side but for OpenSSL.

cheers ./daniel

On 6/1/20 4:33 AM, Peter Eisentraut wrote:

On 2020-05-30 14:34, Andrew Dunstan wrote:

On 5/28/20 6:16 PM, Daniel Gustafsson wrote:

OpenSSL also deprecates DES keys in 3.0.0, which cause our password
callback
tests to fail with the cryptic error "fetch failed", as the test
suite keys are
encrypted with DES.� 0002 fixes this by changing to AES256 (randomly
chosen
among the ciphers supported in 1.0.1+ and likely to be around), and
could be
applied already today as there is nothing 3.0.0 specific about it.

+1 for applying this forthwith. The key in my recent commit 896fcdb230
is encrypted with AES256.

I don't see anything in that commit about how to regenerate those
files, such as a makefile rule.� Is that missing?

You missed these comments in the test file:

# self-signed cert was generated like this:
# system('openssl req -new -x509 -days 10000 -nodes -out server.crt
-keyout server.ckey -subj "/CN=localhost"');
# add the cleartext passphrase to the key, remove the unprotected key
# system("openssl rsa -aes256 -in server.ckey -out server.key -passout
pass:$clearpass");
# unlink "server.ckey";

If you want I can add a rule for it to the Makefile, although who knows
what commands will actually apply when the certificate runs out?

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 1 Jun 2020, at 13:58, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:

If you want I can add a rule for it to the Makefile, although who knows
what commands will actually apply when the certificate runs out?

Being able to easily regenerate the testdata, regardless of expiration status,
has proven very helpful for me when implementing support for new TLS backends.
+1 for adding it to the Makefile.

cheers ./daniel

On 6/1/20 8:03 AM, Daniel Gustafsson wrote:

On 1 Jun 2020, at 13:58, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
If you want I can add a rule for it to the Makefile, although who knows
what commands will actually apply when the certificate runs out?

Being able to easily regenerate the testdata, regardless of expiration status,
has proven very helpful for me when implementing support for new TLS backends.
+1 for adding it to the Makefile.

OK, here's a patch.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:

On 6/1/20 8:03 AM, Daniel Gustafsson wrote:

+1 for adding it to the Makefile.

OK, here's a patch.

Likewise +1 for having it in the makefile. But now you have two copies,
the other being in comments in the test script. The latter should go
away, as we surely won't remember to maintain it. (You could replace it
with a pointer to the makefile rules if you want.)

regards, tom lane

On 2020-06-01 15:23, Andrew Dunstan wrote:

On 6/1/20 8:03 AM, Daniel Gustafsson wrote:

On 1 Jun 2020, at 13:58, Andrew Dunstan <andrew.dunstan@2ndquadrant.com> wrote:
If you want I can add a rule for it to the Makefile, although who knows
what commands will actually apply when the certificate runs out?

Being able to easily regenerate the testdata, regardless of expiration status,
has proven very helpful for me when implementing support for new TLS backends.
+1 for adding it to the Makefile.

OK, here's a patch.

In src/test/ssl/ we have targets sslfiles and sslfiles-clean, and here
we have ssl-files and ssl-files-clean. Let's keep that consistent.

Or, why not actually use the generated files from src/test/ssl/ instead
of making another set?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 6/2/20 4:57 AM, Peter Eisentraut wrote:

On 2020-06-01 15:23, Andrew Dunstan wrote:

On 6/1/20 8:03 AM, Daniel Gustafsson wrote:

On 1 Jun 2020, at 13:58, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
If you want I can add a rule for it to the Makefile, although who
knows
what commands will actually apply when the certificate runs out?

Being able to easily regenerate the testdata, regardless of
expiration status,
has proven very helpful for me when implementing support for new TLS
backends.
+1 for adding it to the Makefile.

OK, here's a patch.

In src/test/ssl/ we have targets sslfiles and sslfiles-clean, and here
we have ssl-files and ssl-files-clean.  Let's keep that consistent.

Or, why not actually use the generated files from src/test/ssl/
instead of making another set?

Honestly, I think we've spent plenty of time on this already. I don't
see a problem with each module having its own certificate(s) - that
makes them more self-contained -  nor any great need to have the targets
named the same.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Jun 02, 2020 at 02:45:11PM -0400, Andrew Dunstan wrote:

Honestly, I think we've spent plenty of time on this already. I don't
see a problem with each module having its own certificate(s) - that
makes them more self-contained -  nor any great need to have the targets
named the same.

Yeah, I don't see much point in combining both of them as those
modules have different assumptions behind the files built. Now I
agree with Peter's point to use the same Makefile rule names in both
files so as it gets easier to grep for all instances.

So, src/test/ssl/ being the oldest one, ssl_passphrase_callback should
just do s/ssl-files/sslfiles/.
--
Michael

On Mon, Jun 01, 2020 at 10:44:17AM +0200, Peter Eisentraut wrote:

Then we can stick a OPENSSL_API_COMPAT=908 into at least PG10, 11, and 12,
and probably also into PG9.5 and 9.6 without harm.

FWIW, I am fine that for PG >= 10, and I don't think that it is worth
bothering with PG <= 9.6.
--
Michael

On 2020-05-30 11:29, Peter Eisentraut wrote:

I suggest to update the test data in PG13+, since we require OpenSSL
1.0.1 there. For the older branches, I would look into changing the
test driver setup so that it loads a custom openssl.cnf that loads the
legacy providers.

I have pushed your 0002 patch (the one that updates the test data) to
master.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services