Twisted: Ticket #6586: Support (and enable by default) ECDHE key-exchangehttps://twistedmatrix.com/trac/ticket/6586
<p>
Twisted's SSL support should enable ECDHE key exchange ciphersuites, to provide perfect forward secrecy. Then clients which support it can't have their traffic passively sniffed even if an adversary compromises your server's private key.
</p>
<p>
I think that in order for it to be usable -- even though the ciphersuites are enabled by default -- you need to call an extra function on the context.
</p>
<p>
It looks like this is how to do it in C:
</p>
<p>
<a class="ext-link" href="https://github.com/vincentbernat/stud/commit/f8a66d888880886fadd4dd05080952c3c1c28541"><span class="icon">​</span>https://github.com/vincentbernat/stud/commit/f8a66d888880886fadd4dd05080952c3c1c28541</a>
</p>
<div class="wiki-code"><div class="code"><pre><span class="gi">+#ifdef NID_X9_62_prime256v1
+ EC_KEY *ecdh = NULL;
+ ecdh = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
+ SSL_CTX_set_tmp_ecdh(ctx,ecdh);
+ EC_KEY_free(ecdh);
+ LOG("{core} ECDH Initialized with NIST P-256\n");
+#endif
</span></pre></div></div><p>
But I don't really know anything more about it.
</p>
en-usTwistedhttps://twistedmatrix.com/trac/chrome/common/trac_banner.pnghttps://twistedmatrix.com/trac/ticket/6586
Trac 1.2Thijs TriemstraWed, 19 Jun 2013 17:39:12 GMTdescription changedhttps://twistedmatrix.com/trac/ticket/6586#comment:1
https://twistedmatrix.com/trac/ticket/6586#comment:1
<ul>
<li><strong>description</strong>
modified (<a href="/trac/ticket/6586?action=diff&amp;version=1">diff</a>)
</li>
</ul>
<p>
Fix description markup.
</p>
TicketJean-Paul CalderoneFri, 21 Jun 2013 16:50:26 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:2
https://twistedmatrix.com/trac/ticket/6586#comment:2
<p>
Do you have a reference to an explanation of the security properties of ECDHE?
</p>
<p>
Separately, pyOpenSSL doesn't expose the C APIs used in the diff in the description. Those will need to be exposed before Twisted can do anything with them.
</p>
TicketjknightMon, 08 Jul 2013 13:53:52 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:3
https://twistedmatrix.com/trac/ticket/6586#comment:3
<p>
This looks like a nice explanation of the issues:
<a class="ext-link" href="http://vincent.bernat.im/en/blog/2011-ssl-perfect-forward-secrecy.html"><span class="icon">​</span>http://vincent.bernat.im/en/blog/2011-ssl-perfect-forward-secrecy.html</a>
</p>
<p>
Twisted could support DHE too, since it's supported in more versions of openssl than ECDHE. It too requires annoying configuration options to be set, including the generation of a "dh paramaters" blob that has to be loaded. And it's slow. Basically the only reason to consider using it is compatibility with older versions of ssl implementations.
</p>
<p>
ECDHE support in python:
<a class="ext-link" href="http://bugs.python.org/issue13627"><span class="icon">​</span>http://bugs.python.org/issue13627</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/8b729d65cfd2"><span class="icon">​</span>http://hg.python.org/cpython/rev/8b729d65cfd2</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/ec44f2e82707"><span class="icon">​</span>http://hg.python.org/cpython/rev/ec44f2e82707</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/c1a07c8092f7"><span class="icon">​</span>http://hg.python.org/cpython/rev/c1a07c8092f7</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/06ed9b3f02af"><span class="icon">​</span>http://hg.python.org/cpython/rev/06ed9b3f02af</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/1a06f0a8120f"><span class="icon">​</span>http://hg.python.org/cpython/rev/1a06f0a8120f</a>
</p>
<p>
DHE support in python:
<a class="ext-link" href="http://bugs.python.org/issue13626"><span class="icon">​</span>http://bugs.python.org/issue13626</a>
<a class="ext-link" href="http://hg.python.org/cpython/rev/33dea851f918"><span class="icon">​</span>http://hg.python.org/cpython/rev/33dea851f918</a>
</p>
TicketzookoMon, 08 Jul 2013 15:09:41 GMTcc sethttps://twistedmatrix.com/trac/ticket/6586#comment:4
https://twistedmatrix.com/trac/ticket/6586#comment:4
<ul>
<li><strong>cc</strong>
<em>zooko@…</em> added
</li>
</ul>
TicketlvhFri, 14 Mar 2014 15:37:55 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:5
https://twistedmatrix.com/trac/ticket/6586#comment:5
<p>
It turns out that it's possible, and, in fact, quite easy, to add best-effort ECDHE support to Twisted *right now*, if the environment happens to have a new enough version of cryptography/openssl installed. This is in the interest of having A-grade TLS in Twisted ASAP (14.0.0), and does not compromise future efforts to do so through PyOpenSSL.
</p>
<p>
We should of course eventually use the PyOpenSSL API once that exists, but that can be done in a separate ticket, which I will file right now. The branch hynek is creating adds support without introducing any new public APIs that we have to maintain. This way, we get improved security right now, without impeding any future tickets.
</p>
<p>
This would have really cool advantages such as magically Twisted being one of the only reasonable ways you can serve WSGI applications and get EC-based PFS (uwsgi does it too, but only with a bunch of gnarly C code), and *the* only way you can get a reasonably programmable one. That means that suddenly we have a great carrot for people to use Twisted: twistd web is a lot nicer than having to install and configure nginx as a reverse proxy, which is the default way of doing it right now.
</p>
TicketHynek SchlawackFri, 14 Mar 2014 15:39:43 GMTbranch, branch_author sethttps://twistedmatrix.com/trac/ticket/6586#comment:6
https://twistedmatrix.com/trac/ticket/6586#comment:6
<ul>
<li><strong>branch</strong>
set to <em>branches/ninja-in-ecdhe-6586</em>
</li>
<li><strong>branch_author</strong>
set to <em>hynek</em>
</li>
</ul>
<p>
(In <a class="missing changeset" title="No changeset 41883 in the repository">[41883]</a>) Branching to ninja-in-ecdhe-6586.
</p>
TicketHynek SchlawackFri, 14 Mar 2014 15:43:03 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:7
https://twistedmatrix.com/trac/ticket/6586#comment:7
<p>
(In <a class="missing changeset" title="No changeset 41884 in the repository">[41884]</a>) Add best-effort ECDHE support to CertificateOptions
</p>
<p>
refs <a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/6586" title="#6586: enhancement: Support (and enable by default) ECDHE key-exchange (closed: fixed)">#6586</a>
</p>
TicketlvhFri, 14 Mar 2014 15:46:53 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:8
https://twistedmatrix.com/trac/ticket/6586#comment:8
<p>
I have filed that ticket about eventually using the PyOpenSSL API whenever PyOpenSSL supports it: <a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/7033" title="#7033: task: Use PyOpenSSL's ECDHE API (closed: fixed)">#7033</a>.
</p>
TicketHynek SchlawackFri, 14 Mar 2014 16:04:32 GMTkeywords sethttps://twistedmatrix.com/trac/ticket/6586#comment:9
https://twistedmatrix.com/trac/ticket/6586#comment:9
<ul>
<li><strong>keywords</strong>
<em>review</em> added
</li>
</ul>
<p>
Welcome to the ecc future where the bots are green and the crypto is pretty!
</p>
<p>
This patch tries in a best-effort fashion to give us the benefits of ECDHE without adding public APIs or options.
</p>
<p>
Laurens said everything important, if this lands, our TLS server story becomes top-notch.
</p>
TicketlvhFri, 14 Mar 2014 16:12:28 GMTowner sethttps://twistedmatrix.com/trac/ticket/6586#comment:10
https://twistedmatrix.com/trac/ticket/6586#comment:10
<ul>
<li><strong>owner</strong>
set to <em>lvh</em>
</li>
</ul>
TicketlvhFri, 14 Mar 2014 16:17:34 GMTowner changed; keywords deletedhttps://twistedmatrix.com/trac/ticket/6586#comment:11
https://twistedmatrix.com/trac/ticket/6586#comment:11
<ul>
<li><strong>keywords</strong>
<em>review</em> removed
</li>
<li><strong>owner</strong>
changed from <em>lvh</em> to <em>Hynek Schlawack</em>
</li>
</ul>
<p>
There is a pydoctor failure: <code>twisted.internet._sslverify._OpenSSLECCurve.addECKeyToContext: invalid ref to OpenSSL.SSL.Context</code> but that looks pretty bogus to me since <code>OpenSSL.SSL.Context</code> is obviously a thing. It also appears in other already existing places; I am assuming this is perhaps because that isn't supposed to work at all, or the API builder just doesn't have PyOpenSSL installed? I will file a ticket for that.
</p>
<p>
There is a twistedchecker failure that you should probably address: <code>W9207:1223,0:FakeLib: Missing a blank line before epytext markups</code>
</p>
<p>
All the other twistedchecker failures are bogus, because it's complaining that some method names don't follow the Twisted coding standard, but that's just because they're OpenSSL methods/functions:
</p>
<pre class="wiki">C0103:1238,4:FakeLib.OBJ_sn2nid: Invalid name "OBJ_sn2nid" (should match ((([a-z])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$|(__[a-z]+__)$|(\_.+)$|(test\_.+)$|((telnet_|sync_|_fromString_|auth_|perspective_|handleStatus_|_parseSVNEntries_|removeTrigger_|_process_|get_|xmlrpc_|iac_|macro_|handle_|ctcpQuery_|visitNode_a_|ctcpReply_|parse_|channel_|_toString_|observe_|func_|async_|spew_|dcc_|_stat_|convert_|state_|ftp_|global_|packet_|start_|ext_|_dataReceived_|class_|opt_|do_|request_|render_|ssh_|_unjelly_|soap_|agentc_|cmd_|key_|generate_|view_|ssh_USERAUTH_PK_OK_|end_|oscar_|type_|irc_|search_|remote_|linemode_|decode_|response_|login_|visitNode_).+)$)
C0103:1251,4:FakeLib.EC_KEY_new_by_curve_name: Invalid name "EC_KEY_new_by_curve_name" (should match ((([a-z])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$|(__[a-z]+__)$|(\_.+)$|(test\_.+)$|((telnet_|sync_|_fromString_|auth_|perspective_|handleStatus_|_parseSVNEntries_|removeTrigger_|_process_|get_|xmlrpc_|iac_|macro_|handle_|ctcpQuery_|visitNode_a_|ctcpReply_|parse_|channel_|_toString_|observe_|func_|async_|spew_|dcc_|_stat_|convert_|state_|ftp_|global_|packet_|start_|ext_|_dataReceived_|class_|opt_|do_|request_|render_|ssh_|_unjelly_|soap_|agentc_|cmd_|key_|generate_|view_|ssh_USERAUTH_PK_OK_|end_|oscar_|type_|irc_|search_|remote_|linemode_|decode_|response_|login_|visitNode_).+)$)
C0103:1266,4:FakeLib.EC_KEY_free: Invalid name "EC_KEY_free" (should match ((([a-z])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$|(__[a-z]+__)$|(\_.+)$|(test\_.+)$|((telnet_|sync_|_fromString_|auth_|perspective_|handleStatus_|_parseSVNEntries_|removeTrigger_|_process_|get_|xmlrpc_|iac_|macro_|handle_|ctcpQuery_|visitNode_a_|ctcpReply_|parse_|channel_|_toString_|observe_|func_|async_|spew_|dcc_|_stat_|convert_|state_|ftp_|global_|packet_|start_|ext_|_dataReceived_|class_|opt_|do_|request_|render_|ssh_|_unjelly_|soap_|agentc_|cmd_|key_|generate_|view_|ssh_USERAUTH_PK_OK_|end_|oscar_|type_|irc_|search_|remote_|linemode_|decode_|response_|login_|visitNode_).+)$)
C0103:1282,4:FakeLib.SSL_CTX_set_tmp_ecdh: Invalid name "SSL_CTX_set_tmp_ecdh" (should match ((([a-z])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$|(__[a-z]+__)$|(\_.+)$|(test\_.+)$|((telnet_|sync_|_fromString_|auth_|perspective_|handleStatus_|_parseSVNEntries_|removeTrigger_|_process_|get_|xmlrpc_|iac_|macro_|handle_|ctcpQuery_|visitNode_a_|ctcpReply_|parse_|channel_|_toString_|observe_|func_|async_|spew_|dcc_|_stat_|convert_|state_|ftp_|global_|packet_|start_|ext_|_dataReceived_|class_|opt_|do_|request_|render_|ssh_|_unjelly_|soap_|agentc_|cmd_|key_|generate_|view_|ssh_USERAUTH_PK_OK_|end_|oscar_|type_|irc_|search_|remote_|linemode_|decode_|response_|login_|visitNode_).+)$)
C0103:1393,8:TestECCurve.test_nonECbinding: Invalid name "OBJ_sn2nid" (should match ((([a-z_])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$)
C0103:1423,8:TestECCurve.test_keyFails: Invalid name "EC_KEY_new_by_curve_name" (should match ((([a-z_])|([a-z]+_[a-z]))[a-zA-Z0-9]+)$)
</pre><p>
... so I'm ignoring those.
</p>
<p>
WinXP buildbot fails because of that ridiculous ENOSPC bug, so whatever.
</p>
<p>
LGTM. Please merge.
</p>
TicketlvhFri, 14 Mar 2014 16:18:16 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:12
https://twistedmatrix.com/trac/ticket/6586#comment:12
<p>
I'm sorry, I should probably clarify: please merge after addressing that one epytext markup thing.
</p>
TicketHynek SchlawackFri, 14 Mar 2014 16:32:23 GMThttps://twistedmatrix.com/trac/ticket/6586#comment:13
https://twistedmatrix.com/trac/ticket/6586#comment:13
<p>
Thanks lvh. The twistedchecker error is bogus, because the docstring in question looks like this:
</p>
<pre class="wiki"> """
An introspectable fake of cryptography's lib object.
@ivar _createdKey: A set of keys that have been created by this instance.
@type _createdKey: L{set} of L{FakeKey}
@cvar NID_undef: A symbolic constant for undefined NIDs.
@type NID_undef: L{FakeNID}
"""
</pre><p>
Therefore merging.
</p>
TicketHynek SchlawackFri, 14 Mar 2014 16:38:29 GMTstatus changed; resolution sethttps://twistedmatrix.com/trac/ticket/6586#comment:14
https://twistedmatrix.com/trac/ticket/6586#comment:14
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
(In <a class="missing changeset" title="No changeset 41888 in the repository">[41888]</a>) Merge ninja-in-ecdhe-6586
</p>
<p>
Author: hynek
Reviewer: lvh
Fixes: <a class="closed ticket" href="https://twistedmatrix.com/trac/ticket/6586" title="#6586: enhancement: Support (and enable by default) ECDHE key-exchange (closed: fixed)">#6586</a>
</p>
<p>
OpenSSLCertificateOptions now tries best-effort to support ECDHE for servers.
</p>
Ticket