refactor(auth): replace pyOpenSSL with standard ssl and cryptography by nbayati · Pull Request #16976 · googleapis/google-cloud-python

nbayati · 2026-05-07T04:16:32Z

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.

gemini-code-assist

Code Review

This pull request refactors the library to remove the pyOpenSSL dependency, migrating certificate and key management to the cryptography library and Python's standard ssl module. It introduces a secure three-tier fallback strategy for handling mTLS credentials via the new secure_cert_key_paths utility, which utilizes RAM-backed virtual files on Linux to minimize disk exposure. Review feedback suggests simplifying the manual context manager orchestration within this utility and using os.fdopen instead of os.write for more robust and idiomatic file descriptor operations.

parthea · 2026-05-26T17:51:27Z

@nbayati , Please could you address the failing presubmits?

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

nbayati · 2026-06-17T18:39:09Z

/gemini review

gemini-code-assist

Code Review

This pull request removes the dependency on pyOpenSSL across the library, replacing it with Python's standard ssl library and the cryptography package. It also introduces a secure three-tier fallback strategy for handling client certificates and private keys. The review feedback highlights a potential NameError in urllib3.py due to an unimported module reference, suggests simplifying context manager exit logic in _mtls_helper.py, recommends verifying write permissions for /dev/shm to prevent failures in restricted environments, and advises defensively encoding leaf certificate callback values to bytes.

daniel-sanche · 2026-06-17T23:01:28Z

+            cleanup_fds.append(fd_key)
+            with os.fdopen(fd_key, "wb", closefd=False) as f:
+                f.write(key_bytes)
+            key_path = f"/proc/self/fd/{fd_key}"


nit: I wonder if making a helper or loop for these could be cleaner?

Done! Added a loop.

… checks

…actor file writing into loops

…ean standard with block

nbayati · 2026-06-18T18:46:45Z

I tested the refactored paths manually by using the External Account X.509 provider to exchange certificates for access tokens and retrieve GCS buckets across different transports and credential initiation paths. The default transport (requests), urllib3, and aiohttp (async) are all verified and working as expected after the refactoring. I've also verified the encrypted private key loading/decryption flow using the new cryptography-based helper.

A copy of the test script can be found at: paste/5209556048281600
Results: screenshot/7tvfZhJ6qoYEGbY

…s in RefreshError

…nnection resets

…ed runtimes

macastelaz · 2026-06-25T14:32:04Z

+            ctx.load_cert_chain(
+                certfile=cert_path,
+                keyfile=key_path,
+                password=passphrase or "",


Nit: Based on the definition of load_cert_chain, password can safely be None (https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain) so I don't think we need to do this type casting here.

…s in tests

parthea · 2026-06-25T21:16:04Z

/gemini review

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 handling mTLS certificates and keys. It introduces a new secure context manager, secure_cert_key_paths, in _mtls_helper.py to securely manage certificate and key paths using Linux in-memory virtual files (memfd_create) or encrypted temporary files as a fallback. The setup configuration, tests, and environment files are updated accordingly. The feedback recommends removing a redundant import os statement inside the _secure_wipe_and_remove function since os is already imported at the module level.

gemini-code-assist · 2026-06-25T21:18:41Z

+    import os
+
+    if not os.path.exists(file_path):


The os module is already imported at the module level (line 20). Please remove the redundant import os inside _secure_wipe_and_remove to adhere to the project's coding standards.

Suggested change

import os

if not os.path.exists(file_path):

if not os.path.exists(file_path):

References

Do not import modules or classes inside functions if they are already imported at the module level.

lsirac

1. Critical Packaging Bug in `setup.py`

File: packages/google-auth/setup.py (PR-level)
Technical Note:
The list cryptography_base_require is placed directly inside the DEPENDENCIES tuple without unpacking on line 27. This results in a nested structure: ("pyasn1-modules>=0.2.1", ["cryptography >= 38.0.3"]). When setuptools parses install_requires during packaging or installation, it iterates over the requirements and expects each item to be a flat string specifier. Encountering a nested list triggers a TypeError (e.g., TypeError: expected string or bytes-like object), completely blocking building or installing the package from source. Unpack the list inside DEPENDENCIES:
```
DEPENDENCIES = (
    "pyasn1-modules>=0.2.1",
    *cryptography_base_require,
)
```

2. Standard SSLContext Pointer Alignment and Safety

File: packages/google-auth/google/auth/transport/_custom_tls_signer.py (Line 270)
Technical Note:
The pointer calculation in _cast_ssl_ctx_to_void_p_stdlib relies on standard CPython release alignment id(context) + ctypes.sizeof(ctypes.c_void_p) * 2 to obtain the raw pointer to the underlying OpenSSL structure SSL_CTX. This offset assumes a 2-pointer PyObject_HEAD header structure. This alignment is highly fragile and shifts under debug builds with Py_TRACE_REFS enabled (increasing header size to 4 pointers), free-threaded CPython (Python 3.13+, PEP 703), or alternative runtimes (PyPy). Reading from the wrong offset results in arbitrary memory pointer retrieval and uncatchable segmentation faults. Reject unsupported environments or enforce safe alternative C-API casting.

3. Missing Type Validation on Context Parameter

File: packages/google-auth/google/auth/transport/_custom_tls_signer.py (Line 270)
Technical Note:
In attach_to_ssl_context, there is no type validation on ctx. If a caller passes None or an invalid object type, _cast_ssl_ctx_to_void_p_stdlib calculates a memory address offset beyond the bounds of the object structure and reads raw heap garbage as an SSL_CTX pointer. When passed to the Go offload library ConfigureSslContext, this bogus address is dereferenced, causing a hard segmentation fault that crashes the entire Python host process instead of raising a clean Python TypeError. Add an explicit assertion before pointer casting:
```
if not isinstance(ctx, ssl.SSLContext):
    raise TypeError("ctx must be an instance of ssl.SSLContext")
```

lsirac · 2026-06-26T03:35:14Z

+def _encrypt_key_if_plaintext(
+    key_bytes: bytes, passphrase: Optional[bytes]
+) -> Tuple[bytes, Optional[bytes]]:
+    """Encrypts a plaintext PEM key if necessary, returning the bytes and passphrase.


We are returning plaintext if encryption fails. Is that intentional? If so should be documented

lsirac · 2026-06-26T03:36:17Z

-            OpenSSL.crypto.Error,
+            ValueError,
        ) as caught_exc:
+            self._is_mtls = False


Can we make this atomic: build the new adapter first, then update is_mtls and mount it only after success?

lsirac · 2026-06-26T03:37:12Z

-            OpenSSL.crypto.Error,
+            ValueError,
        ) as caught_exc:
+            self._is_mtls = False


Same issue here - I think if mTLS was already configured and a later configure call fails, _is_mtls becomes false but self.http is still the old mTLS PoolManager.

lsirac · 2026-06-26T03:37:48Z

-        leaf_cert = crypto.load_certificate(
-            crypto.FILETYPE_PEM, self._leaf_cert_callback()
-        )
+        try:


This wraps too much code in the parse error.

The callback can fail for config or file-read reasons, but because it is inside this try, those errors become Failed to parse leaf certificate.

Can we call the callback before this parse try, so only actual cert parsing errors get this message?

gemini-code-assist Bot reviewed May 7, 2026

View reviewed changes

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

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

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

parthea assigned nbayati May 26, 2026

nbayati marked this pull request as ready for review June 10, 2026 05:59

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

nbayati added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:run Add this label to force Kokoro to re-run the tests. labels Jun 10, 2026

nbayati and others added 4 commits June 10, 2026 06:07

fix: resolve mypy and lint issues

48f4e26

fix: suppress interactive OpenSSL stdin passphrase prompts during mTL…

41282b8

…S cert loading fallbacks

add unit tests to mtls_helper

1704dd8

nbayati force-pushed the remove-pyopenssl-dependency branch from f7e5146 to 1704dd8 Compare June 10, 2026 06:11

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 10, 2026

nbayati added 5 commits June 16, 2026 19:03

refactor(auth): use os.fdopen for writing to memfd in mtls helper and…

a6882e4

… update tests accordingly

test(auth): fix failing test by updating mock_mds_mtls_config asserti…

f9a741a

…on to include empty password for client_combined_cert_path

fix nox failures

04776d6

test: add edge case and error handling tests for _mtls_helper functions

980527e

fix lint error, again!

105b02a

This was referenced Jun 17, 2026

feat(auth): add mTLS helper methods and endpoint configuration for ha… nbayati/google-cloud-python#1

Closed

feat(auth): Implement python mtls helpers #17495

Open

gemini-code-assist Bot reviewed Jun 17, 2026

View reviewed changes

daniel-sanche requested changes Jun 17, 2026

View reviewed changes

nbayati added 3 commits June 17, 2026 20:20

refactor(auth): address PR comments on imports, exit call, and safety…

0a9e02d

… checks

docs/refactor(auth): improve secure_cert_key_paths docstrings and ref…

7b77812

…actor file writing into loops

refactor(auth): simplify fallback logic using custom exception and cl…

31ae507

…ean standard with block

nbayati requested a review from daniel-sanche June 18, 2026 05:45

refactor: add type annotations to paths variable in _mtls_helper.py

759f6c2

macastelaz reviewed Jun 18, 2026

View reviewed changes

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

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

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

lsirac requested changes Jun 22, 2026

View reviewed changes

nbayati mentioned this pull request Jun 23, 2026

fix(auth): align mTLS discovery and enforce fail-fast transport configuration. #17470

Merged

nbayati added 6 commits June 25, 2026 01:09

fix: unpack cryptography_base_require in DEPENDENCIES

9c31523

fix(auth): wrap callback, certificate load and trust chain read error…

30dd701

…s in RefreshError

verify certificate path readability to prevent AppArmor/LSM crashes

3eaaf22

wrap secure_cert_key_paths inside transport exception handlers

56cde12

fix: catch ConnectionError in MDS client to allow HTTP fallback on co…

22e7f0d

…nnection resets

raise MutualTLSChannelError if custom TLS signer is used on unsupport…

1305d2f

…ed runtimes

nbayati requested review from lsirac and macastelaz June 25, 2026 03:22

fix lint and mypy failre

3ff3750

macastelaz approved these changes Jun 25, 2026

View reviewed changes

fix(auth): handle FileNotFoundError for trust chain and mock os.acces…

21d0254

…s in tests

gemini-code-assist Bot reviewed Jun 25, 2026

View reviewed changes

lsirac reviewed Jun 25, 2026

View reviewed changes

lsirac reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

nbayati commented May 7, 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!

parthea commented May 26, 2026

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-sanche Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nbayati commented Jun 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

macastelaz Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

parthea commented Jun 25, 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 Jun 25, 2026

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

1. Critical Packaging Bug in setup.py

2. Standard SSLContext Pointer Alignment and Safety

3. Missing Type Validation on Context Parameter

Uh oh!

lsirac Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati commented May 7, 2026 •

edited

Loading

lsirac left a comment •

edited

Loading

1. Critical Packaging Bug in `setup.py`