Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-109945: Enable spec of multiple curves/groups for TLS#119244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base:main
Are you sure you want to change the base?
Changes from all commits
403b9178ac384265490a28ec8608ebad50caf71eee8459b661af4b23b48e5bc42ec0c867fd580fae26208a2cbdeFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1375,11 +1375,29 @@ def test_set_ecdh_curve(self): | ||
| ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) | ||
| ctx.set_ecdh_curve("prime256v1") | ||
| ctx.set_ecdh_curve(b"prime256v1") | ||
| # Only OpenSSL 3 and above supported for multiple curves | ||
| if (IS_OPENSSL_3_0_0 >= 3): | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
| ||
| ctx.set_ecdh_curve("prime256v1:brainpoolP384r1") | ||
| ctx.set_ecdh_curve(b"prime256v1:brainpoolP384r1") | ||
| self.assertRaises(TypeError, ctx.set_ecdh_curve) | ||
| self.assertRaises(TypeError, ctx.set_ecdh_curve, None) | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, "foo") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, b"foo") | ||
| # Multiple bad curves should cause error for any OpenSSL version | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, "foo:bar") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, b"foo:bar") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, "prime256v1:bar") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, b"prime256v1:bar") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, "foo:prime256v1") | ||
| self.assertRaises(ValueError, ctx.set_ecdh_curve, b"foo:prime256v1") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, ":") | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's up with the commented out test cases? are these not valuable? it seems like a set of edge cases worth covering to define behavior on unusual inputs. | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, b":") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, "::") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, b"::") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, "prime256v1:") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, b"prime256v1:") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, ":prime256v1") | ||
| #self.assertRaises(ValueError, ctx.set_ecdh_curve, b":prime256v1") | ||
| def test_sni_callback(self): | ||
| ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Adds support for multiple curves to be specified in SSLContext.set_ecdh_curve() for OpenSSL 3.0 and above by setting curve_name to a colon separated list of curves. This allows multiple curves to be passed on a TLS client hello. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -4379,18 +4379,19 @@ _ssl__SSLContext_set_ecdh_curve(PySSLContext *self, PyObject *name) | ||
| /*[clinic end generated code: output=23022c196e40d7d2 input=c2bafb6f6e34726b]*/ | ||
| { | ||
| PyObject *name_bytes; | ||
| int nid; | ||
| 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; | ||
| } | ||
| #if OPENSSL_VERSION_MAJOR < 3 | ||
| EC_KEY *key = EC_KEY_new_by_curve_name(nid); | ||
| if (key == NULL){ | ||
| _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); | ||
| @@ -4399,8 +4400,10 @@ _ssl__SSLContext_set_ecdh_curve(PySSLContext *self, PyObject *name) | ||
| SSL_CTX_set_tmp_ecdh(self->ctx, key); | ||
| EC_KEY_free(key); | ||
| #else | ||
| if (!SSL_CTX_set1_groups(self->ctx, &nid, 1)){ | ||
| _setSSLError(get_state_ctx(self), NULL, 0, __FILE__, __LINE__); | ||
| int res = SSL_CTX_set1_groups_list(self->ctx, PyBytes_AS_STRING(name_bytes)); | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SSL_CTX_set1_groups_list also supports another syntax, adding a
This isn't tested in our test suite, it would be useful to test that so future contributors know that syntax exists. | ||
| Py_DECREF(name_bytes); | ||
| if (!res){ | ||
| PyErr_Format(PyExc_ValueError,"unknown elliptic curves %R", name); | ||
| return NULL; | ||
| } | ||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need a "new in version 3.14" admonition.