-
Notifications
You must be signed in to change notification settings - Fork 880
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
fix for python3-requests 2.32.2 #5435
Conversation
Thank you for this contribution! Would you mind adding your GitHub username to the tools/.github-cla-signers file and committing that change to this PR? Additionally, you need to sign the Canonical contributor licence agreement. This grants us your permission to use your contributions in the project. Thanks in advance! |
Oh also, would you mind editing your PR description so that it matches the desired pull request template as such: Proposed Commit Message
Additional ContextFixes: #5434 Test StepsChecklist
Merge type
|
I'm trying. |
get_connection() is deprecated in requests 2.32.2+ so this will allow for the LXDSocketAdapter to avoid using a deprecated api. Fixed:canonical#5434 Signed-off-by: eaglegai <eaglegai@163.com>
@a-dubs hello, I have finished the CLA, and changed the commit message. |
@eaglegai @a-dubs I just saw the "Check ruff" job failed because we didn't run |
Thank you @blackboxsw ! |
Thanks for proposing this @eaglegai! To quote the description in psf/requests#6710, the proposed solution (which matches the example code) should be backwards compatible between versions of Requests. Addressing the CVE in requests and its impact on cloud-init:From the description of CVE-2024-34195:
The proposes remediations are:
In cloud-init, this code is used only to communicate over a local unix socket to a trusted service, the LXD IMDS. It does not use While requiring a dependency on 2.32.0 is ultimately a downstream choice to make, from an upstream perspective we already have a recommended remediation in place. |
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.
I've requested Canonical's security team take a look at this, but I feel comfortable with merging this since cloud-init already remediates the issue and the change in this PR is simply implementing the recommended way to work with the api changes in 2.32.2 and greater that resulted from this CVE.
ACK from the Security Team, change looks reasonable, and is backwards compatible. |
get_connection() is deprecated in requests 2.32.2+ so this will allow for the LXDSocketAdapter to avoid using a deprecated api. Fixes GH-5434 Signed-off-by: eaglegai <eaglegai@163.com>
Fix for requests 2.32.2+:
Add get_connection_with_tls_context() for requests 2.32.2 as suggests: psf/requests#6710