gh-131050: Allow CPython test to handle TLS libraries lacking FFDHE ciphersuites #131051
+16
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WillChilds-Klein
force-pushed
the
detect-dhe-support
branch
from
145d860 to
d4121d7
Compare
picnixz
reviewed
Mar 12, 2025
There was a problem hiding this comment.
Strictly speaking, the (FF) part is usually used to distinguish between FF-based DH and elliptic curve DH. Rather than skipping the test, I think we should actually add a test for checking ECDHE in this case. The reason is that ADH and EDH/DHE do not contain EC-based DH.
Otherwise, it's as if we're skipping a test that shouldn't really be skipped (we're not testing loading of DH parameters). Are AWS-LC and BoringSSL only supporting EC-based DH?
Misc/NEWS.d/next/Tests/2025-03-10-18-58-03.gh-issue-131050.FMBAPN.rst
Outdated
Show resolved
Hide resolved
…APN.rst Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz
reviewed
Mar 13, 2025
picnixz
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @picnixz for commit 9bf7e6c 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131051%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
Until the build bots are done, don't update the PR. TiA |
|
Some build bots may fail due to other issues (like the BigMem one as I think it doesn't contain the fix from yesterday) but I'm just interested in testing the various supported build bots. |
|
Hi @picnixz, do you think the buildbot failures are caused by this PR? Going by the buildbot logs, it doesn't look like the reference leaks are related to |
|
Click on "Update branch" and I'll rerun them! I don't think they are related though but some fixes were added recently. |
|
Ok, branch updated. Thanks @picnixz! |
gpshead
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @gpshead for commit 781d531 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131051%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
picnixz
reviewed
Mar 22, 2025
There was a problem hiding this comment.
I have a suggestion where we can remove the return tpe annotation and also extract supports_dhe into a more general function of the form:
def supports_kx_alias(context, aliases):
for cipher in ctx.get_ciphers():
for alias in aliases:
if f"Kx={alias}" in cipher['description']:
return True
return False
Maybe we can use it later or in some other parts of the code. But first, let the build bots finish before committing (namely, don't touch this PR!)
|
Everything looks good now. Maybe you want to extract the little helper to help refactoring the test suite, otherwise we can let it like this. |
picnixz
reviewed
Mar 23, 2025
picnixz
reviewed
Mar 29, 2025
picnixz
approved these changes
Mar 29, 2025
|
Sorry for the back-and-forth with the auto squash-merge but I changed the commit message quite a lot and since I can't preview it... |
picnixz
added
needs backport to 3.12
|
Thanks @WillChilds-Klein for the PR, and @picnixz for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Mar 29, 2025
…iphersuites (pythonGH-131051) (cherry picked from commit be2d218) Co-authored-by: Will Childs-Klein <willck93@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-131874 is a backport of this pull request to the 3.13 branch. |
miss-islington
pushed a commit
to miss-islington/cpython
that referenced
this pull request
Mar 29, 2025
…iphersuites (pythonGH-131051) (cherry picked from commit be2d218) Co-authored-by: Will Childs-Klein <willck93@gmail.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
|
GH-131875 is a backport of this pull request to the 3.12 branch. |
|
seehwan
pushed a commit
to seehwan/cpython
that referenced
this pull request
Apr 16, 2025
…iphersuites (python#131051) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notes
Please see #131050's description.
Testing