HTTPAdapter now updates its PoolManager connection_pool_kw

With the addition of https://github.com/shazow/urllib3/pull/830 requests
should update the connection_pool_kw on the PoolManager so that new
ConnectionPools get created when TLS/SSL settings change. This ensures
that users can update the CA certificates used to verify servers as well
as the client certificate and key it uses to authenticate with servers.

This fixes issue #2863

AUTHORS.rst

@@ -171,3 +171,4 @@ Patches and Suggestions
 - Nate Prewitt <nate.prewitt@gmail.com> (`@nateprewitt <https://github.com/nateprewitt>`_)
 - Maik Himstedt
 - Michael Hunsinger
+- Jeremy Cline <jcline@redhat.com> (`@jeremycline <https://github.com/jeremycline>`_)

requests/adapters.py

@@ -10,6 +10,10 @@ and maintain connections.
 
 import os.path
 import socket
+try:
+    from threading import RLock
+except ImportError:  # threading is an optional module and may not be present.
+    from dummy_threading import RLock
 
 from .models import Response
 from .packages.urllib3.poolmanager import PoolManager, proxy_from_url
@@ -119,6 +123,7 @@ class HTTPAdapter(BaseAdapter):
         self._pool_connections = pool_connections
         self._pool_maxsize = pool_maxsize
         self._pool_block = pool_block
+        self._pool_kw_lock = RLock()
 
         self.init_poolmanager(pool_connections, pool_maxsize, block=pool_block)
 
@@ -131,6 +136,7 @@ class HTTPAdapter(BaseAdapter):
         # self.poolmanager uses a lambda function, which isn't pickleable.
         self.proxy_manager = {}
         self.config = {}
+        self._pool_kw_lock = RLock()
 
         for attr, value in state.items():
             setattr(self, attr, value)
@@ -195,17 +201,21 @@ class HTTPAdapter(BaseAdapter):
 
         return manager
 
-    def cert_verify(self, conn, url, verify, cert):
-        """Verify a SSL certificate. This method should not be called from user
-        code, and is only exposed for use when subclassing the
-        :class:`HTTPAdapter <requests.adapters.HTTPAdapter>`.
+    def _update_poolmanager_ssl_kw(self, verify, cert):
+        """Update the :class:`PoolManager <urllib3.poolmanager.PoolManager>`
+        connection_pool_kw with the necessary SSL configuration. This method
+        should not be called from user code, and is only exposed for use when
+        subclassing the :class:`HTTPAdapter <requests.adapters.HTTPAdapter>`.
 
-        :param conn: The urllib3 connection object associated with the cert.
-        :param url: The requested URL.
-        :param verify: Whether we should actually verify the certificate.
-        :param cert: The SSL certificate to verify.
+        :param verify: Whether we should actually verify the certificate;
+                       optionally a path to a CA certificate bundle or
+                       directory of CA certificates.
+        :param cert: The path to the client certificate and key, if any.
+                     This can either be the path to the certificate and
+                     key concatenated in a single file, or as a tuple of
+                     (cert_file, key_file).
         """
-        if url.lower().startswith('https') and verify:
+        if verify:
 
             cert_loc = None
 
@@ -219,23 +229,25 @@ class HTTPAdapter(BaseAdapter):
             if not cert_loc:
                 raise Exception("Could not find a suitable SSL CA certificate bundle.")
 
-            conn.cert_reqs = 'CERT_REQUIRED'
+            self.poolmanager.connection_pool_kw['cert_reqs'] = 'CERT_REQUIRED'
 
             if not os.path.isdir(cert_loc):
-                conn.ca_certs = cert_loc
+                self.poolmanager.connection_pool_kw['ca_certs'] = cert_loc
+                self.poolmanager.connection_pool_kw['ca_cert_dir'] = None
             else:
-                conn.ca_cert_dir = cert_loc
+                self.poolmanager.connection_pool_kw['ca_cert_dir'] = cert_loc
+                self.poolmanager.connection_pool_kw['ca_certs'] = None
         else:
-            conn.cert_reqs = 'CERT_NONE'
-            conn.ca_certs = None
-            conn.ca_cert_dir = None
+            self.poolmanager.connection_pool_kw['cert_reqs'] = 'CERT_NONE'
+            self.poolmanager.connection_pool_kw['ca_certs'] = None
+            self.poolmanager.connection_pool_kw['ca_cert_dir'] = None
 
         if cert:
             if not isinstance(cert, basestring):
-                conn.cert_file = cert[0]
-                conn.key_file = cert[1]
+                self.poolmanager.connection_pool_kw['cert_file'] = cert[0]
+                self.poolmanager.connection_pool_kw['key_file'] = cert[1]
             else:
-                conn.cert_file = cert
+                self.poolmanager.connection_pool_kw['cert_file'] = cert
 
     def build_response(self, req, resp):
         """Builds a :class:`Response <requests.Response>` object from a urllib3
