cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Enable specification of multiple curves(/groups) for TLS

Open martinschmatz opened this issue 2 years ago • 0 comments

Feature or enhancement

Proposal:

Context: Setting multiple curves/groups in the client hello of TLS clients. Enabling OpenSSLv3 providers.

Issue: Function set_ecdh_curve allows only to specify a single curve for an SSL context, which must support the generation of an EC key via OpenSSL function EC_KEY_new_by_curve_name(nid). This makes it impossible that a client offers more than one curve/group in the client hello (if not specified via openssl.cnf) and also is not supported by some OpenSSL providers (e.g. OQS).

Effect: When addressed, this would add the possibility to specify one or several curves (aka 'groups') by a colon separated string of curve names.

Implementation options: 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().

As an alternative, we could add a new function 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())

I'm fine either way, with a slight preference for the former: It does NOT introduce a new function, which must be maintained going forward. But it isn't 110% guaranteed not to interfere with legacy code.

Preferred Implementation: ==> Code change for changing existing function (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.

Usage: If implemented as above, usage could look like this when using an SSL context:

myContext.set_ecdh_curve('x25519_kyber768:X25519:secp521r1')

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

This is a follow-up to #77063, an issue raised in 2018 which seemingly is no longer monitored.

Linked PRs

  • gh-119244

martinschmatz avatar Sep 27 '23 07:09 martinschmatz