Update minimum SSL version

Started by Peter Eisentrautabout 6 years ago38 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

I propose to change the default of ssl_min_protocol_version to TLSv1.2
(from TLSv1, which means 1.0). Older versions would still be supported,
just not by default.

The reason is that TLS 1.0 and 1.1 are either already discouraged or
deprecated or will be by the time PostgreSQL 13 comes out. So this move
would be in the direction of "secure by default". Specifically, PCI DSS
disallows the use of TLS 1.0 and discourages 1.1 [0]https://blog.pcisecuritystandards.org/are-you-ready-for-30-june-2018-sayin-goodbye-to-ssl-early-tls, and browser
vendors are set to disable 1.0 and 1.1 in their products sometime soon [1]https://arstechnica.com/gadgets/2018/10/browser-vendors-unite-to-end-support-for-20-year-old-tls-1-0/.

Using TLS 1.2 requires OpenSSL 1.0.1, released in 2012. I find this to
be satisfied in CentOS 6 and Debian jessie (oldoldstable), for example.

More details also in my recent blog post [2]https://www.2ndquadrant.com/en/blog/setting-ssl-tls-protocol-versions-with-postgresql-12/.

[0]: https://blog.pcisecuritystandards.org/are-you-ready-for-30-june-2018-sayin-goodbye-to-ssl-early-tls
https://blog.pcisecuritystandards.org/are-you-ready-for-30-june-2018-sayin-goodbye-to-ssl-early-tls
[1]: https://arstechnica.com/gadgets/2018/10/browser-vendors-unite-to-end-support-for-20-year-old-tls-1-0/
https://arstechnica.com/gadgets/2018/10/browser-vendors-unite-to-end-support-for-20-year-old-tls-1-0/
[2]: https://www.2ndquadrant.com/en/blog/setting-ssl-tls-protocol-versions-with-postgresql-12/
https://www.2ndquadrant.com/en/blog/setting-ssl-tls-protocol-versions-with-postgresql-12/

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

Attachments:

0001-Update-minimum-SSL-version.patchtext/plain; charset=UTF-8; name=0001-Update-minimum-SSL-version.patch; x-mac-creator=0; x-mac-type=0Download
From e5d71a3048a8eb17d68ef34caac05ce0bc0b156e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 29 Nov 2019 08:08:03 +0100
Subject: [PATCH] Update minimum SSL version

Change default of ssl_min_protocol_version to TLSv1.2 (from TLSv1,
which means 1.0).  Older versions are still supported, just not by
default.

TLS 1.0 is widely deprecated, and TLS 1.1 only slightly less so.  All
OpenSSL versions that support TLS 1.1 also support TLS 1.2, so there
would be very little reason to, say, set the default to TLS 1.1
instead on grounds of better compatibility.
---
 doc/src/sgml/config.sgml                      | 6 ++----
 src/backend/utils/misc/guc.c                  | 2 +-
 src/backend/utils/misc/postgresql.conf.sample | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d4d1fe45cc..901744958c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1365,10 +1365,8 @@ <title>SSL</title>
        </para>
 
        <para>
-        The default is <literal>TLSv1</literal>, mainly to support older
-        versions of the <productname>OpenSSL</productname> library.  You might
-        want to set this to a higher value if all software components can
-        support the newer protocol versions.
+        The default is <literal>TLSv1.2</literal>, which satisfies industry
+        best practices as of this writing.
        </para>
       </listitem>
      </varlistentry>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ba4edde71a..adb277d8f2 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4573,7 +4573,7 @@ static struct config_enum ConfigureNamesEnum[] =
 			GUC_SUPERUSER_ONLY
 		},
 		&ssl_min_protocol_version,
-		PG_TLS1_VERSION,
+		PG_TLS1_2_VERSION,
 		ssl_protocol_versions_info + 1, /* don't allow PG_TLS_ANY */
 		NULL, NULL, NULL
 	},
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 46a06ffacd..9541879c1f 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -105,7 +105,7 @@
 #ssl_ciphers = 'HIGH:MEDIUM:+3DES:!aNULL' # allowed SSL ciphers
 #ssl_prefer_server_ciphers = on
 #ssl_ecdh_curve = 'prime256v1'
-#ssl_min_protocol_version = 'TLSv1'
+#ssl_min_protocol_version = 'TLSv1.2'
 #ssl_max_protocol_version = ''
 #ssl_dh_params_file = ''
 #ssl_passphrase_command = ''
-- 
2.24.0

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#1)
Re: Update minimum SSL version

On 29 Nov 2019, at 08:36, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

I propose to change the default of ssl_min_protocol_version to TLSv1.2 (from TLSv1, which means 1.0). Older versions would still be supported, just not by default.

+1 for having a sane default with a way to fall back to older versions in case
they are required.

cheers ./daniel

