feat(auth): Implement python mtls helpers by nbayati · Pull Request #17495 · googleapis/google-cloud-python

nbayati · 2026-06-17T17:59:02Z

This PR provides helper methods to allow custom HTTP and WebSocket connection pools (such as those in google-genai and google-adk) to load default client certificates and resolve the GOOGLE_API_USE_MTLS_ENDPOINT env var. Changes include:

Introduced GOOGLE_API_USE_MTLS_ENDPOINT environment variable to control whether an mTLS endpoint should be used (always, never, or auto).
Added several new helper functions in google.auth.transport.mtls to facilitate SSL context creation and client certificate loading:
- _load_client_cert_into_context: Loads a client certificate and key into a provided SSL context.
- load_default_client_cert: Discovers and loads the default client certificate into a provided SSL context if mTLS is enabled.
- get_default_ssl_context: Returns a default SSL context pre-loaded with the default client certificate, or None if unavailable.
- should_use_mtls_endpoint: Determines if an mTLS endpoint should be used based on the new environment variable and certificate availability.
Fixed outdated docstrings for default_client_cert_source and default_client_encrypted_cert_source to correctly state they raise MutualTLSChannelError instead of DefaultClientCertSourceError.
Added comprehensive unit tests for the new mTLS helper methods.

gemini-code-assist

Code Review

This pull request removes the dependency on pyOpenSSL and cffi across the codebase, transitioning to the standard library ssl module and the cryptography library for mutual TLS (mTLS) support. It introduces a secure three-tier fallback strategy for handling client certificates and private keys via secure_cert_key_paths in _mtls_helper.py. Feedback on these changes includes addressing a potential NameError in urllib3.py due to an unimported module, utilizing contextlib.ExitStack instead of manual context manager calls to prevent resource leaks, and verifying write permissions for /dev/shm to avoid runtime crashes in restricted environments.

macastelaz

The last commit, bc6d2b8, LGTM. I do also wonder if everything needs to be public or not though.

I am withholding approval on the PR as it currently contains much more than just this last commit.

lsirac

will post on line numbers

…ndwritten SDK mTLS support - Introduced `GOOGLE_API_USE_MTLS_ENDPOINT` environment variable to control whether an mTLS endpoint should be used (`always`, `never`, or `auto`). - Added several new helper functions in `google.auth.transport.mtls` to facilitate SSL context creation and client certificate loading: - `load_client_cert_into_context`: Loads a client certificate and key into a provided SSL context. - `make_client_cert_ssl_context`: Creates a default SSL context loaded with a specific client certificate and key. - `load_default_client_cert`: Discovers and loads the default client certificate into a provided SSL context if mTLS is enabled. - `get_default_ssl_context`: Returns a default SSL context pre-loaded with the default client certificate, or `None` if unavailable. - `should_use_mtls_endpoint`: Determines if an mTLS endpoint should be used based on the new environment variable and certificate availability. - Fixed outdated docstrings for `default_client_cert_source` and `default_client_encrypted_cert_source` to correctly state they raise `MutualTLSChannelError` instead of `DefaultClientCertSourceError`. - Updated `default_client_cert_source` to also catch `ClientCertError` when loading credentials. - Added comprehensive unit tests for the new mTLS helper methods.

…ntext typing

Removes the private `_make_client_cert_ssl_context` method and its corresponding unit test from `google.auth.transport.mtls`. Initially, this method was added to mirror the async API (`aio.transport.mtls.make_client_cert_ssl_context`). However, upon further review, we decided that external client libraries do not need it (they should use `get_default_ssl_context` or `load_default_client_cert` instead), and there is no need for it internally within the synchronous transports (e.g., `requests`, `urllib3`) which handle their mTLS context initialization independently. Removing this dead code reduces the internal API surface area and eliminates unnecessary maintenance overhead.

daniel-sanche · 2026-07-06T18:52:26Z


    Raises:
-        google.auth.exceptions.DefaultClientCertSourceError: If the default
+        google.auth.exceptions.MutualTLSChannelError: If the default


What is DefaultClientCertSourceError? I was concerned changing the exception could be a breaking change. But I don't actually see the old one in the codebase?

It seems like DefaultClientCertSourceError was never defined in this repository. Looking at the PR that introduced the method back in 2020, it has always raised MutualTLSChannelError, but the docstring incorrectly said DefaultClientCertSourceError.
This inconsistency was flagged when I was updating the file and I decided to update the docstring to match the real behavior of the method.

daniel-sanche · 2026-07-06T18:56:50Z

+    if not has_cert:
+        return False
+    _load_client_cert_into_context(ctx, cert_bytes, key_bytes, passphrase)
+    return True


nit: The scoping seems a bit strange here. Could these lines be inside the try block, since that's where the variables are defined? Or at least in an else block?

Done, moved them to an else block!

