Re: Replacing the EDH SKIP primes
On Thu, Jul 04, 2019 at 08:24:13AM +0200, Daniel Gustafsson wrote: > LGTM, thanks. Okay, done, after rechecking the shape of the key. Thanks! -- Michael signature.asc Description: PGP signature
Re: Replacing the EDH SKIP primes
> On 04 Jul 2019, at 02:58, Michael Paquier wrote: > >> On Wed, Jul 03, 2019 at 08:56:42PM +0200, Daniel Gustafsson wrote: >> Agreed, I’ve updated the patch with a comment on this formulated such that it >> should stand the test of time even as OpenSSL changes etc. > > I'd like to think that we had rather mention the warning issue > explicitely, so as people don't get surprised, like that for example: > > * This is the 2048-bit DH parameter from RFC 3526. The generation of the > * prime is specified in RFC 2412, which also discusses the design choice > * of the generator. Note that when loaded with OpenSSL this causes > * DH_check() to fail on with DH_NOT_SUITABLE_GENERATOR, where leaking > * a bit is preferred. > > Now this makes an OpenSSL-specific issue pop up within a section of > the code where we want to make things more generic with SSL, so your > simpler version has good arguments as well. > > I have just rechecked the shape of the key, and we have an exact > match. LGTM, thanks. cheers ./daniel
Re: Replacing the EDH SKIP primes
On Wed, Jul 03, 2019 at 08:56:42PM +0200, Daniel Gustafsson wrote: > Agreed, I’ve updated the patch with a comment on this formulated such that it > should stand the test of time even as OpenSSL changes etc. I'd like to think that we had rather mention the warning issue explicitely, so as people don't get surprised, like that for example: * This is the 2048-bit DH parameter from RFC 3526. The generation of the * prime is specified in RFC 2412, which also discusses the design choice * of the generator. Note that when loaded with OpenSSL this causes * DH_check() to fail on with DH_NOT_SUITABLE_GENERATOR, where leaking * a bit is preferred. Now this makes an OpenSSL-specific issue pop up within a section of the code where we want to make things more generic with SSL, so your simpler version has good arguments as well. I have just rechecked the shape of the key, and we have an exact match. -- Michael diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 96415a9c8b..93581acb26 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -206,19 +206,20 @@ typedef struct Port * Hardcoded DH parameters, used in ephemeral DH keying. (See also * README.SSL for more details on EDH.) * - * If you want to create your own hardcoded DH parameters - * for fun and profit, review "Assigned Number for SKIP - * Protocols" (http://www.skip-vpn.org/spec/numbers.html) - * for suggestions. + * This is the 2048-bit DH parameter from RFC 3526. The generation of the + * prime is specified in RFC 2412, which also discusses the design choice + * of the generator. Note that when loaded with OpenSSL this causes + * DH_check() to fail on with DH_NOT_SUITABLE_GENERATOR, where leaking + * a bit is preferred. */ #define FILE_DH2048 \ "-BEGIN DH PARAMETERS-\n\ -MIIBCAKCAQEA9kJXtwh/CBdyorrWqULzBej5UxE5T7bxbrlLOCDaAadWoxTpj0BV\n\ -89AHxstDqZSt90xkhkn4DIO9ZekX1KHTUPj1WV/cdlJPPT2N286Z4VeSWc39uK50\n\ -T8X8dryDxUcwYc58yWb/Ffm7/ZFexwGq01uejaClcjrUGvC/RgBYK+X0iP1YTknb\n\ -zSC0neSRBzZrM2w4DUUdD3yIsxx8Wy2O9vPJI8BD8KVbGI2Ou1WMuF040zT9fBdX\n\ -Q6MdGGzeMyEstSr/POGxKUAYEY18hKcKctaGxAMZyAcpesqVDNmWn6vQClCbAkbT\n\ -CD1mpF1Bn5x8vYlLIhkmuquiXsNV6TILOwIBAg==\n\ +MIIBCAKCAQEA///JD9qiIWjCNMTGYouA3BzRKQJOCIpnzHQCC76mOxOb\n\ +IlFKCHmONATd75UZs806QxswKwpt8l8UN0/hNW1tUcJF5IW1dmJefsb0TELppjft\n\ +awv/XLb0Brft7jhr+1qJn6WunyQRfEsf5kkoZlHs5Fs9wgB8uKFjvwWY2kg2HFXT\n\ +mmkWP6j9JM9fg2VdI9yjrZYcYvNWIIVSu57VKQdwlpZtZww1Tkq8mATxdGwIyhgh\n\ +fDKQXkYuNs474553LBgOhgObJ4Oi7Aeij7XFXfBvTFLJ3ivL9pVYFxg5lUl86pVq\n\ +5RXSJhiY+gUQFXKOWoqsqmj//wIBAg==\n\ -END DH PARAMETERS-\n" /* signature.asc Description: PGP signature
Re: Replacing the EDH SKIP primes
> On 3 Jul 2019, at 12:11, Michael Paquier wrote: > It would be nice to add a comment on that though, perhaps in > libpq-be.h where the key is defined. Agreed, I’ve updated the patch with a comment on this formulated such that it should stand the test of time even as OpenSSL changes etc. cheers ./daniel skip_primes-v2.patch Description: Binary data
Re: Replacing the EDH SKIP primes
On Wed, Jul 03, 2019 at 10:56:41AM +0200, Daniel Gustafsson wrote: > OpenSSL provides DH_check() which we use in load_dh_file() to ensure that the > user is passing a valid prime in the DH file. Adding this to the hardcoded > blob seems overkill though, once the validity has been verified before it > being > committed. Agreed, and I didn't notice this check... There could be an argument for having DH_check within an assertion block but as this changes once per decade there is little value. > A DH param in PEM (or DER) format can be checked with the openssl dhparam > tool. > Assuming the PEM is extracted from the patch into a file, one can do: > > openssl dhparam -inform PEM -in /tmp/dh.pem -check -text > > The prime is returned and can be diffed against the one in the RFC. If you > modify the blob you will see that the check complains about it not being > prime. Ah, thanks. I can see that the new key matches the RFC. > There is an expected warning in the output however: "the g value is not a > generator” (this is also present when subjecting the PEM for the 2048 MODP in > OpenSSL). Indeed, I saw that also from OpenSSL. That looks to come from dh.c (there are two other code paths, haven't checked in details). Thanks for the pointer. > I’m far from a cryptographer, but AFAICT from reading it essentially means > that > the RFC authors chose to limit the search space of the shared secret rather > than leaking a bit of it, and OpenSSL has chosen in DH_check() that leaking a > bit is preferrable. (This makes me wonder if we should downgrade the check in > load_dh_file() "codes & DH_NOT_SUITABLE_GENERATOR” to WARNING, but the lack of > reports of it being a problem either shows that most people are just using > openssl dhparam generated parameters which can leak a bit, or aren’t using DH > files at all.) Yeah, no objections per the arguments from the RFC. I am not sure if we actually need to change that part though. We'd still get a complaint for a key which is not a prime, and for the default one this is not something we care much about, because we know its properties, no? It would be nice to add a comment on that though, perhaps in libpq-be.h where the key is defined. -- Michael signature.asc Description: PGP signature
Re: Replacing the EDH SKIP primes
> On 2 Jul 2019, at 09:49, Michael Paquier wrote: > On Tue, Jul 02, 2019 at 08:14:25AM +0100, Peter Eisentraut wrote: >> >> I was wondering whether the provided binary blob contained any checksums >> or other internal checks. How would we know whether it contains >> transposed characters or replaces a 1 by a I or a l? If I just randomly >> edit the blob, the ssl tests still pass. (The relevant load_dh_buffer() >> call does get called by the tests.) How can we make sure we actually >> got a good copy? >> > > PEM_read_bio_DHparams() has some checks on the Diffie-Hellman key, but > it is up to the caller to make sure that it is normally providing a > prime number in this case to make the cracking harder, no? OpenSSL provides DH_check() which we use in load_dh_file() to ensure that the user is passing a valid prime in the DH file. Adding this to the hardcoded blob seems overkill though, once the validity has been verified before it being committed. > RFC 3526 > has a small formula in this case, which we can use to double-check the > patch. A DH param in PEM (or DER) format can be checked with the openssl dhparam tool. Assuming the PEM is extracted from the patch into a file, one can do: openssl dhparam -inform PEM -in /tmp/dh.pem -check -text The prime is returned and can be diffed against the one in the RFC. If you modify the blob you will see that the check complains about it not being prime. There is an expected warning in the output however: "the g value is not a generator” (this is also present when subjecting the PEM for the 2048 MODP in OpenSSL). From reading RFC2412 which outlines how the primes are generated, this is by design. In Appendix E: "Because these two primes are congruent to 7 (mod 8), 2 is a quadratic residue of each prime. All powers of 2 will also be quadratic residues. This prevents an opponent from learning the low order bit of the Diffie-Hellman exponent (AKA the subgroup confinement problem). Using 2 as a generator is efficient for some modular exponentiation algorithms. [Note that 2 is technically not a generator in the number theory sense, because it omits half of the possible residues mod P. From a cryptographic viewpoint, this is a virtue.]" I’m far from a cryptographer, but AFAICT from reading it essentially means that the RFC authors chose to limit the search space of the shared secret rather than leaking a bit of it, and OpenSSL has chosen in DH_check() that leaking a bit is preferrable. (This makes me wonder if we should downgrade the check in load_dh_file() "codes & DH_NOT_SUITABLE_GENERATOR” to WARNING, but the lack of reports of it being a problem either shows that most people are just using openssl dhparam generated parameters which can leak a bit, or aren’t using DH files at all.) cheers ./daniel
Re: Replacing the EDH SKIP primes
On Tue, Jul 02, 2019 at 08:14:25AM +0100, Peter Eisentraut wrote: > It appears that we have consensus to go ahead with this. Yeah, I was planning to look at that one next. Or perhaps you would like to take care of it, Peter? > > I was wondering whether the provided binary blob contained any checksums > or other internal checks. How would we know whether it contains > transposed characters or replaces a 1 by a I or a l? If I just randomly > edit the blob, the ssl tests still pass. (The relevant load_dh_buffer() > call does get called by the tests.) How can we make sure we actually > got a good copy? > PEM_read_bio_DHparams() has some checks on the Diffie-Hellman key, but it is up to the caller to make sure that it is normally providing a prime number in this case to make the cracking harder, no? RFC 3526 has a small formula in this case, which we can use to double-check the patch. -- Michael signature.asc Description: PGP signature
Re: Replacing the EDH SKIP primes
On 2019-06-18 13:05, Daniel Gustafsson wrote: > This was touched upon, but never really discussed AFAICT, back when then EDH > parameters were reworked a few years ago. Instead of replacing with custom > ones, as suggested in [1] it we might as well replace with standardized ones > as > this is a fallback. Custom ones wont make it more secure, just add more work > for the project. The attached patch replace the SKIP prime with the 2048 bit > MODP group from RFC 3526, which is the same change that OpenSSL did a few > years > back [2]. It appears that we have consensus to go ahead with this. I was wondering whether the provided binary blob contained any checksums or other internal checks. How would we know whether it contains transposed characters or replaces a 1 by a I or a l? If I just randomly edit the blob, the ssl tests still pass. (The relevant load_dh_buffer() call does get called by the tests.) How can we make sure we actually got a good copy? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Replacing the EDH SKIP primes
On Wed, Jun 19, 2019 at 07:44:46AM +0200, Daniel Gustafsson wrote: > I think this is v13 material, I’ll stick it in the next commitfest. Thanks! -- Michael signature.asc Description: PGP signature
Re: Replacing the EDH SKIP primes
> On 19 Jun 2019, at 05:40, Michael Paquier wrote: > Fine by me. Let's stick with the 2048b-long one for now as we did in > c0a15e0. I am wondering if we should sneak that into v12, but I'd > rather just wait for v13 to open. I think this is v13 material, I’ll stick it in the next commitfest. cheers ./daniel
Re: Replacing the EDH SKIP primes
On Tue, Jun 18, 2019 at 02:05:00PM +0200, Daniel Gustafsson wrote: > The current hardcoded EDH parameter fallback use the old SKIP primes, for > which > the source disappeared from the web a long time ago. Referencing a known dead > source seems a bit silly, so I think we should either switch to a non-dead > source of MODP primes or use an archive.org link for SKIP. Personally I > prefer > the former. I agree with you that it sounds more sensible to switch to a new prime instead of relying on an archive of the past one. > This was touched upon, but never really discussed AFAICT, back when then EDH > parameters were reworked a few years ago. Instead of replacing with custom > ones, as suggested in [1] it we might as well replace with standardized ones > as > this is a fallback. Custom ones wont make it more secure, just add more work > for the project. The attached patch replace the SKIP prime with the 2048 bit > MODP group from RFC 3526, which is the same change that OpenSSL did a few > years > back [2]. Fine by me. Let's stick with the 2048b-long one for now as we did in c0a15e0. I am wondering if we should sneak that into v12, but I'd rather just wait for v13 to open. -- Michael signature.asc Description: PGP signature
