Should we back-patch SSL renegotiation fixes?

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Those of you who have been following
/messages/by-id/1d3bc192-970d-4b70-a5fe-38d2a9f762b3@me.com
are aware that Red Hat shipped a rather broken version of openssl last
week. While waiting for them to fix it, I've been poking at the behavior,
and have found out that PG 9.4 and later are much less badly broken than
older branches. In the newer branches you'll see a failure only after
transmitting 2GB within a session, whereas the older branches fail at
the second renegotiation attempt, which would typically be 1GB of data
and could be a lot less.

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654. (There are more changes
in master, but since those haven't yet shipped in any released branch,
and there's been a lot of other rework in the same area, those probably
are not back-patch candidates.)

Thoughts?

I have no clear idea how safe it is to back-port these fixes.

Just as a point of reference, we had a customer hit a problem similar
to bug #12769 on 9.3.x. I think (but am not sure) that 272923a0a may
have been intended to fix that issue. In a quick search, I didn't
find any other complaints about renegotiation-related issues from our
customers.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654. (There are more changes
in master, but since those haven't yet shipped in any released branch,
and there's been a lot of other rework in the same area, those probably
are not back-patch candidates.)

Thoughts?

I have no clear idea how safe it is to back-port these fixes.

Well, it would mean that pre-9.5 branches all behave the same, which
would be an improvement in my book. Also, ISTM that the 9.4 code
for renegotiation assumes a whole lot less than prior branches about
OpenSSL's internal behavior; so it ought to be more robust, even if
some bugs remain.

Just as a point of reference, we had a customer hit a problem similar
to bug #12769 on 9.3.x. I think (but am not sure) that 272923a0a may
have been intended to fix that issue. In a quick search, I didn't
find any other complaints about renegotiation-related issues from our
customers.

The problem with trying to adopt code from HEAD is that it probably
depends on the rather invasive changes explained here:
/messages/by-id/20150126101405.GA31719@awork2.anarazel.de
Even assuming that there's no dependency on the immediate-interrupt
changes, I'm afraid to back-patch anything that invasive.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas wrote:

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654. (There are more changes
in master, but since those haven't yet shipped in any released branch,
and there's been a lot of other rework in the same area, those probably
are not back-patch candidates.)

Yes, +1 for backpatching. Don't forget 5674460b and b1aebbb6.

I could reproduce problems trivially with COPY in psql without that and
a small renegotiation limit, as I recall.

Thoughts?

I have no clear idea how safe it is to back-port these fixes.

Just as a point of reference, we had a customer hit a problem similar
to bug #12769 on 9.3.x. I think (but am not sure) that 272923a0a may
have been intended to fix that issue.

Maybe we should *also* backpatch that, then (and if so, then also its
fixup 1c2b7c087). I do not think that the failure was introduced by
the fixes cited above.

In a quick search, I didn't find any other complaints about
renegotiation-related issues from our customers.

Other issues Andres was investigating seemed related to nonblocking
connections (which as I recall are used mostly by replication code).
Bug #12769 contained a link to the previous discussion.

--
�lvaro Herrera http://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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654.

Yes, +1 for backpatching. Don't forget 5674460b and b1aebbb6.

Huh? 5674460b is ancient, and we concluded that b1aebbb6 didn't represent
anything much more than cosmetic fixes.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Jun 23, 2015 at 3:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654. (There are more changes
in master, but since those haven't yet shipped in any released branch,
and there's been a lot of other rework in the same area, those probably
are not back-patch candidates.)

Thoughts?

I have no clear idea how safe it is to back-port these fixes.

Well, it would mean that pre-9.5 branches all behave the same, which
would be an improvement in my book. Also, ISTM that the 9.4 code
for renegotiation assumes a whole lot less than prior branches about
OpenSSL's internal behavior; so it ought to be more robust, even if
some bugs remain.

Just as a point of reference, we had a customer hit a problem similar
to bug #12769 on 9.3.x. I think (but am not sure) that 272923a0a may
have been intended to fix that issue. In a quick search, I didn't
find any other complaints about renegotiation-related issues from our
customers.

The problem with trying to adopt code from HEAD is that it probably
depends on the rather invasive changes explained here:
/messages/by-id/20150126101405.GA31719@awork2.anarazel.de
Even assuming that there's no dependency on the immediate-interrupt
changes, I'm afraid to back-patch anything that invasive.

What commits actually resulted from that?

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-06-23 14:33:21 -0400, Tom Lane wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4.

I, by now, have come to a different conclusion. I think it's time to
entirely drop the renegotiation support.

While there's a security benefit of renegotiation by limiting the amount
of leaked data in case either client or server is exploited while the
connection is ongoing, the reality is that the renegotiation support in
openssl just isn't up to the task.

Both Heikki and I have spent a considerable amount of time trying to
find workarounds for the renegotiation bugs in openssl, but so far I
don't think that's bullet proof. Additionally the track record of
renegotiation both in ssl specification and in the openssl specification
is that it opens many more security holes than it fixes.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

