Opened 4 years ago

Closed 2 weeks ago

Last modified 2 weeks ago

#7033 task closed fixed (fixed)

Use PyOpenSSL's ECDHE API

Reported by: lvh Owned by: mark williams
Priority: normal Milestone:
Component: core Keywords: tls ecdhe
Cc: tom@… Branch:
Author:

Description (last modified by lvh)

In ticket #6586, hynek and I introduced temporary ECDHE support for Twisted, greatly improving Twisted's default behavior for TLS servers. However, it does so with some custom code. It'd be preferable if we just inherited that behavior from PyOpenSSL instead, since it would be less code in Twisted to maintain.

For this to be possible, several things have to happen, all but the last of them upstream:

  • One of the ECDHE support branches needs to land in PyOpenSSL. Right now it looks most likely it will be https://github.com/pyca/pyopenssl/pull/57
  • cryptography needs to make a release >= 0.2.2
  • PyOpenSSL needs to make a release that depends on a cryptography release >= 0.2.2
  • Twisted needs to depend on that new PyOpenSSL release

Once all of that happens, we can just use the behavior from PyOpenSSL, and remove the workaround.

Change History (6)

comment:1 Changed 4 years ago by lvh

Description: modified (diff)

comment:2 Changed 2 years ago by Tom

Cc: tom@… added

pyopenssl 0.15 now supports ECDHE ciphersuites via the Context.set_tmp_ecdh function: https://github.com/pyca/pyopenssl/blob/0.15.1/OpenSSL/SSL.py#L808 However the current usage in twisted does not use it.

comment:3 Changed 3 weeks ago by mark williams

Keywords: review added

comment:4 Changed 2 weeks ago by Adi Roiban

Keywords: review removed
Owner: set to mark williams

See PR comments. It looks good and ready to merge.

I assume that Twisted already depends on modern pyopenssl and cryptography so the info from the description is outdated.

comment:5 Changed 2 weeks ago by Mark Williams <mrw@…>

Resolution: fixed
Status: newclosed

In a2e8623a:

Merge pull request #928 from twisted/7033-pyopenssl-ecdh

Author: markrwilliams
Reviewer: adiroiban
Fixes: ticket:7033

Remove _OpenSSLECCurve in favor of pyOpenSSL's public ECDHE curve API.

comment:6 in reply to:  4 Changed 2 weeks ago by mark williams

Replying to Adi Roiban:

See PR comments. It looks good and ready to merge.

I assume that Twisted already depends on modern pyopenssl and cryptography so the info from the description is outdated.

Yep! Twisted depends on pyOpenSSL 16.0.0, and set_tmp_ecdh and get_elliptic_curve have been around since at least 16.0.0.

Note: See TracTickets for help on using tickets.