Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,18 @@
)


# Cast SSL_CTX* to void*
def _cast_ssl_ctx_to_void_p_pyopenssl(ssl_ctx):
try:
import cffi
except ImportError as caught_exc:
raise exceptions.MutualTLSChannelError(
"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)


# Cast SSL_CTX* to void*
def _cast_ssl_ctx_to_void_p_stdlib(context):
if not issubclass(type(context), ssl.SSLContext):
Expand Down Expand Up @@ -281,7 +293,7 @@ def attach_to_ssl_context(self, ctx):
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),
):
Comment on lines 293 to 297

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.

high

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
  1. 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.
  2. 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.

raise exceptions.MutualTLSChannelError(
"failed to configure ECP Offload SSL context"
Expand Down
7 changes: 6 additions & 1 deletion packages/google-auth/google/auth/transport/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ class _MutualTlsOffloadAdapter(requests.adapters.HTTPAdapter):
}

Raises:
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.
"""
Expand All @@ -290,6 +290,11 @@ def __init__(self, enterprise_cert_file_path):
self.signer = _custom_tls_signer.CustomTlsSigner(enterprise_cert_file_path)
self.signer.load_libraries()

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

urllib3.contrib.pyopenssl.inject_into_urllib3()

poolmanager = create_urllib3_context()
poolmanager.load_verify_locations(cafile=certifi.where())
self.signer.attach_to_ssl_context(poolmanager)
Expand Down
3 changes: 2 additions & 1 deletion packages/google-auth/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

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"]

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.

high

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",
]


urllib3_extra_require = [
"urllib3 >= 1.26.15, < 3.0.0",
Expand All @@ -65,6 +65,7 @@
*reauth_extra_require,
"responses",
*urllib3_extra_require,
*enterprise_cert_extra_require,
# Async Dependencies
*aiohttp_extra_require,
"aioresponses",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ def test_get_cert():


def test_custom_tls_signer():
urllib3_pyopenssl = pytest.importorskip("urllib3.contrib.pyopenssl")
urllib3_pyopenssl.inject_into_urllib3()

offload_lib = mock.MagicMock()
signer_lib = mock.MagicMock()

Expand Down Expand Up @@ -238,7 +241,9 @@ def test_custom_tls_signer_failed_to_attach():
signer_object._sign_callback = mock.MagicMock()
signer_object._cert = b"mock cert"
signer_object._offload_lib.ConfigureSslContext.return_value = False
signer_object.attach_to_ssl_context(ssl.SSLContext())
ctx = mock.Mock()
ctx._ctx._context = 123456
signer_object.attach_to_ssl_context(ctx)
assert excinfo.match("failed to configure ECP Offload SSL context")


Expand Down Expand Up @@ -366,3 +371,12 @@ def test_cast_ssl_ctx_to_void_p_stdlib_mock_error():
TypeError, match="context must be an instance of ssl.SSLContext, not a mock"
):
_custom_tls_signer._cast_ssl_ctx_to_void_p_stdlib(context)


def test_cast_ssl_ctx_to_void_p_pyopenssl():
urllib3_pyopenssl = pytest.importorskip("urllib3.contrib.pyopenssl")
urllib3_pyopenssl.inject_into_urllib3()

context = create_urllib3_context()
res = _custom_tls_signer._cast_ssl_ctx_to_void_p_pyopenssl(context._ctx._context)
assert isinstance(res, ctypes.c_void_p)
1 change: 1 addition & 0 deletions packages/google-auth/tests/transport/test_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,7 @@ def test_success(
mock_proxy_manager_for,
mock_init_poolmanager,
):
pytest.importorskip("urllib3.contrib.pyopenssl")
enterprise_cert_file_path = "/path/to/enterprise/cert/json"
adapter = google.auth.transport.requests._MutualTlsOffloadAdapter(
enterprise_cert_file_path
Expand Down
Loading