-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[ARM] Fix #20842: az bicep
: Fix to use requests environment variables for CA bundle
#21807
Conversation
Thank you for your contribution wwmoraes! We will review the pull request and get back to you soon. |
Azure CLI relies on the requests python package, which allows users to set custom CA bundle paths through two environment variables: CURL_CA_BUNDLE and REQUESTS_CA_BUNDLE. This is useful for cases where the host is behind a MITM proxy that re-signs all SSL traffic for DPI, or due to a corporate policy that defines which CA roots are to be trusted. Enforcing a hard-coded bundle, like the one provided by the certifi package, only forces users under similar use cases to modify the Python site packages files, which is meant to be controlled only by the package manager and thus it is not a good practice to configure the tool.
Bicep |
Could someone check this, please? |
@shenglol Could you please help to review this PR? |
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.
az bicep
: Fix to use requests environment variables for CA bundle
Please include PR ID in the commit message of e0bb745. |
Has this fix been released? If so, it does not work, still seeing the same error. |
@inaun This PR has been released in version |
os.environ.setdefault("CURL_CA_BUNDLE", certifi.where()) | ||
request = urlopen(_get_bicep_download_url(system, release_tag, target_platform=target_platform)) |
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.
Copying @emitor's comment from e0bb745#r71858253:
Hi @wwmoraes, urlopen
does not use the OS environment CURL_CA_BUNDLE
as request
does. So if you remove the cafile=ca_file
parameter this break the command az bicep
if you are behind a proxy using a custom CA.
I'm trying to using this at work behind a corporate proxt and does not work anymore and throws the [SSL: CERTIFICATE_VERIFY_FAILED]
. I've manually modified the file locally and is working.
In order to do it properly so urlopen
does not throw any warnings, you should change it to something like this:
...
import ssl
...
context = ssl.create_default_context(cafile=certifi.where())
request = urlopen(_get_bicep_download_url(system, release_tag, target_platform=target_platform), context=context)
...
Would you be able to fix this back please?
Thanks!
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.
In short, urlopen
doesn't support REQUESTS_CA_BUNDLE
or CURL_CA_BUNDLE
and we shouldn't make urlopen
support them either.
os.environ.setdefault("CURL_CA_BUNDLE", certifi.where()) | ||
response = requests.get("https://aka.ms/BicepReleases") |
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.
Per https://requests.readthedocs.io/en/latest/user/advanced/#ssl-cert-verification, there is no need to set certifi.where()
to CURL_CA_BUNDLE
at all.
requests
checks CA bundles in following order:
REQUESTS_CA_BUNDLE
CURL_CA_BUNDLE
certifi.where()
If REQUESTS_CA_BUNDLE
or CURL_CA_BUNDLE
is not set, request
by default uses certifi.where()
.
Description
Azure CLI relies on the requests python package, which allows users to set custom CA bundle paths through two environment variables:
CURL_CA_BUNDLE
andREQUESTS_CA_BUNDLE
. https://2.python-requests.org/en/master/user/advanced/#ssl-cert-verificationThis is useful for cases where the host is behind a MITM proxy that re-signs all SSL traffic for DPI, or due to a corporate policy that defines which CA roots are to be trusted.
Enforcing a hard-coded bundle, like the one provided by the certifi package, only leads users under similar use cases to modify the Python site packages files, which is meant to be controlled only by the package manager and thus it is not a good practice to configure any Python solution.
The proposed change here leverages Python
setdefault
mechanism available on dictionaries, which allows to set a default value for a key that is not set. This makes any lookups return the default value instead of their usual result (KeyError
exception on an index lookup for a missing key, orNone
on aget
method call).Setting the
CURL_CA_BUNDLE
environment variable default value to the certifi bundle path results in the following resolution order:REQUESTS_CA_BUNDLE
environment variable is used if setCURL_CA_BUNDLE
environment variable is used if setcertifi.where()
path is used otherwisefixes #20842.
Testing Guide
A clean environment should use the certifi CA bundle path:
If either
REQUESTS_CA_BUNDLE
orCURL_CA_BUNDLE
is set, then it should be picked up:If both
REQUESTS_CA_BUNDLE
andCURL_CA_BUNDLE
are set, then it should useREQUESTS_CA_BUNDLE
(this is by definition of the requests package).History Notes
[ARM] Fix #20842:
az bicep
: Fix to use requests environment variables for CA bundleThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.