#3Magnus Hagander
magnus@hagander.net
In reply to: Daniel Gustafsson (#2)
Re: Update minimum SSL version

On Fri, Nov 29, 2019 at 11:10 AM Daniel Gustafsson <daniel@yesql.se> wrote:

On 29 Nov 2019, at 08:36, Peter Eisentraut <

peter.eisentraut@2ndquadrant.com> wrote:

I propose to change the default of ssl_min_protocol_version to TLSv1.2

(from TLSv1, which means 1.0). Older versions would still be supported,
just not by default.

+1 for having a sane default with a way to fall back to older versions in
case
they are required.

+1. As long as we still have support to change it down if needed, it's a
good thing to ship with a proper default.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#4Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#3)
Re: Update minimum SSL version

On Fri, Nov 29, 2019 at 01:40:48PM +0100, Magnus Hagander wrote:

+1. As long as we still have support to change it down if needed, it's a
good thing to ship with a proper default.

+1.
--
Michael
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#4)
Re: Update minimum SSL version

Michael Paquier <michael@paquier.xyz> writes:

On Fri, Nov 29, 2019 at 01:40:48PM +0100, Magnus Hagander wrote:

+1. As long as we still have support to change it down if needed, it's a
good thing to ship with a proper default.

+1.

What's the impact going to be on buildfarm members with older openssl
installations? Perhaps "none", if they aren't running the ssl test
suite, but we should be clear about it.

regards, tom lane

#6Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#5)
Re: Update minimum SSL version

On Fri, Nov 29, 2019 at 10:30:47AM -0500, Tom Lane wrote:

What's the impact going to be on buildfarm members with older openssl
installations? Perhaps "none", if they aren't running the ssl test
suite, but we should be clear about it.

The buildfarm logs don't directly report the version of OpenSSL used
as far as I recalled, and a quick lookup shows that.. Anyway, I
recall that all Windows buildfarm members linking to OpenSSL use at
least 1.0.2 on HEAD. For the others, I would be ready to suspect that
some of them are still using 0.9.8 and 1.0.0.

Anyway, as we still support OpenSSL down to 0.9.8 on HEAD, shouldn't
we just patch the SSL TAP tests to make sure that we don't enforce an
incorrect minimum version at configuration time?

[... thinks more ...]

Actually, no, what I am writing here is incorrect. We should make
sure of that the default configuration is correct at initdb time, and
the patch does not do that.
--
Michael

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#6)
Re: Update minimum SSL version

Michael Paquier <michael@paquier.xyz> writes:

On Fri, Nov 29, 2019 at 10:30:47AM -0500, Tom Lane wrote:

What's the impact going to be on buildfarm members with older openssl
installations? Perhaps "none", if they aren't running the ssl test
suite, but we should be clear about it.

Actually, no, what I am writing here is incorrect. We should make
sure of that the default configuration is correct at initdb time, and
the patch does not do that.

Yeah, that's sort of what I was getting at, but not quite. On newer
openssl versions, this doesn't seem like it's really changing anything
at all --- AFAIK, the client and server will already negotiate the
highest jointly-supported TLS version. OTOH, with an openssl version
old enough to not understand TLS >= 1.2, this change likewise won't do
anything, except break configurations that used to work (for some
not-too-secure value of "work").

I think the real question we have to answer is this: are we intent on
making people upgrade ancient openssl installations? If so, shouldn't
we be doing something even more aggressive than this? If not, wouldn't
the patch need to try to autoconfigure the minimum TLS version? As
proposed, the patch seems to be somewhere in a passive-aggressive middle
ground of being annoying without really enforcing anything. So I don't
quite see the point.

regards, tom lane

#8Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#5)
Re: Update minimum SSL version

On 2019-11-29 16:30, Tom Lane wrote:

Michael Paquier <michael@paquier.xyz> writes:

On Fri, Nov 29, 2019 at 01:40:48PM +0100, Magnus Hagander wrote:

+1. As long as we still have support to change it down if needed, it's a
good thing to ship with a proper default.

+1.

What's the impact going to be on buildfarm members with older openssl
installations? Perhaps "none", if they aren't running the ssl test
suite, but we should be clear about it.

If they aren't running the ssl tests, then none.

We could add an override of ssl_min_protocol_version in
src/test/ssl/t/SSLServer.pm so that the tests still work with very old
OpenSSL versions by default. That might be a good idea.

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

#9Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#7)
Re: Update minimum SSL version

On 2019-11-30 04:06, Tom Lane wrote:

I think the real question we have to answer is this: are we intent on
making people upgrade ancient openssl installations? If so, shouldn't
we be doing something even more aggressive than this? If not, wouldn't
the patch need to try to autoconfigure the minimum TLS version? As
proposed, the patch seems to be somewhere in a passive-aggressive middle
ground of being annoying without really enforcing anything. So I don't
quite see the point.

The trade-off is that this makes the defaults better for the vast
majority of users and gives users of really old systems a nudge that
they are no longer in compliance with industry best practices. You need
manual steps to set up SSL anyway, so this doesn't introduce an entirely
new kind of requirement for the latter group of users.

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

#10Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#6)
Re: Update minimum SSL version

On 30 Nov 2019, at 03:43, Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Nov 29, 2019 at 10:30:47AM -0500, Tom Lane wrote:

What's the impact going to be on buildfarm members with older openssl
installations? Perhaps "none", if they aren't running the ssl test
suite, but we should be clear about it.

The buildfarm logs don't directly report the version of OpenSSL used
as far as I recalled, and a quick lookup shows that..

Not explicitly, but it would be a nice if it did. Since the version depends on
the optional FIPS module, running "openssl version" is really the safe option,
which in itself is hard since the libraries pointed to with --with-libs aren't
guaranteed to have an openssl command installed etc. OpenSSL might also these
days be LibreSSL (or potentially even BoringSSL perhaps if someone twists the
arm of their installation enough).

However, looking at the signatures detected by autoconf we can however get an
idea of which version is used. SSL_clear_options and X509_get_signature_nid()
first shipped in 1.0.2, while SSL_get_current_compression first shipped in
0.9.8. There are also a set of functions which are new in 1.1.0 (BIO_get_data
et.al).

This tells us that for example alewife is likely running 1.0.2:

checking for SSL_new in -lssl... (cached) yes
checking for SSL_clear_options... (cached) no
checking for SSL_get_current_compression... (cached) yes
checking for X509_get_signature_nid... (cached) yes
checking for OPENSSL_init_ssl... (cached) no
checking for BIO_get_data... (cached) no
checking for BIO_meth_new... (cached) no
checking for ASN1_STRING_get0_data... (cached) no

(the careful observer notes that the SSL_clear_options() check fails even
though it should be in 1.0.2, and thats probably because SSL_clear_options is a
macro until 1.1.0 where it becomes a function).

gaur however looks like it is running 0.9.8:

checking for SSL_new in -lssl... yes
checking for SSL_clear_options... no
checking for SSL_get_current_compression... yes
checking for X509_get_signature_nid... no
checking for OPENSSL_init_ssl... no
checking for BIO_get_data... no
checking for BIO_meth_new... no
checking for ASN1_STRING_get0_data... no
checking for CRYPTO_lock... yes

scorpionfly running OpenBSD 6.6 configures as a LibreSSL on par with what we
expect for 1.1.0 (SSL_clear_options again fail here since it's still a macro in
LibreSSL):

checking for SSL_new in -lssl... (cached) yes
checking for SSL_clear_options... (cached) no
checking for SSL_get_current_compression... (cached) yes
checking for X509_get_signature_nid... (cached) yes
checking for OPENSSL_init_ssl... (cached) yes
checking for BIO_get_data... (cached) yes
checking for BIO_meth_new... (cached) yes
checking for ASN1_STRING_get0_data... (cached) yes
checking for CRYPTO_lock... (cached) yes

Randomly picking animals, and trying to target platforms where older versions
could be expected, I didn't see any <= 0.9.7; a small number 0.9.8 and most at
1.0.2 or higher (with the 0.9.8 animals being: gaur, sungazer and prairiedog).
This is not an exhaustive list of course, maybe someone with better access to
the buildfarm data can do some more clever analysis.

cheers ./daniel

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#10)
Re: Update minimum SSL version

Daniel Gustafsson <daniel@yesql.se> writes:

On 30 Nov 2019, at 03:43, Michael Paquier <michael@paquier.xyz> wrote:

The buildfarm logs don't directly report the version of OpenSSL used
as far as I recalled, and a quick lookup shows that..

Not explicitly, but it would be a nice if it did. Since the version depends on
the optional FIPS module, running "openssl version" is really the safe option,
which in itself is hard since the libraries pointed to with --with-libs aren't
guaranteed to have an openssl command installed etc. OpenSSL might also these
days be LibreSSL (or potentially even BoringSSL perhaps if someone twists the
arm of their installation enough).

Yeah, I do not think that would be a good solution --- it would give wrong
answers on three of my four buildfarm animals :-(, for precisely the
reason that they're using --with-libs to point to a non-system openssl
installation.

Is there a simple way to ask the library itself for version info?
It might be worth the cycles to have configure run a small test
program to extract and print that data (not on cross-compile
builds, of course).

(the careful observer notes that the SSL_clear_options() check fails even
though it should be in 1.0.2, and thats probably because SSL_clear_options is a
macro until 1.1.0 where it becomes a function).

Hmm, is it worth the trouble to fix that?

gaur however looks like it is running 0.9.8:

gaur and prairiedog are both building with 0.9.8x, as you can tell
from their --with-libs options.

Randomly picking animals, and trying to target platforms where older versions
could be expected, I didn't see any <= 0.9.7; a small number 0.9.8 and most at
1.0.2 or higher (with the 0.9.8 animals being: gaur, sungazer and prairiedog).

According to the commit log (see 593d4e47d), we require 0.9.8 or later
in v10 and up, so any older animals got upgraded or retired some time
ago.

regards, tom lane

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#9)
Re: Update minimum SSL version

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-11-30 04:06, Tom Lane wrote:

I think the real question we have to answer is this: are we intent on
making people upgrade ancient openssl installations?

The trade-off is that this makes the defaults better for the vast
majority of users and gives users of really old systems a nudge that
they are no longer in compliance with industry best practices. You need
manual steps to set up SSL anyway, so this doesn't introduce an entirely
new kind of requirement for the latter group of users.

True. I'm okay with this as long as we adapt the ssl test suite as
per your other reply.

regards, tom lane

#13Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#11)
Re: Update minimum SSL version

On 2 Dec 2019, at 15:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Is there a simple way to ask the library itself for version info?
It might be worth the cycles to have configure run a small test
program to extract and print that data (not on cross-compile
builds, of course).

Asking the lib is easy, making that fit cleanly into how autoconf does things might be trickier. I’ll take a look and will report back (on the SSL_clear_options thing as well).

cheers ./daniel

#14Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#6)
Re: Update minimum SSL version

On Fri, Nov 29, 2019 at 9:44 PM Michael Paquier <michael@paquier.xyz> wrote:

Actually, no, what I am writing here is incorrect. We should make
sure of that the default configuration is correct at initdb time, and
the patch does not do that.

I think that would be overkill. There shouldn't be many people who are
running with a version of PostgreSQL that is 8 years newer than the
version of OpenSSL they are using, and who are also relying on SSL,
and even if there are such people, it's a pretty minor configuration
change to make it work. However, it would be worth putting in some
effort to make sure that we give a good error message if this happens.
I'm not sure how practical that is. But there's a big difference
between giving an incomprehensible OpenSSL message that says "things
aren't working and good luck figuring out why" and giving a message
that says something like:

ERROR: ssl_min_protocol_version specifies TLSv1.2, but your OpenSSL
library does not support protocol versions beyond TLSv1.1

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#14)
Re: Update minimum SSL version

Robert Haas <robertmhaas@gmail.com> writes:

... However, it would be worth putting in some
effort to make sure that we give a good error message if this happens.

That's an excellent point, but it looks like we're pretty good
already. I tried the patch with openssl 0.9.8x, and got this
failure at server start:

FATAL: ssl_min_protocol_version setting TLSv1.2 not supported by this build

Maybe it'd be worth extending that to show the max supported
version, with some rats-nest of #ifdefs, but I'm not sure if
it's worth the trouble.

regards, tom lane

#16Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#15)
Re: Update minimum SSL version

On Mon, Dec 2, 2019 at 11:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

That's an excellent point, but it looks like we're pretty good
already. I tried the patch with openssl 0.9.8x, and got this
failure at server start:

FATAL: ssl_min_protocol_version setting TLSv1.2 not supported by this build

Oh, that's pretty good.

Maybe it'd be worth extending that to show the max supported
version, with some rats-nest of #ifdefs, but I'm not sure if
it's worth the trouble.

Especially if we mess up the #ifdefs. :-)

I don't have super-strong feelings that we have to try to do that. It
would be worth doing if it were easy, I think, but if our hypothesis
that this will affect relatively few people is correct, it may not
matter very much.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#16)
Re: Update minimum SSL version

Robert Haas <robertmhaas@gmail.com> writes:

On Mon, Dec 2, 2019 at 11:39 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Maybe it'd be worth extending that to show the max supported
version, with some rats-nest of #ifdefs, but I'm not sure if
it's worth the trouble.

Especially if we mess up the #ifdefs. :-)

Yah. Although, looking at the code in be-secure-openssl.c,
it doesn't look that hard to do in an extensible way.
Something like (untested)

static int
ssl_protocol_version_to_openssl(int v, const char *guc_name, int loglevel)
{
switch (v)
{
case PG_TLS_ANY:
return 0;
case PG_TLS1_VERSION:
+#define PG_MAX_TLS_VERSION "TLSv1"
return TLS1_VERSION;
case PG_TLS1_1_VERSION:
#ifdef TLS1_1_VERSION
+#undef PG_MAX_TLS_VERSION
+#define PG_MAX_TLS_VERSION "TLSv1.1"
return TLS1_1_VERSION;
#else
break;
#endif
case PG_TLS1_2_VERSION:
#ifdef TLS1_2_VERSION
+#undef PG_MAX_TLS_VERSION
+#define PG_MAX_TLS_VERSION "TLSv1.2"
return TLS1_2_VERSION;
#else
break;
#endif
case PG_TLS1_3_VERSION:
#ifdef TLS1_3_VERSION
+#undef PG_MAX_TLS_VERSION
+#define PG_MAX_TLS_VERSION "TLSv1.3"
return TLS1_3_VERSION;
#else
break;
#endif
}

     ereport(loglevel,
             (errmsg("%s setting %s not supported by this build",
                     guc_name,
-                    GetConfigOption(guc_name, false, false))));
+                    GetConfigOption(guc_name, false, false)),
+             errdetail("Maximum supported TLS version is %s.",
+                       PG_MAX_TLS_VERSION)));
     return -1;
 }

