feat: MDS connections use mTLS #1856

nolanleastin · 2025-11-03T21:32:18Z

Feature Gating
The GCE_METADATA_MTLS_MODE environment variable is introduced, which can be set to strict, none, or default.

The should_use_mds_mtls function determines whether to use mTLS based on the environment variable and the existence of the certificate files.

Using MDS mTLS
A custom MdsMtlsAdapter for requests is implemented to handle the SSL context for mTLS.

The create_session function mounts an MdsMtlsAdapter into the request.Session

MdsMtlsAdapter
Loads MDS mTLS certificates from well-known location (https://docs.cloud.google.com/compute/docs/metadata/overview#https-mds-certificates).

If HTTPS request fails, MdsMtlsAdapter will fallback to HTTP.

Integrating with existing code
compute_engine/_metadata.py:

The metadata server URL construction is now dynamic, supporting both http and https schemes based on whether mTLS is enabled.
ping and get functions are updated to use mTLS when it's enabled.

google/auth/compute_engine/_mtls.py

daniel-sanche

Nothing major, but left a few suggestions to consider

google/auth/compute_engine/_metadata.py

daniel-sanche · 2025-11-06T17:26:04Z

google/auth/compute_engine/_metadata.py

+
+    Returns:
+        google.auth.transport.Request: Request
+            object to use.


nit: the line break seems unnecessary here

google/auth/compute_engine/_metadata.py

daniel-sanche · 2025-11-06T17:40:28Z

google/auth/compute_engine/_mtls.py

+            "C:\\", "ProgramData", "Google", "ComputeEngine", "mds-mtls-root.crt"
+        )
+    else:
+        return os.path.join("/", "run", "google-mds-mtls", "root.crt")


These paths are a bit unweildy, especially since the directory is referenced multiple times in this file. I'd suggest doing something like:

WIN_BASE_PATH_COMPONENTS = ("C:\\", "ProgramData", "Google", "ComputeEngine") BASE_PATH_COMPONENTS = ("/", "run", "google-mds-mtls") def _get_mds_root_crt_path(): if os.name == "nt": return os.path.join(*WIN_BASE_PATH_COMPONENTS, "mds-mtls-root.crt") else: return os.path.join(*BASE_PATH_COMPONENTS, "root.crt")

Or, preferably, consider using Path objects instead of strings. It could simplify some of the weirdness of paths on different platforms

from pathlib import Path WIN_BASE_PATH = PATH("C:/ProgramData/Google/ComputeEngine") BASE_PATH = PATH("/run/google-mds-mtls") def _get_mds_root_crt_path(): if os.name == "nt": return WIN_BASE_PATH / "mds-mtls-root.crt" else: return BASE_PATH / "root.crt"

daniel-sanche · 2025-11-06T17:51:30Z

google/auth/compute_engine/_mtls.py

+
+
+def _get_mds_root_crt_path():
+    if os.name == "nt":


nit: The "nt" string is used in a few places, and could be confusing if you're not familiar

Maybe consider making this a constant. Something like WINDOWS_OS_NAME="nt"

daniel-sanche

Looking at this a bit closer, it looks like this change would contradict the public API as it currently stands. Custom request objects constructed and passed in by users would be bypassed after the next release, which could break existing code. I think we need to consider that more before merging

Or let me know if I'm misunterstanding

daniel-sanche · 2025-11-17T22:58:08Z

google/auth/compute_engine/_metadata.py

+
+    if root is None:
+        root = _get_metadata_root(use_mtls)
+    _validate_gce_mds_configured_environment()


nit: maybe add a comment explaining what this does

daniel-sanche · 2025-11-17T22:58:11Z

