Skip to content

Commit 5780387

Browse files
committed
gh-158907: Do not send proxy credentials to a direct origin on 407
ProxyBasicAuthHandler looked up passwords with the scheme-less origin host, so an HTTP 407 could match credentials stored for HTTPS and retry in cleartext.
1 parent 2639fd6 commit 5780387

3 files changed

Lines changed: 58 additions & 0 deletions

File tree

‎Lib/test/test_urllib2.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,54 @@ def test_proxy_basic_auth(self):
17351735
"proxy.example.com:3128",
17361736
)
17371737

1738+
def test_proxy_basic_auth_ignores_direct_407(self):
1739+
# gh-158907: a direct HTTP origin that returns 407 must not receive
1740+
# credentials registered for the same authority over HTTPS.
1741+
opener = OpenerDirector()
1742+
opener.add_handler(urllib.request.ProxyHandler({}))
1743+
password_manager = urllib.request.HTTPPasswordMgr()
1744+
auth_handler = urllib.request.ProxyBasicAuthHandler(password_manager)
1745+
realm = "test-realm"
1746+
http_handler = MockHTTPHandlerRedirect(
1747+
407, 'Proxy-Authenticate: Basic realm="%s"\r\n\r\n' % realm)
1748+
opener.add_handler(auth_handler)
1749+
opener.add_handler(http_handler)
1750+
1751+
password_manager.add_password(
1752+
realm, "https://example.com/", "victim-user", "victim-secret")
1753+
opener.open("http://example.com/resource")
1754+
1755+
self.assertEqual(len(http_handler.requests), 1)
1756+
self.assertFalse(
1757+
http_handler.requests[0].has_header("Proxy-authorization"))
1758+
1759+
def test_proxy_basic_auth_https_tunnel_still_authenticates(self):
1760+
# 407 from the proxy during an HTTPS tunnel must still be answered.
1761+
class MockHTTPSHandlerRedirect(MockHTTPHandlerRedirect):
1762+
def https_open(self, req):
1763+
return self.http_open(req)
1764+
1765+
opener = OpenerDirector()
1766+
opener.add_handler(urllib.request.ProxyHandler(
1767+
dict(https="proxy.example.com:3128")))
1768+
password_manager = urllib.request.HTTPPasswordMgr()
1769+
auth_handler = urllib.request.ProxyBasicAuthHandler(password_manager)
1770+
realm = "ACME Networks"
1771+
http_handler = MockHTTPSHandlerRedirect(
1772+
407, 'Proxy-Authenticate: Basic realm="%s"\r\n\r\n' % realm)
1773+
opener.add_handler(auth_handler)
1774+
opener.add_handler(http_handler)
1775+
1776+
password_manager.add_password(
1777+
realm, "proxy.example.com:3128", "wile", "coyote")
1778+
opener.open("https://acme.example.com/protected")
1779+
1780+
self.assertEqual(len(http_handler.requests), 2)
1781+
self.assertFalse(
1782+
http_handler.requests[0].has_header("Proxy-authorization"))
1783+
self.assertTrue(
1784+
http_handler.requests[1].has_header("Proxy-authorization"))
1785+
17381786
def test_basic_and_digest_auth_handlers(self):
17391787
# HTTPDigestAuthHandler raised an exception if it couldn't handle a 40*
17401788
# response (https://bugs.python.org/issue1479302), where it should instead

‎Lib/urllib/request.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,12 @@ class ProxyBasicAuthHandler(AbstractBasicAuthHandler, BaseHandler):
10481048
auth_header = 'Proxy-authorization'
10491049

10501050
def http_error_407(self, req, fp, code, msg, headers):
1051+
# A direct origin is not a proxy. req.host has no scheme, so a
1052+
# password stored for https://HOST/ would match and be sent in
1053+
# cleartext on the retry (gh-158907). HTTPS tunnels set
1054+
# _tunnel_host instead of making has_proxy() true.
1055+
if not req.has_proxy() and not req._tunnel_host:
1056+
return None
10511057
# http_error_auth_reqed requires that there is no userinfo component in
10521058
# authority. Assume there isn't one, since urllib.request does not (and
10531059
# should not, RFC 3986 s. 3.2.1) support requests for URLs containing
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
:class:`~urllib.request.ProxyBasicAuthHandler` no longer sends credentials
2+
in response to ``407`` from a direct origin. A scheme-less lookup against
3+
the origin host could match passwords stored for an HTTPS URL and retry
4+
the cleartext request with ``Proxy-Authorization``.

0 commit comments

Comments
 (0)