regards, tom lane

#18Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#11)
Re: Update minimum SSL version

On Mon, Dec 02, 2019 at 09:59:44AM -0500, Tom Lane wrote:

Is there a simple way to ask the library itself for version info?
It might be worth the cycles to have configure run a small test
program to extract and print that data (not on cross-compile
builds, of course).

SSLeay_version():
https://www.openssl.org/docs/man1.0.2/man3/SSLeay_version.html
--
Michael

#19Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#17)
Re: Update minimum SSL version

On Mon, Dec 02, 2019 at 12:51:26PM -0500, Tom Lane wrote:

Yah. Although, looking at the code in be-secure-openssl.c,
it doesn't look that hard to do in an extensible way.
Something like (untested)

While we are on the topic... Here is another wild idea. We discussed
not so long ago about removing support for OpenSSL 0.9.8 from the
tree. What if we removed support for 1.0.0 and 0.9.8 for 13~. This
would solve a couple of compatibility headaches, and we have TLSv1.2
support automatically for all the versions supported. Note that 1.0.0
has been retired by upstream in February 2014.
--
Michael

#20Magnus Hagander
magnus@hagander.net
In reply to: Michael Paquier (#19)
Re: Update minimum SSL version

On Tue, Dec 3, 2019 at 4:53 AM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Dec 02, 2019 at 12:51:26PM -0500, Tom Lane wrote:

Yah. Although, looking at the code in be-secure-openssl.c,
it doesn't look that hard to do in an extensible way.
Something like (untested)

While we are on the topic... Here is another wild idea. We discussed
not so long ago about removing support for OpenSSL 0.9.8 from the
tree. What if we removed support for 1.0.0 and 0.9.8 for 13~. This
would solve a couple of compatibility headaches, and we have TLSv1.2
support automatically for all the versions supported. Note that 1.0.0
has been retired by upstream in February 2014.

Is 1.0.1 considered a separate major from 1.0.0, in this reasoning? Because
while retiring 1.0.0 should probably not be that terrible, 1.0.1 is still
in very widespread use on most long term supported distributions.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#21Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#20)
Re: Update minimum SSL version

On Tue, Dec 03, 2019 at 10:10:57AM +0100, Magnus Hagander wrote:

Is 1.0.1 considered a separate major from 1.0.0, in this reasoning? Because
while retiring 1.0.0 should probably not be that terrible, 1.0.1 is still
in very widespread use on most long term supported distributions.

1.0.1 and 1.0.0 are two different major releases in the OpenSSL world,
so my suggestion would be to cut support for everything which does not
have TLSv1.2, meaning that we keep compatibility with 1.0.1 for
a longer period.
--
Michael

#22Magnus Hagander
magnus@hagander.net
In reply to: Michael Paquier (#21)
Re: Update minimum SSL version

On Tue, Dec 3, 2019 at 12:09 PM Michael Paquier <michael@paquier.xyz> wrote:

On Tue, Dec 03, 2019 at 10:10:57AM +0100, Magnus Hagander wrote:

Is 1.0.1 considered a separate major from 1.0.0, in this reasoning?

Because

while retiring 1.0.0 should probably not be that terrible, 1.0.1 is still
in very widespread use on most long term supported distributions.

1.0.1 and 1.0.0 are two different major releases in the OpenSSL world,
so my suggestion would be to cut support for everything which does not
have TLSv1.2, meaning that we keep compatibility with 1.0.1 for
a longer period.

Good, that's what I thought you meant :) And that makes it sound like a
working plan to me.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

#23Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#15)
Re: Update minimum SSL version

On 2019-12-02 17:39, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

... However, it would be worth putting in some
effort to make sure that we give a good error message if this happens.

That's an excellent point, but it looks like we're pretty good
already. I tried the patch with openssl 0.9.8x, and got this
failure at server start:

FATAL: ssl_min_protocol_version setting TLSv1.2 not supported by this build

That's the easy part, since it's under our control. The other situation
is if you connect with an old library to a newer server that has the
raised ssl_min_protocol_version setting. Then you get something like this:

psql: SSL error: tlsv1 alert protocol version

and on the server:

LOG: could not accept SSL connection: unsupported protocol

Not great, but usable.

(What actually happens due to the default of PGSSLMODE=prefer is that
psql/libpq will have the SSL connection attempt rejected and will
connect using a non-SSL connection.)

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

#24Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Magnus Hagander (#22)
Re: Update minimum SSL version

On 2019-12-03 12:44, Magnus Hagander wrote:

On Tue, Dec 3, 2019 at 12:09 PM Michael Paquier <michael@paquier.xyz
<mailto:michael@paquier.xyz>> wrote:

On Tue, Dec 03, 2019 at 10:10:57AM +0100, Magnus Hagander wrote:

Is 1.0.1 considered a separate major from 1.0.0, in this

reasoning? Because

while retiring 1.0.0 should probably not be that terrible, 1.0.1

is still

in very widespread use on most long term supported distributions.

1.0.1 and 1.0.0 are two different major releases in the OpenSSL world,
so my suggestion would be to cut support for everything which does not
have TLSv1.2, meaning that we keep compatibility with 1.0.1 for
a longer period.

Good, that's what I thought you meant :) And that makes it sound like a
working plan to me.

This would mean we'd stop support for RHEL 5, which is probably OK,
seeing that even the super-extended support ends in November 2020.

Dropping RHEL 5 would also allow us to drop support for Python 2.4,
which is something I've been itching to do. ;-)

In both of these cases, maintaining support for all these ancient
versions is a significant burden IMO, so it would be good to clean up
the tail end a bit.

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

#25Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#24)
Re: Update minimum SSL version

On Wed, Dec 04, 2019 at 09:10:04AM +0100, Peter Eisentraut wrote:

This would mean we'd stop support for RHEL 5, which is probably OK, seeing
that even the super-extended support ends in November 2020.

Sounds like a plan. I can work on the OpenSSL part, if you need help
of course. And if others don't object in doing that. Of course.

Dropping RHEL 5 would also allow us to drop support for Python 2.4, which is
something I've been itching to do. ;-)

In both of these cases, maintaining support for all these ancient versions
is a significant burden IMO, so it would be good to clean up the tail end a
bit.

Good to know.
--
Michael

#26Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#24)
Re: Update minimum SSL version

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-12-03 12:44, Magnus Hagander wrote:

On Tue, Dec 3, 2019 at 12:09 PM Michael Paquier <michael@paquier.xyz
<mailto:michael@paquier.xyz>> wrote:
On Tue, Dec 03, 2019 at 10:10:57AM +0100, Magnus Hagander wrote:

Is 1.0.1 considered a separate major from 1.0.0, in this reasoning? Because
while retiring 1.0.0 should probably not be that terrible, 1.0.1
is still in very widespread use on most long term supported distributions.

This would mean we'd stop support for RHEL 5, which is probably OK,
seeing that even the super-extended support ends in November 2020.

Dropping RHEL 5 would also allow us to drop support for Python 2.4,
which is something I've been itching to do. ;-)

In both of these cases, maintaining support for all these ancient
versions is a significant burden IMO, so it would be good to clean up
the tail end a bit.

So, what exactly are we going to set as the new minimum version in
each case? I'll have to go update my trailing-edge-Johnnie buildfarm
critters, and it'd make sense to have them continue to test the
oldest nominally-supported versions.

For OpenSSL it seems like 1.0.1a is the target, per the above
discussion.

For Python, I'll just observe that RHEL6 ships 2.6.6, so we can't
bump up to 2.7.

regards, tom lane

#27Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#26)
Re: Update minimum SSL version

On 2019-12-04 13:53, Tom Lane wrote:

So, what exactly are we going to set as the new minimum version in
each case? I'll have to go update my trailing-edge-Johnnie buildfarm
critters, and it'd make sense to have them continue to test the
oldest nominally-supported versions.

For OpenSSL it seems like 1.0.1a is the target, per the above
discussion.

For Python, I'll just observe that RHEL6 ships 2.6.6, so we can't
bump up to 2.7.

Yes, it would be Python 2.6.

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

#28Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#25)
Re: Update minimum SSL version

On 2019-12-04 09:20, Michael Paquier wrote:

On Wed, Dec 04, 2019 at 09:10:04AM +0100, Peter Eisentraut wrote:

This would mean we'd stop support for RHEL 5, which is probably OK, seeing
that even the super-extended support ends in November 2020.

Sounds like a plan. I can work on the OpenSSL part, if you need help
of course. And if others don't object in doing that. Of course.

Please go ahead and propose a patch.

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

#29Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Tom Lane (#12)
Re: Update minimum SSL version

On 2019-12-02 16:13, Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-11-30 04:06, Tom Lane wrote:

I think the real question we have to answer is this: are we intent on
making people upgrade ancient openssl installations?

The trade-off is that this makes the defaults better for the vast
majority of users and gives users of really old systems a nudge that
they are no longer in compliance with industry best practices. You need
manual steps to set up SSL anyway, so this doesn't introduce an entirely
new kind of requirement for the latter group of users.

True. I'm okay with this as long as we adapt the ssl test suite as
per your other reply.

I have committed this with that change. The discussion on which OpenSSL
versions to support and how will continue.

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

#30Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#10)
Re: Update minimum SSL version

On Mon, Dec 02, 2019 at 02:09:51PM +0100, Daniel Gustafsson wrote:

However, looking at the signatures detected by autoconf we can however get an
idea of which version is used. SSL_clear_options and X509_get_signature_nid()
first shipped in 1.0.2, while SSL_get_current_compression first shipped in
0.9.8. There are also a set of functions which are new in 1.1.0 (BIO_get_data
et.al).

I was just looking at this problem, and something does not match with
what you wrote here. SSL_clear_options() is defined in OpenSSL from
0.9.8 to 1.0.2 as a macro (see ssl/ssl.h), and is defined as a
function since 1.1.0. So it seems to me that we are able to correctly
detect the presence of this function in the configure checks if
building with 1.1.0~, but not other versions.

In LibreSSL, the code has visibly always used a macro, even on their
latest HEAD since the code has been forked from OpenSSL 1.0.1g:
https://github.com/libressl-portable/openbsd. So we should be able
to compile our code, still we fail to detect that we can use the
macro.

It seems to me that we have quite a couple of arguments in favor of
dropping this configure check all together. (I saw the business
around a364dfa as well regarding NetBSD 5.1).

We can do more cleanup, and the discussion is quite different than the
original intent of this thread, so I am going to create a new one on
the matter.
--
Michael

#31Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#27)
Re: Update minimum SSL version

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 2019-12-04 13:53, Tom Lane wrote:

So, what exactly are we going to set as the new minimum version in
each case? I'll have to go update my trailing-edge-Johnnie buildfarm
critters, and it'd make sense to have them continue to test the
oldest nominally-supported versions.

For OpenSSL it seems like 1.0.1a is the target, per the above
discussion.

For Python, I'll just observe that RHEL6 ships 2.6.6, so we can't
bump up to 2.7.

Yes, it would be Python 2.6.

So the upshot, after a fair amount of hair-pulling, is

* Somebody maybe should be testing openssl 1.0.1, but it won't be
me, because neither 1.0.1 nor 1.0.1a will even build on non-Intel
platforms. After closer study of their release notes, I've settled
on 1.0.1e as being the best compromise between being old and not
having unreasonable teething pains. (I wonder how coincidental
it is that that's also what Red Hat is now shipping in RHEL6.)
I've successfully installed 1.0.1e on prairiedog and gaur, so
I can flip them to start building HEAD with that whenever we
break compatibility with 0.9.8.

* Python 2.6.x also suffered from an unreasonable amount of
teething pains --- 2.6.2 is the oldest version that seems
to know how to build a shared library on Darwin. I've now
got a reasonably functional 2.6 on gaur and 2.6.2 on prairiedog,
and again will adjust those buildfarm members to use those
installations when/if our support for their current versions
goes away.

regards, tom lane

#32Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#30)
Re: Update minimum SSL version

On 5 Dec 2019, at 02:48, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Dec 02, 2019 at 02:09:51PM +0100, Daniel Gustafsson wrote:

However, looking at the signatures detected by autoconf we can however get an
idea of which version is used. SSL_clear_options and X509_get_signature_nid()
first shipped in 1.0.2, while SSL_get_current_compression first shipped in
0.9.8. There are also a set of functions which are new in 1.1.0 (BIO_get_data
et.al).

I was just looking at this problem, and something does not match with
what you wrote here. SSL_clear_options() is defined in OpenSSL from
0.9.8 to 1.0.2 as a macro (see ssl/ssl.h), and is defined as a
function since 1.1.0.

Yes, I confused myself regarding the version for SSL_clear_options, except for
when it turned into a function.

So it seems to me that we are able to correctly
detect the presence of this function in the configure checks if
building with 1.1.0~, but not other versions.

In LibreSSL, the code has visibly always used a macro, even on their
latest HEAD since the code has been forked from OpenSSL 1.0.1g:
https://github.com/libressl-portable/openbsd. So we should be able
to compile our code, still we fail to detect that we can use the
macro.

Yes, we can't use AC_CHECK_FUNCS but would need to use AC_COMPILE_IFELSE (or a
similar check) in order to detect the macro.

It seems to me that we have quite a couple of arguments in favor of
dropping this configure check all together. (I saw the business
around a364dfa as well regarding NetBSD 5.1).

We can do more cleanup, and the discussion is quite different than the
original intent of this thread, so I am going to create a new one on
the matter.

Yes, if we're dropping older versions such that all supported versions have the
function, then keeping the autoconf check would be quite pointless.

cheers ./daniel

#33Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#32)
Re: Update minimum SSL version

Daniel Gustafsson <daniel@yesql.se> writes:

On 5 Dec 2019, at 02:48, Michael Paquier <michael@paquier.xyz> wrote:
So it seems to me that we are able to correctly
detect the presence of this function in the configure checks if
building with 1.1.0~, but not other versions.

Yes, we can't use AC_CHECK_FUNCS but would need to use AC_COMPILE_IFELSE (or a
similar check) in order to detect the macro.

configure already has a similar issue for isinf(). (I thought there
were more cases, actually, but I don't see another right now.)
We could just duplicate that logic, or maybe it's time to wrap it
up in an autoconf macro?

Yes, if we're dropping older versions such that all supported versions have the
function, then keeping the autoconf check would be quite pointless.

True as far as HEAD goes. What I'd like to know is whether not
realizing that SSL_clear_options is present causes any functional
issues that would justify back-patching a fix.

regards, tom lane

#34Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#33)
1 attachment(s)
Re: Update minimum SSL version

On 5 Dec 2019, at 15:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 5 Dec 2019, at 02:48, Michael Paquier <michael@paquier.xyz> wrote:
So it seems to me that we are able to correctly
detect the presence of this function in the configure checks if
building with 1.1.0~, but not other versions.

Yes, we can't use AC_CHECK_FUNCS but would need to use AC_COMPILE_IFELSE (or a
similar check) in order to detect the macro.

configure already has a similar issue for isinf(). (I thought there
were more cases, actually, but I don't see another right now.)
We could just duplicate that logic, or maybe it's time to wrap it
up in an autoconf macro?

Yes, if we're dropping older versions such that all supported versions have the
function, then keeping the autoconf check would be quite pointless.

True as far as HEAD goes.

Good point.

What I'd like to know is whether not
realizing that SSL_clear_options is present causes any functional
issues that would justify back-patching a fix.

ISTM that SSL_clear_options is required for turning on compression. Since
compression was introduced in 1.0.0 and SSL_clear_options was turned into a
function in 1.1.0, it affects 1.0.0, 1.0.1 and 1.0.2 with the latter two being
quite heavily used. I'm not sure how common it is to enable compression, and
especially how common it is post-CRIME, but since the option is there it seems
silly for it not to work with highly common library versions. Removing the
check only affects NetBSD 5, but breaking compilation in a stable release, even
for a rare OS, is I assume/hope a no-no. So thats a +1 from me for back-
patching a fix, while removing the check altogether in master.

The attached converts the check to use AC_LINK_IFELSE, in order to detect the
macro as well as the function (the compiled code is omitted for readability).
The patch is against master, but the check applies against backbranches except
for the AC_CHECK_FUNCS hunk which need tailoring per backbranch. I didn't
convert it to an autoconf macro, as there are only two callers in the
backbranches and it won't go into HEAD.

cheers ./daniel

Attachments:

ssl_clear_options_check.patchapplication/octet-stream; name=ssl_clear_options_check.patch; x-unix-mode=0644Download
From b6a6d29dbff2538c2faddbfb6cb176548a02e6cc Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Thu, 5 Dec 2019 23:25:22 +0100
Subject: [PATCH] Fix autoconf check for SSL_clear_options

SSL_clear_options was implemented as a macro up until OpenSSL 1.1.0,
which means it cannot be reliably tested for with AC_CHECK_FUNCS as
that will only work for 1.1.0 and later.  Reimplement the check with
a test that compiles to ensure the signature is present.
---
 configure.in | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/configure.in b/configure.in
index a2cb20b5e3..37b59a2de4 100644
--- a/configure.in
+++ b/configure.in
@@ -1186,7 +1186,7 @@ if test "$with_openssl" = yes ; then
      AC_SEARCH_LIBS(CRYPTO_new_ex_data, [eay32 crypto], [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for OpenSSL])])
      AC_SEARCH_LIBS(SSL_new, [ssleay32 ssl], [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for OpenSSL])])
   fi
