fix(auth): restore pyOpenSSL for ECP offload flow by nbayati · Pull Request #18085 · googleapis/google-cloud-python

nbayati · 2026-08-12T05:31:57Z

Resolves an issue where Enterprise Certificate Proxy (ECP) offload failed with "failed to configure ECP Offload SSL context" after pyOpenSSL was removed in #16976.

The tls_offload C library requires a PyOpenSSL context pointer and does not support standard library CPython ssl.SSLContext. This change:

Restores _cast_ssl_ctx_to_void_p_pyopenssl in _custom_tls_signer.py for the offload branch.
Conditionally calls urllib3.contrib.pyopenssl.inject_into_urllib3() in _MutualTlsOffloadAdapter only when the offload flow is used, leaving the ECP Provider flow on standard library ssl.SSLContext.
Restores pyopenssl and cffi in enterprise_cert extra requirements in setup.py.
Updates unit tests

Fixes #17791

Resolves an issue where Enterprise Certificate Proxy (ECP) offload failed with "failed to configure ECP Offload SSL context" after pyOpenSSL was removed in googleapis#16976. The `tls_offload` C library requires a PyOpenSSL context pointer and does not support standard library CPython `ssl.SSLContext`. This change: - Restores `_cast_ssl_ctx_to_void_p_pyopenssl` in `_custom_tls_signer.py` for the offload branch. - Conditionally calls `urllib3.contrib.pyopenssl.inject_into_urllib3()` in `_MutualTlsOffloadAdapter` only when the offload flow is used, leaving the ECP Provider flow on standard library `ssl.SSLContext`. - Restores `pyopenssl` and `cffi` in `enterprise_cert` extra requirements in `setup.py`. - Updates unit tests Fixes googleapis#17791

nbayati · 2026-08-12T05:32:45Z

I've verified the fix on the ECP offload flow locally. I still need to verify that there was no regression on the ECP provider flow.

gemini-code-assist

Code Review

This pull request introduces support for pyOpenSSL ECP by casting the SSL context using cffi and pyOpenSSL, injecting pyOpenSSL into urllib3 when necessary, and updating the setup dependencies and tests accordingly. The review feedback highlights two critical issues: first, the cryptography dependency should be combined with, rather than replaced by, the new pyOpenSSL and cffi dependencies in setup.py; second, strict type checking should be enforced on the SSL context in _custom_tls_signer.py to prevent security risks from duck-typed wrapper objects, while gracefully returning False if the type is unexpected to maintain backwards compatibility.

gemini-code-assist · 2026-08-12T05:34:09Z

 reauth_extra_require = ["pyu2f>=0.1.5"]

-enterprise_cert_extra_require = cryptography_base_require
+enterprise_cert_extra_require = ["pyopenssl>=20.0.0", "cffi>=1.0.0"]


Replacing cryptography_base_require entirely with ["pyopenssl>=20.0.0", "cffi>=1.0.0"] removes the cryptography dependency from the enterprise_cert extra. Since the enterprise certificate functionality still relies on cryptography for parsing and handling certificates, we should combine them instead of replacing.

enterprise_cert_extra_require = cryptography_base_require + [ "pyopenssl>=20.0.0", "cffi>=1.0.0", ]

gemini-code-assist · 2026-08-12T05:34:09Z

            if not self._offload_lib.ConfigureSslContext(
                self._sign_callback,
                ctypes.c_char_p(self._cert),
-                _cast_ssl_ctx_to_void_p_stdlib(ctx),
+                _cast_ssl_ctx_to_void_p_pyopenssl(ctx._ctx._context),
            ):


To prevent security risks such as passphrase leakage from arbitrary duck-typed wrapper objects, we should enforce strict type checking on the SSL context instead of using duck typing. Additionally, to maintain backwards compatibility and avoid introducing breaking changes, we should gracefully return False (or fall back) instead of raising an exception if the context is not of the expected type.

Suggested change

if not self._offload_lib.ConfigureSslContext(

self._sign_callback,

ctypes.c_char_p(self._cert),

_cast_ssl_ctx_to_void_p_stdlib(ctx),

_cast_ssl_ctx_to_void_p_pyopenssl(ctx._ctx._context),

):

if not isinstance(ctx, OpenSSL.SSL.Context):

return False

ssl_ctx = ctx._ctx._context

if not self._offload_lib.ConfigureSslContext(

self._sign_callback,

ctypes.c_char_p(self._cert),

_cast_ssl_ctx_to_void_p_pyopenssl(ssl_ctx),

):

References

