Skip to content

feat(auth): add _MtlsCertManager for mTLS client certificate rotation - #18925

Open
nbayati wants to merge 2 commits into
mainfrom
grpc_phase1_pr1
Open

nbayati wants to merge 2 commits into
mainfrom
grpc_phase1_pr1

Conversation

@nbayati

@nbayati nbayati commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Adds _MtlsCertManager in google.auth.transport._mtls_helper to track the active mTLS client certificate, private key, leaf certificate SHA-256 fingerprint, and generation counter across certificate reloads.

This is the first change in the gRPC mTLS client certificate rotation series. Subsequent PRs wire _MtlsCertManager into requests.Request and Compute Engine bound-token refreshes, AuthMetadataPlugin, google.api_core.grpc_helpers.create_channel, and gapic-generator so that TLS channel credentials and certificate-bound token refreshes stay on the same certificate generation.

Adds `_MtlsCertManager` in `google.auth.transport._mtls_helper` to track
the active mTLS client certificate, private key, leaf certificate
SHA-256 fingerprint, and generation counter across certificate reloads.

This is the first change in the gRPC mTLS client certificate rotation
series. Subsequent PRs wire `_MtlsCertManager` into `requests.Request`
and Compute Engine bound-token refreshes, `AuthMetadataPlugin`,
`google.api_core.grpc_helpers.create_channel`, and `gapic-generator` so
that TLS channel credentials and certificate-bound token refreshes stay
on the same certificate generation.
@nbayati
nbayati requested review from a team as code owners October 30, 2026 06:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces _MtlsCertManager, a thread-safe manager for handling mTLS client certificates, tracking their generation, and managing reloading. It also adds comprehensive unit tests for this new class. The feedback suggests catching all standard exceptions (Exception) during certificate reloads rather than a specific subset, ensuring that unexpected errors from custom certificate sources do not crash the application and are instead gracefully handled as transient reload errors.

Comment on lines +1067 to +1083
except (
exceptions.ClientCertError,
exceptions.MutualTLSChannelError,
OSError,
ValueError,
) as caught_exc:
if self._generation_id > 0:
_LOGGER.debug(
"Failed to reload mTLS client certificate; retaining "
"previous certificate (generation %d): %s",
self._generation_id,
caught_exc,
)
return False
if isinstance(caught_exc, exceptions.ClientCertError):
raise
raise exceptions.ClientCertError(caught_exc) from caught_exc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The except block currently catches a specific list of exceptions (ClientCertError, MutualTLSChannelError, OSError, ValueError). However, when a custom client_cert_source callback is provided, it may raise other unexpected or custom exceptions (e.g., network errors, custom HSM/KMS exceptions, or even TypeError if the callback returns an unexpected format).

If such an exception is raised during a certificate reload (when self._generation_id > 0), it will propagate and crash the application/request instead of being gracefully handled as a transient reload error. To ensure maximum robustness and successfully retain the last known good certificate, we should catch all standard exceptions (Exception) during reload.

            except Exception as caught_exc:
                if self._generation_id > 0:
                    _LOGGER.debug(
                        "Failed to reload mTLS client certificate; retaining "
                        "previous certificate (generation %d): %s",
                        self._generation_id,
                        caught_exc,
                    )
                    return False
                if isinstance(caught_exc, exceptions.ClientCertError):
                    raise
                raise exceptions.ClientCertError(caught_exc) from caught_exc
References
  1. Do not replace historical graceful fallback behaviors (such as returning False/falling back to standard TLS) with exceptions if doing so would introduce breaking changes for downstream users and violate backwards compatibility.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants