-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(google-auth): fail loudly when dynamically disabling mTLS on active sessions #18005
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,6 +182,13 @@ async def _do_configure(): | |
| google.auth.transport._mtls_helper.check_use_client_cert | ||
| ) | ||
| if not use_client_cert: | ||
| # Dynamically disabling mTLS on an active session is unsafe in concurrent | ||
| # environments and can cause a zombie state mismatch where mTLS contexts | ||
| # remain attached while auth checks believe mTLS is disabled. | ||
| if getattr(self, "_is_mtls", False): | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedSession must be created." | ||
| ) | ||
| return | ||
|
|
||
| try: | ||
|
|
@@ -191,6 +198,12 @@ async def _do_configure(): | |
| key, | ||
| ) = await mtls.get_client_cert_and_key(client_cert_callback) | ||
|
|
||
| # Prevent mid-lifecycle transition from mTLS-enabled to mTLS-disabled state. | ||
| if getattr(self, "_is_mtls", False) and not is_mtls: | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedSession must be created." | ||
| ) | ||
|
Comment on lines
+202
to
+205
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raising a References
|
||
|
|
||
| if is_mtls: | ||
| # Re-create the auth request with the new SSL context | ||
| if AIOHTTP_INSTALLED and isinstance( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -469,6 +469,13 @@ def configure_mtls_channel(self, client_cert_callback=None): | |
| """ | ||
| use_client_cert = google.auth.transport._mtls_helper.check_use_client_cert() | ||
| if not use_client_cert: | ||
| # Dynamically disabling mTLS on an active session is unsafe in concurrent | ||
| # environments and can cause a zombie state mismatch where mTLS adapters | ||
| # remain attached while auth checks believe mTLS is disabled. | ||
| if getattr(self, "_is_mtls", False): | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedSession must be created." | ||
| ) | ||
|
Comment on lines
+475
to
+478
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raising a References
|
||
| return | ||
|
|
||
| try: | ||
|
|
@@ -480,6 +487,12 @@ def configure_mtls_channel(self, client_cert_callback=None): | |
| client_cert_callback | ||
| ) | ||
|
|
||
| # Prevent mid-lifecycle transition from mTLS-enabled to mTLS-disabled state. | ||
| if getattr(self, "_is_mtls", False) and not is_mtls: | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedSession must be created." | ||
| ) | ||
|
Comment on lines
+491
to
+494
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raising a References
|
||
|
|
||
| old_adapter = self.adapters.get("https://") | ||
|
|
||
| kwargs = {} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,13 +350,26 @@ def configure_mtls_channel(self, client_cert_callback=None): | |
| """ | ||
| use_client_cert = transport._mtls_helper.check_use_client_cert() | ||
| if not use_client_cert: | ||
| # Dynamically disabling mTLS on an active session is unsafe in concurrent | ||
| # environments and can cause a zombie state mismatch where mTLS connection | ||
| # pools remain attached while auth checks believe mTLS is disabled. | ||
| if getattr(self, "_is_mtls", False): | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedHttp must be created." | ||
| ) | ||
|
Comment on lines
+356
to
+359
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raising a References
|
||
| return False | ||
|
|
||
| try: | ||
| found_cert_key, cert, key = transport._mtls_helper.get_client_cert_and_key( | ||
| client_cert_callback | ||
| ) | ||
|
|
||
| # Prevent mid-lifecycle transition from mTLS-enabled to mTLS-disabled state. | ||
| if getattr(self, "_is_mtls", False) and not found_cert_key: | ||
| raise exceptions.MutualTLSChannelError( | ||
| "Cannot disable mTLS on an active session. A new AuthorizedHttp must be created." | ||
| ) | ||
|
Comment on lines
+368
to
+371
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Raising a References
|
||
|
|
||
| if found_cert_key: | ||
| new_http = _make_mutual_tls_http(cert, key) | ||
| new_is_mtls = True | ||
|
|
||
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.
Raising a
MutualTLSChannelErrorexception here replaces the historical graceful fallback behavior with an exception. This introduces breaking changes for downstream users and violates backwards compatibility.References