google/auth/compute_engine/_metadata.py

    Raises:
        google.auth.exceptions.TransportError: if an error occurred while
            retrieving metadata.
    """


Raises doesn't mention MutualTLSChannelError

daniel-sanche · 2025-11-17T23:16:22Z

google/auth/compute_engine/_metadata.py

    request,
    path,
-    root=_METADATA_ROOT,
+    root=None,


the docstring for this says root (str):. It looks like it should be root (Optional[str]):. And the docstring should mention what happens if its left as None

daniel-sanche · 2025-11-17T23:17:37Z

google/auth/compute_engine/_mtls.py

+    )  # path to CA certificate
+    client_combined_cert_path: str = field(
+        default_factory=_get_mds_client_combined_cert_path
+    )  # path to file containing client certificate and key


these are marked as str, but it looks like they are paths now?

Are you running nox -s mypy checks? I'm not sure if they're configured to run in CI

Yes I have been running mypy checks! Not sure why this isn't flagged though.

Either way, updated

sai-sunder-s

This is a great addition to the library. The implementation of mTLS for the GCE metadata server is well-structured and thoroughly tested.

Here's a summary of my review:

Good separation of concerns: The mTLS logic is nicely encapsulated in the new _mtls.py module.
Clear configuration: The use of the GCE_METADATA_MTLS_MODE environment variable provides a clear and flexible way to configure the mTLS behavior.
Robustness: The fallback mechanism in default mode is a good feature to ensure that the library can still function even if mTLS is not properly configured.
Thorough testing: The addition of new test files and the extension of existing tests provide good coverage for the new functionality.

I have one minor suggestion for improvement in google/auth/compute_engine/_metadata.py, which I've added as a line comment.

Overall, this looks ready to merge.

sai-sunder-s · 2025-11-19T06:49:03Z

google/auth/compute_engine/_metadata.py

+        if _GCE_METADATA_HOST != _GCE_DEFAULT_HOST:
+            # mTLS is only supported when connecting to the default metadata host.
+            # Raise an exception if we are in strict mode (which requires mTLS)
+            # but the metadata host has been overridden. (which means mTLS will fail)


This validation is good. For extra robustness, you could consider checking if _GCE_METADATA_HOST is in _GCE_DEFAULT_MDS_HOSTS instead of just comparing it to _GCE_DEFAULT_HOST. This would also cover the case where the IP address is used as the host. However, the current implementation is likely sufficient as it's improbable for the IP to be set as the host via the environment variable.

This is a good point. I will make this edit.

sai-sunder-s · 2025-11-19T06:50:06Z

This is a great addition to the library. The implementation of mTLS for the GCE metadata server is well-structured and thoroughly tested.

Here's a summary of my review:

Good separation of concerns: The mTLS logic is nicely encapsulated in the new _mtls.py module.

Clear configuration: The use of the GCE_METADATA_MTLS_MODE environment variable provides a clear and flexible way to configure the mTLS behavior.

Robustness: The fallback mechanism in default mode is a good feature to ensure that the library can still function even if mTLS is not properly configured.

Thorough testing: The addition of new test files and the extension of existing tests provide good coverage for the new functionality.

I have one minor suggestion for improvement in google/auth/compute_engine/_metadata.py, which I've added as a line comment.

Overall, this looks ready to merge.

This is Gemini CLI's review, not mine :)

sai-sunder-s

Overall LGTM. Left couple of comments.

sai-sunder-s · 2025-11-19T07:06:37Z

google/auth/compute_engine/_metadata.py

+        request.session = requests.Session()
+    for host in _GCE_DEFAULT_MDS_HOSTS:
+        request.session.mount(f"https://{host}/", adapter)
+    return request


you are returning the same request object as the input. All changes are inside the object.

Do we really have to return?

Ok, this is a good point. Changed.

sai-sunder-s · 2025-11-19T07:08:30Z

google/auth/compute_engine/_metadata.py

+            has been overridden in strict mTLS mode).
+
    """
+    use_mtls = _mtls.should_use_mds_mtls()


can we determine whether to use mtls or not at cred initialization time and put it in an attribute instead of determining each time?

Do we expect the value to change after cred initilization?

daniel-sanche

LGTM

daniel-sanche · 2025-11-19T18:59:00Z

google/auth/compute_engine/_metadata.py

            retrieving metadata.
+        google.auth.exceptions.MutualTLSChannelError: if the environment
+            configuration is invalid for mTLS (for example, the metadata host
+            has been overridden in strict mTLS mode).


nit: this is only if mtls is requested and the configuration is invalid, right?

yes that's right. updated that.

1. now we do not create a new request. instead, create an mds mtls adapter and mount it on the request session. 2. added _validate_gce_mds_configured_environment, which ensures if we are using strict, that the host being contacted is default 3. fix unit tests and add new tests

PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: v0.7.0 Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator:latest <details><summary>google-auth: 2.44.0</summary> ## [2.44.0](v2.43.0...v2.44.0) (2025-12-12) ### Features * MDS connections use mTLS (#1856) ([0387bb9](0387bb95)) * support Python 3.14 (#1822) ([0f7097e](0f7097e7)) * add ecdsa p-384 support (#1872) ([39c381a](39c381a5)) * Add shlex to correctly parse executable commands with spaces (#1855) ([cf6fc3c](cf6fc3cc)) * Implement token revocation in STS client and add revoke() metho… (#1849) ([d563898](d5638986)) ### Bug Fixes * Add temporary patch to workload cert logic to accomodate Cloud Run mis-configuration (#1880) ([78de790](78de7907)) * Delegate workload cert and key default lookup to helper function (#1877) ([b0993c7](b0993c7e)) * Use public refresh method for source credentials in ImpersonatedCredentials (#1884) ([e0c3296](e0c3296f)) </details>

nolanleastin requested review from a team as code owners November 3, 2025 21:32

nolanleastin force-pushed the neastin/mds-mtls branch from 076c937 to 0d550bc Compare November 3, 2025 21:38

nolanleastin requested a review from sai-sunder-s November 3, 2025 21:58

nolanleastin force-pushed the neastin/mds-mtls branch 3 times, most recently from 8947e48 to 3689ea7 Compare November 4, 2025 20:16

sai-sunder-s reviewed Nov 4, 2025

View reviewed changes

google/auth/compute_engine/_mtls.py Show resolved Hide resolved

google/auth/compute_engine/_mtls.py Show resolved Hide resolved

nolanleastin requested a review from daniel-sanche November 4, 2025 23:31

sai-sunder-s previously approved these changes Nov 5, 2025

View reviewed changes

nolanleastin dismissed sai-sunder-s’s stale review via e56d755 November 5, 2025 19:33

sai-sunder-s previously approved these changes Nov 5, 2025

View reviewed changes

daniel-sanche reviewed Nov 6, 2025

View reviewed changes

daniel-sanche requested changes Nov 6, 2025

View reviewed changes

nolanleastin dismissed sai-sunder-s’s stale review via 0569d90 November 17, 2025 19:59

nolanleastin force-pushed the neastin/mds-mtls branch 5 times, most recently from 09c61a6 to 8f7e197 Compare November 17, 2025 20:11

daniel-sanche reviewed Nov 17, 2025

View reviewed changes

nolanleastin requested a review from daniel-sanche November 18, 2025 23:36

sai-sunder-s reviewed Nov 19, 2025

View reviewed changes

sai-sunder-s previously approved these changes Nov 19, 2025

View reviewed changes

nolanleastin dismissed sai-sunder-s’s stale review via 110e535 November 19, 2025 18:51

daniel-sanche previously approved these changes Nov 19, 2025

View reviewed changes

nolanleastin dismissed daniel-sanche’s stale review via 3cdf91a November 19, 2025 19:20

sai-sunder-s approved these changes Nov 19, 2025

View reviewed changes

daniel-sanche approved these changes Nov 19, 2025

View reviewed changes

nolanleastin added 14 commits November 19, 2025 21:05

feat: MDS connections use mTLS

02657fa

add custom root unit test to increase coverage

b49aad1

add unit test to increase coverage

a6a2cc3

add explanation to mtlsmds modes

6e00ea4

Update mds mtls certificate well-known locations

df1378e

modify contants

e3a311a

add fallback to mds mtls

c34a17a

add fallback for http error codes from mds

b1d7021

Add more unit tests to address coverage requirements

e01ea09

update docstrings

c3caa16

update MdsMtlsConfig fields to be Path type

e0b31e8

address feedback

eaf8435

minor modification to docstring

9f4bbaa

nolanleastin force-pushed the neastin/mds-mtls branch from 3cdf91a to 9f4bbaa Compare November 19, 2025 21:06

nolanleastin merged commit 0387bb9 into main Nov 19, 2025
14 checks passed

nolanleastin deleted the neastin/mds-mtls branch November 19, 2025 21:37

vchudnov-g mentioned this pull request Dec 13, 2025

chore: librarian release pull request: 20251212T161150Z #1888

Merged

feat: MDS connections use mTLS #1856

feat: MDS connections use mTLS #1856

Uh oh!

Conversation

nolanleastin commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche Nov 6, 2025 • 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 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!

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 Nov 17, 2025 • 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!

sai-sunder-s left a comment

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!

sai-sunder-s commented Nov 19, 2025

Uh oh!

sai-sunder-s left a comment

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 left a comment

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!

Reviewers

Assignees

nolanleastin commented Nov 3, 2025 •

edited

Loading

daniel-sanche Nov 6, 2025 •

edited

Loading

daniel-sanche left a comment •

edited

Loading

daniel-sanche Nov 17, 2025 •

edited

Loading