-  AC_CHECK_FUNCS([SSL_clear_options SSL_get_current_compression X509_get_signature_nid])
+  AC_CHECK_FUNCS([SSL_get_current_compression X509_get_signature_nid])
   # Functions introduced in OpenSSL 1.1.0. We used to check for
   # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
   # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
@@ -1197,6 +1197,20 @@ if test "$with_openssl" = yes ; then
   # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
   # function was removed.
   AC_CHECK_FUNCS([CRYPTO_lock])
+  # SSL_clear_options is a macro in OpenSSL from 0.9.8 up until 1.0.2, so we
+  # can't test for it with AC_CHECK_FUNCS
+  AC_CACHE_CHECK([for SSL_clear_options], ac_cv_func_ssl_clear_options,
+  [AC_LINK_IFELSE([AC_LANG_PROGRAM([
+  #include <openssl/ssl.h>
+  #include <openssl/bio.h>
+  SSL *ssl;
+  ],
+  [return SSL_clear_options(ssl, 0);])],
+  [ac_cv_func_ssl_clear_options=yes],
+  [ac_cv_func_ssl_clear_options=no])])
+  if test $ac_cv_func_ssl_clear_options = yes ; then
+    AC_DEFINE(HAVE_SSL_CLEAR_OPTIONS, 1, [Define to 1 if you have SSL_clear_options()])
+  fi
 fi
 
 if test "$with_pam" = yes ; then
