RomanNum3ral
/
scrapy
mirror of https://github.com/scrapy/scrapy.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request from GHSA-9x8m-2xpf-crp3 

* Enforce matching proxy request meta and Proxy-Authorization header

* Cover proxy credential security fix in the release notes

* Remove extra empty line

* Reword the security issue description

* Address scenario where Proxy-Authorization is unexpectedly removed by a prior middleware

* Set the release date of Scrapy 2.6.2 and 1.8.3
This commit is contained in:
Adrián Chaves 2022-07-25 13:15:17 +02:00 committed by GitHub
parent 4205609051
commit af7dd16d8d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 439 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
docs/news.rst
View File

@ -5,10 +5,57 @@ Release notes
.. _release-2.6.2:
Scrapy 2.6.2 (to be determined)
-------------------------------
Scrapy 2.6.2 (2022-07-25)
-------------------------
Fixes additional regressions introduced in 2.6.0:
**Security bug fix:**
- When :class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware`
processes a request with :reqmeta:`proxy` metadata, and that
:reqmeta:`proxy` metadata includes proxy credentials,
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` sets
the ``Proxy-Authentication`` header, but only if that header is not already
set.
There are third-party proxy-rotation downloader middlewares that set
different :reqmeta:`proxy` metadata every time they process a request.
Because of request retries and redirects, the same request can be processed
by downloader middlewares more than once, including both
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` and
any third-party proxy-rotation downloader middleware.
These third-party proxy-rotation downloader middlewares could change the
:reqmeta:`proxy` metadata of a request to a new value, but fail to remove
the ``Proxy-Authentication`` header from the previous value of the
:reqmeta:`proxy` metadata, causing the credentials of one proxy to be sent
to a different proxy.
To prevent the unintended leaking of proxy credentials, the behavior of
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` is now
as follows when processing a request:
- If the request being processed defines :reqmeta:`proxy` metadata that
includes credentials, the ``Proxy-Authorization`` header is always
updated to feature those credentials.
- If the request being processed defines :reqmeta:`proxy` metadata
without credentials, the ``Proxy-Authorization`` header is removed
*unless* it was originally defined for the same proxy URL.
To remove proxy credentials while keeping the same proxy URL, remove
the ``Proxy-Authorization`` header.
- If the request has no :reqmeta:`proxy` metadata, or that metadata is a
falsy value (e.g. ``None``), the ``Proxy-Authorization`` header is
removed.
It is no longer possible to set a proxy URL through the
:reqmeta:`proxy` metadata but set the credentials through the
``Proxy-Authorization`` header. Set proxy credentials through the
:reqmeta:`proxy` metadata instead.
Also fixes the following regressions introduced in 2.6.0:
- :class:`~scrapy.crawler.CrawlerProcess` supports again crawling multiple
spiders (:issue:`5435`, :issue:`5436`)
@ -1925,6 +1972,59 @@ affect subclasses:
(:issue:`3884`)
.. _release-1.8.3:
Scrapy 1.8.3 (2022-07-25)
-------------------------
**Security bug fix:**
- When :class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware`
processes a request with :reqmeta:`proxy` metadata, and that
:reqmeta:`proxy` metadata includes proxy credentials,
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` sets
the ``Proxy-Authentication`` header, but only if that header is not already
set.
There are third-party proxy-rotation downloader middlewares that set
different :reqmeta:`proxy` metadata every time they process a request.
Because of request retries and redirects, the same request can be processed
by downloader middlewares more than once, including both
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` and
any third-party proxy-rotation downloader middleware.
These third-party proxy-rotation downloader middlewares could change the
:reqmeta:`proxy` metadata of a request to a new value, but fail to remove
the ``Proxy-Authentication`` header from the previous value of the
:reqmeta:`proxy` metadata, causing the credentials of one proxy to be sent
to a different proxy.
To prevent the unintended leaking of proxy credentials, the behavior of
:class:`~scrapy.downloadermiddlewares.httpproxy.HttpProxyMiddleware` is now
as follows when processing a request:
- If the request being processed defines :reqmeta:`proxy` metadata that
includes credentials, the ``Proxy-Authorization`` header is always
updated to feature those credentials.
- If the request being processed defines :reqmeta:`proxy` metadata
without credentials, the ``Proxy-Authorization`` header is removed
*unless* it was originally defined for the same proxy URL.
To remove proxy credentials while keeping the same proxy URL, remove
the ``Proxy-Authorization`` header.
- If the request has no :reqmeta:`proxy` metadata, or that metadata is a
falsy value (e.g. ``None``), the ``Proxy-Authorization`` header is
removed.
It is no longer possible to set a proxy URL through the
:reqmeta:`proxy` metadata but set the credentials through the
``Proxy-Authorization`` header. Set proxy credentials through the
:reqmeta:`proxy` metadata instead.
.. _release-1.8.2:
Scrapy 1.8.2 (2022-03-01)

56
scrapy/downloadermiddlewares/httpproxy.py
View File

@ -45,31 +45,37 @@ class HttpProxyMiddleware:
return creds, proxy_url
def process_request(self, request, spider):
# ignore if proxy is already set
creds, proxy_url = None, None
if 'proxy' in request.meta:
if request.meta['proxy'] is None:
return
# extract credentials if present
creds, proxy_url = self._get_proxy(request.meta['proxy'], '')
if request.meta['proxy'] is not None:
creds, proxy_url = self._get_proxy(request.meta['proxy'], '')
elif self.proxies:
parsed = urlparse_cached(request)
scheme = parsed.scheme
if (
(
# 'no_proxy' is only supported by http schemes
scheme not in ('http', 'https')
or not proxy_bypass(parsed.hostname)
)
and scheme in self.proxies
):
creds, proxy_url = self.proxies[scheme]
self._set_proxy_and_creds(request, proxy_url, creds)
def _set_proxy_and_creds(self, request, proxy_url, creds):
if proxy_url:
request.meta['proxy'] = proxy_url
if creds and not request.headers.get('Proxy-Authorization'):
request.headers['Proxy-Authorization'] = b'Basic ' + creds
return
elif not self.proxies:
return
parsed = urlparse_cached(request)
scheme = parsed.scheme
# 'no_proxy' is only supported by http schemes
if scheme in ('http', 'https') and proxy_bypass(parsed.hostname):
return
if scheme in self.proxies:
self._set_proxy(request, scheme)
def _set_proxy(self, request, scheme):
creds, proxy = self.proxies[scheme]
request.meta['proxy'] = proxy
elif request.meta.get('proxy') is not None:
request.meta['proxy'] = None
if creds:
request.headers['Proxy-Authorization'] = b'Basic ' + creds
request.headers[b'Proxy-Authorization'] = b'Basic ' + creds
request.meta['_auth_proxy'] = proxy_url
elif '_auth_proxy' in request.meta:
if proxy_url != request.meta['_auth_proxy']:
if b'Proxy-Authorization' in request.headers:
del request.headers[b'Proxy-Authorization']
del request.meta['_auth_proxy']
elif b'Proxy-Authorization' in request.headers:
del request.headers[b'Proxy-Authorization']

313
tests/test_downloadermiddleware_httpproxy.py
View File

@ -65,12 +65,12 @@ class TestHttpProxyMiddleware(TestCase):
mw = HttpProxyMiddleware()
req = Request('http://scrapytest.org')
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic dXNlcjpwYXNz')
# proxy from request.meta
req = Request('http://scrapytest.org', meta={'proxy': 'https://username:password@proxy:3128'})
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic dXNlcm5hbWU6cGFzc3dvcmQ=')
def test_proxy_auth_empty_passwd(self):
@ -78,12 +78,12 @@ class TestHttpProxyMiddleware(TestCase):
mw = HttpProxyMiddleware()
req = Request('http://scrapytest.org')
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic dXNlcjo=')
# proxy from request.meta
req = Request('http://scrapytest.org', meta={'proxy': 'https://username:@proxy:3128'})
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic dXNlcm5hbWU6')
def test_proxy_auth_encoding(self):
@ -92,26 +92,26 @@ class TestHttpProxyMiddleware(TestCase):
mw = HttpProxyMiddleware(auth_encoding='utf-8')
req = Request('http://scrapytest.org')
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic bcOhbjpwYXNz')
# proxy from request.meta
req = Request('http://scrapytest.org', meta={'proxy': 'https://\u00FCser:pass@proxy:3128'})
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic w7xzZXI6cGFzcw==')
# default latin-1 encoding
mw = HttpProxyMiddleware(auth_encoding='latin-1')
req = Request('http://scrapytest.org')
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic beFuOnBhc3M=')
# proxy from request.meta, latin-1 encoding
req = Request('http://scrapytest.org', meta={'proxy': 'https://\u00FCser:pass@proxy:3128'})
assert mw.process_request(req, spider) is None
self.assertEqual(req.meta, {'proxy': 'https://proxy:3128'})
self.assertEqual(req.meta['proxy'], 'https://proxy:3128')
self.assertEqual(req.headers.get('Proxy-Authorization'), b'Basic /HNlcjpwYXNz')
def test_proxy_already_seted(self):
@ -152,3 +152,300 @@ class TestHttpProxyMiddleware(TestCase):
# '/var/run/docker.sock' may be used by the user for
# no_proxy value but is not parseable and should be skipped
assert 'no' not in mw.proxies
def test_add_proxy_without_credentials(self):
middleware = HttpProxyMiddleware()
request = Request('https://example.com')
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_add_proxy_with_credentials(self):
middleware = HttpProxyMiddleware()
request = Request('https://example.com')
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user1:password1@example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_remove_proxy_without_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = None
assert middleware.process_request(request, spider) is None
self.assertIsNone(request.meta['proxy'])
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_remove_proxy_with_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = None
assert middleware.process_request(request, spider) is None
self.assertIsNone(request.meta['proxy'])
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_add_credentials(self):
"""If the proxy request meta switches to a proxy URL with the same
proxy and adds credentials (there were no credentials before), the new
credentials must be used."""
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user1:password1@example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_change_credentials(self):
"""If the proxy request meta switches to a proxy URL with different
credentials, those new credentials must be used."""
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user2:password2@example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
encoded_credentials = middleware._basic_auth_header(
'user2',
'password2',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_remove_credentials(self):
"""If the proxy request meta switches to a proxy URL with the same
proxy but no credentials, the original credentials must be still
used.
To remove credentials while keeping the same proxy URL, users must
delete the Proxy-Authorization header.
"""
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
request.meta['proxy'] = 'https://example.com'
del request.headers[b'Proxy-Authorization']
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_change_proxy_add_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user1:password1@example.org'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.org')
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_change_proxy_keep_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user1:password1@example.org'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.org')
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
# Make sure, indirectly, that _auth_proxy is updated.
request.meta['proxy'] = 'https://example.com'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_change_proxy_change_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://user2:password2@example.org'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.org')
encoded_credentials = middleware._basic_auth_header(
'user2',
'password2',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_change_proxy_remove_credentials(self):
"""If the proxy request meta switches to a proxy URL with a different
proxy and no credentials, no credentials must be used."""
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://example.org'
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta, {'proxy': 'https://example.org'})
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_change_proxy_remove_credentials_preremoved_header(self):
"""Corner case of proxy switch with credentials removal where the
credentials have been removed beforehand.
It ensures that our implementation does not assume that the credentials
header exists when trying to remove it.
"""
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
request.meta['proxy'] = 'https://example.org'
del request.headers[b'Proxy-Authorization']
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta, {'proxy': 'https://example.org'})
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_proxy_authentication_header_undefined_proxy(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
headers={'Proxy-Authorization': 'Basic foo'},
)
assert middleware.process_request(request, spider) is None
self.assertNotIn('proxy', request.meta)
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_proxy_authentication_header_disabled_proxy(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
headers={'Proxy-Authorization': 'Basic foo'},
meta={'proxy': None},
)
assert middleware.process_request(request, spider) is None
self.assertIsNone(request.meta['proxy'])
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_proxy_authentication_header_proxy_without_credentials(self):
middleware = HttpProxyMiddleware()
request = Request(
'https://example.com',
headers={'Proxy-Authorization': 'Basic foo'},
meta={'proxy': 'https://example.com'},
)
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
self.assertNotIn(b'Proxy-Authorization', request.headers)
def test_proxy_authentication_header_proxy_with_same_credentials(self):
middleware = HttpProxyMiddleware()
encoded_credentials = middleware._basic_auth_header(
'user1',
'password1',
)
request = Request(
'https://example.com',
headers={'Proxy-Authorization': b'Basic ' + encoded_credentials},
meta={'proxy': 'https://user1:password1@example.com'},
)
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials,
)
def test_proxy_authentication_header_proxy_with_different_credentials(self):
middleware = HttpProxyMiddleware()
encoded_credentials1 = middleware._basic_auth_header(
'user1',
'password1',
)
request = Request(
'https://example.com',
headers={'Proxy-Authorization': b'Basic ' + encoded_credentials1},
meta={'proxy': 'https://user2:password2@example.com'},
)
assert middleware.process_request(request, spider) is None
self.assertEqual(request.meta['proxy'], 'https://example.com')
encoded_credentials2 = middleware._basic_auth_header(
'user2',
'password2',
)
self.assertEqual(
request.headers['Proxy-Authorization'],
b'Basic ' + encoded_credentials2,
)