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

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

This PR builds on top of PR #16976. Once that PR is merged to main, this PR will be rebased and would only have the commit that introduces the helper methods (commit bc6d2b8), as the commits before this one are all from the other PR.

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.
- 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.

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.

daniel-sanche · 2026-06-17T23:49:10Z

       bool: indicating whether the client certificate should be used for mTLS.
    """
    return _mtls_helper.check_use_client_cert()
+


Do all of these new methods need to be public?

3 of them need to be public. I made _load_client_cert_into_context private and decided to remove make_client_cert_ssl_context altogether.

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

lsirac · 2026-07-01T20:53:14Z

+
+
+def load_client_cert_into_context(
+    ctx: ssl.SSLContext,


Statically annotating parameter ctx strictly as ssl.SSLContext contradicts the runtime duck-typing check at line 175 (hasattr(ctx, "load_cert_chain")). To align static typing with runtime duck typing, annotate ctx using typing.Protocol or Union[ssl.SSLContext, Any].

Done. Updated the runtime check to strictly use isinstance(ctx, ssl.SSLContext) to align with the static typing.

lsirac · 2026-07-01T20:53:14Z

+            ctx.load_cert_chain(
+                certfile=cert_path, keyfile=key_path, password=passphrase_val
+            )
+    except (


The exception handler for ctx.load_cert_chain() omits TypeError. If cert_bytes, key_bytes, or passphrase is None (or an invalid type), Python's ssl module raises TypeError, which escapes uncaught instead of raising the documented MutualTLSChannelError.

Done. Added TypeError to the caught exceptions.

lsirac · 2026-07-01T20:53:14Z

+        ValueError,
+        RuntimeError,
+    ) as caught_exc:
+        new_exc = exceptions.MutualTLSChannelError(


Raising MutualTLSChannelError with a static string ("Failed to load client certificate and key for mTLS.") instead of passing the underlying caught_exc is inconsistent with lines 75 and 120, losing low-level OpenSSL failure specifics when exception messages are logged without tracebacks.

Done. Updated to pass the underlying exception directly to MutualTLSChannelError.

lsirac · 2026-07-01T20:53:14Z

+        cert_bytes,
+        key_bytes,
+        passphrase,
+    ) = _mtls_helper.get_client_ssl_credentials()


Invoking _mtls_helper.get_client_ssl_credentials() in load_default_client_cert without exception handling allows raw OSError, RuntimeError, and ValueError exceptions to escape out of get_default_ssl_context() (line 277), violating the function's documented contract.

Done. Wrapped the call in a try/except block to catch and raise them as a MutualTLSChannelError.

lsirac · 2026-07-01T20:53:14Z

+        ("invalid_val", False, False),
+    ],
+)
+@mock.patch(


In tests evaluating environment variable lookups, the code patches the module string constant GOOGLE_API_USE_MTLS_ENDPOINT with its own literal string value ("GOOGLE_API_USE_MTLS_ENDPOINT"), which is redundant boilerplate.

Done. Removed the redundant mock.

lsirac · 2026-07-01T20:53:14Z

+    @mock.patch.object(os, "memfd_create", create=True)
+    @mock.patch.object(_mtls_helper, "_memfd_cert_key_paths", autospec=True)
+    @mock.patch.object(_mtls_helper, "_tempfile_cert_key_paths", autospec=True)
+    def test_tier2_fallback_to_tier3_on_oserror(


While test_tier2_fallback_to_tier3_on_oserror verifies fallback when RAM descriptors fail, the test suite lacks a negative test case verifying exception propagation when Tier 3 disk fallback (_tempfile_cert_key_paths) fails with OSError or ValueError.

Done. Added test_tier3_fallback_failure_propagation to verify exception propagation when Tier 3 fails.

lsirac · 2026-07-01T20:53:14Z

+        mock_close.assert_called_once_with(15)
+
+
+class TestTempfileCertKeyPaths(object):


In TestTempfileCertKeyPaths, no test verifies that an exception raised during key_path creation or writing triggers cleanup of the already-created cert_path file in the context manager's error handler.

Done. Added a test case to verify that os.remove is called on the cert file if an exception occurs before the key file finishes.

lsirac · 2026-07-01T20:53:14Z

+        decrypted = serialization.load_pem_private_key(
+            encrypted_bytes, password=b"my_passphrase"
+        )
+        assert decrypted


In test_encrypt_key_if_plaintext, assert decrypted performs a weak truthy check on the loaded private key object rather than serializing and comparing key parameters against pytest.private_key_bytes to verify exact key equality.

Done. Updated to serialize the decrypted key object back to bytes and use an equality assertion (==) against the original plaintext key for exact verification.

lsirac · 2026-07-01T20:53:14Z

+    @mock.patch("builtins.open", autospec=True)
+    @mock.patch.object(os, "fsync")
+    @mock.patch.object(os, "remove")
+    def test_success(


In TestSecureWipeAndRemove, the test verifies write(), fsync(), and remove(), but never asserts mock_fh.flush.assert_called_once(), even though _secure_wipe_and_remove explicitly calls f.flush() before os.fsync().

Done. Added the missing assertion for mock_fh.flush.

…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.

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

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

		mock_close.assert_called_once_with(15)


		class TestTempfileCertKeyPaths(object):

Uh oh!

Conversation

nbayati commented Jun 17, 2026

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lsirac left a comment •

edited

Loading