From 2769cb607d4e696e2fe70802d4246ccc5abd64a8 Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Wed, 29 May 2024 12:48:48 -0700 Subject: [PATCH 1/3] Consider cert settings when using default context --- src/requests/adapters.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/requests/adapters.py b/src/requests/adapters.py index 9a58b16025..991b7e21c9 100644 --- a/src/requests/adapters.py +++ b/src/requests/adapters.py @@ -87,6 +87,23 @@ def SOCKSProxyManager(*args, **kwargs): _preloaded_ssl_context = None +def _should_use_default_context( + verify: "bool | str | None", + client_cert: "typing.Tuple[str, str] | str | None", + poolmanager_kwargs: typing.Dict[str, typing.Any], +) -> bool: + # Determine if we have and should use our default SSLContext + # to optimize performance on standard requests. + has_poolmanager_ssl_context = poolmanager_kwargs.get("ssl_context") + should_use_default_ssl_context = ( + verify is True + and _preloaded_ssl_context is not None + and not has_poolmanager_ssl_context + and client_cert is None + ) + return should_use_default_ssl_context + + def _urllib3_request_context( request: "PreparedRequest", verify: "bool | str | None", @@ -98,19 +115,12 @@ def _urllib3_request_context( parsed_request_url = urlparse(request.url) scheme = parsed_request_url.scheme.lower() port = parsed_request_url.port - - # Determine if we have and should use our default SSLContext - # to optimize performance on standard requests. poolmanager_kwargs = getattr(poolmanager, "connection_pool_kw", {}) - has_poolmanager_ssl_context = poolmanager_kwargs.get("ssl_context") - should_use_default_ssl_context = ( - _preloaded_ssl_context is not None and not has_poolmanager_ssl_context - ) cert_reqs = "CERT_REQUIRED" if verify is False: cert_reqs = "CERT_NONE" - elif verify is True and should_use_default_ssl_context: + elif _should_use_default_context(verify, client_cert, poolmanager_kwargs): pool_kwargs["ssl_context"] = _preloaded_ssl_context elif isinstance(verify, str): if not os.path.isdir(verify): From e341df3efa0323072fab5d16307e2a20295675b9 Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Fri, 31 May 2024 11:41:48 -0700 Subject: [PATCH 2/3] Set default ca_cert bundle if verify is True --- src/requests/adapters.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/requests/adapters.py b/src/requests/adapters.py index 991b7e21c9..ba5a0ec4f0 100644 --- a/src/requests/adapters.py +++ b/src/requests/adapters.py @@ -118,15 +118,23 @@ def _urllib3_request_context( poolmanager_kwargs = getattr(poolmanager, "connection_pool_kw", {}) cert_reqs = "CERT_REQUIRED" + cert_loc = None if verify is False: cert_reqs = "CERT_NONE" elif _should_use_default_context(verify, client_cert, poolmanager_kwargs): pool_kwargs["ssl_context"] = _preloaded_ssl_context + elif verify is True: + # Set default ca cert location if none provided + cert_loc = extract_zipped_paths(DEFAULT_CA_BUNDLE_PATH) elif isinstance(verify, str): - if not os.path.isdir(verify): - pool_kwargs["ca_certs"] = verify + cert_loc = verify + + if cert_loc is not None: + if not os.path.isdir(cert_loc): + pool_kwargs["ca_certs"] = cert_loc else: - pool_kwargs["ca_cert_dir"] = verify + pool_kwargs["ca_cert_dir"] = cert_loc + pool_kwargs["cert_reqs"] = cert_reqs if client_cert is not None: if isinstance(client_cert, tuple) and len(client_cert) == 2: From da96a92e2eb6dfe7c74704267bcb8f9fd6fb92b0 Mon Sep 17 00:00:00 2001 From: Nate Prewitt Date: Fri, 31 May 2024 12:20:11 -0700 Subject: [PATCH 3/3] Correct comment to match actual behavior --- src/requests/adapters.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/requests/adapters.py b/src/requests/adapters.py index ba5a0ec4f0..54143f9e6b 100644 --- a/src/requests/adapters.py +++ b/src/requests/adapters.py @@ -334,10 +334,8 @@ def cert_verify(self, conn, url, verify, cert): if url.lower().startswith("https") and verify: conn.cert_reqs = "CERT_REQUIRED" - # Only load the CA certificates if 'verify' is a string indicating the CA bundle to use. - # Otherwise, if verify is a boolean, we don't load anything since - # the connection will be using a context with the default certificates already loaded, - # and this avoids a call to the slow load_verify_locations() + # Only load the CA certificates if `verify` is a + # string indicating the CA bundle to use. if verify is not True: # `verify` must be a str with a path then cert_loc = verify