feat: Add new classification for single (leaf) key #7030
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
Ten0
reviewed
Sep 14, 2025
| } | ||
| return host_params, pool_kwargs | ||
|
|
||
| def is_single_certificate(cert_path): |
There was a problem hiding this comment.
It seems unlikely that special-casing single certs is the best approach 🤔: we probably want to not care about the part of the chain that's above the trusted certificate.
I wouldn't expect curl to special-case single certs either...
Probably we want to match curl's behavior with regards to the openssl config here.
Ten0
reviewed
Sep 14, 2025
|
|
||
| conn.cert_reqs = "CERT_REQUIRED" | ||
|
|
||
| if is_single_certificate(cert_loc): |
There was a problem hiding this comment.
Doesn't this just disable certificate verification if there is a single cert in the chain, without actually verifying that the cert provided by the server matches?
sigmavirus24
requested changes
Sep 14, 2025
There was a problem hiding this comment.
@DarthCeltic the LLMs have led you far astray. You're wrecking the security here rather than doing what was reported as a bug
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.
What this PR does
This pull request addresses the issue of key management being too restrictive due to hardcoded, finite parameters for both leaf and chain keys. It introduces a more flexible approach to handling different key types, specifically a single (leaf) key.
Why this change is needed
The existing implementation was too limited and failed when a single key was passed, as the parameters were not designed to handle that specific use case. This change makes the key management logic more robust and versatile, allowing it to work with a wider range of inputs without breaking.
How this was accomplished
if/thenstatements) to properly classify and handle single (leaf) keys.pytestsuite, which now passes with the new test cases I added for single key scenarios.Related Issue
Closes #6978
Note to reviewer: I've added new tests to cover the single-key use case, which you can see in the
tests/directory. Please let me know if you have any questions or need further clarification.