Horton

Thread

Commits

  1. Fix handling of OpenSSL's SSL_clear_options

  2. Remove configure check for OpenSSL's SSL_get_current_compression()

  3. Update minimum SSL version

  1. Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-11-29T07:36:00Z

    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], and browser 
vendors are set to disable 1.0 and 1.1 in their products sometime soon [1].

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].


[0]: 
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/
[2]: 
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

  2. Re: Update minimum SSL version

    Daniel Gustafsson <daniel@yesql.se> — 2019-11-29T10:10:32Z

    > 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

  3. Re: Update minimum SSL version

    Magnus Hagander <magnus@hagander.net> — 2019-11-29T12:40:48Z

    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/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  4. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-11-29T12:57:27Z

    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

  5. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-29T15:30:47Z

    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

  6. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-11-30T02:43:45Z

    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

  7. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-30T03:06:54Z

    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

  8. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-02T10:39:48Z

    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

  9. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-02T10:56:25Z

    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

  10. Re: Update minimum SSL version

    Daniel Gustafsson <daniel@yesql.se> — 2019-12-02T13:09:51Z

    > 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

  11. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-02T14:59:44Z

    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

  12. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-02T15:13:26Z

    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

  13. Re: Update minimum SSL version

    Daniel Gustafsson <daniel@yesql.se> — 2019-12-02T15:28:39Z

    > 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

  14. Re: Update minimum SSL version

    Robert Haas <robertmhaas@gmail.com> — 2019-12-02T15:31:40Z

    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

  15. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-02T16:39:28Z

    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

  16. Re: Update minimum SSL version

    Robert Haas <robertmhaas@gmail.com> — 2019-12-02T17:40:18Z

    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

  17. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-02T17:51:26Z

    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

  18. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-03T03:47:13Z

    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

  19. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-03T03:53:23Z

    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

  20. Re: Update minimum SSL version

    Magnus Hagander <magnus@hagander.net> — 2019-12-03T09:10:57Z

    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/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  21. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-03T11:08:54Z

    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

  22. Re: Update minimum SSL version

    Magnus Hagander <magnus@hagander.net> — 2019-12-03T11:44:32Z

    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/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  23. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-04T07:52:14Z

    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

  24. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-04T08:10:04Z

    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

  25. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-04T08:20:49Z

    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

  26. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-04T12:53:21Z

    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

  27. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-04T15:24:01Z

    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

  28. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-04T15:24:37Z

    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

  29. Re: Update minimum SSL version

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2019-12-04T22:58:22Z

    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

  30. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-05T01:48:23Z

    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

  31. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-05T01:50:01Z

    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

  32. Re: Update minimum SSL version

    Daniel Gustafsson <daniel@yesql.se> — 2019-12-05T09:03:27Z

    > 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

  33. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-05T14:50:50Z

    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

  34. Re: Update minimum SSL version

    Daniel Gustafsson <daniel@yesql.se> — 2019-12-05T22:29:36Z

    > 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

  35. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-06T00:41:14Z

    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] 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] https://www.postgresql.org/message-id/20191205083252.GE5064%40paquier.xyz

  36. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-06T00:59:51Z

    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

  37. Re: Update minimum SSL version

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-12-06T04:40:37Z

    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

  38. Re: Update minimum SSL version

    Michael Paquier <michael@paquier.xyz> — 2019-12-06T06:20:32Z

    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