On 07/13/2017 01:07 PM, Simon Riggs wrote: > On 13 July 2017 at 16:32, Heikki Linnakangas <hlinn...@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-763f8e410...@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
signature.asc
Description: OpenPGP digital signature