daniel-sanche · 2026-07-06T19:05:37Z

+    if use_mtls_endpoint == "never":
+        return False
+    if use_mtls_endpoint == "auto":
+        return client_cert_available


nit: it might be cleaner to define an enum instead of these magic strings

Great suggestion, done!

daniel-sanche · 2026-07-06T19:07:13Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces environment variables and helper functions to configure and load mutual TLS (mTLS) client certificates into SSL contexts, along with determining when to use mTLS endpoints. Key feedback points out an inconsistency in exception handling within the tests, suggests using duck typing instead of strict type checking for the SSL context, and recommends passing the passphrase directly as bytes to avoid potential decoding errors. Additionally, reviewers advised gracefully falling back to standard TLS upon configuration failures to maintain backward compatibility, and improving the robustness of environment variable parsing for empty or whitespace values.

gemini-code-assist · 2026-07-06T19:10:31Z

+    # Test bad callback which throws ClientCertError.
+    get_client_cert_and_key.side_effect = exceptions.ClientCertError()
+    callback = mtls.default_client_cert_source()
+    with pytest.raises(exceptions.ClientCertError):


There is an inconsistency between the PR description, the docstring update, and this test. The PR description states: 'Updated default_client_cert_source to also catch ClientCertError when loading credentials.' and the docstring of default_client_cert_source was updated to state that it raises MutualTLSChannelError. However, this test asserts that calling the callback raises ClientCertError directly instead of wrapping it in MutualTLSChannelError. If default_client_cert_source is intended to raise MutualTLSChannelError on failure, the callback implementation should catch ClientCertError and wrap it, and this test should assert that MutualTLSChannelError is raised.

I reverted the change to catch and wrap ClientCertError based on the conversation with Matt. I've updated the PR description and removed that line. We'll decide if clean up is needed in the future to catch and wrap ClientCertError similar to how other transports handle those cases.

gemini-code-assist · 2026-07-06T19:10:31Z