@@ -274,7 +286,7 @@ class HTTPAdapter(BaseAdapter):
 
         return response
 
-    def get_connection(self, url, proxies=None):
+    def get_connection(self, url, proxies=None, verify=None, cert=None):
         """Returns a urllib3 connection for the given URL. This should not be
         called from user code, and is only exposed for use when subclassing the
         :class:`HTTPAdapter <requests.adapters.HTTPAdapter>`.
@@ -283,17 +295,21 @@ class HTTPAdapter(BaseAdapter):
         :param proxies: (optional) A Requests-style dictionary of proxies used on this request.
         :rtype: requests.packages.urllib3.ConnectionPool
         """
-        proxy = select_proxy(url, proxies)
+        with self._pool_kw_lock:
+            if url.lower().startswith('https'):
+                self._update_poolmanager_ssl_kw(verify, cert)
 
-        if proxy:
-            proxy = prepend_scheme_if_needed(proxy, 'http')
-            proxy_manager = self.proxy_manager_for(proxy)
-            conn = proxy_manager.connection_from_url(url)
-        else:
-            # Only scheme should be lower case
-            parsed = urlparse(url)
-            url = parsed.geturl()
-            conn = self.poolmanager.connection_from_url(url)
+            proxy = select_proxy(url, proxies)
+
+            if proxy:
+                proxy = prepend_scheme_if_needed(proxy, 'http')
+                proxy_manager = self.proxy_manager_for(proxy)
+                conn = proxy_manager.connection_from_url(url)
+            else:
+                # Only scheme should be lower case
+                parsed = urlparse(url)
+                url = parsed.geturl()
+                conn = self.poolmanager.connection_from_url(url)
 
         return conn
 
@@ -386,10 +402,8 @@ class HTTPAdapter(BaseAdapter):
         :param proxies: (optional) The proxies dictionary to apply to the request.
         :rtype: requests.Response
         """
+        conn = self.get_connection(request.url, proxies, verify, cert)
 
-        conn = self.get_connection(request.url, proxies)
-
-        self.cert_verify(conn, request.url, verify, cert)
         url = self.request_url(request, proxies)
         self.add_headers(request)
 

tests/test_requests.py

@@ -14,6 +14,7 @@ import warnings
 import io
 import requests
 import pytest
+import pytest_httpbin
 from requests.adapters import HTTPAdapter
 from requests.auth import HTTPDigestAuth, _basic_auth_str
 from requests.compat import (
@@ -1487,6 +1488,34 @@ class TestRequests:
         resp.close()
         assert resp.raw.closed
 
+    def test_updating_ca_cert(self, httpbin_secure):
+        """Assert that requests use the latest configured CA certificates."""
+        session = requests.session()
+        session.verify = pytest_httpbin.certs.where()
+        session.get(httpbin_secure('/'))
+        session.verify = True
+        with pytest.raises(requests.exceptions.SSLError) as e:
+            session.get(httpbin_secure('/'))
+        assert 'certificate verify failed' in str(e)
+
+    def test_updating_client_cert(self, httpbin_secure):
+        """Assert that requests use the latest configured client certificates."""
+        ca_file = pytest_httpbin.certs.where()
+        cert_dir = os.path.dirname(ca_file)
+        # All we need is a valid certificate and key to make a request. httpbin_secure
+        # won't check the signature or subject name, so it's okay that these happen to
+        # be the server's certificate and key.
+        cert = os.path.join(cert_dir, 'cert.pem')
+        key = os.path.join(cert_dir, 'key.pem')
+        session = requests.session()
+        session.verify = ca_file
+        resp = session.get(httpbin_secure('/'))
+        resp_with_cert = session.get(httpbin_secure('/'), cert=(cert, key))
+        assert resp_with_cert.raw._pool.cert_file == cert
+        assert resp_with_cert.raw._pool.key_file == key
+        assert resp.raw._pool is not resp_with_cert.raw._pool
+
+
 class TestCaseInsensitiveDict:
 
     @pytest.mark.parametrize(