-- 
2.21.0 (Apple Git-122.2)

#35Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#34)
Re: Update minimum SSL version

Daniel Gustafsson <daniel@yesql.se> writes:

On 5 Dec 2019, at 15:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:

What I'd like to know is whether not
realizing that SSL_clear_options is present causes any functional
issues that would justify back-patching a fix.

ISTM that SSL_clear_options is required for turning on compression. Since
compression was introduced in 1.0.0 and SSL_clear_options was turned into a
function in 1.1.0, it affects 1.0.0, 1.0.1 and 1.0.2 with the latter two being
quite heavily used. I'm not sure how common it is to enable compression, and
especially how common it is post-CRIME, but since the option is there it seems
silly for it not to work with highly common library versions. Removing the
check only affects NetBSD 5, but breaking compilation in a stable release, even
for a rare OS, is I assume/hope a no-no. So thats a +1 from me for back-
patching a fix, while removing the check altogether in master.

Agreed that we should do something about this. However, our requirement
for 0.9.8 or newer has been there since v10 (cf. 593d4e47d). So I think
what we should do is

(1) Back-patch Michael's
0002-Remove-configure-checks-for-SSL_clear_options-in-Ope.patch
from the other thread [1]/messages/by-id/20191205083252.GE5064@paquier.xyz as far as v10.