On Tue, Jun 23, 2015 at 2:33 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4. Specifically, I think maybe we should
back-patch 31cf1a1a4, 86029b31e, and 36a3be654.

Yes, +1 for backpatching. Don't forget 5674460b and b1aebbb6.

Huh? 5674460b is ancient, and we concluded that b1aebbb6 didn't represent
anything much more than cosmetic fixes.

Sorry, I mixed up 5674460b with 36a3be65 which you already mentioned ...
and I see that because of the conclusions from 272923a0a695 then the
one-char change in b1aebbb6 is not very interesting.

I do think that perhaps we should simplify the code down to what
272923a0a695 + 1c2b7c0879d8 do.

I also agree that the other changes by Andres and Heikki, which involve
making all communication use a nonblocking socket, seem too invasive to
backpatch; even with the insurance provided by beta+release, the
disruptiveness of changes seems a bit too high, considering that
387da18874a apparently cannot be used without 4f85fde8eb which is a bit
scary in itself.

--
�lvaro Herrera http://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

Andres Freund <andres@anarazel.de> writes:

I, by now, have come to a different conclusion. I think it's time to
entirely drop the renegotiation support.

Well, that's a radical proposal, but I think we should take it seriously.

On balance I think I agree that SSL renegotiation has not been worth the
trouble. And we definitely aren't testing it adequately, so if we wanted
to keep it then there's even *more* work that somebody ought to expend.

I assume we'd back-patch it, too? (Probably not remove the
ssl_renegotiation_limit variable, but always act as though it were zero.)
If we still have to maintain the code in the back branches then we'd
continue to have to deal with its bugs for some time.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-06-24 11:11:16 -0400, Tom Lane wrote:

On balance I think I agree that SSL renegotiation has not been worth the
trouble. And we definitely aren't testing it adequately, so if we wanted
to keep it then there's even *more* work that somebody ought to expend.

Right. Our code was nearly entirely broken for streaming replication for
*years* without anybody noticing. And even now it doesn't reliably
work. It's also pretty hard to test due to the required data volumes and
the vast number of different behaviours across openssl versions.

I assume we'd back-patch it, too? (Probably not remove the
ssl_renegotiation_limit variable, but always act as though it were
zero.)

Yes, I think so. Maybe log a warning at startup if set to nonzero
(startup is probably the best we can do).

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 6/23/15 2:33 PM, Tom Lane wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4.

If Red Hat fixes their bug, then PostgreSQL doesn't have any actual
problem anymore, does it?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-06-24 11:57:53 -0400, Peter Eisentraut wrote:

On 6/23/15 2:33 PM, Tom Lane wrote:

I do not know at this point whether these behaviors are really the same
bug or not, but I wonder whether it's time to consider back-patching the
renegotiation fixes we did in 9.4.

If Red Hat fixes their bug, then PostgreSQL doesn't have any actual
problem anymore, does it?

It does, there are numerous bugs around renegotiation that exist with
upstream openssl and postgres. More in the older branches, but even in
HEAD we break regularly. Most only occur in replication connections (due
to copy both) and/or when using more complex clients where clients and
servers send data at the same time due to pipelining.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:

On 2015-06-24 11:57:53 -0400, Peter Eisentraut wrote:

If Red Hat fixes their bug, then PostgreSQL doesn't have any actual
problem anymore, does it?

It does, there are numerous bugs around renegotiation that exist with
upstream openssl and postgres. More in the older branches, but even in
HEAD we break regularly. Most only occur in replication connections (due
to copy both) and/or when using more complex clients where clients and
servers send data at the same time due to pipelining.

The lesson to learn from the Red Hat fiasco is that vendors are not
adequately testing renegotiation either. All the more reason to get
out from under it. I did not like being told that "Postgres fails and
$randomapp doesn't, therefore it's Postgres' problem" when actually
the difference was that $randomapp doesn't invoke renegotiation.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Jun 24, 2015 5:13 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

I, by now, have come to a different conclusion. I think it's time to
entirely drop the renegotiation support.

Well, that's a radical proposal, but I think we should take it seriously.

Yes.

Just on my phone right now, but wasn't renegotiation also an attack vector
in one of the recent openssl bugs?

/Magnus

On Wed, Jun 24, 2015 at 11:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

I, by now, have come to a different conclusion. I think it's time to
entirely drop the renegotiation support.

Well, that's a radical proposal, but I think we should take it seriously.

On balance I think I agree that SSL renegotiation has not been worth the
trouble. And we definitely aren't testing it adequately, so if we wanted
to keep it then there's even *more* work that somebody ought to expend.

I'd like to know what factors we are balancing against each other.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-06-24 12:57:03 -0400, Robert Haas wrote:

On Wed, Jun 24, 2015 at 11:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

I, by now, have come to a different conclusion. I think it's time to
entirely drop the renegotiation support.

