CVE-2018-20060

@@ -4,6 +4,10 @@ Changes
 dev (master)
 ------------
 
+* Allow providing a list of headers to strip from requests when redirecting
+  to a different host. Defaults to the ``Authorization`` header. Different
+  headers can be set via ``Retry.remove_headers_on_redirect``. (Issue #1316)
+
 * Fix ``util.selectors._fileobj_to_fd`` to accept ``long`` (Issue #1247).
 
 * Dropped Python 3.3 support. (Pull #1242)

@@ -249,3 +249,13 @@ def test_retry_method_not_in_whitelist(self):
         retry = Retry()
         with pytest.raises(ReadTimeoutError):
             retry.increment(method='POST', error=error)
+
+    def test_retry_default_remove_headers_on_redirect(self):
+        retry = Retry()
+
+        assert list(retry.remove_headers_on_redirect) == ['Authorization']
+
+    def test_retry_set_remove_headers_on_redirect(self):
+        retry = Retry(remove_headers_on_redirect=['X-API-Secret'])
+
+        assert list(retry.remove_headers_on_redirect) == ['X-API-Secret']

@@ -109,6 +109,52 @@ def test_too_many_redirects(self):
         except MaxRetryError:
             pass
 
+    def test_redirect_cross_host_remove_headers(self):
+        http = PoolManager()
+        self.addCleanup(http.clear)
+
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'Authorization': 'foo'})
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('Authorization', data)
+
+    def test_redirect_cross_host_no_remove_headers(self):
+        http = PoolManager()
+        self.addCleanup(http.clear)
+
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'Authorization': 'foo'},
+                         retries=Retry(remove_headers_on_redirect=[]))
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertEqual(data['Authorization'], 'foo')
+
+    def test_redirect_cross_host_set_removed_headers(self):
+        http = PoolManager()
+        self.addCleanup(http.clear)
+
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'X-API-Secret': 'foo',
+                                  'Authorization': 'bar'},
+                         retries=Retry(remove_headers_on_redirect=['X-API-Secret']))
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('X-API-Secret', data)
+        self.assertEqual(data['Authorization'], 'bar')
+
     def test_raise_on_redirect(self):
         http = PoolManager()
         self.addCleanup(http.clear)

@@ -312,8 +312,9 @@ def urlopen(self, method, url, redirect=True, **kw):
 
         kw['assert_same_host'] = False
         kw['redirect'] = False
+
         if 'headers' not in kw:
-            kw['headers'] = self.headers
+            kw['headers'] = self.headers.copy()
 
         if self.proxy is not None and u.scheme == "http":
             response = conn.urlopen(method, url, **kw)
@@ -335,6 +336,14 @@ def urlopen(self, method, url, redirect=True, **kw):
         if not isinstance(retries, Retry):
             retries = Retry.from_int(retries, redirect=redirect)
 
+        # Strip headers marked as unsafe to forward to the redirected location.
+        # Check remove_headers_on_redirect to avoid a potential network call within
+        # conn.is_same_host() which may use socket.gethostbyname() in the future.
+        if (retries.remove_headers_on_redirect
+                and not conn.is_same_host(redirect_location)):
+            for header in retries.remove_headers_on_redirect:
+                kw['headers'].pop(header, None)
+
         try:
             retries = retries.increment(method, url, response=response, _pool=conn)
         except MaxRetryError:

@@ -19,6 +19,7 @@
 
 log = logging.getLogger(__name__)
 
+
 # Data structure for representing the metadata of requests that result in a retry.
 RequestHistory = namedtuple('RequestHistory', ["method", "url", "error",
                                                "status", "redirect_location"])
@@ -139,20 +140,27 @@ class Retry(object):
         Whether to respect Retry-After header on status codes defined as
         :attr:`Retry.RETRY_AFTER_STATUS_CODES` or not.
 
+    :param iterable remove_headers_on_redirect:
+        Sequence of headers to remove from the request when a response
+        indicating a redirect is returned before firing off the redirected
+        request.
     """
 
     DEFAULT_METHOD_WHITELIST = frozenset([
         'HEAD', 'GET', 'PUT', 'DELETE', 'OPTIONS', 'TRACE'])
 
     RETRY_AFTER_STATUS_CODES = frozenset([413, 429, 503])
 
+    DEFAULT_REDIRECT_HEADERS_BLACKLIST = frozenset(['Authorization'])
+
     #: Maximum backoff time.
     BACKOFF_MAX = 120
 
     def __init__(self, total=10, connect=None, read=None, redirect=None, status=None,
                  method_whitelist=DEFAULT_METHOD_WHITELIST, status_forcelist=None,
                  backoff_factor=0, raise_on_redirect=True, raise_on_status=True,
-                 history=None, respect_retry_after_header=True):
+                 history=None, respect_retry_after_header=True,
+                 remove_headers_on_redirect=DEFAULT_REDIRECT_HEADERS_BLACKLIST):
 
         self.total = total
         self.connect = connect
@@ -171,6 +179,7 @@ def __init__(self, total=10, connect=None, read=None, redirect=None, status=None
         self.raise_on_status = raise_on_status
         self.history = history or tuple()
         self.respect_retry_after_header = respect_retry_after_header
+        self.remove_headers_on_redirect = remove_headers_on_redirect
 
     def new(self, **kw):
         params = dict(
@@ -182,6 +191,7 @@ def new(self, **kw):
             raise_on_redirect=self.raise_on_redirect,
             raise_on_status=self.raise_on_status,
             history=self.history,
+            remove_headers_on_redirect=self.remove_headers_on_redirect
         )
         params.update(kw)
         return type(self)(**params)