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