SSLContext.set_ecdh_curve() not accepting x25519 #77063

Activity

1. sruester commented on Feb 20, 2018

   

   sruestermannequin

   on Feb 20, 2018

   MannequinAuthor

   More actions

   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.
2. sruester

   assigned

   tiran

   on Feb 20, 2018
3. sruester

   added

   topic-SSL

   on Feb 20, 2018
4. tiran commented on Feb 20, 2018

   

   tiran

   on Feb 20, 2018

   Member

   More actions

   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 ?
5. tiran

   added

   3.7 (EOL)end of lifeend of life

   3.8 (EOL)end of lifeend of life

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Feb 20, 2018
6. sruester commented on Apr 21, 2021

   

   sruestermannequin

   on Apr 21, 2021

   MannequinAuthor

   More actions

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

   added

   3.9 (EOL)end of lifeend of life

   3.10 (EOL)end of lifeend of life

   3.11only security fixesonly security fixes

   on Apr 21, 2021
8. ezio-melotti

   transferred this issue fromon Apr 10, 2022
9. OrionCodeDev commented on Apr 22, 2022

   

   OrionCodeDev

   on Apr 22, 2022

   More actions

   Any updates?
10. sla-te commented on Jun 24, 2022

    

    sla-te

    on Jun 24, 2022

    Last edited by sla-te

    More actions

    +
11. gschaffner commented on Jun 9, 2023

    

    gschaffner

    on Jun 9, 2023

    More actions

    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.
12. martinschmatz commented on Sep 12, 2023

    

    martinschmatz

    on Sep 12, 2023

    More actions

    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....
13. martinschmatz commented on Sep 12, 2023

    

    martinschmatz

    on Sep 12, 2023

    More actions

    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.