Recent vendor SSL renegotiation patches break PostgreSQL

Greetings, hackers!

The flurry of patches that vendors have recently been making to OpenSSL to address the potential man-in-the-middle attack during SSL renegotiation have disabled SSL renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL renegotiation, such as PostgreSQL, start failing.

I’ve noticed such failures on Mac OS X 10.6.2 after installing Security Update 2010-001 (which is when Apple distributed their OpenSSL patch):

http://support.apple.com/kb/HT4004

> OpenSSL
>
> CVE-ID: CVE-2009-3555
>
> Available for: Mac OS X v10.5.8, Mac OS X Server v10.5.8, Mac OS X v10.6.2, Mac OS X Server v10.6.2
>
> Impact: An attacker with a privileged network position may capture data or change the operations performed in sessions protected by SSL
>
> Description: A man-in-the-middle vulnerability exists in the SSL and TLS protocols. Further information is available at http://www.phonefactor.com/sslgap A change to the renegotiation protocol is underway within the IETF. This update disables renegotiation in OpenSSL as a preventive security measure.

After installing Security Update 2010-001, any libpq connection to the server that exchanges more than 512MB of data (the RENEGOTIATION_LIMIT defined in src/backend/libpq/be-secure.c) will trigger an SSL renegotiation, which fails, which disconnects the client. I observed the problem on both PostgreSQL 8.1.19 and PostgreSQL 8.4.2 (those are the only versions I have in production).

I have been working around the problem by disabling SSL renegotiation entirely in my PostgreSQL servers, commenting out lines 316-339 in src/backend/libpq/be-secure.c.

There have been reports of such SSL-related breakage on other platforms, too:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=560205

Thanks! Happy hacking!

- Chris

On Wed, Feb 3, 2010 at 6:24 AM, Chris Campbell <chris_campbell(at)mac(dot)com> wrote:
> The flurry of patches that vendors have recently been making to OpenSSL to address
> the potential man-in-the-middle attack during SSL renegotiation have disabled SSL
> renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL
> renegotiation, such as PostgreSQL, start failing.

Should we think about adding a GUC to disable renegotiation until this
blows over?

...Robert

Robert Haas wrote:
> On Wed, Feb 3, 2010 at 6:24 AM, Chris Campbell <chris_campbell(at)mac(dot)com> wrote:
>> The flurry of patches that vendors have recently been making to OpenSSL to address
>> the potential man-in-the-middle attack during SSL renegotiation have disabled SSL
>> renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL
>> renegotiation, such as PostgreSQL, start failing.
>
> Should we think about adding a GUC to disable renegotiation until this
> blows over?

hmm I wonder if we should not go as far as removing the whole
renegotiation code, from the field it seems that there are very very few
daemons actually doing that kind forced renegotiation.

Stefan

On Feb 3, 2010, at 10:16 AM, Stefan Kaltenbrunner wrote:

> Robert Haas wrote:
>> On Wed, Feb 3, 2010 at 6:24 AM, Chris Campbell <chris_campbell(at)mac(dot)com> wrote:
>>> The flurry of patches that vendors have recently been making to OpenSSL to address
>>> the potential man-in-the-middle attack during SSL renegotiation have disabled SSL
>>> renegotiation altogether in the OpenSSL libraries. Applications that make use of SSL
>>> renegotiation, such as PostgreSQL, start failing.
>> Should we think about adding a GUC to disable renegotiation until this
>> blows over?
>
> hmm I wonder if we should not go as far as removing the whole renegotiation code, from the field it seems that there are very very few daemons actually doing that kind forced renegotiation.

There was a discussion about the relevance and consequences of SSL renegotiation on this list back in 2003:

http://archives.postgresql.org/pgsql-interfaces/2003-04/msg00075.php

Personally, my production servers have been patched to remove renegotiation completely, and I’m comfortable with the consequences of that for my usage.

- Chris

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Should we think about adding a GUC to disable renegotiation until this
> blows over?

Bad idea: once set, it'll never get unset, thus leaving installations
with a weakened security posture even after they've installed fixed
versions of openssl.

regards, tom lane

On Wed, Feb 3, 2010 at 10:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Should we think about adding a GUC to disable renegotiation until this
>> blows over?
>
> Bad idea: once set, it'll never get unset, thus leaving installations
> with a weakened security posture even after they've installed fixed
> versions of openssl.

That's a problem, but our current posture of holding our breath
doesn't seem to be working either. If we insist on shipping code that
doesn't work with currently-distributed versions of OpenSSL, people
will do things like, say, shut SSL off. Or packagers of PostgreSQL
will apply patches that disable it unconditionally, leaving us with no
control.

...Robert

Is there a way to detect when the SSL library has renegotiation disabled? (Either at compile-time or runtime, although runtime would definitely be better because we’ll change our behavior if/when the user updates their SSL library.)

If so, we could skip renegotiation when it’s disabled in the library, but otherwise perform renegotiation like we normally do (every 512 MB, I think it is).

Also, the official OpenSSL patch provides a way for the application to re-enable renegotiation. I don’t think all implementations will do so, though (e.g., some vendors might have patched it differently).

- Chris

On Wed, Feb 3, 2010 at 10:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Bad idea: once set, it'll never get unset, thus leaving installations
> with a weakened security posture even after they've installed fixed
> versions of openssl.
>
>                        regards, tom lane

One might argue that the current method is already weakened as it is
measured by the amount of data sent instead of of a length of time. A
session could live a long time under the 512MB threshold depending on
the queries that are being performed.

Michael

Michael Ledford <mledford(at)gmail(dot)com> writes:
> One might argue that the current method is already weakened as it is
> measured by the amount of data sent instead of of a length of time. A
> session could live a long time under the 512MB threshold depending on
> the queries that are being performed.

Renegotiation after X amount of data is the recommended method AFAIK,
because it limits the volume of data available to cryptanalysis.
What makes you think that elapsed time is relevant at all?

regards, tom lane

On Wed, Feb 3, 2010 at 11:09 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Renegotiation after X amount of data is the recommended method AFAIK,
> because it limits the volume of data available to cryptanalysis.
> What makes you think that elapsed time is relevant at all?
>
>                        regards, tom lane

You are correct. In that volume of data also matters. It depends on
what kind of attack you are trying to minimize here. In my particular
use case I fluctuate between idle and busy but mostly low bandwidth.

You have four different primary cases that you are possible:

1) timed method on an idle link (or low usage)
2) timed method on a busy link (or high usage)
3) data limit on an idle link (or low usage)
4) data limit on a busy link (or high usage)

The timed method is more optimal for case 1.
The data limit is more optimal for case 4.

Case 2 gives an attacker more data to do crypto-analysis.
Case 3 gives an attacker more time to work with the current secret key
on a live link.

Depending on your use case, being able to work with a live link is
worse than working with the data postmortem. There is I'm sure some
hybrid in the middle between these where optimal lives.

Michael

Chris Campbell <chris_campbell(at)mac(dot)com> writes:
> Is there a way to detect when the SSL library has renegotiation disabled?

Probably not. The current set of emergency security patches would
certainly not have exposed any new API that would help us tell this :-(

If said patches were done properly they'd have also turned an
application-level renegotiation request into a no-op, instead of
breaking apps by making it fail --- but apparently they were not done
properly.

regards, tom lane

On Wed, Feb 3, 2010 at 11:52 AM, Michael Ledford <mledford(at)gmail(dot)com> wrote:
> On Wed, Feb 3, 2010 at 11:09 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Renegotiation after X amount of data is the recommended method AFAIK,
>> because it limits the volume of data available to cryptanalysis.
>> What makes you think that elapsed time is relevant at all?
>
> You are correct. In that volume of data also matters. It depends on
> what kind of attack you are trying to minimize here. In my particular
> use case I fluctuate between idle and busy but mostly low bandwidth.
>
> You have four different primary cases that you are possible:

This may all be true, but I think we're getting off track. If we
force ANY negotiation (whether based on time or bytes transferred), we
will, apparently, break things. So I think that means we should have
a way to disable that behavior, for fear of dissuading people from
using SSL (or PostgreSQL) altogether, or hacking their own copies of
the source in ways that may be even uglier.

...Robert

Tom Lane wrote:
> Chris Campbell <chris_campbell(at)mac(dot)com> writes:
> > Is there a way to detect when the SSL library has renegotiation disabled?
>
> Probably not. The current set of emergency security patches would
> certainly not have exposed any new API that would help us tell this :-(
>
> If said patches were done properly they'd have also turned an
> application-level renegotiation request into a no-op, instead of
> breaking apps by making it fail --- but apparently they were not done
> properly.

Yea, and also keep in mind any SSL library checks need to be done at
run-time (because I believe openssl is usually linked as a shared
object), which even further limits our options.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane escribió:
> Michael Ledford <mledford(at)gmail(dot)com> writes:
> > One might argue that the current method is already weakened as it is
> > measured by the amount of data sent instead of of a length of time. A
> > session could live a long time under the 512MB threshold depending on
> > the queries that are being performed.
>
> Renegotiation after X amount of data is the recommended method AFAIK,
> because it limits the volume of data available to cryptanalysis.
> What makes you think that elapsed time is relevant at all?

FWIW I think there's another problem with streaming replication here,
which is that most data flows from client to server, so it would take
quite some time for the threshold to be reached. Note that there's no
size check in the libpq frontend code. Normally this is not an issue
because the bulk of data is expected to flow in the other direction.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> FWIW I think there's another problem with streaming replication here,
> which is that most data flows from client to server, so it would take
> quite some time for the threshold to be reached. Note that there's no
> size check in the libpq frontend code. Normally this is not an issue
> because the bulk of data is expected to flow in the other direction.

Huh? I thought the slaves connect to the master, rather than the other
way round?

It's true that libpq doesn't contain any such code, but that seems like
a fortunate thing right at the moment, as it limits the number of places
we might have to hack something.

regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>> FWIW I think there's another problem with streaming replication here,
>> which is that most data flows from client to server, so it would take
>> quite some time for the threshold to be reached. Note that there's no
>> size check in the libpq frontend code. Normally this is not an issue
>> because the bulk of data is expected to flow in the other direction.
>
> Huh? I thought the slaves connect to the master, rather than the other
> way round?

Correct, slave connects to the master.

Alvaro is pointing out that most data flows from client to server, like
in COPY FROM. But the server counts both in- and out-going data against
the renegotiation limit, so the server will initiate renegotiation just
fine in that case too.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Tom Lane wrote:
> Chris Campbell <chris_campbell(at)mac(dot)com> writes:
> > Is there a way to detect when the SSL library has renegotiation disabled?
>
> Probably not. The current set of emergency security patches would
> certainly not have exposed any new API that would help us tell this :-(
>
> If said patches were done properly they'd have also turned an
> application-level renegotiation request into a no-op, instead of
> breaking apps by making it fail --- but apparently they were not done
> properly.

Is there anything remaining to do on this issue?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> Chris Campbell <chris_campbell(at)mac(dot)com> writes:
>>> Is there a way to detect when the SSL library has renegotiation disabled?
>>
>> Probably not. The current set of emergency security patches would
>> certainly not have exposed any new API that would help us tell this :-(
>>
>> If said patches were done properly they'd have also turned an
>> application-level renegotiation request into a no-op, instead of
>> breaking apps by making it fail --- but apparently they were not done
>> properly.

> Is there anything remaining to do on this issue?

I'm not sure. My impression is that by the time we had anything in the
field, there will be real fixes for the SSL renegotiation problem.
So all we'd be accomplishing is to weaken security for people who have
those fixes, to cater to people who are using copies of openssl they'd
obtained in the past couple of months and then not updated to latest.
However, if anyone thinks that the SSL problem isn't going to get fixed
promptly, maybe it needs more consideration.

regards, tom lane

2010/2/20 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> Tom Lane wrote:
>>> Chris Campbell <chris_campbell(at)mac(dot)com> writes:
>>>> Is there a way to detect when the SSL library has renegotiation disabled?
>>>
>>> Probably not.  The current set of emergency security patches would
>>> certainly not have exposed any new API that would help us tell this :-(
>>>
>>> If said patches were done properly they'd have also turned an
>>> application-level renegotiation request into a no-op, instead of
>>> breaking apps by making it fail --- but apparently they were not done
>>> properly.
>
>> Is there anything remaining to do on this issue?
>
> I'm not sure.  My impression is that by the time we had anything in the
> field, there will be real fixes for the SSL renegotiation problem.
> So all we'd be accomplishing is to weaken security for people who have
> those fixes, to cater to people who are using copies of openssl they'd
> obtained in the past couple of months and then not updated to latest.
> However, if anyone thinks that the SSL problem isn't going to get fixed
> promptly, maybe it needs more consideration.

The problem with this is, I think, that there are "semi-patched"
versions of OpenSSL out there, that simply *break*. Instead of not
doing renegotiation when we ask for it, they break it and return an
error, thus canceling the connection. Correct?

If so, shouldn't we try to disable renegotiation for all versions
*before* it was properly fixed?

Which today means all versions released. The proper fix is in 0.9.8m,
which is currently in beta. At least that's my understanding.

The way I read it, the code now does:
* If the user has an old version of openssl, exposes the user to the
security issue and makes the connection insecure
* OpenSSL 0.9.8l, breaks the connection whenever renegotiation
happens, but doesn't compromise security
* OpenSSL 0.9.8m-beta, does the right thing.

Do we have any idea of what people like RHEL are doing wrt
backpatching these things?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> If so, shouldn't we try to disable renegotiation for all versions
> *before* it was properly fixed?

If we could tell that, sure. But I don't believe there is any way to
identify whether a given installation of openssl has this patched.
Please don't suggest looking at the version number --- Red Hat and
other vendors are in the habit of back-patching security fixes without
changing the version number.

> Which today means all versions released. The proper fix is in 0.9.8m,
> which is currently in beta. At least that's my understanding.

Red Hat's already shipping the patch. Dunno about other vendors.

The real bottom line here is that this isn't our bug. It's unfortunate
that we're affected by it, but that doesn't mean that we should be
installing kluges to work around it.

regards, tom lane

2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> If so, shouldn't we try to disable renegotiation for all versions
>> *before* it was properly fixed?
>
> If we could tell that, sure.  But I don't believe there is any way to
> identify whether a given installation of openssl has this patched.
> Please don't suggest looking at the version number --- Red Hat and
> other vendors are in the habit of back-patching security fixes without
> changing the version number.

That, if anything, is a bug :( But yes, it's a bug lots of linux
distros seem to consider a feature :(

>> Which today means all versions released. The proper fix is in 0.9.8m,
>> which is currently in beta. At least that's my understanding.
>
> Red Hat's already shipping the patch.  Dunno about other vendors.

Which patch? The one that breaks it, or the one that changes the protocol?

> The real bottom line here is that this isn't our bug.  It's unfortunate
> that we're affected by it, but that doesn't mean that we should be
> installing kluges to work around it.

True. But people will call it our problem.

One way to deal with it would be to expose the whole renegotiation
setting as a user configuratble option. So they can set *when* we
renegotiate, which would also let them turn it off completely. There
are probably people who would like to change it, but there certainly
haven't been enough of them so we've heard lots of complains. And it's
definitely not back-patchable.

We also have to consider our Windows users, where *we* ship the
OpenSSL library. Where there is no library we can ship right now that
fixes it.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> Red Hat's already shipping the patch. Dunno about other vendors.

> Which patch? The one that breaks it, or the one that changes the protocol?

The one with the protocol change.

I think we already missed the window where it would have been sensible
to install a hack workaround for this. If we'd done that in November
it might have been reasonable, but by now it's too late for any hack
we install to spread much faster than fixed openssl libraries.

> One way to deal with it would be to expose the whole renegotiation
> setting as a user configuratble option. So they can set *when* we
> renegotiate, which would also let them turn it off completely.

Well, that might be a reasonable thing to do, because it's not just a
temporary kluge (that we won't know when to remove) but is adding an
arguably-useful-in-other-ways knob.

> And it's definitely not back-patchable.

Why not? We certainly wouldn't back-patch such a thing if we didn't
have a problem to deal with, but it's not like there's no precedent
for adding back-patched GUCs in response to security issues. We
did that with backslash_quote.

regards, tom lane

On Feb 22, 2010, at 12:25 PM, Tom Lane wrote:

> I think we already missed the window where it would have been sensible
> to install a hack workaround for this. If we'd done that in November
> it might have been reasonable, but by now it's too late for any hack
> we install to spread much faster than fixed openssl libraries.

Could we simply ignore renegotiation errors? Or change them to warnings? That may enable us to work with the semi-fixed OpenSSL libraries that are currently in the field, without disabling the functionality altogether.

- Chris

On Mon, 22 Feb 2010 12:25:08 -0500, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> Red Hat's already shipping the patch.  Dunno about other vendors.
>
>> Which patch? The one that breaks it, or the one that changes the
>> protocol?
>
> The one with the protocol change.
>
> I think we already missed the window where it would have been sensible
> to install a hack workaround for this. If we'd done that in November
> it might have been reasonable, but by now it's too late for any hack
> we install to spread much faster than fixed openssl libraries.

Perhaps I am missing something here but as it is not our bug but is a
known bug, why can't we just say:

ERROR: SSL FATAL: Renogiation failed. Check OpenSSL bug list

Yes the wording is miserable, change it but the point I think is clear.

I think it is completely reasonable to have warnings or errors that point
to other areas. If nothing else when it comes to our list we can say, "What
is the error message you get" and then we say,
"Did you check the OpenSSL bug list?".

I have to do similar things with PITRTools because of various unknown but
possibly successful states (like files changing underneath rsync).

Joshua D. Drake

--
PostgreSQL - XMPP: jdrake(at)jabber(dot)postgresql(dot)org
Consulting, Development, Support, Training
503-667-4564 - http://www.commandprompt.com/
The PostgreSQL Company, serving since 1997

On Mon, 22 Feb 2010 18:00:33 +0100, Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

> We also have to consider our Windows users, where *we* ship the
> OpenSSL library. Where there is no library we can ship right now that
> fixes it.

We do? I mean I know that we provide the old 8.2/8.3 pginstaller, but EDB
is the provider of w32 binaries, not the community.

Joshua D. Drake

--
PostgreSQL - XMPP: jdrake(at)jabber(dot)postgresql(dot)org
Consulting, Development, Support, Training
503-667-4564 - http://www.commandprompt.com/
The PostgreSQL Company, serving since 1997

2010/2/22 Joshua D. Drake <jd(at)commandprompt(dot)com>:
> On Mon, 22 Feb 2010 18:00:33 +0100, Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
>
>> We also have to consider our Windows users, where *we* ship the
>> OpenSSL library. Where there is no library we can ship right now that
>> fixes it.
>
> We do? I mean I know that we provide the old 8.2/8.3 pginstaller, but EDB
> is the provider of w32 binaries, not the community.

How does that change the fact even a tiny bit for the end user?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Mon, 2010-02-22 at 18:45 +0100, Magnus Hagander wrote:
> 2010/2/22 Joshua D. Drake <jd(at)commandprompt(dot)com>:
> > On Mon, 22 Feb 2010 18:00:33 +0100, Magnus Hagander <magnus(at)hagander(dot)net>
> > wrote:
> >
> >> We also have to consider our Windows users, where *we* ship the
> >> OpenSSL library. Where there is no library we can ship right now that
> >> fixes it.
> >
> > We do? I mean I know that we provide the old 8.2/8.3 pginstaller, but EDB
> > is the provider of w32 binaries, not the community.
>
> How does that change the fact even a tiny bit for the end user?

Only that EDB may chose to put in there own solution.

Joshua D. Drake

--
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 503.667.4564
Consulting, Training, Support, Custom Development, Engineering
Respect is earned, not gained through arbitrary and repetitive use or Mr. or Sir.

On 2/22/10, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
> On Mon, 2010-02-22 at 18:45 +0100, Magnus Hagander wrote:
>> 2010/2/22 Joshua D. Drake <jd(at)commandprompt(dot)com>:
>> > On Mon, 22 Feb 2010 18:00:33 +0100, Magnus Hagander
>> > <magnus(at)hagander(dot)net>
>> > wrote:
>> >
>> >> We also have to consider our Windows users, where *we* ship the
>> >> OpenSSL library. Where there is no library we can ship right now that
>> >> fixes it.
>> >
>> > We do? I mean I know that we provide the old 8.2/8.3 pginstaller, but
>> > EDB
>> > is the provider of w32 binaries, not the community.
>>
>> How does that change the fact even a tiny bit for the end user?
>
> Only that EDB may chose to put in there own

We're certainly not going to be shipping anything but standard
PostgreSQL, and i don't have the cycles or energy to try building
bespoke versions of openssl.

2010/2/22 Chris Campbell <chris_campbell(at)mac(dot)com>:
> On Feb 22, 2010, at 12:25 PM, Tom Lane wrote:
>
>> I think we already missed the window where it would have been sensible
>> to install a hack workaround for this.  If we'd done that in November
>> it might have been reasonable, but by now it's too late for any hack
>> we install to spread much faster than fixed openssl libraries.
>
> Could we simply ignore renegotiation errors? Or change them to warnings? That may enable us to work with the semi-fixed OpenSSL libraries that are currently in the field, without disabling the functionality altogether.

I guess we could, but if we do that then we've opened a window where
someone can attack us if we *have* a properly working openssl, haven't
we?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> Red Hat's already shipping the patch.  Dunno about other vendors.
>
>> Which patch? The one that breaks it, or the one that changes the protocol?
>
> The one with the protocol change.

Ok. If RedHat has done it, I think we're in reasonably good shape.
>From what I can tell, Debian doesn't have the broken *or* non-broken
patch in, but I'm not certain.

> I think we already missed the window where it would have been sensible
> to install a hack workaround for this.  If we'd done that in November
> it might have been reasonable, but by now it's too late for any hack
> we install to spread much faster than fixed openssl libraries.

Yeah, seems so.

>> One way to deal with it would be to expose the whole renegotiation
>> setting as a user configuratble option. So they can set *when* we
>> renegotiate, which would also let them turn it off completely.
>
> Well, that might be a reasonable thing to do, because it's not just a
> temporary kluge (that we won't know when to remove) but is adding an
> arguably-useful-in-other-ways knob.

Yeah, the question is, how useful is it?

>> And it's definitely not back-patchable.
>
> Why not?  We certainly wouldn't back-patch such a thing if we didn't
> have a problem to deal with, but it's not like there's no precedent
> for adding back-patched GUCs in response to security issues.  We
> did that with backslash_quote.

Hmm, I guess. It's a new feature, but if it's necessary..

That would take care of things on Windows as well. We could then just
disable renegotiation when we ship the known broken binaries.

You'd still have to turn it off on the server side if you have a
*single* client that has the broken patch, but that's still a lot
better than nothing.

Think it's worth taking a stab at?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>> One way to deal with it would be to expose the whole renegotiation
>>> setting as a user configuratble option. So they can set *when* we
>>> renegotiate, which would also let them turn it off completely.
>>
>> Well, that might be a reasonable thing to do, because it's not just a
>> temporary kluge (that we won't know when to remove) but is adding an
>> arguably-useful-in-other-ways knob.

> You'd still have to turn it off on the server side if you have a
> *single* client that has the broken patch, but that's still a lot
> better than nothing.

Well, if it's a GUC it can be set per-user or per-database, so there's
at least some hope of not having to turn it off for everyone.

> Think it's worth taking a stab at?

If you want to do it, I'd be fine with it.

regards, tom lane

Tom Lane wrote:
>>>> One way to deal with it would be to expose the whole renegotiation
>>>> setting as a user configuratble option. So they can set *when* we
>>>> renegotiate, which would also let them turn it off completely.
>>>
>>> Well, that might be a reasonable thing to do, because it's not just a
>>> temporary kluge (that we won't know when to remove) but is adding an
>>> arguably-useful-in-other-ways knob.
>
>> You'd still have to turn it off on the server side if you have a
>> *single* client that has the broken patch, but that's still a lot
>> better than nothing.
>
> Well, if it's a GUC it can be set per-user or per-database, so there's
> at least some hope of not having to turn it off for everyone.
>
> > Think it's worth taking a stab at?
>
> If you want to do it, I'd be fine with it.

+1

That would help me with a different problem:
SSL renegotiation is broken with Npgsql, the cause is Bug 321325
in the Mono security library
https://bugzilla.novell.com/show_bug.cgi?id=321325
which does not look like it is ever going to be fixed.

Up to now I have crippled SSL renegotiation in our servers with a patch,
because I figured that bad SSL is better than no SSL.

Yours,
Laurenz Albe

2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> You'd still have to turn it off on the server side if you have a
>> *single* client that has the broken patch, but that's still a lot
>> better than nothing.
>
> Well, if it's a GUC it can be set per-user or per-database, so there's
> at least some hope of not having to turn it off for everyone.
>
>> Think it's worth taking a stab at?
>
> If you want to do it, I'd be fine with it.

Seems easy enough, see attached. Comments?

This version is set to superuser only. It's a security related
feature, so just letting a random user turn it off may be seen as
wrong. On the other hand, this is just about the connection security,
and if we have a malicious user on the other end, he can do a lot
worse things than disable renegotiation (such as resending the
plaintext after it's been decrypted).

I'd therefore suggest we make it USERSET. Anything wrong in that discussion?

(That would also for example allow npgsql to always set it to 0, if
it's known to be broken)

Also, do we want to add a specific <note> to the documentation saying
this is the way around broken SSL libraries? Or leave that to release
notes? Or just leave it to the mailinglist archives?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

2010/2/23 Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>:
> Tom Lane wrote:
>>>>> One way to deal with it would be to expose the whole renegotiation
>>>>> setting as a user configuratble option. So they can set *when* we
>>>>> renegotiate, which would also let them turn it off completely.
>>>>
>>>> Well, that might be a reasonable thing to do, because it's not just a
>>>> temporary kluge (that we won't know when to remove) but is adding an
>>>> arguably-useful-in-other-ways knob.
>>
>>> You'd still have to turn it off on the server side if you have a
>>> *single* client that has the broken patch, but that's still a lot
>>> better than nothing.
>>
>> Well, if it's a GUC it can be set per-user or per-database, so there's
>> at least some hope of not having to turn it off for everyone.
>>
>> > Think it's worth taking a stab at?
>>
>> If you want to do it, I'd be fine with it.
>
> +1
>
> That would help me with a different problem:
> SSL renegotiation is broken with Npgsql, the cause is Bug 321325
> in the Mono security library
> https://bugzilla.novell.com/show_bug.cgi?id=321325
> which does not look like it is ever going to be fixed.

*ouch*

> Up to now I have crippled SSL renegotiation in our servers with a patch,
> because I figured that bad SSL is better than no SSL.

Given the major security hole in the whole project, SSL without
renegotiation was a *lot* more secure than SSL *with* renegotiation,
until very recently :-) But patching the server is always annoying...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> If you want to do it, I'd be fine with it.

> Seems easy enough, see attached. Comments?

Looks mostly okay to me, a few notes:

+ if (ssl_renegotiation_limit && port->count > ssl_renegotiation_limit * 1024)

You need "1024L" there to avoid risk of integer overflow when long is
wider than int --- MAX_KILOBYTES is set on the assumption that any such
multiplies will be done in long arithmetic. It doesn't matter that
count will never be wider than int, it can still get the wrong answer.

Also, the comment attached to ssl_renegotiation_limit needs to be fixed
to mention that zero disables renegotiation.

Also, the coding seems a bit confused about whether the
ssl_renegotiation_limit GUC exists when USE_SSL isn't set. I think we
have a project policy about whether GUCs should still exist when the
underlying support isn't compiled, but I forget what it is :-(.
Anyway you need to test that the patch compiles with USE_SSL off.

Also the xreflabel for the variable in the docs isn't consistent,
and you failed to mention the default value.

> This version is set to superuser only. It's a security related
> feature, so just letting a random user turn it off may be seen as
> wrong. On the other hand, this is just about the connection security,
> and if we have a malicious user on the other end, he can do a lot
> worse things than disable renegotiation (such as resending the
> plaintext after it's been decrypted).

> I'd therefore suggest we make it USERSET. Anything wrong in that discussion?

SUSET seems less surprising to me. I agree that it's hard to make
a concrete case for a user doing anything terribly bad with it,
but on the other hand is there much value in letting it be USERSET?
Anyway it's not very important to me either way.

> Also, do we want to add a specific <note> to the documentation saying
> this is the way around broken SSL libraries? Or leave that to release
> notes? Or just leave it to the mailinglist archives?

I think a short note in the description of the variable wouldn't be out
of place.

regards, tom lane

2010/2/24 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> 2010/2/22 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> If you want to do it, I'd be fine with it.
>
>> Seems easy enough, see attached. Comments?
>
> Looks mostly okay to me, a few notes:
>
> +               if (ssl_renegotiation_limit && port->count > ssl_renegotiation_limit * 1024)

> You need "1024L" there to avoid risk of integer overflow when long is

Check.

> Also, the comment attached to ssl_renegotiation_limit needs to be fixed
> to mention that zero disables renegotiation.

Check.

> Also, the coding seems a bit confused about whether the
> ssl_renegotiation_limit GUC exists when USE_SSL isn't set.  I think we
> have a project policy about whether GUCs should still exist when the
> underlying support isn't compiled, but I forget what it is :-(.
> Anyway you need to test that the patch compiles with USE_SSL off.

Yeah, that's clearly wrong. The extern is there, but not the
definition. Fixed, and will test.

I personally find it highly annoying when a GUC goes away, so I'm all
for always having them there. And I thought that was our policy for
new ones, but I can't find a reference to it...

> Also the xreflabel for the variable in the docs isn't consistent,
> and you failed to mention the default value.

You mean add _limit to it, right?

I looked at the parameter next to it, and none of them include the
default value in the description. But I see now that's the exception,
rather than the rule. Fixed.

>> This version is set to superuser only. It's a security related
>> feature, so just letting a random user turn it off may be seen as
>> wrong. On the other hand, this is just about the connection security,
>> and if we have a malicious user on the other end, he can do a lot
>> worse things than disable renegotiation (such as resending the
>> plaintext after it's been decrypted).
>
>> I'd therefore suggest we make it USERSET. Anything wrong in that discussion?
>
> SUSET seems less surprising to me.  I agree that it's hard to make
> a concrete case for a user doing anything terribly bad with it,
> but on the other hand is there much value in letting it be USERSET?
> Anyway it's not very important to me either way.

The use case would be for example npgsql (or npgsql clients) being
able to disable it from the client side, because they know they can't
deal with it. Even in the case that the server doesn't know that.

>> Also, do we want to add a specific <note> to the documentation saying
>> this is the way around broken SSL libraries? Or leave that to release
>> notes? Or just leave it to the mailinglist archives?
>
> I think a short note in the description of the variable wouldn't be out
> of place.

Ok, will add.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> 2010/2/24 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>> Also, the coding seems a bit confused about whether the
>> ssl_renegotiation_limit GUC exists when USE_SSL isn't set. I think we
>> have a project policy about whether GUCs should still exist when the
>> underlying support isn't compiled, but I forget what it is :-(.

> I personally find it highly annoying when a GUC goes away, so I'm all
> for always having them there. And I thought that was our policy for
> new ones, but I can't find a reference to it...

I see that ssl_ciphers is made to go away when USE_SSL isn't set,
so the most consistent thing in the near term would be to do the same.
Revisiting the whole issue seems like not material for back-patching.

>> Also the xreflabel for the variable in the docs isn't consistent,

> You mean add _limit to it, right?

Right.

>> SUSET seems less surprising to me. I agree that it's hard to make
>> a concrete case for a user doing anything terribly bad with it,
>> but on the other hand is there much value in letting it be USERSET?

> The use case would be for example npgsql (or npgsql clients) being
> able to disable it from the client side, because they know they can't
> deal with it. Even in the case that the server doesn't know that.

Fair enough, USERSET it is then.

regards, tom lane

On Wed, Feb 24, 2010 at 17:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> 2010/2/24 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>> Also, the coding seems a bit confused about whether the
>>> ssl_renegotiation_limit GUC exists when USE_SSL isn't set.  I think we
>>> have a project policy about whether GUCs should still exist when the
>>> underlying support isn't compiled, but I forget what it is :-(.
>
>> I personally find it highly annoying when a GUC goes away, so I'm all
>> for always having them there. And I thought that was our policy for
>> new ones, but I can't find a reference to it...
>
> I see that ssl_ciphers is made to go away when USE_SSL isn't set,
> so the most consistent thing in the near term would be to do the same.

The difference is that ssl_ciphers is only set in postgresql.conf, so
it doesn't have the same exposure. I can certainly see a use-case
where a naive application will just disable ssl renegotiation because
it knows it can't deal with it (or the driver can't) uncondinionally -
but the use of SSL or not is controlled by the server at the other end
of the connection. Not failing then would be good..

> Revisiting the whole issue seems like not material for back-patching.

Is this something we should consider looking over for 9.0,or is it too
late already? (For other parameters, that is - a check of all the ones
we have that are #ifdef:ed out today, to see if they can be made
available even when the support isn't compiled in)

>>> SUSET seems less surprising to me.  I agree that it's hard to make
>>> a concrete case for a user doing anything terribly bad with it,
>>> but on the other hand is there much value in letting it be USERSET?
>
>> The use case would be for example npgsql (or npgsql clients) being
>> able to disable it from the client side, because they know they can't
>> deal with it. Even in the case that the server doesn't know that.
>
> Fair enough, USERSET it is then.

Done. Will run some tests and then apply.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Thu, Feb 25, 2010 at 10:42, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Wed, Feb 24, 2010 at 17:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> Fair enough, USERSET it is then.
>
> Done. Will run some tests and then apply.

And applied.

I backpatched it to 8.2, which is as far as it applied fairly cleanly.
Before that, we don't have GUC_UNIT_KB for example, so it'll be a
different format of the patch as well. I'm not sure it's important
enough to go back beyond that...

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Wed, Feb 24, 2010 at 17:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I see that ssl_ciphers is made to go away when USE_SSL isn't set,
>> so the most consistent thing in the near term would be to do the same.

> The difference is that ssl_ciphers is only set in postgresql.conf, so
> it doesn't have the same exposure. I can certainly see a use-case
> where a naive application will just disable ssl renegotiation because
> it knows it can't deal with it (or the driver can't) uncondinionally -
> but the use of SSL or not is controlled by the server at the other end
> of the connection. Not failing then would be good..

Hm, well, surely the client ought to know if the connection is actually
SSL or not. But it's not important enough to argue about.

>> Revisiting the whole issue seems like not material for back-patching.

> Is this something we should consider looking over for 9.0,or is it too
> late already? (For other parameters, that is - a check of all the ones
> we have that are #ifdef:ed out today, to see if they can be made
> available even when the support isn't compiled in)

I don't think it's appropriate to worry about it right now. We have
bigger issues to deal with.

regards, tom lane

On Thu, Feb 25, 2010 at 15:27, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Wed, Feb 24, 2010 at 17:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I see that ssl_ciphers is made to go away when USE_SSL isn't set,
>>> so the most consistent thing in the near term would be to do the same.
>
>> The difference is that ssl_ciphers is only set in postgresql.conf, so
>> it doesn't have the same exposure. I can certainly see a use-case
>> where a naive application will just disable ssl renegotiation because
>> it knows it can't deal with it (or the driver can't) uncondinionally -
>> but the use of SSL or not is controlled by the server at the other end
>> of the connection. Not failing then would be good..
>
> Hm, well, surely the client ought to know if the connection is actually
> SSL or not.  But it's not important enough to argue about.

You'd think so, but our entire setup of SSL on the client is designed
on the assumption that it doesn't ;)

>>> Revisiting the whole issue seems like not material for back-patching.
>
>> Is this something we should consider looking over for 9.0,or is it too
>> late already? (For other parameters, that is - a check of all the ones
>> we have that are #ifdef:ed out today, to see if they can be made
>> available even when the support isn't compiled in)
>
> I don't think it's appropriate to worry about it right now.  We have
> bigger issues to deal with.

Agreed.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> I backpatched it to 8.2, which is as far as it applied fairly cleanly.
> Before that, we don't have GUC_UNIT_KB for example, so it'll be a
> different format of the patch as well. I'm not sure it's important
> enough to go back beyond that...

Hm, I'd kinda like to have it in 8.1 at least, since I'm on the hook for
support of 8.1 in RHEL5. If you don't feel like messing with a further
back-patch I can take a look.

regards, tom lane