From f1a8bd4ad6d51283b261bd188e946ae67bd9ae59 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Mon, 23 Mar 2020 09:27:55 +0200 Subject: [PATCH 1/5] session: Fix comment about add_insecure_host, rename to add_trusted_host --- src/pip/_internal/network/session.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pip/_internal/network/session.py b/src/pip/_internal/network/session.py index f5eb15ef2f6..b311a2762b9 100644 --- a/src/pip/_internal/network/session.py +++ b/src/pip/_internal/network/session.py @@ -280,7 +280,7 @@ def __init__(self, *args, **kwargs): # well as any https:// host that we've marked as ignoring TLS errors # for. insecure_adapter = InsecureHTTPAdapter(max_retries=retries) - # Save this for later use in add_insecure_host(). + # Save this for later use in add_trusted_host(). self._insecure_adapter = insecure_adapter self.mount("https://", secure_adapter) From c936ed01655ea85ba408d064c2167fd0cb255c1e Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Mon, 23 Mar 2020 09:30:55 +0200 Subject: [PATCH 2/5] session: Rename _insecure_adapter to _trusted_host_adapter Currently it is just the insecure adapter, but this can change in the future --- src/pip/_internal/network/session.py | 9 ++++++--- tests/unit/test_network_session.py | 12 ++++++------ tests/unit/test_req_file.py | 7 +++++-- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/pip/_internal/network/session.py b/src/pip/_internal/network/session.py index b311a2762b9..1e895a15c62 100644 --- a/src/pip/_internal/network/session.py +++ b/src/pip/_internal/network/session.py @@ -281,7 +281,7 @@ def __init__(self, *args, **kwargs): # for. insecure_adapter = InsecureHTTPAdapter(max_retries=retries) # Save this for later use in add_trusted_host(). - self._insecure_adapter = insecure_adapter + self._trusted_host_adapter = insecure_adapter self.mount("https://", secure_adapter) self.mount("http://", insecure_adapter) @@ -310,12 +310,15 @@ def add_trusted_host(self, host, source=None, suppress_logging=False): if host_port not in self.pip_trusted_origins: self.pip_trusted_origins.append(host_port) - self.mount(build_url_from_netloc(host) + '/', self._insecure_adapter) + self.mount( + build_url_from_netloc(host) + '/', + self._trusted_host_adapter + ) if not host_port[1]: # Mount wildcard ports for the same host. self.mount( build_url_from_netloc(host) + ':', - self._insecure_adapter + self._trusted_host_adapter ) def iter_secure_origins(self): diff --git a/tests/unit/test_network_session.py b/tests/unit/test_network_session.py index 159a4d4dea1..125ac2d8de2 100644 --- a/tests/unit/test_network_session.py +++ b/tests/unit/test_network_session.py @@ -88,7 +88,7 @@ def test_add_trusted_host(self): # Leave a gap to test how the ordering is affected. trusted_hosts = ['host1', 'host3'] session = PipSession(trusted_hosts=trusted_hosts) - insecure_adapter = session._insecure_adapter + trusted_host_adapter = session._trusted_host_adapter prefix2 = 'https://host2/' prefix3 = 'https://host3/' prefix3_wildcard = 'https://host3:' @@ -97,8 +97,8 @@ def test_add_trusted_host(self): assert session.pip_trusted_origins == [ ('host1', None), ('host3', None) ] - assert session.adapters[prefix3] is insecure_adapter - assert session.adapters[prefix3_wildcard] is insecure_adapter + assert session.adapters[prefix3] is trusted_host_adapter + assert session.adapters[prefix3_wildcard] is trusted_host_adapter assert prefix2 not in session.adapters @@ -108,8 +108,8 @@ def test_add_trusted_host(self): ('host1', None), ('host3', None), ('host2', None) ] # Check that prefix3 is still present. - assert session.adapters[prefix3] is insecure_adapter - assert session.adapters[prefix2] is insecure_adapter + assert session.adapters[prefix3] is trusted_host_adapter + assert session.adapters[prefix2] is trusted_host_adapter # Test that adding the same host doesn't create a duplicate. session.add_trusted_host('host3') @@ -123,7 +123,7 @@ def test_add_trusted_host(self): ('host1', None), ('host3', None), ('host2', None), ('host4', 8080) ] - assert session.adapters[prefix4] is insecure_adapter + assert session.adapters[prefix4] is trusted_host_adapter def test_add_trusted_host__logging(self, caplog): """ diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py index c12cdb64bf6..57b50017a7f 100644 --- a/tests/unit/test_req_file.py +++ b/tests/unit/test_req_file.py @@ -401,10 +401,13 @@ def test_set_finder_trusted_host( ) assert list(finder.trusted_hosts) == ['host1', 'host2:8080'] session = finder._link_collector.session - assert session.adapters['https://host1/'] is session._insecure_adapter + assert ( + session.adapters['https://host1/'] + is session._trusted_host_adapter + ) assert ( session.adapters['https://host2:8080/'] - is session._insecure_adapter + is session._trusted_host_adapter ) # Test the log message. From 65a9bec7a444e8b2690197b00e8b8fbb89beb896 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Mon, 23 Mar 2020 09:05:22 +0200 Subject: [PATCH 3/5] session: Add InsecureCacheControlAdapter --- src/pip/_internal/network/session.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/pip/_internal/network/session.py b/src/pip/_internal/network/session.py index 1e895a15c62..151f01c72fd 100644 --- a/src/pip/_internal/network/session.py +++ b/src/pip/_internal/network/session.py @@ -217,6 +217,14 @@ def cert_verify(self, conn, url, verify, cert): ) +class InsecureCacheControlAdapter(CacheControlAdapter): + + def cert_verify(self, conn, url, verify, cert): + super(InsecureCacheControlAdapter, self).cert_verify( + conn=conn, url=url, verify=False, cert=cert + ) + + class PipSession(requests.Session): timeout = None # type: Optional[int] From 544c307ebf1073d33840305b59f754b3d33e8825 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Thu, 26 Mar 2020 21:23:32 +0200 Subject: [PATCH 4/5] session: Always cache responses from trusted-host source news: Add news about default behaviour change --- news/7847.feature | 1 + src/pip/_internal/network/session.py | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 news/7847.feature diff --git a/news/7847.feature b/news/7847.feature new file mode 100644 index 00000000000..8f1a69b6fd2 --- /dev/null +++ b/news/7847.feature @@ -0,0 +1 @@ +Change default behaviour to always cache responses from trusted-host source. diff --git a/src/pip/_internal/network/session.py b/src/pip/_internal/network/session.py index 151f01c72fd..39a4a546edc 100644 --- a/src/pip/_internal/network/session.py +++ b/src/pip/_internal/network/session.py @@ -271,8 +271,16 @@ def __init__(self, *args, **kwargs): backoff_factor=0.25, ) - # We want to _only_ cache responses on securely fetched origins. We do - # this because we can't validate the response of an insecurely fetched + # Our Insecure HTTPAdapter disables HTTPS validation. It does not + # support caching so we'll use it for all http:// URLs. + # If caching is disabled, we will also use it for + # https:// hosts that we've marked as ignoring + # TLS errors for (trusted-hosts). + insecure_adapter = InsecureHTTPAdapter(max_retries=retries) + + # We want to _only_ cache responses on securely fetched origins or when + # the host is specified as trusted. We do this because + # we can't validate the response of an insecurely/untrusted fetched # origin, and we don't want someone to be able to poison the cache and # require manual eviction from the cache to fix it. if cache: @@ -280,16 +288,13 @@ def __init__(self, *args, **kwargs): cache=SafeFileCache(cache), max_retries=retries, ) + self._trusted_host_adapter = InsecureCacheControlAdapter( + cache=SafeFileCache(cache), + max_retries=retries, + ) else: secure_adapter = HTTPAdapter(max_retries=retries) - - # Our Insecure HTTPAdapter disables HTTPS validation. It does not - # support caching (see above) so we'll use it for all http:// URLs as - # well as any https:// host that we've marked as ignoring TLS errors - # for. - insecure_adapter = InsecureHTTPAdapter(max_retries=retries) - # Save this for later use in add_trusted_host(). - self._trusted_host_adapter = insecure_adapter + self._trusted_host_adapter = insecure_adapter self.mount("https://", secure_adapter) self.mount("http://", insecure_adapter) From 2050ecc7d7ef88770986aca06733e6aaf2fb1c00 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Mon, 23 Mar 2020 11:29:58 +0200 Subject: [PATCH 5/5] tests: session: Remake test_insecure_host into test_trusted_host --- tests/unit/test_network_session.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_network_session.py b/tests/unit/test_network_session.py index 125ac2d8de2..a0d1463b2cf 100644 --- a/tests/unit/test_network_session.py +++ b/tests/unit/test_network_session.py @@ -72,7 +72,7 @@ def test_http_cache_is_not_enabled(self, tmpdir): assert not hasattr(session.adapters["http://"], "cache") - def test_insecure_host_adapter(self, tmpdir): + def test_trusted_hosts_adapter(self, tmpdir): session = PipSession( cache=tmpdir.joinpath("test-cache"), trusted_hosts=["example.com"], @@ -81,8 +81,8 @@ def test_insecure_host_adapter(self, tmpdir): assert "https://example.com/" in session.adapters # Check that the "port wildcard" is present. assert "https://example.com:" in session.adapters - # Check that the cache isn't enabled. - assert not hasattr(session.adapters["https://example.com/"], "cache") + # Check that the cache is enabled. + assert hasattr(session.adapters["https://example.com/"], "cache") def test_add_trusted_host(self): # Leave a gap to test how the ordering is affected.