Well, that's a radical proposal, but I think we should take it seriously.

On balance I think I agree that SSL renegotiation has not been worth the
trouble. And we definitely aren't testing it adequately, so if we wanted
to keep it then there's even *more* work that somebody ought to expend.

I'd like to know what factors we are balancing against each other.

Benefits:

SSL renegotiation limits the exposure of on-the-wire material that's
leaked if either client or server is hijacked during a existing
session. With renegotiation the client/server will hopefully only have
the currently negotiated symetric key, covering only
session_renegotiation_limit bytes, in memory.

That's nice, but from a practical point of view it's not worth all that
much. If the server has been hacked nearly all the data is accessible
anyway, and even if not, the hacker can just continue collecting data
going forward. If the client has been hacked, it's likely that it has
been relegating data for the session to somewhere that's still
accessible.

For the server hacked case all that generally only matters if perfect
forward secrecy (PFS) has been employed, without that the session keys
can be recovered anyway as the private key will be accessible in memory.

All this only matters for hacks that access running processes.

Deficits:

SSL renegotiation has had several weaknesses (c.f. CVE-2009-3555/RFC
5746 , although I'm not 100% sure it's possible to apply to PG) on the
protocol level, at least partially leading to much worse vulnerabilities
than renegotiation in the pg style fixes. The openssl implementation has
had several bugs several of them unfixed years after they were reported
(#3712, #2481, #2146,...). By my reading of openssl's code the current
state is entirely broken for duplex communication.

The current draft of the next version of the TLS standard removes
renegotiation entirely.

Additionally supporting SSL renegotiation requires SSL_write/read
callers to always call the SSL_write/read after either function has
reported the need for additional reads/writes (note that SSL_read can
require writes and the other way round). We don't comply with that rule,
especially in the backbranches, because it's really hard to do that.

Our code currently uses crude hacks (c.f. comment around
SSL_clear_num_renegotiations(), and the loop around SSL_do_handshake()
in the back branches) to manage renegotiations. There's pending patches
to substantially increase the amount of ugly hacking to cope with us
misusing the SSL_read/write protocol.

Greetings,

Andres Freund

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2015-06-24 19:35:51 +0200, Andres Freund wrote:

Our code currently uses crude hacks (c.f. comment around
SSL_clear_num_renegotiations(), and the loop around SSL_do_handshake()
in the back branches) to manage renegotiations. There's pending patches
to substantially increase the amount of ugly hacking to cope with us
misusing the SSL_read/write protocol.

C.f.
http://archives.postgresql.org/message-id/54DE6FAF.6050005%40vmware.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 6/24/15 12:26 PM, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2015-06-24 11:57:53 -0400, Peter Eisentraut wrote:

If Red Hat fixes their bug, then PostgreSQL doesn't have any actual
problem anymore, does it?

It does, there are numerous bugs around renegotiation that exist with
upstream openssl and postgres. More in the older branches, but even in
HEAD we break regularly. Most only occur in replication connections (due
to copy both) and/or when using more complex clients where clients and
servers send data at the same time due to pipelining.

The lesson to learn from the Red Hat fiasco is that vendors are not
adequately testing renegotiation either. All the more reason to get
out from under it. I did not like being told that "Postgres fails and
$randomapp doesn't, therefore it's Postgres' problem" when actually
the difference was that $randomapp doesn't invoke renegotiation.

I'm fine with removing renegotiation. But the original proposal was to
backpatch renegation changes, which seemed like replacing one problem
variation with another, and does not sound comfortable given recent
backpatching record.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On June 24, 2015 9:07:35 PM GMT+02:00, Peter Eisentraut <peter_e@gmx.net> wrote:

On 6/24/15 12:26 PM, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

On 2015-06-24 11:57:53 -0400, Peter Eisentraut wrote:

If Red Hat fixes their bug, then PostgreSQL doesn't have any actual
problem anymore, does it?

It does, there are numerous bugs around renegotiation that exist

with

upstream openssl and postgres. More in the older branches, but even

in

HEAD we break regularly. Most only occur in replication connections

(due

to copy both) and/or when using more complex clients where clients

and

servers send data at the same time due to pipelining.

The lesson to learn from the Red Hat fiasco is that vendors are not
adequately testing renegotiation either. All the more reason to get
out from under it. I did not like being told that "Postgres fails

and

$randomapp doesn't, therefore it's Postgres' problem" when actually
the difference was that $randomapp doesn't invoke renegotiation.

I'm fine with removing renegotiation. But the original proposal was to
backpatch renegation changes, which seemed like replacing one problem
variation with another, and does not sound comfortable given recent
backpatching record.

Meh. The relevant branches already exist, as you can disable it today.

We could also just change the default in the back branches.

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 6/24/15 3:13 PM, Andres Freund wrote:

Meh. The relevant branches already exist, as you can disable it today.

We could also just change the default in the back branches.

One more argument for leaving everything alone. If users don't like it,
they can turn it off themselves.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers