Re: [HACKERS] PostgreSQL - Weak DH group

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