Thread: Update minimum SSL version
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
Attachment
> 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 wouldstill 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
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.
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
Attachment
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
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
Attachment
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
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
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
> 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
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
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
> 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 willreport back (on the SSL_clear_options thing as well). cheers ./daniel
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
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
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
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
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
Attachment
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
Attachment
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.
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
Attachment
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.
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
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
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
Attachment
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
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
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
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
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
Attachment
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
> 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
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
> 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
Attachment
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
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
Attachment
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
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