feat(auth): Implement python mtls helpers#17495
Conversation
Replace pyOpenSSL with standard library ssl for mTLS transport and update key decryption to use cryptography library. This change also enhances security for handling private keys by: - Using Linux memfd_create for RAM-backed in-memory files to avoid writing secrets to physical storage. - Encrypting plaintext keys on-the-fly before writing to fallback temporary files on disk. - Securely wiping temporary files with null bytes before deletion.
…S cert loading fallbacks
… update tests accordingly
…on to include empty password for client_combined_cert_path
…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.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
| if sys.platform == "linux" and hasattr(os, "memfd_create"): | ||
| cm = _memfd_cert_key_paths(cert_bytes, key_bytes) | ||
| try: | ||
| cert_path, key_path = cm.__enter__() | ||
| except OSError: | ||
| pass # Fallback to Tier 3 on failure. | ||
| else: | ||
| try: | ||
| # Handle cases where path exists but might be restricted. | ||
| if (cert_path is None or os.path.exists(cert_path)) and ( | ||
| key_path is None or os.path.exists(key_path) | ||
| ): | ||
| yield cast(str, cert_path or cert), cast( | ||
| str, key_path or key | ||
| ), passphrase | ||
| return | ||
| finally: | ||
| import sys | ||
|
|
||
| exc_info = sys.exc_info() | ||
| cm.__exit__( | ||
| *(exc_info if exc_info[0] is not None else (None, None, None)) | ||
| ) | ||
| # If verification failed, fall through to Tier 3. |
There was a problem hiding this comment.
Manual context manager entry and exit (__enter__ and __exit__) is error-prone and can lead to resource leaks or incorrect exception propagation. Using contextlib.ExitStack is a much safer and more idiomatic way to handle conditional context managers in Python.
| if sys.platform == "linux" and hasattr(os, "memfd_create"): | |
| cm = _memfd_cert_key_paths(cert_bytes, key_bytes) | |
| try: | |
| cert_path, key_path = cm.__enter__() | |
| except OSError: | |
| pass # Fallback to Tier 3 on failure. | |
| else: | |
| try: | |
| # Handle cases where path exists but might be restricted. | |
| if (cert_path is None or os.path.exists(cert_path)) and ( | |
| key_path is None or os.path.exists(key_path) | |
| ): | |
| yield cast(str, cert_path or cert), cast( | |
| str, key_path or key | |
| ), passphrase | |
| return | |
| finally: | |
| import sys | |
| exc_info = sys.exc_info() | |
| cm.__exit__( | |
| *(exc_info if exc_info[0] is not None else (None, None, None)) | |
| ) | |
| # If verification failed, fall through to Tier 3. | |
| if sys.platform == "linux" and hasattr(os, "memfd_create"): | |
| memfd_failed = False | |
| with contextlib.ExitStack() as stack: | |
| try: | |
| cert_path, key_path = stack.enter_context( | |
| _memfd_cert_key_paths(cert_bytes, key_bytes) | |
| ) | |
| except OSError: | |
| memfd_failed = True | |
| if not memfd_failed: | |
| # Handle cases where path exists but might be restricted. | |
| if (cert_path is None or os.path.exists(cert_path)) and ( | |
| key_path is None or os.path.exists(key_path) | |
| ): | |
| yield cast(str, cert_path or cert), cast( | |
| str, key_path or key | |
| ), passphrase | |
| return |
| with _mtls_helper.secure_cert_key_paths(cert, key) as ( | ||
| cert_path, | ||
| key_path, | ||
| passphrase, | ||
| ): |
There was a problem hiding this comment.
The module _mtls_helper is not imported directly in this file; instead, it is accessed via transport._mtls_helper (as seen on line 353). Using _mtls_helper directly here will result in a NameError at runtime when _make_mutual_tls_http is called.
| with _mtls_helper.secure_cert_key_paths(cert, key) as ( | |
| cert_path, | |
| key_path, | |
| passphrase, | |
| ): | |
| with transport._mtls_helper.secure_cert_key_paths(cert, key) as ( | |
| cert_path, | |
| key_path, | |
| passphrase, | |
| ): |
| import tempfile | ||
|
|
||
| # Prioritize RAM-backed /dev/shm to avoid writing secrets to physical storage. | ||
| tmp_dir = "/dev/shm" if os.path.isdir("/dev/shm") else None |
There was a problem hiding this comment.
In restricted or containerized environments (such as certain Docker/Kubernetes setups or serverless environments like Cloud Run), /dev/shm might exist but not be writable by the current user. If tempfile.mkstemp is called with a non-writable directory, it will raise a PermissionError and crash. Checking os.access for write permissions ensures a safe fallback to the default temp directory.
| tmp_dir = "/dev/shm" if os.path.isdir("/dev/shm") else None | |
| tmp_dir = ( | |
| "/dev/shm" | |
| if os.path.isdir("/dev/shm") and os.access("/dev/shm", os.W_OK) | |
| else None | |
| ) |
| bool: indicating whether the client certificate should be used for mTLS. | ||
| """ | ||
| return _mtls_helper.check_use_client_cert() | ||
|
|
There was a problem hiding this comment.
Do all of these new methods need to be public?
daniel-sanche
changed the title
2 participants
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:
GOOGLE_API_USE_MTLS_ENDPOINTenvironment variable to control whether an mTLS endpoint should be used (always,never, orauto).google.auth.transport.mtlsto 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, orNoneif unavailable.should_use_mtls_endpoint: Determines if an mTLS endpoint should be used based on the new environment variable and certificate availability.default_client_cert_sourceanddefault_client_encrypted_cert_sourceto correctly state they raiseMutualTLSChannelErrorinstead ofDefaultClientCertSourceError.default_client_cert_sourceto also catchClientCertErrorwhen loading credentials.