When passing sensitive cryptographic material (such as private keys and passphrases) to an SSL context, enforce strict type checking (e.g., isinstance(ctx, ssl.SSLContext)) instead of duck typing. This prevents security risks, such as passphrase leakage, that could be introduced by arbitrary duck-typed wrapper objects.

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.

I looked into this, and it seems like this is a false flag.
First, the recommended solution would not work, because ctx is a urllib3.contrib.pyopenssl.PyOpenSSLContext wrapper object (where the OpenSSL.SSL.Context instance lives at ctx._ctx). Enforcing isinstance(ctx, OpenSSL.SSL.Context) directly on ctx evaluates to False, which would disable ECP offload configuration entirely. Also attach_to_ssl_context is designed to raise exceptions.MutualTLSChannelError whenever ECP configuration fails. Transport callers (such as requests.py) expect an exception on failure and do not evaluate boolean return values, so returning False would cause setup failures to fail silently.

Second, there isn't really a security risk here. The SSL context is only used to pass OpenSSL's underlying C-level SSL_CTX* pointer (ctx._ctx._context) to the C++ offload library. Private key operations and signing are offloaded via _sign_callback (communicating with hardware/TPM), so no private keys or passphrases are stored on or accessed from ctx.

While I think I understand the conclusion of this being, largely, a false flag - I do think there may be some validity here with regards to the existing change leading to the possibility of an AttributeError being raised and escaping. We probably should be more defensive about extracting this.

…sts and urllib3

…transport tests

macastelaz · 2026-08-14T03:27:21Z

+            "cffi is required for pyOpenSSL ECP support."
+        ) from caught_exc
+
+    return ctypes.cast(int(cffi.FFI().cast("intptr_t", ssl_ctx)), ctypes.c_void_p)


Do we need to use an unsigned ptr here (uintptr_t) to avoid overflow if the address of ssl_ctx has a most significant bit of "1"?

macastelaz · 2026-08-14T03:31:22Z

            if not self._offload_lib.ConfigureSslContext(
                self._sign_callback,
                ctypes.c_char_p(self._cert),
-                _cast_ssl_ctx_to_void_p_stdlib(ctx),
+                _cast_ssl_ctx_to_void_p_pyopenssl(ctx._ctx._context),
            ):


While I think I understand the conclusion of this being, largely, a false flag - I do think there may be some validity here with regards to the existing change leading to the possibility of an AttributeError being raised and escaping. We probably should be more defensive about extracting this.

macastelaz · 2026-08-14T03:35:44Z

+try:
+    import OpenSSL.SSL  # type: ignore
+
+    _OPENSSL_SSL_ERROR = (OpenSSL.SSL.Error,)


Do we need to worry about OpenSSL.crypto.Error?

macastelaz · 2026-08-14T03:39:22Z

-        ImportError: if certifi is not installed
+        ImportError: if certifi or pyOpenSSL is not installed
        google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel
            creation failed for any reason.


or if cffi is not installed

macastelaz · 2026-08-14T03:41:38Z

+        if not self.signer.should_use_provider():
+            import urllib3.contrib.pyopenssl
+
+            urllib3.contrib.pyopenssl.inject_into_urllib3()


I believe this has a process-wide side effect where it permanently switches all subsequent HTTPS requests made anywhere across the Python process to PyOpenSSL.

gemini-code-assist Bot reviewed Aug 12, 2026

View reviewed changes

parthea assigned nbayati Aug 12, 2026

nbayati added 2 commits August 13, 2026 20:22

fix(auth): catch OpenSSL.SSL.Error during mTLS channel setup in reque…

0f0d97b

…sts and urllib3

fix(test): teardown pyopenssl injection and isolate gcloud config in …

e0c06a0

…transport tests

nbayati marked this pull request as ready for review August 13, 2026 22:55

nbayati requested review from a team as code owners August 13, 2026 22:55

nbayati requested a review from nolanleastin August 14, 2026 01:57

macastelaz reviewed Aug 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(auth): restore pyOpenSSL for ECP offload flow - #18085

fix(auth): restore pyOpenSSL for ECP offload flow#18085
nbayati wants to merge 3 commits into
googleapis:mainfrom
nbayati:ecp-pyopenssl-restore

nbayati commented Aug 12, 2026

Uh oh!

nbayati commented Aug 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Aug 12, 2026

Uh oh!

gemini-code-assist Bot Aug 12, 2026

Uh oh!

nbayati Aug 13, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

macastelaz Aug 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nbayati commented Aug 12, 2026

Uh oh!

nbayati commented Aug 12, 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 Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

macastelaz Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants