Horton

Thread

Commits

  1. Always use 2048 bit DH parameters for OpenSSL ephemeral DH ciphers.

  1. PostgreSQL - Weak DH group

    Nicolas Guini <nicolasguini@gmail.com> — 2016-10-05T14:15:34Z

    Hello everyone,

I sent few days ago to the security DL a mail reporting a vulnerability in
how Postgres is requesting DH params to be used later for encryption
algorithms. So, due to there is no problem sharing with this group, here is
what I sent:

------------------------------------------------------------------------------------------------------------------------------------------
 Hi folks,



                We are working with Postgres 9.3.14 and executing nmap we
found that it is using “weak DH group” (nmap –script ssl-dh-params). Weak =
1024 bits.

                See nmap output (1)



                We don’t know if other versions are affected or not. The
environment used is a RHEL 6 x86_6, OpenSSL version 1.0.2i with FIPS module.

    This issue is similar to what this post explains about using weak DH
parameters: http://www.usefuljs.net/2016/09/29/imperfect-forward-secrecy/



                Following with the code, it seems that PostgreSQL has
missed the keyLength OpenSSL parameter, and it delivers into a weak crypto
configuration.. Affected Code:

https://git.postgresql.org/gitweb/?p=postgresql.git;a=
blob;f=src/backend/libpq/be-secure-openssl.c;h=
8d8f12952a4a4f14a15f8647b96935e13d68fb39;hb=48d50840d53eb62842c0d9b54eab9c
d7c9a3a46d



                (Thanks to Damian in order to found the affected code)




(1) nmap output:


# nmap –script ssl-dh-params -p 5432 <ip>


