Skip to content

Commit 81b9ae4

Browse files
nbayatihebaalazzeh
authored andcommitted
fix(transport): propagate mTLS adapter to auth session and fix connection leaks (#17689)
This PR is fixing two issues found in the auth library: 1. Adapter Connection Leaks Whenever the `configure_mtls_channel()` method was called, the code would create a new mTLS `HTTPAdapter` and mount it to `"https://"`. However, it never closed the *old* adapter it was replacing. Because of how the underlying `requests` and `urllib3` libraries work, the old adapter's connection pool was left open, creating dangling sockets and eventually causing file descriptor exhaustion. **The Fix:** I updated the method to retrieve the existing `"https://"` adapter (via `self.get_adapter()`) before replacing it. If an old adapter is found, we now explicitly call `old_adapter.close()` to shut down its connection pool and cleanly release the sockets back to the OS. 2. Missing Auth Request Session Update (Issue #17680) The `AuthorizedSession` class maintains an internal session called `_auth_request_session`. This hidden session is specifically responsible for doing background work, like fetching new OAuth tokens or signing IAM requests. When mTLS was enabled, the new secure adapter was *only* applied to the main user-facing session, completely missing this internal session. As a result, critical token refreshes were bypassing mTLS and failing when they hit strict Certificate-Based Access (CBA) endpoints. **The Fix:** I added logic to check if `self._auth_request_session` exists during the mTLS configuration. If it does, we now apply the exact same cleanup (closing its old adapter) and then explicitly mount the new mTLS adapter to it. This guarantees that all background token refreshes are routed securely over the mTLS channel. Also added a documentation warning to clarify that dynamically reconfiguring the channel mutates the underlying session and is not natively thread-safe.
1 parent 26e7820 commit 81b9ae4

4 files changed

Lines changed: 274 additions & 5 deletions

File tree

packages/google-auth/google/auth/transport/requests.py

Lines changed: 70 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class _MutualTlsAdapter(requests.adapters.HTTPAdapter):
208208
google.auth.exceptions.MutualTLSChannelError: If the cert or key is invalid.
209209
"""
210210

211-
def __init__(self, cert, key):
211+
def __init__(self, cert, key, **kwargs):
212212
import certifi
213213
import ssl
214214

@@ -250,7 +250,7 @@ def __init__(self, cert, key):
250250
self._ctx_poolmanager = ctx_poolmanager
251251
self._ctx_proxymanager = ctx_proxymanager
252252

253-
super(_MutualTlsAdapter, self).__init__()
253+
super(_MutualTlsAdapter, self).__init__(**kwargs)
254254

255255
def init_poolmanager(self, *args, **kwargs):
256256
kwargs["ssl_context"] = self._ctx_poolmanager
@@ -457,6 +457,11 @@ def configure_mtls_channel(self, client_cert_callback=None):
457457
If the callback is None, application default SSL credentials
458458
will be used.
459459
460+
.. warning::
461+
Calling this method mutates the underlying `requests.Session` adapter
462+
dictionary. It is not thread-safe to call this explicitly while other
463+
threads are making requests.
464+
460465
Raises:
461466
google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel
462467
creation failed for any reason. The existing session state (such
@@ -475,10 +480,58 @@ def configure_mtls_channel(self, client_cert_callback=None):
475480
client_cert_callback
476481
)
477482

483+
old_adapter = self.adapters.get("https://")
484+
485+
kwargs = {}
486+
if old_adapter is not None:
487+
kwargs["max_retries"] = getattr(old_adapter, "max_retries", 0)
488+
kwargs["pool_connections"] = getattr(
489+
old_adapter, "_pool_connections", requests.adapters.DEFAULT_POOLSIZE
490+
)
491+
kwargs["pool_maxsize"] = getattr(
492+
old_adapter, "_pool_maxsize", requests.adapters.DEFAULT_POOLSIZE
493+
)
494+
kwargs["pool_block"] = getattr(
495+
old_adapter, "_pool_block", requests.adapters.DEFAULT_POOLBLOCK
496+
)
497+
498+
old_auth_adapter = None
499+
auth_kwargs = {}
500+
if self._auth_request_session is not None:
501+
old_auth_adapter = self._auth_request_session.adapters.get("https://")
502+
503+
if old_auth_adapter is not None:
504+
auth_kwargs["max_retries"] = getattr(
505+
old_auth_adapter, "max_retries", 0
506+
)
507+
auth_kwargs["pool_connections"] = getattr(
508+
old_auth_adapter,
509+
"_pool_connections",
510+
requests.adapters.DEFAULT_POOLSIZE,
511+
)
512+
auth_kwargs["pool_maxsize"] = getattr(
513+
old_auth_adapter,
514+
"_pool_maxsize",
515+
requests.adapters.DEFAULT_POOLSIZE,
516+
)
517+
auth_kwargs["pool_block"] = getattr(
518+
old_auth_adapter,
519+
"_pool_block",
520+
requests.adapters.DEFAULT_POOLBLOCK,
521+
)
522+
478523
if is_mtls:
479-
new_adapter = _MutualTlsAdapter(cert, key)
524+
new_adapter = _MutualTlsAdapter(cert, key, **kwargs)
525+
if self._auth_request_session is not None:
526+
new_auth_adapter = _MutualTlsAdapter(cert, key, **auth_kwargs)
527+
else:
528+
new_auth_adapter = None
480529
else:
481-
new_adapter = requests.adapters.HTTPAdapter()
530+
new_adapter = requests.adapters.HTTPAdapter(**kwargs)
531+
if self._auth_request_session is not None:
532+
new_auth_adapter = requests.adapters.HTTPAdapter(**auth_kwargs)
533+
else:
534+
new_auth_adapter = None
482535
except (
483536
exceptions.ClientCertError,
484537
ImportError,
@@ -489,6 +542,19 @@ def configure_mtls_channel(self, client_cert_callback=None):
489542
raise new_exc from caught_exc
490543

491544
self.mount("https://", new_adapter)
545+
546+
if old_adapter is not None and old_adapter is not new_adapter:
547+
old_adapter.close()
548+
549+
if self._auth_request_session is not None and new_auth_adapter is not None:
550+
self._auth_request_session.mount("https://", new_auth_adapter)
551+
552+
if (
553+
old_auth_adapter is not None
554+
and old_auth_adapter is not new_auth_adapter
555+
):
556+
old_auth_adapter.close()
557+
492558
self._is_mtls = is_mtls
493559
if is_mtls:
494560
self._cached_cert = cert

packages/google-auth/google/auth/transport/urllib3.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,11 @@ def configure_mtls_channel(self, client_cert_callback=None):
335335
If the callback is None, application default SSL credentials
336336
will be used.
337337
338+
.. warning::
339+
Calling this method mutates the underlying `urllib3.PoolManager`.
340+
It is not thread-safe to call this explicitly while other
341+
threads are making requests.
342+
338343
Returns:
339344
True if the channel is mutual TLS and False otherwise.
340345
@@ -367,9 +372,15 @@ def configure_mtls_channel(self, client_cert_callback=None):
367372
new_exc = exceptions.MutualTLSChannelError(caught_exc)
368373
raise new_exc from caught_exc
369374

375+
old_http = self.http
376+
370377
self.http = new_http
371378
self._is_mtls = new_is_mtls
372379
self._request.http = new_http
380+
381+
if old_http is not None and old_http is not new_http:
382+
getattr(old_http, "clear", getattr(old_http, "close", lambda: None))()
383+
373384
if new_is_mtls:
374385
self._cached_cert = cert
375386
else:
@@ -491,7 +502,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
491502

492503
def __del__(self):
493504
if hasattr(self, "http") and self.http is not None:
494-
self.http.clear()
505+
getattr(self.http, "clear", getattr(self.http, "close", lambda: None))()
495506

496507
@property
497508
def headers(self):

packages/google-auth/tests/transport/test_requests.py

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,141 @@ def test_configure_mtls_channel_with_metadata(self, mock_get_client_cert_and_key
453453
google.auth.transport.requests._MutualTlsAdapter,
454454
)
455455

456+
@mock.patch(
457+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
458+
)
459+
def test_configure_mtls_channel_closes_old_adapters(
460+
self, mock_get_client_cert_and_key
461+
):
462+
mock_get_client_cert_and_key.return_value = (
463+
True,
464+
pytest.public_cert_bytes,
465+
pytest.private_key_bytes,
466+
)
467+
468+
auth_session = google.auth.transport.requests.AuthorizedSession(
469+
credentials=mock.Mock()
470+
)
471+
old_main_adapter = mock.Mock(spec=requests.adapters.HTTPAdapter)
472+
old_auth_adapter = mock.Mock(spec=requests.adapters.HTTPAdapter)
473+
474+
auth_session.mount("https://", old_main_adapter)
475+
auth_session._auth_request_session.mount("https://", old_auth_adapter)
476+
477+
with mock.patch.dict(
478+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
479+
):
480+
auth_session.configure_mtls_channel()
481+
482+
old_main_adapter.close.assert_called_once()
483+
old_auth_adapter.close.assert_called_once()
484+
485+
@mock.patch(
486+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
487+
)
488+
def test_configure_mtls_channel_mounts_adapter_to_auth_request_session(
489+
self, mock_get_client_cert_and_key
490+
):
491+
mock_get_client_cert_and_key.return_value = (
492+
True,
493+
pytest.public_cert_bytes,
494+
pytest.private_key_bytes,
495+
)
496+
497+
auth_session = google.auth.transport.requests.AuthorizedSession(
498+
credentials=mock.Mock()
499+
)
500+
501+
with mock.patch.dict(
502+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
503+
):
504+
auth_session.configure_mtls_channel()
505+
506+
assert auth_session.is_mtls
507+
# Main session gets the mTLS adapter
508+
assert isinstance(
509+
auth_session.adapters["https://"],
510+
google.auth.transport.requests._MutualTlsAdapter,
511+
)
512+
# _auth_request_session gets a separate adapter instance
513+
assert isinstance(
514+
auth_session._auth_request_session.adapters["https://"],
515+
google.auth.transport.requests._MutualTlsAdapter,
516+
)
517+
assert (
518+
auth_session.adapters["https://"]
519+
is not auth_session._auth_request_session.adapters["https://"]
520+
)
521+
522+
@mock.patch(
523+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
524+
)
525+
def test_configure_mtls_channel_without_https_adapter(
526+
self, mock_get_client_cert_and_key
527+
):
528+
mock_get_client_cert_and_key.return_value = (
529+
True,
530+
pytest.public_cert_bytes,
531+
pytest.private_key_bytes,
532+
)
533+
534+
auth_session = google.auth.transport.requests.AuthorizedSession(
535+
credentials=mock.Mock()
536+
)
537+
538+
# Remove the 'https://' adapter to trigger InvalidSchema
539+
auth_session.adapters.pop("https://", None)
540+
auth_session._auth_request_session.adapters.pop("https://", None)
541+
542+
with mock.patch.dict(
543+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
544+
):
545+
auth_session.configure_mtls_channel()
546+
547+
assert auth_session.is_mtls
548+
# Main session gets the mTLS adapter
549+
assert isinstance(
550+
auth_session.adapters["https://"],
551+
google.auth.transport.requests._MutualTlsAdapter,
552+
)
553+
# _auth_request_session gets the exact same adapter
554+
assert isinstance(
555+
auth_session._auth_request_session.adapters["https://"],
556+
google.auth.transport.requests._MutualTlsAdapter,
557+
)
558+
559+
@mock.patch(
560+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
561+
)
562+
def test_configure_mtls_channel_without_auth_request_session(
563+
self, mock_get_client_cert_and_key
564+
):
565+
mock_get_client_cert_and_key.return_value = (
566+
True,
567+
pytest.public_cert_bytes,
568+
pytest.private_key_bytes,
569+
)
570+
571+
auth_session = google.auth.transport.requests.AuthorizedSession(
572+
credentials=mock.Mock(), auth_request=mock.Mock()
573+
)
574+
assert auth_session._auth_request_session is None
575+
576+
old_main_adapter = mock.Mock(spec=requests.adapters.HTTPAdapter)
577+
auth_session.mount("https://", old_main_adapter)
578+
579+
with mock.patch.dict(
580+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
581+
):
582+
auth_session.configure_mtls_channel()
583+
584+
old_main_adapter.close.assert_called_once()
585+
assert auth_session.is_mtls
586+
assert isinstance(
587+
auth_session.adapters["https://"],
588+
google.auth.transport.requests._MutualTlsAdapter,
589+
)
590+
456591
@mock.patch.object(google.auth.transport.requests._MutualTlsAdapter, "__init__")
457592
@mock.patch(
458593
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True

packages/google-auth/tests/transport/test_urllib3.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,63 @@ def test_configure_mtls_channel_with_metadata(
243243
cert=pytest.public_cert_bytes, key=pytest.private_key_bytes
244244
)
245245

246+
@mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True)
247+
@mock.patch(
248+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
249+
)
250+
def test_configure_mtls_channel_closes_old_poolmanager(
251+
self, mock_get_client_cert_and_key, mock_make_mutual_tls_http
252+
):
253+
mock_get_client_cert_and_key.return_value = (
254+
True,
255+
pytest.public_cert_bytes,
256+
pytest.private_key_bytes,
257+
)
258+
259+
old_http = mock.create_autospec(urllib3.PoolManager)
260+
authed_http = google.auth.transport.urllib3.AuthorizedHttp(
261+
credentials=mock.Mock(), http=old_http
262+
)
263+
264+
with mock.patch.dict(
265+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
266+
):
267+
is_mtls = authed_http.configure_mtls_channel()
268+
269+
assert is_mtls
270+
old_http.clear.assert_called_once()
271+
mock_make_mutual_tls_http.assert_called_once_with(
272+
cert=pytest.public_cert_bytes, key=pytest.private_key_bytes
273+
)
274+
275+
@mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True)
276+
@mock.patch(
277+
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True
278+
)
279+
def test_configure_mtls_channel_with_none_http(
280+
self, mock_get_client_cert_and_key, mock_make_mutual_tls_http
281+
):
282+
mock_get_client_cert_and_key.return_value = (
283+
True,
284+
pytest.public_cert_bytes,
285+
pytest.private_key_bytes,
286+
)
287+
288+
authed_http = google.auth.transport.urllib3.AuthorizedHttp(
289+
credentials=mock.Mock()
290+
)
291+
authed_http.http = None # Force old_http to be None
292+
293+
with mock.patch.dict(
294+
os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}
295+
):
296+
is_mtls = authed_http.configure_mtls_channel()
297+
298+
assert is_mtls
299+
mock_make_mutual_tls_http.assert_called_once_with(
300+
cert=pytest.public_cert_bytes, key=pytest.private_key_bytes
301+
)
302+
246303
@mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True)
247304
@mock.patch(
248305
"google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True

0 commit comments

Comments
 (0)