Repository navigation
Add support in SSL module for getting/setting groups used for key agreement #136306
Description
Add support for getting/setting groups used for key agreement
Proposal:
This feature proposal is an expansion of the feature proposed in issue #109945. It began as a discussion on the PR where I provided some suggestions on generalizing that feature to include supporting more than just EC curves, and I provided some rough example code. Since then, I've put together a more complete version of this which I'll be submitting shortly as a PR attached to this issue.
The basic idea is to add three new methods related to getting & setting groups used for key agreement:
SSLContext.get_groups() -> List[str]:
"""Get a list of groups implemented for key agreement, taking into account
the SSLContext's current TLS `minimum_version` and `maximum_version` values."""
SSLContext.set_groups(groups: str) -> None:
"""Set the groups allowed for key agreement for sockets created with this context."""
SSLSocket.group() -> str:
"""Return the group used for key agreement, after the TLS handshake completes."""These methods are designed to directly mimic the existing methods for getting and setting ciphers suites. Prior to TLS 1.3, all of this could be done with just setting ciphers, but that's no longer the case.
This proposal provides a superset of the functionality requested in #109945, allowing not only multiple EC curves to be specified but also allowing other mechanisms like fixed field DHE and post-quantum algorithms added in OpenSSL 3.5. In fact, once the set_groups() method is available the existing set_ecdh_curve() method could be deprecated, as the methods it calls are available all the way to OpenSSL 1.1.1, which is now the minimum supported OpenSSL version for Python.
The group() and get_groups() methods require later versions of OpenSSL (3.2 and 3.5, respectively), but the code can check for this and raise a NotImplemented exception if the version of OpenSSL that Python is built against is too old to support them.
Links to previous discussion of this feature:
Previous discussion occurred in PR #119244, and it was suggested that it might be best to create a new issue and PR, since the previous request might not be monitored any more.
Activity
pythongh-136306: Initial cut at SSL groups support
pythongh-136306: Address first round of comments
pythongh-136306: Add back indentation of example
pythongh-136306: Handle another allocation failure in get_groups()
pythongh-136306: Address additional review comments
pythongh-136306: Fix method references in whatsnew entry.
pythongh-136306: Add missing documentation for SSLSocket.group()
pythongh-136306: Address additional review comments
pythongh-136306: Minor doc updates
pythongh-136306: Missed one
pythongh-136306: Address additional review comments
pythongh-136306: Regenerate clinic signature to fix missing end fo line
3 remaining items
picnixz commentedon Aug 4, 2025
I'm re-opening the issue because this broke the AMD64 CentOS9 FIPS Only Blake2 Builtin Hash 3.x which was working until now. I don't think the issue is with the PR itself, but rather that something is not supported due to the OpenSSL build configuration (outside Python's control).
picnixz commentedon Aug 4, 2025
So the error is:
The OpenSSL version of this buildbot is configured as:
that is, for the options:
Interestingly, the other FIPS buildbot that is working has another version:
For the options, that is
ronf commentedon Aug 4, 2025
Since this is a FIPS build, X25519 will not be available. However, P-256 should be available. To avoid getting an error here, we might need to add a '?' in front of X25519 in the test (e.g. 'P-256:?X25519'). Alternately, we could odd in a #if check for whether this is a FIPS compile if that's available, and request only 'P-256' in that case.
picnixz commentedon Aug 4, 2025
Wait X25519 is not allowed? I thought that a recent draft allowed its usage. Anyway, to know whether the FIPS module is enabled, there is
_hashlib.get_fips_mode(). So yes, let's skip the test if this is in FIPS mode.Or .. maybe it's a stupid idea but how about "P-256:P-256"? would it work actually?
ronf commentedon Aug 4, 2025
FIPS recently allowed x25519mlkem768, but they haven't gone back and allowed plain X25519. Even if they did at some point in the future I'm not sure OpenSSL 1.1.1 would pick up those changes.
My recommendation would be to add the '?' flag in front of x25519, which says to just ignore X25519 on OpenSSL versions/configurations that don't support it. The other option would be something like "P-256:P-384". While I expect 'P-256:P-256' would probably also work, I don't think we need to resort to that.
picnixz commentedon Aug 4, 2025
Ok, let's just do the ? trick. Could you open a PR for that and add a comment indicating why we need it?
ronf commentedon Aug 5, 2025
Sure - I'll try to get to that in the next day or two. In the meantime, only FIPS builds should be affected, and it should really only affect the unit tests. The resulting binary should still be completely usable.
picnixz commentedon Aug 5, 2025
Yes that I agree. I have a few hours so I'll do it myself
picnixz commentedon Aug 5, 2025
Ah actually, ignoring unknow groups is not possible before OpenSSL 3.3:
gh-136306: fix `test_ssl.ContextTests.test_set_groups` on FIPS builds (…
picnixz commentedon Aug 5, 2025
Fixed in 0af7556.
pythongh-136306: Add support for getting and setting SSL groups (pyth…
pythongh-136306: fix `test_ssl.ContextTests.test_set_groups` on FIPS …