(2) Use this patch in 9.4-9.6.

It'd be possible to also backpatch the other thread's
0001-Remove-configure-checks-for-SSL_get_current_compress.patch
as far as v10, but I'm less excited about that -- it'd just save
a few configure cycles, no?

regards, tom lane

[1]: /messages/by-id/20191205083252.GE5064@paquier.xyz

#36Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#35)
Re: Update minimum SSL version

On Thu, Dec 05, 2019 at 07:41:14PM -0500, Tom Lane wrote:

It'd be possible to also backpatch the other thread's
0001-Remove-configure-checks-for-SSL_get_current_compress.patch
as far as v10, but I'm less excited about that -- it'd just save
a few configure cycles, no?

Yeah. I'd try not to meddle with stable branches more than necessary,
and the removal of the part for get_current_compression is just a
cleanup so I would just do that on HEAD and be done with it.

About clear_options, my take is to remove the check on HEAD, and to
apply Daniel's patch on *all* stable branches because I think that we
should not break the business that happened with NetBSD 5 on already
released branches. Does that sound good?
--
Michael

#37Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#36)
Re: Update minimum SSL version

Michael Paquier <michael@paquier.xyz> writes:

About clear_options, my take is to remove the check on HEAD, and to
apply Daniel's patch on *all* stable branches because I think that we
should not break the business that happened with NetBSD 5 on already
released branches. Does that sound good?

OK, re-reading the thread, I see the point --- old NetBSD has a weird
OpenSSL version that this would break. OK, let's stay compatible
with that on the back branches. So, your patch on HEAD and Daniel's
in the back branches is the right thing. Please push.

(Note: I didn't actually test Daniel's patch or read it closely ---
it looks like about the right thing, but please double check.)

regards, tom lane

#38Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#37)
Re: Update minimum SSL version

On Thu, Dec 05, 2019 at 11:40:37PM -0500, Tom Lane wrote:

OK, re-reading the thread, I see the point --- old NetBSD has a weird
OpenSSL version that this would break. OK, let's stay compatible
with that on the back branches. So, your patch on HEAD and Daniel's
in the back branches is the right thing. Please push.

Thanks, applied. I have tested Daniel's version with OpenSSL 0.9.8,
1.0.2 and 1.1.0 and the test was able to detect correctly the
macro/function in all cases.
--
Michael