Opened 12 months ago

Closed 9 months ago

#6799 enhancement closed fixed (fixed)

OpenSSLCertificateOptions doesn’t support DHE

Reported by: hynek Owned by: hynek
Priority: normal Milestone:
Component: core Keywords:
Cc: tobias.oberstein@… Branch: branches/add-dhe-to-certificateoptions-6799
(diff, github, buildbot, log)
Author: hynek Launchpad Bug:

Description

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 12 months 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 12 months ago by oberstet

FWIW, the man page http://linux.die.net/man/3/ssl_ctx_set_tmp_dh uses this command to generate the parameters

openssl dhparam -out dh_param_1024.pem -2 1024

The actual command reference is here: http://linux.die.net/man/1/dhparam

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 12 months ago by oberstet

  • Cc tobias.oberstein@… added

comment:4 Changed 9 months ago by hynek

  • Author set to hynek
  • Branch set to branches/add-dhe-to-certificateoptions-6799

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

comment:5 Changed 9 months ago by hynek

  • 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 9 months ago by exarkun

  • Keywords review removed
  • Owner set to hynek

Thanks!

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 9 months ago by exarkun

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

comment:8 Changed 9 months ago by hynek

  • Resolution set to fixed
  • Status changed from new to closed

(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.