Merge commit from fork

docs/news.rst

@@ -10,10 +10,30 @@ Scrapy 2.17.0 (unreleased)
 
 Highlights:
 
+-   Security bug fixes
+
 -   HTTP/2 and SOCKS proxy support for ``HttpxDownloadHandler``
 
 -   Improved settings for changing allowed TLS versions
 
+Security bug fixes
+~~~~~~~~~~~~~~~~~~
+
+-   ``s3://`` requests now use HTTPS by default, instead of plaintext HTTP.
+
+    Previously, :class:`~scrapy.core.downloader.handlers.s3.S3DownloadHandler`
+    sent signed S3 requests over plaintext HTTP unless
+    ``request.meta["is_secure"]`` was set to a true value, exposing the request
+    path, the AWS ``Authorization`` header, the ``X-Amz-Security-Token`` header
+    (when using temporary credentials), and the response contents to network
+    attackers, who could also tamper with responses. See the `76g3-c3x4-crvx`_
+    security advisory for details.
+
+    To restore the previous behavior for a given request, set
+    ``request.meta["is_secure"]`` to ``False``.
+
+    .. _76g3-c3x4-crvx: https://github.com/scrapy/scrapy/security/advisories/GHSA-76g3-c3x4-crvx
+
 Deprecations
 ~~~~~~~~~~~~
 

scrapy/core/downloader/handlers/s3.py

@@ -49,7 +49,7 @@ class S3DownloadHandler(BaseDownloadHandler):
 
     async def download_request(self, request: Request) -> Response:
         p = urlparse_cached(request)
-        scheme = "https" if request.meta.get("is_secure") else "http"
+        scheme = "http" if request.meta.get("is_secure") is False else "https"
         bucket = p.hostname
         path = p.path + "?" + p.query if p.query else p.path
         url = f"{scheme}://{bucket}.s3.amazonaws.com{path}"

tests/test_downloader_handlers.py

@@ -170,6 +170,12 @@ class TestS3Anon:
         httpreq = await self.download_request(req)
         assert hasattr(self.s3reqh, "anon")
         assert self.s3reqh.anon
+        assert httpreq.url == "https://aws-publicdatasets.s3.amazonaws.com/"
+
+    @coroutine_test
+    async def test_anon_request_insecure(self):
+        req = Request("s3://aws-publicdatasets/", meta={"is_secure": False})
+        httpreq = await self.download_request(req)
         assert httpreq.url == "http://aws-publicdatasets.s3.amazonaws.com/"
 
 
@@ -200,6 +206,18 @@ class TestS3:
             mock_formatdate.return_value = date
             yield
 
+    @coroutine_test
+    async def test_secure_by_default(self):
+        req = Request("s3://johnsmith/photos/puppy.jpg")
+        httpreq = await self.download_request(req)
+        assert httpreq.url == "https://johnsmith.s3.amazonaws.com/photos/puppy.jpg"
+
+    @coroutine_test
+    async def test_insecure_opt_out(self):
+        req = Request("s3://johnsmith/photos/puppy.jpg", meta={"is_secure": False})
+        httpreq = await self.download_request(req)
+        assert httpreq.url == "http://johnsmith.s3.amazonaws.com/photos/puppy.jpg"
+
     @coroutine_test
     async def test_request_signing1(self):
         # gets an object from the johnsmith bucket.