From the OpenSSL changelog:

  *) Support for automatic EC temporary key parameter selection. If enabled
     the most preferred EC parameters are automatically used instead of
     hardcoded fixed parameters. Now a server just has to call:
     SSL_CTX_set_ecdh_auto(ctx, 1) and the server will automatically
     support ECDH and use the most appropriate parameters.
     [Steve Henson]

We could probably call this function automatically on SSL contexts, when possible.

Besides, Apache's mod_ssl has the following code:

#if defined(SSL_CTX_set_ecdh_auto)
        SSL_CTX_set_ecdh_auto(mctx->ssl_ctx, 1);
#else
        SSL_CTX_set_tmp_ecdh(mctx->ssl_ctx,
                             EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
#endif

So perhaps we can also reuse the same fallback to "prime256v1" (which would allow prioritizing ECDH in the cipher string).

Heh, I was just getting ready to figure out if I should write this ticket or not :)

I think we should do this. Nginx also defaults to prime256v1 (which is NIST P-256), DJB seems to think that's an unsafe curve though (http://safecurves.cr.yp.to/).

Ok, looks like everyone just uses prime256v1, so we should use that!

I don't have a new enough OpenSSL to test against SSL_CTX_set_ecdh_auto and I don't really know C very well so I didn't want to do try to handle that.

That being said, here is a patch that allows you to call SSLContext().set_ecdh_curve() without any argument, and if you do that it'll use the "default" curve. This is currently hardcoded to prime256v1 but ideally at some point it would use SSL_CTX_set_ecdh_auto if available.

I don't really know C, so I had to Google around to learn enough to write this patch, so please look it over closely?

Marking this as "buildbot", as Donald suspects it's the culprit for the current SSL related buildbot failures after merging issue 20995.

Also Donald, welcome to the "I broke (some of) the buildbots" club, although you did come up with something more exotic than the somewhat traditional "I forgot to 'hg add' a new file". Your next challenge is to break them when a release is imminent and you need to abjectly apologise to a release manager while you try to fix it or decide to revert it :)

You're also getting to see first hand why I decided to write PEP 462 to start down the path of improving our core development workflows :)

I always have to do things the hard way ;)

Note that my patch changes the set_ecdh_curve() method to no longer require a name. If that is too big of a change for Python 3.4 we can just hardcode the same name inside of ssl.py for 3.4

I don't think it makes much sense to have a default argument to set_ecdh_curve(). It's probably better to just copy mod_ssl's initialization (which selects prime256v1 on not-so-new OpenSSLs).

Here is a patch working with both 1.0.2 (set_ecdh_auto) and 1.0.1 (fallback on prime256v1).

Perhaps we should add a test for this.

New patch with a test.

The reason I made a default argument to set_ecdh_curve is that I couldn't find any information about what happens if you set a ecdh curve _twice_ within a a OpenSSL SSL Context. So I played it safe and made it a default argument that only gets called if you haven't called it. That preserves the users ability to specify their own curve for sure.

I know it doesn't segfault or raise an error if you do that, but I don't know if it:

1) Replaces the already called ECDH Curve
2) Adds to the already called ECDH Curve
3) Silently Does Nothing.

If it's 2 or 3 then your patch will make SSLContext().set_ecdh_curve() a no-op and take away users ability to set their own curve.

> I know it doesn't segfault or raise an error if you do that, but I don't know if it:
> 
> 1) Replaces the already called ECDH Curve
> 2) Adds to the already called ECDH Curve
> 3) Silently Does Nothing.

Judging by OpenSSL's code, it replaces the already called ECDH curve.

(from s3_lib.c:

		if (s->cert->ecdh_tmp != NULL)
			EC_KEY_free(s->cert->ecdh_tmp);
		s->cert->ecdh_tmp = ecdh;

)

Works for me, that's what it appears like to me too. +1

Here is an updated patch with proper releasing the EC_KEY structure.

(note that the patch has a slight performance implication: creating a SSLContext becomes more costly - around 100µs more here)

Looks good to me, do you want to commit it Antoine?

I will :)

New changeset 869277faf3dc by Antoine Pitrou in branch '3.4':
Issue #21015: SSL contexts will now automatically select an elliptic curve for ECDH key exchange on OpenSSL 1.0.2 and later, and otherwise default to "prime256v1".
http://hg.python.org/cpython/rev/869277faf3dc

New changeset 3b81d1b3f9d1 by Antoine Pitrou in branch 'default':
Issue #21015: SSL contexts will now automatically select an elliptic curve for ECDH key exchange on OpenSSL 1.0.2 and later, and otherwise default to "prime256v1".
http://hg.python.org/cpython/rev/3b81d1b3f9d1

The Ubuntu LTS buildbot seems to feel better now, closing.

