Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On Wed, Jul 29, 2015 at 2:00 AM, Andres Freund wrote: > On 2015-07-28 18:59:02 +0200, Andres Freund wrote: >> Attached are: >> >> a) a slightly evolved version of Michael's patch disabling renegotiation >>by default that I'm planning to apply to 9.4 - 9.0 >> >> b) a patch removing renegotiation entirely from master and 9.5 Note: there was already upthread a patch for that: http://www.postgresql.org/message-id/cab7npqqnjidixr5qnj86qnm++skpytedtnlf_vnpmvtu5xo...@mail.gmail.com But it doesn't matter much. Thanks for the final push. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
Andres Freund wrote: > On 2015-07-28 18:59:02 +0200, Andres Freund wrote: > > Unless somebody protests soon I'm going to push something like that > > after having dinner. > > Done. Yay! -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On 2015-07-28 18:59:02 +0200, Andres Freund wrote: > Unless somebody protests soon I'm going to push something like that > after having dinner. Done. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On 2015-07-28 18:59:02 +0200, Andres Freund wrote: > Attached are: > > a) a slightly evolved version of Michael's patch disabling renegotiation >by default that I'm planning to apply to 9.4 - 9.0 > > b) a patch removing renegotiation entirely from master and 9.5 > > Unless somebody protests soon I'm going to push something like that > after having dinner. > > I am wondering whether b) ought to remove Port->count, but I'm currently > leaning to leaving it in place for now; perhaps adding a comment in the > struct. I'm actually thinking we very well might want to add something > like it to all backends, but more importantly it'd make the diff larger > with mostly unrelated changes. And really attached. >From 768dd6560e262d7597d0efb37043a96e1594508e Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 28 Jul 2015 18:41:32 +0200 Subject: [PATCH] Disable ssl renegotiation by default. While postgres' use of SSL renegotiation is a good idea in theory, it turned out to not work well in practice. The specification and openssl's implementation of it have lead to several security issues. Postgres' use of renegotiation also had its share of bugs. Additionally OpenSSL has a bunch of bugs around renegotiation, reported and open for years, that regularly lead to connections breaking with obscure error messages. We tried increasingly complex workarounds around these bugs, but we didn't find anything complete. Since these connection breakages often lead to hard to debug problems, e.g. spuriously failing base backups and significant latency spikes when synchronous replication is used, we have decided to change the default setting for ssl renegotiation to 0 (disabled) in the released backbranches and remove it entirely in 9.5 and master.. Author: Michael Paquier, with changes by me Discussion: 20150624144148.gq4...@alap3.anarazel.de Backpatch: 9.0-9.4; 9.5 and master get a different patch --- doc/src/sgml/config.sgml | 10 +- src/backend/utils/misc/guc.c | 2 +- src/backend/utils/misc/postgresql.conf.sample | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index c669f75..871b04a 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1040,7 +1040,7 @@ include_dir 'conf.d' cryptanalysis when large amounts of traffic can be examined, but it also carries a large performance penalty. The sum of sent and received traffic is used to check the limit. If this parameter is set to 0, -renegotiation is disabled. The default is 512MB. +renegotiation is disabled. The default is 0. @@ -1052,6 +1052,14 @@ include_dir 'conf.d' disabled. + + + + Due to bugs in OpenSSL enabling ssl renegotiation, by + configuring a non-zero ssl_renegotiation_limit, is likely + to lead to problems like long-lived connections breaking. + + diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 6ad0892..396c68b 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2457,7 +2457,7 @@ static struct config_int ConfigureNamesInt[] = GUC_UNIT_KB, }, &ssl_renegotiation_limit, - 512 * 1024, 0, MAX_KILOBYTES, + 0, 0, MAX_KILOBYTES, NULL, NULL, NULL }, diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 8dfd485..3845d57 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -83,7 +83,7 @@ # (change requires restart) #ssl_prefer_server_ciphers = on # (change requires restart) #ssl_ecdh_curve = 'prime256v1' # (change requires restart) -#ssl_renegotiation_limit = 512MB # amount of data between renegotiations +#ssl_renegotiation_limit = 0 # amount of data between renegotiations #ssl_cert_file = 'server.crt' # (change requires restart) #ssl_key_file = 'server.key' # (change requires restart) #ssl_ca_file = '' # (change requires restart) -- 2.4.0.rc2.1.g3d6bc9a >From 0d488c5f1d7eba48aa19894339c69488094878a1 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 28 Jul 2015 17:59:55 +0200 Subject: [PATCH] Remove ssl renegotiation support. While postgres' use of SSL renegotiation is a good idea in theory, it turned out to not work well in practice. The specification and openssl's implementation of it have lead to several security issues. Postgres' use of renegotiation also had its share of bugs. Additionally OpenSSL has a bunch of bugs around renegotiation, reported and open for years, that regularly lead to connections breaking with obscure error messages. We tried increasingly complex workarounds around these bugs, but we didn't find anything complete. Since these connection breakages often lead to hard to debug problems, e.g. spuriously failin
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
Hi, Attached are: a) a slightly evolved version of Michael's patch disabling renegotiation by default that I'm planning to apply to 9.4 - 9.0 b) a patch removing renegotiation entirely from master and 9.5 Unless somebody protests soon I'm going to push something like that after having dinner. I am wondering whether b) ought to remove Port->count, but I'm currently leaning to leaving it in place for now; perhaps adding a comment in the struct. I'm actually thinking we very well might want to add something like it to all backends, but more importantly it'd make the diff larger with mostly unrelated changes. Regards, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On Sat, Jul 11, 2015 at 9:28 PM, Andres Freund wrote: > On 2015-07-11 21:09:05 +0900, Michael Paquier wrote: > > Something like the patches attached > > Thanks for that! > > > could be considered, one is for master > > and REL9_5_STABLE to remove ssl_renegotiation_limit, the second one for > > ~REL9_4_STABLE to change the default to 0. > > > diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml > > index c669f75..16c0ce5 100644 > > --- a/doc/src/sgml/config.sgml > > +++ b/doc/src/sgml/config.sgml > > @@ -1040,7 +1040,7 @@ include_dir 'conf.d' > > cryptanalysis when large amounts of traffic can be examined, > but it > > also carries a large performance penalty. The sum of sent and > received > > traffic is used to check the limit. If this parameter is set to > 0, > > -renegotiation is disabled. The default is 512MB. > > +renegotiation is disabled. The default is 0. > > I think we should put in a warning or at least note about the dangers of > enabling it (connection breaks, exposure to several open openssl bugs). > This sounds like a good idea to me. Here is an idea: + + + Enabling ssl_renegotiation_limit can cause various + problems endangering the stability of a PostgreSQL + instance like connection breaking suddendly and exposes the + server to bugs related to the internal implementation of renegotiation + done in the SSL libraries used. + + Attached is v2 for ~9.4. Regards, -- Michael 20150712_ssl_renegotiation_remove-94_v2.patch Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On 2015-07-11 21:09:05 +0900, Michael Paquier wrote: > Something like the patches attached Thanks for that! > could be considered, one is for master > and REL9_5_STABLE to remove ssl_renegotiation_limit, the second one for > ~REL9_4_STABLE to change the default to 0. > diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml > index c669f75..16c0ce5 100644 > --- a/doc/src/sgml/config.sgml > +++ b/doc/src/sgml/config.sgml > @@ -1040,7 +1040,7 @@ include_dir 'conf.d' > cryptanalysis when large amounts of traffic can be examined, but it > also carries a large performance penalty. The sum of sent and > received > traffic is used to check the limit. If this parameter is set to 0, > -renegotiation is disabled. The default is 512MB. > +renegotiation is disabled. The default is 0. I think we should put in a warning or at least note about the dangers of enabling it (connection breaks, exposure to several open openssl bugs). Thanks, Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On Fri, Jul 10, 2015 at 7:47 PM, Andres Freund wrote: > On 2015-07-01 23:32:23 -0400, Noah Misch wrote: > > We'd need to be triply confident that we know better than the DBA before > > removing flexibility in back branches. > > +1 for just changing the default. > > I think we do. But I also think that I pretty clearly lost this > argument, so let's just change the default. > > Is anybody willing to work on this? > Something like the patches attached could be considered, one is for master and REL9_5_STABLE to remove ssl_renegotiation_limit, the second one for ~REL9_4_STABLE to change the default to 0. Regards, -- Michael 20150710_ssl_renegotiation_remove-94.patch Description: binary/octet-stream 20150710_ssl_renegotiation_remove-master.patch Description: binary/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On 2015-07-01 23:32:23 -0400, Noah Misch wrote: > We'd need to be triply confident that we know better than the DBA before > removing flexibility in back branches. > +1 for just changing the default. I think we do. But I also think that I pretty clearly lost this argument, so let's just change the default. Is anybody willing to work on this? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)
On Sat, Jun 27, 2015 at 06:13:36PM +0200, Andres Freund wrote: > On 2015-06-27 12:10:49 -0400, Tom Lane wrote: > > Andres Freund writes: > > > On 2015-06-27 15:07:05 +0900, Michael Paquier wrote: > > >> +1 for removing on master and just disabling on back-branches. > > > > > The problem with that approach is that it leaves people hanging in the > > > dry if they've uncommented the default value, or changed it. That > > > doesn't seem nice to me. > > > > I think at least 99% of the people who are using a nondefault value of > > ssl_renegotiation_limit are using zero and so would have no problem with > > this at all. Possibly 100% of them; there's not really much use-case for > > changing from 512MB to some other nonzero value, is there? > > While still at 2ndq I've seen some increase it to nonzero values to cope > with the connection breaks. We'd need to be triply confident that we know better than the DBA before removing flexibility in back branches. +1 for just changing the default. Suppose some security policy mandates a particular key rotation interval; the minor release would force an awkward decision on that user. DBAs who have customized ssl_renegotiation_limit are more likely than most to notice the release note and make an informed decision. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
