Re: PostgreSQL - Weak DH group

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