SSLContext.set_ecdh_curve() not accepting x25519

[9c63a5ca-3c96-4ba4-bf79-17db4cacd9c3]

BPO	32882
Nosy	@tiran, @sruester
PRs	python/cpython#5770 python/cpython#5771
Superseder	bpo-32858: Improve OpenSSL ECDH support

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = 'https://github.com/tiran' closed_at = None created_at = labels = ['expert-SSL', 'type-bug', '3.8', '3.9', '3.10', '3.11', '3.7'] title = 'SSLContext.set_ecdh_curve() not accepting x25519' updated_at = user = 'https://github.com/sruester' ``` bugs.python.org fields: ```python activity = actor = 'sruester' assignee = 'christian.heimes' closed = False closed_date = None closer = None components = ['SSL'] creation = creator = 'sruester' dependencies = [] files = [] hgrepos = [] issue_num = 32882 keywords = ['patch'] message_count = 3.0 messages = ['312405', '312408', '391504'] nosy_count = 2.0 nosy_names = ['christian.heimes', 'sruester'] pr_nums = ['5770', '5771'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = '32858' type = 'behavior' url = 'https://bugs.python.org/issue32882' versions = ['Python 2.7', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11'] ```

[9c63a5ca-3c96-4ba4-bf79-17db4cacd9c3]

Using SSLContext.set_ecdh_curve() it is neither possible to choose X25519, nor to choose a list of curves to be used for key agreement.

[tiran]

This bug was originally the more generic issue bpo-32858.

SSLContext.set_ecdh_curve() uses EC_KEY_new_by_curve_name() and SSL_CTX_set_tmp_ecdh() to configure the ECDH curve parameters. The current approach has multiple downsides. It doesn't work with X25519 and can only set one curve. OpenSSL 1.0.2+ has SSL_CTX_set1_curves_list(), which supports a list of curve names.

Proposal:

SSLContext.set_ecdh_curve() is changed from taking one curve name to an *arg of curve names. With OpenSSL 1.0.2+, 1..n curves are supported. For OpenSSL \< 1.0.2 on 2.7-3.6, one curve is supported. Perhaps it makes sense to map an empty *arg to auto-configuration?

I like to cover the issue in PEP-543, too.

Cory, what do you think about another enum of IANA groups of EC groups, https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-8 ?

[9c63a5ca-3c96-4ba4-bf79-17db4cacd9c3]

PEP-543 was withdrawn in the meantime. Any suggestion how to proceed with this?

[ChronoBrake]

Any updates?

[sla-te]

+

[gschaffner]

GH-103378 partially fixed this: set_ecdh_curve("X25519") now works on OpenSSL 3! it still doesn't work on OpenSSL 1, however. set_ecdh_curve also still only supports one group.

[martinschmatz]

Is there interest in a PR which would add SSLContext.set_ecdh_curves_list() which under the hood would use (for e.g OpenSSL >v3) SSL_CTX_set1_curves_list() (actually the more modern SSL_CTX_set1_groups_list())?

This would add the possibility to specify one or several curves (aka 'groups') by a colon separated string of curve names.

As an alternative, one could change the existing code to avoid the somewhat cumbersome "string -> NID --> set single group" approach, but rather directly use the string value from the PyObject *name function argument and SSL_CTX_set1_groups_list().

I'm fine either way, with a slight preference for the former: It introduces a new function, hence is guaranteed not to interfere with legacy code. But it's a new function which must be maintained going forward....

[martinschmatz]

For the latter approach above, code would be replaced with this (tested on v3.11.5):

static PyObject *
_ssl__SSLContext_set_ecdh_curve(PySSLContext *self, PyObject *name)
/*[clinic end generated code: output=23022c196e40d7d2 input=c2bafb6f6e34726b]*/
{
    PyObject *name_bytes;

    if (!PyUnicode_FSConverter(name, &name_bytes))
        return NULL;
    assert(PyBytes_Check(name_bytes));
#if OPENSSL_VERSION_MAJOR < 3
    int nid;
    nid = OBJ_sn2nid(PyBytes_AS_STRING(name_bytes));
    Py_DECREF(name_bytes);
    if (nid == 0) {
        PyErr_Format(PyExc_ValueError,
                     "unknown elliptic curve name %R", name);
        return NULL;
    }
    EC_KEY *key = EC_KEY_new_by_curve_name(nid);
    if (key == NULL) {
        _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
        return NULL;
    }
    SSL_CTX_set_tmp_ecdh(self->ctx, key);
    EC_KEY_free(key);
#else
    int res = SSL_CTX_set1_groups_list(self->ctx, PyBytes_AS_STRING(name_bytes));
    Py_DECREF(name_bytes);
    if (!res) {
        _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__);
        return NULL;
    }
#endif
    Py_RETURN_NONE;
}

Remark: This will also enable to use the OQS OpenSSL provider to achieve a 'quantum-hard' TLS key agreement, e.g. using x25519_kyber768.