Skip to content

Add AuthorityKeyIdentifier extension for compatibility with Python 3.13#23

Merged
tgmachina merged 4 commits intollnl:mainfrom
minrk:py3.13
Mar 27, 2025
Merged

Add AuthorityKeyIdentifier extension for compatibility with Python 3.13#23
tgmachina merged 4 commits intollnl:mainfrom
minrk:py3.13

Conversation

@minrk
Copy link
Collaborator

@minrk minrk commented Mar 27, 2025

3.13 fails with Missing Authority Key Identifier due to new default strictness

I'm not sure what needs to change to fix this, but certipy certificates are not accepted by default with Python 3.13

also adds test coverage for oldest supported Python (3.7) to make sure it really works. Needed some metadata updates to keep working.

minrk added 4 commits March 27, 2025 12:51
3.13 fails with Missing Authority Key Identifier due to new default strictness
required for default ssl.VERIFY_X509_STRICT in Python 3.13
ssl_context.load_cert_chain(ca_record["files"]["cert"], ca_record["files"]["key"])
# currently required to pass on 3.13
# if hasattr(ssl, "VERIFY_X509_STRICT"):
# ssl_context.verify_flags &= ~ssl.VERIFY_X509_STRICT
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERIFY_X509_STRICT is in the default flags as of Python 3, which checks AuthorityKeyIdentity

@minrk minrk changed the title test with default SSL configuration Add AuthorityKeyIdentifier extension for compatibility with Python 3.13 Mar 27, 2025
cakey = ca_bundle.key.load()

extensions.append(
(x509.AuthorityKeyIdentifier.from_issuer_public_key(cacert.public_key()), False)
Copy link
Collaborator Author

@minrk minrk Mar 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what was requires to pass default ssl checks in Python 3.13

@tgmachina
Copy link
Collaborator

Thanks @minrk! I appreciate you putting this PR together and for updating the tests. Sorry I hadn't covered 3.7 previously. LGTM.

I'll open up a follow on issue to test on a schedule so I catch this next time.

@tgmachina tgmachina merged commit 735bf1e into llnl:main Mar 27, 2025
4 checks passed
ssl_context.load_cert_chain(ca_record["files"]["cert"], ca_record["files"]["key"])

# Succeeds when supplying the CA cert
requests.get(url, verify=ca_record["files"]["cert"])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, the reason I switched from requests to urlopen here is that requests does not use the default ssl config and this test still passed, even on 3.13. Only http libraries that use more default ssl setup (urllib, httpx, tornado, etc.) see this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I figured that might have been why you did that. So it seems the default ssl setup is more strict than what requests is enforcing. Thanks for letting me know.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, requests predates a lot of the standard library SSL stuff and is pretty hardcore about not changing default behavior no matter what, so changes in the standard library often don't affect requests.

@minrk minrk deleted the py3.13 branch March 28, 2025 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants