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
