Opened 3 years ago

Closed 3 years ago

#6799 enhancement closed fixed (fixed)

OpenSSLCertificateOptions doesn’t support DHE

Reported by: Hynek Schlawack Owned by: Hynek Schlawack
Priority: normal Milestone:
Component: core Keywords:
Cc: Tobias Oberstein Branch: branches/add-dhe-to-certificateoptions-6799
branch-diff, diff-cov, branch-cov, buildbot
Author: hynek


Our TLS currently supports only RSA for key exchange. For perfect forward secrecy we need DHE (and later ECDH, but baby steps).

pyOpenSSL supports the necessary APIs, basically all one needs is:

  • a temporary DH file (can be created using openssl dhparam -rand - 1024 >tmp_dh_file)
  • call ctx.load_tmp_dh('tmp_dh_file') on it.

I’ve got a PoC working. I would tackle this as soon as #6663 is resolved since it doesn’t make much sense without.

Change History (8)

comment:1 Changed 3 years ago by lvh

The path to the temporary DH param file should probably be specifiable in the endpoint, and possibly be autogenerated if it doesn't exist. We discovered in #twisted that calling openssl dhparam can take a non-negligible amount of time (and maybe entropy), which has all sorts of issues including screwing up flapping detection (since a service that can't start for reason X but first starts generating a DH param file may take several seconds to do so) and maybe even draining entropy pools (cold boots on wimpy machines with no good entropy sources).

comment:2 Changed 3 years ago by Tobias Oberstein

FWIW, the man page uses this command to generate the parameters

openssl dhparam -out dh_param_1024.pem -2 1024

The actual command reference is here:

It also mentions that reusing the generated param file is no issue when each app does generate its own:

As generating DH parameters is extremely time consuming, an application should not generate the parameters on the fly but supply the parameters. DH parameters can be reused, as the actual key is newly generated during the negotiation. The risk in reusing DH parameters is that an attacker may specialize on a very often used DH group. Applications should therefore generate their own DH parameters during the installation process using the openssl dhparam(1) application.

It also states:

Always using SSL_OP_SINGLE_DH_USE has an impact on the computer time needed during negotiation, but it is not very large, so application authors/users should consider to always enable this option.

So making this option default seems to be good. Same goes probably with OP_SINGLE_ECDH_USE.

comment:3 Changed 3 years ago by Tobias Oberstein

Cc: Tobias Oberstein added

comment:4 Changed 3 years ago by Hynek Schlawack

Author: hynek
Branch: branches/add-dhe-to-certificateoptions-6799

(In [41362]) Branching to add-dhe-to-certificateoptions-6799.

comment:5 Changed 3 years ago by Hynek Schlawack

Keywords: review added

So here we go, out first step toward PFS.

The bots are green, benign, or unrelated. The doc update fixes #6922 as well.

comment:6 Changed 3 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Hynek Schlawack


Just a few easy points, I think:

  1. Can OpenSSLDiffieHellmanParameters.fromFile take a FilePath instance instead of a bytes instance? Strings as path names is an anti-pattern and it'd be great not to perpetuate it further. :) And when you fix this, make sure the path used in the tests is represented as bytes instead of a native string. FilePath` doesn't like unicode yet.
  2. I think you might be able to delete a nice pile of code if you replace self.dhParameters._setParameters(ctx) with ctx.load_tmp_dh(self.dhParameters._dhFilename). Either way it's a private attribute access. I don't think having the method offers an advantages over using the attribute directly? At least not yet. I suppose _setParameters could be implemented differently later after it's possible to represent DH parameters in memory... Up to you.
  3. For the dhParameters @param docs, it might be worth saying that by supplying this value you're enabling DH key exchange? And by omitting it you are disabling it?

Maybe it's also worth linking to, in OpenSSLDiffieHellmanParameters, some pyOpenSSL tickets about better DH parameter APIs.

Thanks again! Please merge after addressing those issues.

comment:7 Changed 3 years ago by Jean-Paul Calderone

Oh, and add a 6922.misc news fragment as well so that you can Fixes: #6922 as well.

comment:8 Changed 3 years ago by Hynek Schlawack

Resolution: fixed
Status: newclosed

(In [41375]) Merge add-dhe-to-certificateoptions-6799

Author: hynek Reviewer: exarkun Fixes: #6799, #6922

twisted.internet.ssl.CertificateOptions now supports Diffie-Hellman key exchange.

Note: See TracTickets for help on using tickets.