| msg304577 - (view) |
Author: Andy (grrrrrrrrr) |
Date: 2017-10-18 13:15 |
| When using elliptic curves in combination with the ssl module to wrap a socket, the only curve the client accepts is prime256v1. Expected behavior is to accept all curves that are on the default list and supported by the server. We noticed the issue when connecting to a server that is configured with a non standard curve, we get [SSL: WRONG_CURVE] wrong curve (_ssl.c:661) unless we explicitly specify to use the correct curve for the context using context.set_ecdh_curve("secp521r1") The bug happens both when using a context or ssl.wrap_socket directly. The bug is that cpython calls OpenSSL (or whatever lib is linked) server methods even when the context is purely for client communications. The flow is: - A ssl.SSLContext gets instantiated that will later be used in SSLContext.wrap_socket with server_side either True or False (bug happens when this is client side, so server_side=False). - In the context's constructor, where it's not yet clear if the context is for a client or a server socket, there is this code: https://github.com/python/cpython/blob/b9a860f3bf80b0d4a6c25d0f2f6ef849d9bf3594/Modules/_ssl.c#L2180 That code calls (in many cases, depending on the Python and OpenSSL versions involved) server side only methods, SSL_CTX_set_ecdh_auto or SSL_CTX_set_tmp_ecdh. Those methods should in theory not influence the client side of the context, there is even a comment in the docs for SSLContext.set_ecdh_curve which does similar things that says "This setting doesn’t apply to client sockets". However, this being OpenSSL, this is not true. The methods actually set the list of acceptable curves that is used for both, server and client side connections, to exactly a single curve, prime256v1. This happens for both, SSL_CTX_set_tmp_ecdh and SSL_CTX_set_ecdh_auto. Versions affected: OpenSSL has changed the API twice here. Before 1.0.2, SSL_CTX_set_tmp_ecdh was mandatory, 1.0.2 - <1.1.0 used SSL_CTX_set_ecdh_auto and 1.1.0+ doesn't need this setting at all. Python 2.7.14+ added a check for 1.1.0+ in https://github.com/python/cpython/commit/f1a696efd6ca674579e25de29ec4053ff5a5ade1 so starting from that version the issue only happens when using OpenSSL <1.1.0. I suspect this is the case for many machines still, including all Macs (haven't confirmed the actual bug there). For LibreSSL the server side methods will be called too, I can't confirm if that combination is vulnerable too. Python 3.5.3 gives me the same error, no reason to believe that higher versions are not affected. |
|
|
| msg304579 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2017-10-18 14:06 |
| Which version of OpenSSL are you using? Please note that macOS' system python uses either an ancient version of OpenSSL 0.9.8 or an ancient version of LibreSSL (IIRC 2.3.x). The code in question is: if !defined(OPENSSL_NO_ECDH) && !defined(OPENSSL_VERSION_1_1) /* Allow automatic ECDH curve selection (on OpenSSL 1.0.2+), or use prime256v1 by default. This is Apache mod_ssl's initialization policy, so we should be safe. OpenSSL 1.1 has it enabled by default. */ #if defined(SSL_CTX_set_ecdh_auto) SSL_CTX_set_ecdh_auto(self->ctx, 1); #else { EC_KEY *key = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1); SSL_CTX_set_tmp_ecdh(self->ctx, key); EC_KEY_free(key); } #endif #endif The block is executed for all SSLContexts (server and client) because . The behavior depends on the version of OpenSSL: OpenSSL >= 1.1: not executed OpenSSL >= 1.0.2, < 1.1: SL_CTX_set_ecdh_auto(ctx, 1) LibreSSL: SSL_CTX_set_ecdh_auto(ctx, 1) OpenSSL < 1.0.2: hard-code prime256v1 Since we have no mean to distinguish between a server context and a client context at the moment, we unconditionally call SSL_CTX_set_ecdh_auto(). It may not be perfect under some condition. But it gives most users a sane and secure default to start with. https://www.openssl.org/docs/man1.0.2/ssl/SSL_CTX_set_ecdh_auto.html |
|
|
| msg304581 - (view) |
Author: Andy (grrrrrrrrr) |
Date: 2017-10-18 15:10 |
| While debugging I reproduced this on - 'OpenSSL 1.1.0f 25 May 2017' - 'OpenSSL 1.0.1f 6 Jan 2014' - and 'BoringSSL', latest. using Python 2.7.12, 2.7.13, 2.7.6 and 3.5.3. This was all on Debian. Note that since I used Python <2.7.14 (or equivalent for 3.x) for all tests, the check "... && !defined(OPENSSL_VERSION_1_1)" is missing and therefore the bug *always* triggers regardless of OpenSSL version. I'm not sure I agree that this one curve is a good default. Note that openssl has 81 curves currently (openssl ecparam -list_curves) and probably can use most of them to connect to a server - accommodating a variety of server setups. Restricting this list to one single curve seems suboptimal. Think about it like this, if tomorrow we find an issue with that particular curve, all servers can just migrate to a different one and all clients will be able to connect just fine - except those that use Python, they will not be able to talk to those servers ever again until they are upgraded. I mean in the end it's your call but having a *client* just accepting one single security parameter and nothing else doesn't seem right. |
|
|
| msg304586 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2017-10-18 16:10 |
| - BoringSSL is not a supported libssl/libcrypto library for Python. We only support 100% OpenSSL-compatible libraries. - OpenSSL 1.0.1 is no longer supported by upstream. Python's semi-official support policy for 1.0.1 and 0.9.8 is "use at your own risk". You should upgrade. - Python < 2.7.13 does not support OpenSSL 1.1 - Python >= 2.7.13 has #if !defined(OPENSSL_VERSION_1_1) before SSL_CTX_set_ecdh_auto() I added a new test that verifies connections with various curve settings. I cannot reproduce your problem with the test case. |
|
|
| msg304978 - (view) |
Author: Andy (grrrrrrrrr) |
Date: 2017-10-25 11:42 |
| Thanks for adding the test! If the official stance is that only the latest OpenSSL is supported then this is definitely WAI. Sounds like a good policy... I'll close this issue. |
|
|
| msg312785 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2018-02-25 08:49 |
| New changeset b7b9225831a729bff84eb7c43bad138416b994fe by Christian Heimes in branch 'master': bpo-31809: test secp ECDH curves (#4036) https://github.com/python/cpython/commit/b7b9225831a729bff84eb7c43bad138416b994fe |
|
|
| msg312794 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2018-02-25 09:56 |
| New changeset ff7528f089b60f8372c658f3bc3b14b059114da9 by Christian Heimes (Miss Islington (bot)) in branch '3.7': [3.7] bpo-31809: test secp ECDH curves (GH-4036) (#5872) https://github.com/python/cpython/commit/ff7528f089b60f8372c658f3bc3b14b059114da9 |
|
|