Add ECDH support

Bug #1233810 reported by Andy Lutomirski
24
This bug affects 4 people
Affects Status Importance Assigned to Milestone
pyOpenSSL
New
Undecided
Unassigned

Bug Description

OpenSSL supports ECDH, but PyOpenSSL does not. ECDH is needed to get forward secrecy on IE, since IE does not support DHE.

This patch adds Context.set_tmp_ecdh_by_curve_name that encapsulates EC_KEY_new_by_curve_name and SSL_CTX_set_tmp_ecdh. If anyone needs to load a non-named curve, they're welcome to add their own bindings to access the raw API directly. Almost nothing supports TLS with custom curves, though, so this should cover pretty much all use cases.

Supporting ECDH is a bit hairy because Fedora disables it in openssl for patent reasons. This patch compiles on Fedora 19, but set_tmp_ecdh_by_curve_name is replaced by a stub that always raises ValueError.

Revision history for this message
Andy Lutomirski (luto-mit) wrote :
Revision history for this message
Tobias Oberstein (tobias-oberstein) wrote :

2 notes:

1) It works for me with pyOpenSSL HEAD. With a suitable ECDH enabled OpenSSL, this indeed provides ECDH (tested with https://www.ssllabs.com/ssltest/analyze.html and `openssl s_client`

2) There is a small typo in

```
:param curve_name: The curve name constant (e.g. SSL.NID_X9_64_prime256v1)
```

This should be `SSL.NID_X9_62_prime256v1`.

Probably it would also be helpful to list the available curves in the function docstring.

3) Besides having the constant for curve identification, it would also be nice to have the human readable curve names accesible (for UI reasons .. let the user choose the curve).

SSL.SN_X9_62_prime192v1 = "prime192v1"
SSL.SN_X9_62_prime192v2 = "prime192v2"
SSL.SN_X9_62_prime192v3 = "prime192v3"
SSL.SN_X9_62_prime239v1 = "prime239v1"
SSL.SN_X9_62_prime239v2 = "prime239v2"
SSL.SN_X9_62_prime239v3 = "prime239v3"
SSL.SN_X9_62_prime256v1 = "prime256v1"

so one can do things like

```
ELLIPTIC_CURVES = {
   SSL.SN_X9_62_prime192v1: SSL.NID_X9_62_prime192v1,
   SSL.SN_X9_62_prime192v2: SSL.NID_X9_62_prime192v2,
   SSL.SN_X9_62_prime192v3: SSL.NID_X9_62_prime192v3,
   SSL.SN_X9_62_prime239v1: SSL.NID_X9_62_prime239v1,
   SSL.SN_X9_62_prime239v2: SSL.NID_X9_62_prime239v2,
   SSL.SN_X9_62_prime239v3: SSL.NID_X9_62_prime239v3,
   SSL.SN_X9_62_prime256v1: SSL.NID_X9_62_prime256v1
}

ECDH_DEFAULT_CURVE = ELLIPTIC_CURVES["prime256v1"]
```

Revision history for this message
Giovanni Pellerano (evilaliv3) wrote :
Revision history for this message
Tobias Oberstein (tobias-oberstein) wrote :

Well, I prefer the patch by Andy attached to this bug here: it lets me choose the curve, and works with pyOpenSSL HEAD (tested).

Revision history for this message
Andy Lutomirski (luto-mit) wrote :

I'll fix the typo.

Re: curve names, this may be a bit hairy. I already dislike OpenSSL's insistence on using nonstandard names for cipher suites, and even RFC4492 is little confused as to what the curves are called. For example, section 5.1.1 (the normative part) says:

        enum {
            sect163k1 (1), sect163r1 (2), sect163r2 (3),
            sect193r1 (4), sect193r2 (5), sect233k1 (6),
            sect233r1 (7), sect239k1 (8), sect283k1 (9),
            sect283r1 (10), sect409k1 (11), sect409r1 (12),
            sect571k1 (13), sect571r1 (14), secp160k1 (15),
            secp160r1 (16), secp160r2 (17), secp192k1 (18),
            secp192r1 (19), secp224k1 (20), secp224r1 (21),
            secp256k1 (22), secp256r1 (23), secp384r1 (24),
            secp521r1 (25),
            reserved (0xFE00..0xFEFF),
            arbitrary_explicit_prime_curves(0xFF01),
            arbitrary_explicit_char2_curves(0xFF02),
            (0xFFFF)
        } NamedCurve;

The text "prime256v1" only appears in the (informative) Appendix A. If I add the list of names, I'd probably follow 5.1.1, which will confuse everyone who's looking for "P-256". Sigh.

FWIW, my patch works unmodified when built against the newly-sort-of-ECC-supporting Fedora OpenSSL. It might be worth adding a comment that secp256r1 and secp284r1 (the NIST Suite B curves) are the most widely supported.

P.S. If I'm reading RFC4492 right (it's not very clear), the server should be able to choose a curve for key exchange based on the allowed curves listed by the client. Does OpenSSL support that? This will be important if/when TLS gains support for curve25519.

Revision history for this message
Tobias Oberstein (tobias-oberstein) wrote :

I agree curve naming is a mess:

P-256 == secp256r1 == prime256v1

Ah yeah, great;)

Regarding how curve negotiation works:
http://crypto.stackexchange.com/questions/11311/with-tls-and-ecdhe-how-does-curve-selection-work

Regarding non-named, explicit curves: this is unsupported by OpenSSL (only named-curves builtin are supported).

Also:
http://crypto.stackexchange.com/questions/11310/with-openssl-and-ecdhe-how-to-show-the-actual-curve-being-used

curve25519:
Yeah, I'm looking forward to Curve25519+Salsa20/ChaCha20+Poly1305 also. And it is indeed coming to Chrome and Firefox: https://twistedmatrix.com/trac/ticket/6663#comment:39

Revision history for this message
Giovanni Pellerano (evilaliv3) wrote :

yep i agree that Andy patch is really good :)

it would be nice to work together on https://bugs.launchpad.net/pyopenssl/+bug/1244201 and in particular there is a security issue related to this that i've reported but its noy yet public on launchpad but it's disclosed here https://github.com/globaleaks/Tor2web-3.0/issues/124, https://github.com/globaleaks/Tor2web-3.0/commit/52652d1872e944591f9dfebf40935c499f43769d

Revision history for this message
Andy Lutomirski (luto-mit) wrote :

I'm about to submit a new version. It makes several changes:

 - The docstring for set_tmp_ecdh_by_curve_name is improved.

 - There are a lot more curve constants (I think I got all of them). I also expose the SN_ constants.

 - I added SSL.ELLIPTIC_CURVE_DESCRIPTIONS. This is loaded at startup from OpenSSL, so it will reflect the list of actual supported curves. On Fedora, it looks like this:

{415: u'X9.62/SECG curve over a 256 bit prime field',
 715: u'NIST/SECG curve over a 384 bit prime field'}

On Ubuntu, it's a huge list. I confirmed that I added the NID_ and SN_ constants for all of the Ubuntu-supported curves.

Revision history for this message
Andy Lutomirski (luto-mit) wrote :
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.