test_default_ecdh_curve is failing on current OS X systems (10.9 Mavericks and 10.8 Mountain Lion, at least) using the system-supplied OpenSSL libraries:

======================================================================
ERROR: test_default_ecdh_curve (test.test_ssl.ThreadedTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/py/3x/unix/source/Lib/test/test_ssl.py", line 2596, in test_default_ecdh_curve
    context.set_ciphers("ECDH")
ssl.SSLError: ('No cipher can be selected.',)

----------------------------------------------------------------------

The OpenSSL command advertise itself as 0.9.8y but it doesn't include any ECDH ciphers.  It appears from the OpenSSL source that it's possible to specify at build configure time which ciphers are included so I guess the version test in _ssl.c for ECDH isn't sufficient.

$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.9.2
BuildVersion:	13C64
$ /usr/bin/openssl version
OpenSSL 0.9.8y 5 Feb 2013
$ /usr/bin/openssl ciphers -v 'ALL:eNULL'
ADH-SEED-SHA            SSLv3 Kx=DH       Au=None Enc=SEED(128) Mac=SHA1
DHE-RSA-SEED-SHA        SSLv3 Kx=DH       Au=RSA  Enc=SEED(128) Mac=SHA1
DHE-DSS-SEED-SHA        SSLv3 Kx=DH       Au=DSS  Enc=SEED(128) Mac=SHA1
SEED-SHA                SSLv3 Kx=RSA      Au=RSA  Enc=SEED(128) Mac=SHA1
ADH-AES256-SHA          SSLv3 Kx=DH       Au=None Enc=AES(256)  Mac=SHA1
DHE-RSA-AES256-SHA      SSLv3 Kx=DH       Au=RSA  Enc=AES(256)  Mac=SHA1
DHE-DSS-AES256-SHA      SSLv3 Kx=DH       Au=DSS  Enc=AES(256)  Mac=SHA1
AES256-SHA              SSLv3 Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA1
ADH-AES128-SHA          SSLv3 Kx=DH       Au=None Enc=AES(128)  Mac=SHA1
DHE-RSA-AES128-SHA      SSLv3 Kx=DH       Au=RSA  Enc=AES(128)  Mac=SHA1
DHE-DSS-AES128-SHA      SSLv3 Kx=DH       Au=DSS  Enc=AES(128)  Mac=SHA1
AES128-SHA              SSLv3 Kx=RSA      Au=RSA  Enc=AES(128)  Mac=SHA1
ADH-DES-CBC3-SHA        SSLv3 Kx=DH       Au=None Enc=3DES(168) Mac=SHA1
ADH-DES-CBC-SHA         SSLv3 Kx=DH       Au=None Enc=DES(56)   Mac=SHA1
EXP-ADH-DES-CBC-SHA     SSLv3 Kx=DH(512)  Au=None Enc=DES(40)   Mac=SHA1 export
ADH-RC4-MD5             SSLv3 Kx=DH       Au=None Enc=RC4(128)  Mac=MD5
EXP-ADH-RC4-MD5         SSLv3 Kx=DH(512)  Au=None Enc=RC4(40)   Mac=MD5  export
EDH-RSA-DES-CBC3-SHA    SSLv3 Kx=DH       Au=RSA  Enc=3DES(168) Mac=SHA1
EDH-RSA-DES-CBC-SHA     SSLv3 Kx=DH       Au=RSA  Enc=DES(56)   Mac=SHA1
EXP-EDH-RSA-DES-CBC-SHA SSLv3 Kx=DH(512)  Au=RSA  Enc=DES(40)   Mac=SHA1 export
EDH-DSS-DES-CBC3-SHA    SSLv3 Kx=DH       Au=DSS  Enc=3DES(168) Mac=SHA1
EDH-DSS-DES-CBC-SHA     SSLv3 Kx=DH       Au=DSS  Enc=DES(56)   Mac=SHA1
EXP-EDH-DSS-DES-CBC-SHA SSLv3 Kx=DH(512)  Au=DSS  Enc=DES(40)   Mac=SHA1 export
DES-CBC3-SHA            SSLv3 Kx=RSA      Au=RSA  Enc=3DES(168) Mac=SHA1
DES-CBC-SHA             SSLv3 Kx=RSA      Au=RSA  Enc=DES(56)   Mac=SHA1
EXP-DES-CBC-SHA         SSLv3 Kx=RSA(512) Au=RSA  Enc=DES(40)   Mac=SHA1 export
EXP-RC2-CBC-MD5         SSLv3 Kx=RSA(512) Au=RSA  Enc=RC2(40)   Mac=MD5  export
RC4-SHA                 SSLv3 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=SHA1
RC4-MD5                 SSLv3 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=MD5
EXP-RC4-MD5             SSLv3 Kx=RSA(512) Au=RSA  Enc=RC4(40)   Mac=MD5  export
DES-CBC3-MD5            SSLv2 Kx=RSA      Au=RSA  Enc=3DES(168) Mac=MD5
DES-CBC-MD5             SSLv2 Kx=RSA      Au=RSA  Enc=DES(56)   Mac=MD5
EXP-RC2-CBC-MD5         SSLv2 Kx=RSA(512) Au=RSA  Enc=RC2(40)   Mac=MD5  export
RC2-CBC-MD5             SSLv2 Kx=RSA      Au=RSA  Enc=RC2(128)  Mac=MD5
EXP-RC4-MD5             SSLv2 Kx=RSA(512) Au=RSA  Enc=RC4(40)   Mac=MD5  export
RC4-MD5                 SSLv2 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=MD5
NULL-SHA                SSLv3 Kx=RSA      Au=RSA  Enc=None      Mac=SHA1
NULL-MD5                SSLv3 Kx=RSA      Au=RSA  Enc=None      Mac=MD5

test_default_ecdh_curve() is still failing on "x86 Ubuntu Shared 3.x":

http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/9964/steps/test/logs/stdio

======================================================================
ERROR: test_default_ecdh_curve (test.test_ssl.ThreadedTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_ssl.py", line 2596, in test_default_ecdh_curve
    context.set_ciphers("ECDH")
ssl.SSLError: ('No cipher can be selected.',)

FreeBSD 9 is failing as well:

http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.0%203.x/builds/6583/steps/test/logs/stdio

> The OpenSSL command advertise itself as 0.9.8y but it doesn't include 
> any ECDH ciphers.

Really? Apple's packaging looks almost criminal here.

> FreeBSD 9 is failing as well:

It's not necessarily the same issue as on OS X. Stefan, can you post the output of the following commands:
* openssl ciphers -v
* openssl ciphers -v ECDH
* openssl ciphers -v EECDH

This is for FreeBSD-9 (which, to be fair, has EOL status):

[stefan@freebsd-amd64 ~]$ openssl ciphers -v
DHE-RSA-AES256-SHA      SSLv3 Kx=DH       Au=RSA  Enc=AES(256)  Mac=SHA1
DHE-DSS-AES256-SHA      SSLv3 Kx=DH       Au=DSS  Enc=AES(256)  Mac=SHA1
AES256-SHA              SSLv3 Kx=RSA      Au=RSA  Enc=AES(256)  Mac=SHA1
DHE-RSA-CAMELLIA256-SHA SSLv3 Kx=DH       Au=RSA  Enc=Camellia(256) Mac=SHA1
DHE-DSS-CAMELLIA256-SHA SSLv3 Kx=DH       Au=DSS  Enc=Camellia(256) Mac=SHA1
CAMELLIA256-SHA         SSLv3 Kx=RSA      Au=RSA  Enc=Camellia(256) Mac=SHA1
EDH-RSA-DES-CBC3-SHA    SSLv3 Kx=DH       Au=RSA  Enc=3DES(168) Mac=SHA1
EDH-DSS-DES-CBC3-SHA    SSLv3 Kx=DH       Au=DSS  Enc=3DES(168) Mac=SHA1
DES-CBC3-SHA            SSLv3 Kx=RSA      Au=RSA  Enc=3DES(168) Mac=SHA1
DES-CBC3-MD5            SSLv2 Kx=RSA      Au=RSA  Enc=3DES(168) Mac=MD5 
DHE-RSA-AES128-SHA      SSLv3 Kx=DH       Au=RSA  Enc=AES(128)  Mac=SHA1
DHE-DSS-AES128-SHA      SSLv3 Kx=DH       Au=DSS  Enc=AES(128)  Mac=SHA1
AES128-SHA              SSLv3 Kx=RSA      Au=RSA  Enc=AES(128)  Mac=SHA1
DHE-RSA-CAMELLIA128-SHA SSLv3 Kx=DH       Au=RSA  Enc=Camellia(128) Mac=SHA1
DHE-DSS-CAMELLIA128-SHA SSLv3 Kx=DH       Au=DSS  Enc=Camellia(128) Mac=SHA1
CAMELLIA128-SHA         SSLv3 Kx=RSA      Au=RSA  Enc=Camellia(128) Mac=SHA1
RC2-CBC-MD5             SSLv2 Kx=RSA      Au=RSA  Enc=RC2(128)  Mac=MD5 
RC4-SHA                 SSLv3 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=SHA1
RC4-MD5                 SSLv3 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=MD5 
RC4-MD5                 SSLv2 Kx=RSA      Au=RSA  Enc=RC4(128)  Mac=MD5 
EDH-RSA-DES-CBC-SHA     SSLv3 Kx=DH       Au=RSA  Enc=DES(56)   Mac=SHA1
EDH-DSS-DES-CBC-SHA     SSLv3 Kx=DH       Au=DSS  Enc=DES(56)   Mac=SHA1
DES-CBC-SHA             SSLv3 Kx=RSA      Au=RSA  Enc=DES(56)   Mac=SHA1
DES-CBC-MD5             SSLv2 Kx=RSA      Au=RSA  Enc=DES(56)   Mac=MD5 
EXP-EDH-RSA-DES-CBC-SHA SSLv3 Kx=DH(512)  Au=RSA  Enc=DES(40)   Mac=SHA1 export
EXP-EDH-DSS-DES-CBC-SHA SSLv3 Kx=DH(512)  Au=DSS  Enc=DES(40)   Mac=SHA1 export
EXP-DES-CBC-SHA         SSLv3 Kx=RSA(512) Au=RSA  Enc=DES(40)   Mac=SHA1 export
EXP-RC2-CBC-MD5         SSLv3 Kx=RSA(512) Au=RSA  Enc=RC2(40)   Mac=MD5  export
EXP-RC2-CBC-MD5         SSLv2 Kx=RSA(512) Au=RSA  Enc=RC2(40)   Mac=MD5  export
EXP-RC4-MD5             SSLv3 Kx=RSA(512) Au=RSA  Enc=RC4(40)   Mac=MD5  export
EXP-RC4-MD5             SSLv2 Kx=RSA(512) Au=RSA  Enc=RC4(40)   Mac=MD5  export
[stefan@freebsd-amd64 ~]$ openssl ciphers -v ECDH
Error in cipher list
34610:error:1410D0B9:SSL routines:SSL_CTX_set_cipher_list:no cipher match:/usr/src/secure/lib/libssl/../../../crypto/openssl/ssl/ssl_lib.c:1218:
[stefan@freebsd-amd64 ~]$ openssl ciphers -v EECDH
Error in cipher list
34611:error:1410D0B9:SSL routines:SSL_CTX_set_cipher_list:no cipher match:/usr/src/secure/lib/libssl/../../../crypto/openssl/ssl/ssl_lib.c:1218:

The docs[1] for SSL_set_ecdh_auto say: "These functions were first added to OpenSSL 1.0.2."  From looking at Modules/_ssl.c, it looks as though we're trying to use them when the version is >= 0.9.8.


[1] ftp://ftp.ulakbim.gov.tr/pub/openssl/docs/ssl/SSL_CTX_set1_curves.html

> The docs[1] for SSL_set_ecdh_auto say: "These functions were first
added to OpenSSL 1.0.2." From looking at Modules/_ssl.c, it looks as
though we're trying to use them when the version is >= 0.9.8.

If that was the issue at hand we would get a compile error, no?

Yep, true.  Ignore me.

> Really? Apple's packaging looks almost criminal here.

Apple has deprecated their bundled version of OpenSSL. This issue has more details, http://bugs.python.org/issue17128

FreeBSD 9.0 has the same broken install:

$ openssl version                                                
OpenSSL 0.9.8y 5 Feb 2013

$ ls /usr/include/openssl/ecd*
/usr/include/openssl/ecdh.h     /usr/include/openssl/ecdsa.h


I'm inclined to view this as an OS issue. FreeBSD 9.2 (koobs'
buildslave) apparently does not have this problem.

In case anyone wonders why the FreeBSD bot works again: I've
installed OpenSSL from source.

> In case anyone wonders why the FreeBSD bot works again: I've
> installed OpenSSL from source.

Did you install the same version?

Antoine Pitrou <report@bugs.python.org> wrote:
> Did you install the same version?

No, I used the latest version + FIPS. Since FreeBSD 9.0 is EOL, I did not
feel like investigating too much. :)

New changeset d6501421b86b by Antoine Pitrou in branch '3.4':
Try to fix buildbot failures on old OpenSSLs (< 1.0.0) - followup to issue #21015
http://hg.python.org/cpython/rev/d6501421b86b

New changeset 1305410bff2d by Antoine Pitrou in branch 'default':
Try to fix buildbot failures on old OpenSSLs (< 1.0.0) - followup to issue #21015
http://hg.python.org/cpython/rev/1305410bff2d

So, I think I've found the issue. On OpenSSL < 1.0.0, the ECDH ciphers exist but the "ECDH" cipher alias doesn't. I've committed a patch which should fix the issue, although the set_ciphers() call may be entirely useless given our current default cipher list.

That does indeed make the test now pass on OS X 10.9:

test_default_ecdh_curve (test.test_ssl.ThreadedTests) ...  server:  new connection from ('127.0.0.1', 60758)
 server: connection cipher is now ('AECDH-AES256-SHA', 'TLSv1/SSLv3', 256)
 server: selected protocol is now None
ok

Thsnks, Antoine!

The buildbots seem happy as well, so I'm closing this.