+        google.auth.exceptions.MutualTLSChannelError: If the SSL context is invalid,
+            or if loading the certificate and key fails.
+    """
+    if not isinstance(ctx, ssl.SSLContext):


Checking isinstance(ctx, ssl.SSLContext) is overly restrictive and violates Python's duck typing philosophy. It prevents the use of custom SSLContext wrappers or duck-typed objects that implement load_cert_chain. Checking hasattr(ctx, "load_cert_chain") is more flexible and Pythonic.

Suggested change

if not isinstance(ctx, ssl.SSLContext):

if not hasattr(ctx, "load_cert_chain"):

References

Python's general coding style and philosophy favors duck typing over strict type checking with isinstance. ^(link)

We intentionally enforce strict ssl.SSLContext typing here rather than using duck typing. Because this method passes sensitive material (the client private key and its passphrase) directly to the context, allowing arbitrary duck-typed wrapper objects could introduce security risks (like leaking the passphrase). Enforcing a genuine ssl.SSLContext ensures this data goes straight to the standard library.

gemini-code-assist · 2026-07-06T19:10:31Z

+            password_val = (
+                passphrase_val.decode("utf-8")
+                if isinstance(passphrase_val, bytes)
+                else passphrase_val
+            )


The ssl.SSLContext.load_cert_chain method natively accepts the password argument as either a string or a bytes object. Decoding passphrase_val to UTF-8 is unnecessary and can raise a UnicodeDecodeError if the passphrase contains non-UTF-8 bytes. Passing passphrase_val directly as bytes is safer and more robust. Note that the corresponding unit tests in packages/google-auth/tests/transport/test_mtls.py should also be updated to expect bytes instead of str.

password_val = passphrase_val

Good catch. I've fixed this and the other places that were calling the method with the decoded passphrase.

gemini-code-assist · 2026-07-06T19:10:31Z

+    except (
+        ssl.SSLError,
+        OSError,
+        ValueError,
+        RuntimeError,
+        TypeError,
+    ) as caught_exc:


When configuring mTLS, if a custom transport or context is provided that cannot be automatically configured (for example, if loading the client certificate fails), we should gracefully fall back to standard TLS and log a descriptive warning instead of raising a MutualTLSChannelError or other exceptions. This avoids backward-compatibility regressions.

except Exception as caught_exc: _LOGGER.warning("Failed to configure mTLS with custom transport, falling back to standard TLS: %s", caught_exc)

References

When configuring mTLS, if a custom transport is provided that cannot be automatically configured, gracefully fall back to standard TLS and log a descriptive warning instead of raising an exception to avoid backward-compatibility regressions.

I don't think we should gracefully fall back to standard TLS if loading the client certificate fails. If the environment is configured to use mTLS, silently falling back to standard TLS on a failure could be a security vulnerability or lead to confusing downstream authorization errors. It's safer to fail fast by raising a MutualTLSChannelError so the user knows their mTLS configuration is broken.

gemini-code-assist · 2026-07-06T19:10:31Z

+    use_mtls_endpoint = getenv(environment_vars.GOOGLE_API_USE_MTLS_ENDPOINT, "auto")
+    use_mtls_endpoint = (use_mtls_endpoint or "auto").strip().lower()


If GOOGLE_API_USE_MTLS_ENDPOINT is set to an empty string or whitespace (e.g., " "), use_mtls_endpoint becomes "", which will cause a MutualTLSChannelError to be raised. Falling back to "auto" when the stripped value is empty makes the environment variable parsing more robust.

Suggested change

use_mtls_endpoint = getenv(environment_vars.GOOGLE_API_USE_MTLS_ENDPOINT, "auto")

use_mtls_endpoint = (use_mtls_endpoint or "auto").strip().lower()

use_mtls_endpoint = getenv(environment_vars.GOOGLE_API_USE_MTLS_ENDPOINT, "auto")

use_mtls_endpoint = (use_mtls_endpoint or "auto").strip().lower()

if not use_mtls_endpoint:

use_mtls_endpoint = "auto"

An empty string already falls back to "auto" gracefully because ("" or "auto") evaluates to "auto" in Python. The only time it would fail and raise an error is if the user explicitly set the environment variable to just whitespace (e.g., " "), which is considered an invalid value. In that case, it's actually better to fail fast and let the user know their configuration is invalid, rather than silently falling back.
I've updated the unit tests to capture that this behavior is intentional and avoid having regressions in the future.

…word type handling

nbayati requested review from a team as code owners June 17, 2026 17:59

gemini-code-assist Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/_mtls_helper.py Outdated

Comment thread packages/google-auth/google/auth/transport/urllib3.py Outdated

Comment thread packages/google-auth/google/auth/transport/_mtls_helper.py Outdated

daniel-sanche reviewed Jun 17, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/mtls.py

daniel-sanche changed the title ~~Implement python mtls helpers~~ feat(auth): Implement python mtls helpers Jun 17, 2026

macastelaz reviewed Jun 25, 2026

View reviewed changes

lsirac reviewed Jul 1, 2026

View reviewed changes

lsirac requested changes Jul 1, 2026

View reviewed changes

lsirac approved these changes Jul 1, 2026

View reviewed changes

nbayati force-pushed the implement-python-mtls-helpers branch from bc6d2b8 to 782211c Compare July 3, 2026 00:12

nbayati added 9 commits July 3, 2026 13:49

refactor: make mtls cert loaders private and enforce strict ssl.SSLCo…

28bc123

…ntext typing

fix: improve mtls exception handling and edge cases

172dd39

fix: logic polish and docstring correction for mtls

27fae9f

test: fill mtls test suite gaps and improve mock rigor

93d6b82

test: add missing unit tests

805bb04

fix: default to auto for empty mtls endpoint environment variable

ae95056

fix: resolve mypy type narrowing in mtls.py

57b2525

test: mock mtls endpoint checks in get_default_ssl_context tests

f8289d2

nbayati requested review from daniel-sanche and macastelaz July 5, 2026 18:22

macastelaz reviewed Jul 6, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/mtls.py

macastelaz reviewed Jul 6, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/mtls.py Outdated

Comment thread packages/google-auth/google/auth/transport/mtls.py Outdated

Comment thread packages/google-auth/tests/transport/test_mtls.py Outdated

chore: update obsolete ClientCertError docstrings and tests

bca7b9d

nbayati force-pushed the implement-python-mtls-helpers branch from 88b2fa0 to bca7b9d Compare July 6, 2026 18:35

macastelaz approved these changes Jul 6, 2026

View reviewed changes

daniel-sanche reviewed Jul 6, 2026

View reviewed changes

gemini-code-assist Bot reviewed Jul 6, 2026

View reviewed changes

nbayati added 2 commits July 7, 2026 22:19

refactor: remove unnecessary passphrase decoding

ed2fbc0

refactor: introduce UseMtlsEndpointMode enum and fix certificate pass…

db59235

…word type handling

nbayati requested a review from daniel-sanche July 8, 2026 05:28

	if not isinstance(ctx, ssl.SSLContext):
	if not hasattr(ctx, "load_cert_chain"):

		use_mtls_endpoint = getenv(environment_vars.GOOGLE_API_USE_MTLS_ENDPOINT, "auto")
		use_mtls_endpoint = (use_mtls_endpoint or "auto").strip().lower()

Uh oh!

Conversation

nbayati commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

macastelaz left a comment

Choose a reason for hiding this comment

Uh oh!

lsirac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniel-sanche commented Jul 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

nbayati commented Jun 17, 2026 •

edited

Loading

lsirac left a comment •

edited

Loading

daniel-sanche Jul 6, 2026 •

edited

Loading