Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Polish SSLError #11093

Merged
merged 7 commits into from
Nov 5, 2019
Merged

Polish SSLError #11093

merged 7 commits into from
Nov 5, 2019

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Nov 4, 2019 •
edited
Loading

Polish SSLError reported in issues #11069, #11068, #10921, #10860, #10272 ...

Changed 3 places:

  • src/azure-cli/azure/cli/command_modules/profile/custom.py when logging in
  • src/azure-cli-core/azure/cli/core/adal_authentication.py when retrieving tokens
  • src/azure-cli-core/azure/cli/core/util.py when making REST requests

To test, replace <virtual env>\Lib\site-packages\certifi\cacert.pem with some self-signed certificate like Fiddler's:

-----BEGIN CERTIFICATE-----
MIIDsjCCApqgAwIBAgIQP8ptgN302YBPYVf50e6xfDANBgkqhkiG9w0BAQsFADBn
MSswKQYDVQQLDCJDcmVhdGVkIGJ5IGh0dHA6Ly93d3cuZmlkZGxlcjIuY29tMRUw
EwYDVQQKDAxET19OT1RfVFJVU1QxITAfBgNVBAMMGERPX05PVF9UUlVTVF9GaWRk
bGVyUm9vdDAeFw0xODA3MzAxMjQ4MzFaFw0yMTEwMjgxMjQ4MzFaMGcxKzApBgNV
BAsMIkNyZWF0ZWQgYnkgaHR0cDovL3d3dy5maWRkbGVyMi5jb20xFTATBgNVBAoM
DERPX05PVF9UUlVTVDEhMB8GA1UEAwwYRE9fTk9UX1RSVVNUX0ZpZGRsZXJSb290
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAm4kCm94NDkJ91DMcTBjq
z/u1LPkiXlBbcknTEp4CBkEtVr8rfGmm058M01QikoLigh8OISeU+MbCao/8fvCg
+TFYHQFFZONKw6sad4IHN6iHwVpPXgfCtP52ZnCGfntrlNJdikQeycPjDSJNDLpy
XGuNkIwk2jREts+u61EF0459CPtyGn9bpxfDQ5LNP5YJ/1dXnslUJAXTLvP4xBKT
u2QP+5OmOjKTHXP53lqqW0rOioijj6PsvPNf+517SoFJrMR5Q9P+FT6BSYav83kU
F/NvIWNRQLrGgx5BXjc4WXDhnC9mM8pUghj4lEfAEBXn/qmntedp4IZoIWbSbANL
lQIDAQABo1owWDATBgNVHSUEDDAKBggrBgEFBQcDATASBgNVHRMBAf8ECDAGAQH/
AgEAMB0GA1UdDgQWBBS18ByP6P35iRo34tV/nbc8FZ0X9DAOBgNVHQ8BAf8EBAMC
AQYwDQYJKoZIhvcNAQELBQADggEBAGa9svmawn4LYELUrotMQVeZF57hPSExWzpr
7UbHMOtoIfGGu1AMosJFfvTZIEhPePLQuZqIQwdSm9m7kmFegqLqoBYjTJRq9mur
KvJvGDXyyTNN2rQV0qf0JL7LFp5XdiltTABtfRKHKZ5vGZPi6lanQwsvBUryShHe
erNP1JIg0ZXS0yFJUdtLYnhFOUqp/ZutWYJjsBB/GMqpHp8rYl6vJSE0pzhRITjP
xk586Tj0RBCby4DJw35AwQeuk9lBqNCVjtNZkKKhEUmAAut6cnLsHWTbV136OcP1
y5useyKJgGCR9p7TWOW2PVz968WH8U/tq3orip3CaTsstGoyt+c=
-----END CERTIFICATE-----

az login now fails with

Certificate verification failed. This typically happens when using Azure CLI behind a proxy that intercepts traffic with a self-signed certificate. Please add this certificate to the trusted CA bundle: https://github.com/Azure/azure-cli/blob/dev/doc/use_cli_effectively.md#working-behind-a-proxy. Error detail: HTTPSConnectionPool(host='login.microsoftonline.com', port=443): Max retries exceeded with url: /common/oauth2/token (Caused by SSLError(SSLError("bad handshake: Error([('SSL routines', 'tls_process_server_certificate', 'certificate verify failed')])")))

If logged in, az vm list fails with similar message.

This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@jiasli jiasli self-assigned this Nov 4, 2019
fengzhou-msft
fengzhou-msft reviewed Nov 4, 2019
View reviewed changes
src/azure-cli/azure/cli/command_modules/profile/custom.py Outdated
@@ -161,6 +161,12 @@ def login(cmd, username=None, password=None, service_principal=None, tenant=None
# pylint: disable=line-too-long
"More details are available at https://github.com/AzureAD/microsoft-authentication-library-for-python/wiki/Username-Password-Authentication")
raise CLIError(err)
except requests.exceptions.SSLError as err:
raise CLIError('Certificate verification failed. Please typically happens when using Azure CLI behind a proxy '
Copy link
Member

Choose a reason for hiding this comment

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

typo Please typically happens

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. Fixed in new commit.

qwordy
qwordy reviewed Nov 5, 2019
View reviewed changes
@jiasli jiasli merged commit 6494316 into Azure:dev Nov 5, 2019
@jiasli jiasli added this to the S161 milestone Nov 19, 2019
@jiasli jiasli mentioned this pull request Nov 25, 2019
Closed
@yonzhan yonzhan mentioned this pull request Feb 3, 2020
Closed
@jiasli jiasli mentioned this pull request Feb 11, 2020
Closed
@jiasli jiasli deleted the sslerror branch February 11, 2020 12:07
@jiasli jiasli mentioned this pull request Nov 16, 2023
Draft
@jiasli jiasli mentioned this pull request Feb 12, 2025
Merged
@jiasli
Copy link
Member Author

jiasli commented Feb 12, 2025

@jiasli, Why is SSLError handled in 3 places, instead of the centralized azure.cli.core.util.handle_exception?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qwordy qwordy qwordy approved these changes

@fengzhou-msft fengzhou-msft fengzhou-msft approved these changes

@yugangw-msft yugangw-msft Awaiting requested review from yugangw-msft

@yonzhan yonzhan Awaiting requested review from yonzhan

@bim-msft bim-msft Awaiting requested review from bim-msft

Assignees

@jiasli jiasli

Labels
None yet
Projects
None yet
Milestone
S161
Development

Successfully merging this pull request may close these issues.
3 participants
@jiasli @qwordy @fengzhou-msft