Starting Nmap 7.25BETA2 ( https://nmap.org )

Nmap scan report for <ip>

Host is up (0.00035s latency).

PORT     STATE SERVICE

5432/tcp open  postgresql

| ssl-dh-params:

|   VULNERABLE:

|   Diffie-Hellman Key Exchange Insufficient Group Strength

|     State: VULNERABLE

|       Transport Layer Security (TLS) services that use Diffie-Hellman
groups

|       of insufficient strength, especially those using one of a few
commonly

|       shared groups, may be susceptible to passive eavesdropping attacks.

|     Check results:

|       WEAK DH GROUP 1

|             Cipher Suite: TLS_DHE_RSA_WITH_AES_128_GCM_SHA256

|             Modulus Type: Safe prime

|             Modulus Source: Unknown/Custom-generated

|             Modulus Length: 1024

|             Generator Length: 8

|             Public Key Length: 1024

|     References:

|_      https://weakdh.org



------------------------------------------------------------------------------------------------------------------------------------------




Thanks in advance

Nicolas Guini

  2. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2016-10-05T18:57:26Z

    On 10/05/2016 05:15 PM, Nicolas Guini wrote:
>                 We are working with Postgres 9.3.14 and executing nmap we
> found that it is using “weak DH group” (nmap –script ssl-dh-params). Weak =
> 1024 bits.

Yeah, it seems that we're a bit behind the times on this...

>     This issue is similar to what this post explains about using weak DH
> parameters: http://www.usefuljs.net/2016/09/29/imperfect-forward-secrecy/

The blog post points out that, as counterintuitive as it sounds, the 
SSL_CTX_set_tmp_dh_callback() callback should ignore the keylength 
argument, and always return a DH group with 2048 bits, or stronger. As 
you pointed out, that's not what our callback does.

We should fix this in master, at least. I'm not sure about backporting, 
there might be compatibility issues. It seems that at least OpenJDK 
(Java) didn't support DH groups larger than 1024 bits, until version 8. 
That's fairly recent, OpenJDK 8 was released in March 2014.

ECDHE family of ciphers are not affected, and are preferred over plain 
DHE, I believe, so disabling DHE and removing the DH parameter loading 
code altogether is one option. Clearly not backportable, though.

Meanwhile, users can work-around this by creating DH parameters with 
something like "openssl dhparam -out $PGDATA/dh1024.pem 2048". Yes, the 
file needs to be called "dh1024.pem", even though the actual key length 
is 2048 bits.

- Heikki

  3. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2016-10-06T13:52:13Z

    On 10/05/2016 09:57 PM, Heikki Linnakangas wrote:
> On 10/05/2016 05:15 PM, Nicolas Guini wrote:
>>                 We are working with Postgres 9.3.14 and executing nmap we
>> found that it is using “weak DH group” (nmap –script ssl-dh-params). Weak =
>> 1024 bits.
>
> Yeah, it seems that we're a bit behind the times on this...
>
>>     This issue is similar to what this post explains about using weak DH
>> parameters: http://www.usefuljs.net/2016/09/29/imperfect-forward-secrecy/
>
> The blog post points out that, as counterintuitive as it sounds, the
> SSL_CTX_set_tmp_dh_callback() callback should ignore the keylength
> argument, and always return a DH group with 2048 bits, or stronger. As
> you pointed out, that's not what our callback does.

I propose the attached patch. It gives up on trying to deal with 
multiple key lengths (as noted earlier, OpenSSL just always passed 
keylength=1024, so that was useless). Instead of using the callback, it 
just sets fixed DH parameters with SSL_CTX_set_tmp_dh(), like we do for 
the ECDH curve. The DH parameters are loaded from a file called 
"dh_params.pem" (instead of "dh1024.pem"), if present, otherwise the 
built-in 2048 bit parameters are used.

I removed the code for generating DH parameters on-the-fly altogether. 
The OpenSSL man page clearly says that that should not be done, because 
generating the parameters takes a long time. And because OpenSSL always 
passed keylength=1024, AFAICS that had been always dead code.

If we want to get really fancy, we could generate the parameters the 
first time you turn ssl=on, and store them in $PGDATA. But the 
generation takes a very long time, so the admin might think it's stuck.

I note that supplying custom DH parameters in the file is completely 
undocumented :-(. We should add a note in the docs on how to generate 
the custom DH parameters with openssl. Also, there's no easy way of 
telling if your custom parameters are actually been used. If you copy 
the params file in $PGDATA, but you have a typo in the name, you won't 
notice. Perhaps we should print a line in the log when the file is loaded.

I'm afraid of back-patching this, out of fear that older clients would 
stop working, or would downgrade to an even weaker cipher. You could fix 
it by putting weaker 1024 bit params in dh_params.pem, so maybe we could 
do it if we put a warning and instructions for doing that in the release 
notes. Thoughts?

- Heikki

  4. Re: PostgreSQL - Weak DH group

    Christoph Berg <myon@debian.org> — 2016-10-06T19:26:42Z

    Re: Heikki Linnakangas 2016-10-06 <fd6eb3cc-1585-1469-fd9e-763f8e410b19@iki.fi>
> I propose the attached patch. It gives up on trying to deal with multiple
> key lengths (as noted earlier, OpenSSL just always passed keylength=1024, so
> that was useless). Instead of using the callback, it just sets fixed DH
> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The DH
> parameters are loaded from a file called "dh_params.pem" (instead of
> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters are
> used.

Shouldn't this be a GUC pointing to a configurable location like
ssl_cert_file? This way, people reading the security section of the
default postgresql.conf would notice that there's something (new) to
configure. (And I wouldn't want to start creating symlinks from PGDATA
to /etc/ssl/something again...)

Thanks,
Christoph

  5. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2016-10-10T15:24:23Z

    On 10/06/2016 10:26 PM, Christoph Berg wrote:
> Re: Heikki Linnakangas 2016-10-06 <fd6eb3cc-1585-1469-fd9e-763f8e410b19@iki.fi>
>> I propose the attached patch. It gives up on trying to deal with multiple
>> key lengths (as noted earlier, OpenSSL just always passed keylength=1024, so
>> that was useless). Instead of using the callback, it just sets fixed DH
>> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The DH
>> parameters are loaded from a file called "dh_params.pem" (instead of
>> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters are
>> used.
>
> Shouldn't this be a GUC pointing to a configurable location like
> ssl_cert_file? This way, people reading the security section of the
> default postgresql.conf would notice that there's something (new) to
> configure. (And I wouldn't want to start creating symlinks from PGDATA
> to /etc/ssl/something again...)

Perhaps. The DH parameters are not quite like other configuration files, 
though. The actual parameters used don't matter, you just want to 
generate different parameters for every cluster. The key length of the 
parameters can be considered configuration, though.

- Heikki

  6. Re: PostgreSQL - Weak DH group

    Oskari Saarenmaa <os@ohmu.fi> — 2016-10-12T07:29:04Z

    06.10.2016, 16:52, Heikki Linnakangas kirjoitti:
> I propose the attached patch. It gives up on trying to deal with
> multiple key lengths (as noted earlier, OpenSSL just always passed
> keylength=1024, so that was useless). Instead of using the callback, it
> just sets fixed DH parameters with SSL_CTX_set_tmp_dh(), like we do for
> the ECDH curve. The DH parameters are loaded from a file called
> "dh_params.pem" (instead of "dh1024.pem"), if present, otherwise the
> built-in 2048 bit parameters are used.

We've been using the same built-in parameters for 14 years now, they 
apparently came from 
https://web.archive.org/web/20011212141438/http://www.skip-vpn.org/spec/numbers.html 
(the original page is no longer available) and are shared by countless 
other systems.

While we're not using the most common Oakley groups which are presumed 
to have been broken by various parties (https://weakdh.org) I think it'd 
be worthwhile to replace the currently built-in parameters with custom 
ones.  And maybe even regenerate parameters for every minor release.

HAProxy made a similar change last year, see 
https://github.com/haproxy/haproxy/commit/d3a341a96fb6107d2b8e3d7a9c0afa2ff43bb0b6

/ Oskari

  7. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-13T15:32:23Z

    (We dropped the ball back in October, continuing the discussion now)

On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:
> On 10/06/2016 10:26 PM, Christoph Berg wrote:
>> Re: Heikki Linnakangas 2016-10-06 <fd6eb3cc-1585-1469-fd9e-763f8e410b19@iki.fi>
>>> I propose the attached patch. It gives up on trying to deal with multiple
>>> key lengths (as noted earlier, OpenSSL just always passed keylength=1024, so
>>> that was useless). Instead of using the callback, it just sets fixed DH
>>> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The DH
>>> parameters are loaded from a file called "dh_params.pem" (instead of
>>> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters are
>>> used.
>>
>> Shouldn't this be a GUC pointing to a configurable location like
>> ssl_cert_file? This way, people reading the security section of the
>> default postgresql.conf would notice that there's something (new) to
>> configure. (And I wouldn't want to start creating symlinks from PGDATA
>> to /etc/ssl/something again...)
>
> Perhaps. The DH parameters are not quite like other configuration files,
> though. The actual parameters used don't matter, you just want to
> generate different parameters for every cluster. The key length of the
> parameters can be considered configuration, though.

Actually, adding a GUC also solves another grief I had about this. There 
is currently no easy way to tell if your parameter file is being used, 
or if the server is failing to read it and is falling back to the 
hard-coded parameters. With a GUC, if the GUC is set it's a good 
indication that the admin expects the file to really exist and to 
contain valid parameters. So if the GUC is set, we should throw an error 
if it cannot be used. And if it's not set, we use the built-in defaults.

I rebased the patch, did some other clean up of error reporting, and 
added a GUC along those lines, as well as docs. How does this look?

It's late in the release cycle, but it would be nice to sneak this into 
v10. Using weak 1024 bit DH parameters is arguably a security issue; it 
was originally reported as such. There's a work-around for older 
versions: generate custom 2048 bit parameters and place them in a file 
called "dh1024.pem", but that's completely undocumented.

Thoughts? Objections to committing this now, instead of waiting for v11?

- Heikki

  8. Re: PostgreSQL - Weak DH group

    Michael Paquier <michael.paquier@gmail.com> — 2017-07-13T16:43:53Z

    On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> I rebased the patch, did some other clean up of error reporting, and added a
> GUC along those lines, as well as docs. How does this look?
>
> It's late in the release cycle, but it would be nice to sneak this into v10.
> Using weak 1024 bit DH parameters is arguably a security issue; it was
> originally reported as such. There's a work-around for older versions:
> generate custom 2048 bit parameters and place them in a file called
> "dh1024.pem", but that's completely undocumented.
>
> Thoughts?

The patch looks in good shape to me.

 #include "utils/memutils.h"

-
 static int    my_sock_read(BIO *h, char *buf, int size);
That's unnecessary noise.

+ *    Very uncool. Alternatively, the system could refuse to start
+ *    if a DH parameters if not specified, but this would tend to
+ *    piss off DBAs.
"is not specified".

> Objections to committing this now, instead of waiting for v11?

But I am -1 for the sneak part. It is not the time to have a new
feature in 10, the focus is to stabilize.
-- 
Michael

  9. Re: PostgreSQL - Weak DH group

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2017-07-13T17:04:02Z

    Michael Paquier wrote:
> On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

> > Objections to committing this now, instead of waiting for v11?
> 
> But I am -1 for the sneak part. It is not the time to have a new
> feature in 10, the focus is to stabilize.

But if we were treating it as a security issue, would we backpatch it?
If we do, then it definitely makes sense to put something in pg10.  I'm
not sure that this patch is it, though -- perhaps it makes sense to put
a minimal fix in older branches, and let the new feature wait for pg11?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  10. Re: PostgreSQL - Weak DH group

    Simon Riggs <simon@2ndquadrant.com> — 2017-07-13T17:07:17Z

    On 13 July 2017 at 16:32, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> (We dropped the ball back in October, continuing the discussion now)
>
> On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:
>>
>> On 10/06/2016 10:26 PM, Christoph Berg wrote:
>>>
>>> Re: Heikki Linnakangas 2016-10-06
>>> <fd6eb3cc-1585-1469-fd9e-763f8e410b19@iki.fi>
>>>>
>>>> I propose the attached patch. It gives up on trying to deal with
>>>> multiple
>>>> key lengths (as noted earlier, OpenSSL just always passed
>>>> keylength=1024, so
>>>> that was useless). Instead of using the callback, it just sets fixed DH
>>>> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The
>>>> DH
>>>> parameters are loaded from a file called "dh_params.pem" (instead of
>>>> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters
>>>> are
>>>> used.
>>>
>>>
>>> Shouldn't this be a GUC pointing to a configurable location like
>>> ssl_cert_file? This way, people reading the security section of the
>>> default postgresql.conf would notice that there's something (new) to
>>> configure. (And I wouldn't want to start creating symlinks from PGDATA
>>> to /etc/ssl/something again...)
>>
>>
>> Perhaps. The DH parameters are not quite like other configuration files,
>> though. The actual parameters used don't matter, you just want to
>> generate different parameters for every cluster. The key length of the
>> parameters can be considered configuration, though.
>
>
> Actually, adding a GUC also solves another grief I had about this. There is
> currently no easy way to tell if your parameter file is being used, or if
> the server is failing to read it and is falling back to the hard-coded
> parameters. With a GUC, if the GUC is set it's a good indication that the
> admin expects the file to really exist and to contain valid parameters. So
> if the GUC is set, we should throw an error if it cannot be used. And if
> it's not set, we use the built-in defaults.
>
> I rebased the patch, did some other clean up of error reporting, and added a
> GUC along those lines, as well as docs. How does this look?
>
> It's late in the release cycle, but it would be nice to sneak this into v10.
> Using weak 1024 bit DH parameters is arguably a security issue; it was
> originally reported as such. There's a work-around for older versions:
> generate custom 2048 bit parameters and place them in a file called
> "dh1024.pem", but that's completely undocumented.
>
> Thoughts? Objections to committing this now, instead of waiting for v11?

+1 to include important open items such as this

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: PostgreSQL - Weak DH group

    Christoph Berg <myon@debian.org> — 2017-07-13T17:10:37Z

    Re: Alvaro Herrera 2017-07-13 <20170713170402.74uuoivrgd3c6tnw@alvherre.pgsql>
> > > Objections to committing this now, instead of waiting for v11?
> > 
> > But I am -1 for the sneak part. It is not the time to have a new
> > feature in 10, the focus is to stabilize.
> 
> But if we were treating it as a security issue, would we backpatch it?
> If we do, then it definitely makes sense to put something in pg10.  I'm
> not sure that this patch is it, though -- perhaps it makes sense to put
> a minimal fix in older branches, and let the new feature wait for pg11?

Making it user-configurable seems pretty minimal to me. Everything
else would probably require lengthy explanations about which file
could hold which contents, and this confusion seems to be part of the
problem.

Fwiw, wouldn't it make sense to recreate the default 2048 DH group as
well, maybe each time a new major is branched?

Christoph

  12. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-13T17:17:33Z

    On 07/13/2017 08:04 PM, Alvaro Herrera wrote:
> Michael Paquier wrote:
>> On Thu, Jul 13, 2017 at 5:32 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
>>> Objections to committing this now, instead of waiting for v11?
>>
>> But I am -1 for the sneak part. It is not the time to have a new
>> feature in 10, the focus is to stabilize.
>
> But if we were treating it as a security issue, would we backpatch it?
> If we do, then it definitely makes sense to put something in pg10.  I'm
> not sure that this patch is it, though -- perhaps it makes sense to put
> a minimal fix in older branches, and let the new feature wait for pg11?

I don't think this can be backpatched. It changes the default DH 
parameters from 1024 bits to 2048 bits. That's a good thing for 
security, but older clients might not support it, and would refuse to 
connect or would fall back to something less secure. I don't think there 
are many such clients around anymore, but it's nevertheless not 
something we want to do in a stable release I think the best we can do 
is to document the issue and the workaround. To recap, to use stronger 
DH parameters in stable versions, you need to do "openssl dhparam -out 
$PGDATA/dh1024.pem 2048".

But I'd like to take the opportunity to change this for new 
installations, with v10, instead of waiting for another year. Of course, 
you could say that for any new feature, too, but that doesn't 
necessarily mean that it's a bad argument :-). It's a judgment call, for 
sure.

- Heikki

  13. Re: PostgreSQL - Weak DH group

    Joe Conway <mail@joeconway.com> — 2017-07-13T17:40:38Z

    On 07/13/2017 01:07 PM, Simon Riggs wrote:
> On 13 July 2017 at 16:32, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> (We dropped the ball back in October, continuing the discussion now)
>>
>> On 10/10/2016 06:24 PM, Heikki Linnakangas wrote:
>>>
>>> On 10/06/2016 10:26 PM, Christoph Berg wrote:
>>>>
>>>> Re: Heikki Linnakangas 2016-10-06
>>>> <fd6eb3cc-1585-1469-fd9e-763f8e410b19@iki.fi>
>>>>>
>>>>> I propose the attached patch. It gives up on trying to deal with
>>>>> multiple
>>>>> key lengths (as noted earlier, OpenSSL just always passed
>>>>> keylength=1024, so
>>>>> that was useless). Instead of using the callback, it just sets fixed DH
>>>>> parameters with SSL_CTX_set_tmp_dh(), like we do for the ECDH curve. The
>>>>> DH
>>>>> parameters are loaded from a file called "dh_params.pem" (instead of
>>>>> "dh1024.pem"), if present, otherwise the built-in 2048 bit parameters
>>>>> are
>>>>> used.
>>>>
>>>>
>>>> Shouldn't this be a GUC pointing to a configurable location like
>>>> ssl_cert_file? This way, people reading the security section of the
>>>> default postgresql.conf would notice that there's something (new) to
>>>> configure. (And I wouldn't want to start creating symlinks from PGDATA
>>>> to /etc/ssl/something again...)
>>>
>>>
>>> Perhaps. The DH parameters are not quite like other configuration files,
>>> though. The actual parameters used don't matter, you just want to
>>> generate different parameters for every cluster. The key length of the
>>> parameters can be considered configuration, though.
>>
>>
>> Actually, adding a GUC also solves another grief I had about this. There is
>> currently no easy way to tell if your parameter file is being used, or if
>> the server is failing to read it and is falling back to the hard-coded
>> parameters. With a GUC, if the GUC is set it's a good indication that the
>> admin expects the file to really exist and to contain valid parameters. So
>> if the GUC is set, we should throw an error if it cannot be used. And if
>> it's not set, we use the built-in defaults.
>>
>> I rebased the patch, did some other clean up of error reporting, and added a
>> GUC along those lines, as well as docs. How does this look?
>>
>> It's late in the release cycle, but it would be nice to sneak this into v10.
>> Using weak 1024 bit DH parameters is arguably a security issue; it was
>> originally reported as such. There's a work-around for older versions:
>> generate custom 2048 bit parameters and place them in a file called
>> "dh1024.pem", but that's completely undocumented.
>>
>> Thoughts? Objections to committing this now, instead of waiting for v11?
> 
> +1 to include important open items such as this

I have not looked at the patch itself, but conceptually +1 to include in
pg10.


-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

  14. Re: PostgreSQL - Weak DH group

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-07-13T18:30:25Z

    Heikki Linnakangas <hlinnaka@iki.fi> writes:
> I don't think this can be backpatched. It changes the default DH 
> parameters from 1024 bits to 2048 bits. That's a good thing for 
> security, but older clients might not support it, and would refuse to 
> connect or would fall back to something less secure.

Do we have any hard information about which versions of which clients
might not support that?  (In particular I'm wondering if any still exist
in the wild.)

			regards, tom lane

  15. Re: PostgreSQL - Weak DH group

    Robert Haas <robertmhaas@gmail.com> — 2017-07-13T19:13:33Z

    On Thu, Jul 13, 2017 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>> I don't think this can be backpatched. It changes the default DH
>> parameters from 1024 bits to 2048 bits. That's a good thing for
>> security, but older clients might not support it, and would refuse to
>> connect or would fall back to something less secure.
>
> Do we have any hard information about which versions of which clients
> might not support that?  (In particular I'm wondering if any still exist
> in the wild.)

Yeah.  If we break clients for v10 two months from release, some
drivers won't be updated by release time, and that sounds pretty
unfriendly to me.  On the other hand, if there is only a theoretical
risk of breakage and no clients that we actually know about will have
a problem with it, then the argument for waiting is weaker.  I'm not
generally very excited about changing things after beta2, which is
where are, but if this is a security issue then we might need to hold
our nose and go ahead.  I'm against it if it's likely to cause
real-world connectivity problems, though.

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

  16. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-13T20:07:35Z

    On 07/13/2017 10:13 PM, Robert Haas wrote:
> On Thu, Jul 13, 2017 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>>> I don't think this can be backpatched. It changes the default DH
>>> parameters from 1024 bits to 2048 bits. That's a good thing for
>>> security, but older clients might not support it, and would refuse to
>>> connect or would fall back to something less secure.
>>
>> Do we have any hard information about which versions of which clients
>> might not support that?  (In particular I'm wondering if any still exist
>> in the wild.)
>
> Yeah.  If we break clients for v10 two months from release, some
> drivers won't be updated by release time, and that sounds pretty
> unfriendly to me.  On the other hand, if there is only a theoretical
> risk of breakage and no clients that we actually know about will have
> a problem with it, then the argument for waiting is weaker.  I'm not
> generally very excited about changing things after beta2, which is
> where are, but if this is a security issue then we might need to hold
> our nose and go ahead.  I'm against it if it's likely to cause
> real-world connectivity problems, though.

Googling around, I believe Java 6 is the only straggler [1]. So we would 
be breaking that. Java 7 also doesn't support DH parameters > 1024 bits, 
but it supports ECDHE, which is prioritized over DH ciphers, so it 
doesn't matter.

Java 6 was released back in 2006. The last public release was in 2013. 
It wouldn't surprise me to still see it bundled with random proprietary 
software packages, though. The official PostgreSQL JDBC driver still 
supports it, but there has been discussion recently on dropping support 
for it, and even for Java 7. [2]

I would be OK with breaking DH with Java 6 in PostgreSQL 10, especially 
since there's a simple workaround (generate a 1024-bit DH parameters 
file). I would be less enthusiastic about doing that in a minor release, 
although maybe that wouldn't be too bad either, if we put a prominent 
notice with the workaround in the release notes.

[1] https://wiki.mozilla.org/Security/Server_Side_TLS#DHE_and_ECDHE_support

[2] 
https://www.postgresql.org/message-id/69ae857b-15cc-36dd-f380-6620ef1effb9%408kdata.com

- Heikki

  17. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-31T11:27:27Z

    On 07/13/2017 11:07 PM, Heikki Linnakangas wrote:
> On 07/13/2017 10:13 PM, Robert Haas wrote:
>> On Thu, Jul 13, 2017 at 1:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Heikki Linnakangas <hlinnaka@iki.fi> writes:
>>>> I don't think this can be backpatched. It changes the default DH
>>>> parameters from 1024 bits to 2048 bits. That's a good thing for
>>>> security, but older clients might not support it, and would refuse to
>>>> connect or would fall back to something less secure.
>>>
>>> Do we have any hard information about which versions of which clients
>>> might not support that?  (In particular I'm wondering if any still exist
>>> in the wild.)
>>
>> Yeah.  If we break clients for v10 two months from release, some
>> drivers won't be updated by release time, and that sounds pretty
>> unfriendly to me.  On the other hand, if there is only a theoretical
>> risk of breakage and no clients that we actually know about will have
>> a problem with it, then the argument for waiting is weaker.  I'm not
>> generally very excited about changing things after beta2, which is
>> where are, but if this is a security issue then we might need to hold
>> our nose and go ahead.  I'm against it if it's likely to cause
>> real-world connectivity problems, though.
> 
> Googling around, I believe Java 6 is the only straggler [1]. So we would
> be breaking that. Java 7 also doesn't support DH parameters > 1024 bits,
> but it supports ECDHE, which is prioritized over DH ciphers, so it
> doesn't matter.
> 
> Java 6 was released back in 2006. The last public release was in 2013.
> It wouldn't surprise me to still see it bundled with random proprietary
> software packages, though. The official PostgreSQL JDBC driver still
> supports it, but there has been discussion recently on dropping support
> for it, and even for Java 7. [2]
> 
> I would be OK with breaking DH with Java 6 in PostgreSQL 10, especially
> since there's a simple workaround (generate a 1024-bit DH parameters
> file). I would be less enthusiastic about doing that in a minor release,
> although maybe that wouldn't be too bad either, if we put a prominent
> notice with the workaround in the release notes.

Some more information on the situation with JDK version 6: I installed 
Debian wheezy on a VM, with a OpenJDK 6, and tested connecting to a 
patched server with the JDBC driver. It worked! Googling around, it 
seems that this was fixed in later versions of OpenJDK 6 
(https://bugs.openjdk.java.net/browse/JDK-8062834). I then downloaded 
the latest Oracle JRE binary version, 6u45, available from 
http://www.oracle.com/technetwork/java/javase/downloads/java-archive-downloads-javase6-419409.html. 
With that, it does not work, you get errors like:

org.postgresql.util.PSQLException: SSL error: 
java.lang.RuntimeException: Could not generate DH keypair
...
Caused by: java.security.InvalidAlgorithmParameterException: Prime size 
must be multiple of 64, and can only range from 512 to 1024 (inclusive)

So, the last binary version downloadable from Oracle is affected, but 
recent versions of OpenJDK 6 work.

Rebased patch attached, with proposed release notes included. Barring 
new objections or arguments, I'll commit this (only) to v10 later today.

- Heikki

  18. Re: PostgreSQL - Weak DH group

    Heikki Linnakangas <hlinnaka@iki.fi> — 2017-07-31T19:48:13Z

    On 07/31/2017 02:27 PM, Heikki Linnakangas wrote:
> Rebased patch attached, with proposed release notes included. Barring
> new objections or arguments, I'll commit this (only) to v10 later today.

Ok, committed for v10. Thanks Nicolas and Damien, and everyone else 
involved!

- Heikki