[Backport #14522 to 5.1.x] Improvements to URLs parsing

geonode/tests/test_utils.py

@@ -17,6 +17,8 @@
 #
 #########################################################################
 import copy
+import requests
+from requests.models import PreparedRequest, Response
 from unittest import TestCase
 
 from unittest.mock import patch
@@ -38,6 +40,7 @@
     is_safe_url,
     is_safe_url_with_redirects,
     assert_safe_xml,
+    safe_request_url,
     UnsafeXMLError,
 )
 from unittest.mock import MagicMock
@@ -441,3 +444,43 @@ def test_trusted_host_case_insensitive(self):
         with patch("geonode.utils.socket.getaddrinfo") as mock_dns:
             mock_dns.return_value = [(None, None, None, None, ("192.168.1.100", 0))]
             self.assertTrue(is_safe_url("https://internal.private.host:9090/geoserver/ows"))
+
+
+def _fake_response(status_code, url, location=None):
+    """Build a minimal requests.Response usable by resolve_redirects without a socket."""
+    response = Response()
+    request = PreparedRequest()
+    request.prepare(method="GET", url=url)
+    response.status_code = status_code
+    response.url = url
+    response.request = request
+    response.raw = None
+    response._content = b""
+    response._content_consumed = True
+    response.history = []
+    if location is not None:
+        response.headers["Location"] = location
+    return response
+
+
+@override_settings(SAFE_URL_CHECK_ENABLED=True)
+class TestSafeRequestUrl(djangoTestCase):
+    """safe_request_url must validate the initial URL and every redirect hop."""
+
+    @patch("geonode.utils.is_safe_url")
+    def test_rejects_disallowed_initial_url(self, mock_is_safe_url):
+        """An initial URL that is not allowed is rejected before any request is sent."""
+        mock_is_safe_url.return_value = False
+        with self.assertRaises(requests.exceptions.InvalidURL):
+            safe_request_url("GET", "http://first.invalid/resource")
+
+    @patch("requests.adapters.HTTPAdapter.send")
+    @patch("geonode.utils.is_safe_url")
+    def test_rejects_disallowed_redirect_target(self, mock_is_safe_url, mock_adapter_send):
+        """A redirect whose target is not allowed is rejected on the redirect hop."""
+        mock_is_safe_url.side_effect = lambda candidate: "allowed.invalid" in candidate
+        mock_adapter_send.return_value = _fake_response(
+            302, "http://allowed.invalid/resource", location="http://other.invalid/resource"
+        )
+        with self.assertRaises(requests.exceptions.InvalidURL):
+            safe_request_url("GET", "http://allowed.invalid/resource")

geonode/upload/handlers/common/remote.py

@@ -35,6 +35,7 @@
 from geonode.base.enumerations import SOURCE_TYPE_REMOTE
 from geonode.resource.registry import resource_manager_registry
 from geonode.resource.models import ExecutionRequest
+from geonode.utils import safe_request_url
 
 logger = logging.getLogger("importer")
 
@@ -91,7 +92,7 @@ def is_valid_url(url, **kwargs):
         and if the url is valid
         """
         try:
-            r = requests.get(url, timeout=10)
+            r = safe_request_url("GET", url, timeout=10)
             r.raise_for_status()
         except requests.exceptions.Timeout:
             raise ImportException("Timed out")

geonode/upload/handlers/remote/cog.py

@@ -17,9 +17,9 @@
 #
 #########################################################################
 import logging
-import requests
 from osgeo import gdal
 
+from geonode.utils import is_safe_url_with_redirects, safe_request_url
 from geonode.security.utils import init_gdal_security
 from geonode.layers.models import Dataset
 from geonode.upload.handlers.common.remote import BaseRemoteResourceHandler
@@ -58,9 +58,13 @@ def is_valid_url(url, **kwargs):
         Check if the URL is reachable and supports HTTP Range requests
         """
         logger.debug(f"Checking COG URL validity (HEAD): {url}")
+        # Reject disallowed URLs early, before running the remaining validity checks.
+        is_safe, _ = is_safe_url_with_redirects(url)
+        if not is_safe:
+            raise ImportException("The requested URL is not allowed.")
         try:
             # Reachability check using HEAD
-            head_res = requests.head(url, timeout=10, allow_redirects=True)
+            head_res = safe_request_url("HEAD", url, timeout=10)
             logger.debug(f"HTTP HEAD status: {head_res.status_code}")
             head_res.raise_for_status()
 
@@ -73,7 +77,7 @@ def is_valid_url(url, **kwargs):
 
             # Some servers might not return Accept-Ranges in HEAD, so we try a small range request
             logger.debug("Accept-Ranges header missing, trying a small Range GET...")
-            range_res = requests.get(url, headers={"Range": "bytes=0-1"}, timeout=10, stream=True)
+            range_res = safe_request_url("GET", url, headers={"Range": "bytes=0-1"}, timeout=10, stream=True)
             logger.debug(f"Range GET status: {range_res.status_code}")
             try:
                 if range_res.status_code != 206:

geonode/upload/handlers/remote/flatgeobuf.py

@@ -17,9 +17,9 @@
 #
 #########################################################################
 import logging
-import requests
 from osgeo import gdal
 
+from geonode.utils import is_safe_url_with_redirects, safe_request_url
 from geonode.security.utils import init_gdal_security
 from geonode.layers.models import Dataset
 from geonode.upload.handlers.common.remote import BaseRemoteResourceHandler
@@ -59,9 +59,13 @@ def is_valid_url(url, **kwargs):
         Check if the URL is reachable and supports HTTP Range requests
         """
         logger.debug(f"Checking FlatGeobuf URL validity (HEAD): {url}")
+        # Reject disallowed URLs early, before running the remaining validity checks.
+        is_safe, _ = is_safe_url_with_redirects(url)
+        if not is_safe:
+            raise ImportException("The requested URL is not allowed.")
         try:
             # Reachability check using HEAD
-            head_res = requests.head(url, timeout=10, allow_redirects=True)
+            head_res = safe_request_url("HEAD", url, timeout=10)
             logger.debug(f"HTTP HEAD status: {head_res.status_code}")
             head_res.raise_for_status()
 
@@ -74,7 +78,7 @@ def is_valid_url(url, **kwargs):
 
             # Some servers might not return Accept-Ranges in HEAD, so we try a small range request
             logger.debug("Accept-Ranges header missing, trying a small Range GET...")
-            range_res = requests.get(url, headers={"Range": "bytes=0-1"}, timeout=10, stream=True)
+            range_res = safe_request_url("GET", url, headers={"Range": "bytes=0-1"}, timeout=10, stream=True)
             logger.debug(f"Range GET status: {range_res.status_code}")
             try:
                 if range_res.status_code != 206:

geonode/upload/handlers/remote/tests/test_3dtiles.py

@@ -18,7 +18,6 @@
 #########################################################################
 from django.test import TestCase
 from mock import MagicMock, patch
-from geonode.upload.api.exceptions import ImportException
 from django.contrib.auth import get_user_model
 from geonode.upload.handlers.common.serializer import RemoteResourceSerializer
 from geonode.upload.handlers.remote.tiles3d import RemoteTiles3DResourceHandler
@@ -95,7 +94,7 @@ def test_task_list_is_the_expected_one_geojson(self):
         self.assertTupleEqual(expected, self.handler.TASKS["copy"])
 
     def test_is_valid_should_raise_exception_if_the_url_is_invalid(self):
-        with self.assertRaises(ImportException) as _exc:
+        with self.assertRaises(Invalid3DTilesException) as _exc:
             self.handler.is_valid_url(url=self.invalid_files["url"])
 
         self.assertIsNotNone(_exc)

geonode/upload/handlers/remote/tests/test_cog.py

@@ -47,23 +47,35 @@ def test_can_handle_cog(self):
         self.assertTrue(self.handler.can_handle({"url": "http://example.com/y.tiff", "type": "cog"}))
         self.assertFalse(self.handler.can_handle({"url": "http://example.com/y.jpg", "type": "image"}))
 
-    @patch("geonode.upload.handlers.remote.cog.requests.head")
-    @patch("geonode.upload.handlers.remote.cog.requests.get")
-    @patch("geonode.upload.handlers.common.remote.requests.get")
-    def test_is_valid_url_success(self, mock_base_get, mock_get, mock_head):
-        mock_head.return_value.headers = {"Accept-Ranges": "bytes"}
-        mock_head.return_value.status_code = 200
-        mock_base_get.return_value.status_code = 200
+    @patch("geonode.upload.handlers.remote.cog.is_safe_url_with_redirects")
+    @patch("geonode.upload.handlers.remote.cog.safe_request_url")
+    def test_is_valid_url_success(self, mock_safe_request_url, mock_is_safe_url):
+        mock_is_safe_url.return_value = (True, None)
+
+        mock_head_response = MagicMock()
+        mock_head_response.headers = {"Accept-Ranges": "bytes"}
+        mock_head_response.status_code = 200
+        mock_head_response.raise_for_status.return_value = None
+
+        mock_safe_request_url.return_value = mock_head_response
 
         self.assertTrue(self.handler.is_valid_url(self.valid_url))
 
-    @patch("geonode.upload.handlers.remote.cog.requests.head")
-    @patch("geonode.upload.handlers.remote.cog.requests.get")
-    @patch("geonode.upload.handlers.common.remote.requests.get")
-    def test_is_valid_url_no_range_support(self, mock_base_get, mock_get, mock_head):
-        mock_head.return_value.headers = {}
-        mock_get.return_value.status_code = 404  # Not 206
-        mock_base_get.return_value.status_code = 200
+    @patch("geonode.upload.handlers.remote.cog.is_safe_url_with_redirects")
+    @patch("geonode.upload.handlers.remote.cog.safe_request_url")
+    def test_is_valid_url_no_range_support(self, mock_safe_request_url, mock_is_safe_url):
+        mock_is_safe_url.return_value = (True, None)
+
+        mock_head_response = MagicMock()
+        mock_head_response.headers = {}
+        mock_head_response.status_code = 200
+        mock_head_response.raise_for_status.return_value = None
+
+        mock_get_response = MagicMock()
+        mock_get_response.status_code = 404
+        mock_get_response.close.return_value = None
+
+        mock_safe_request_url.side_effect = [mock_head_response, mock_get_response]
 
         with self.assertRaises(ImportException):
             self.handler.is_valid_url(self.valid_url)

geonode/upload/handlers/remote/tests/test_flatgeobuf.py

@@ -48,23 +48,35 @@ def test_can_handle_flatgeobuf(self):
         self.assertTrue(self.handler.can_handle({"url": "http://example.com/y.fgb", "type": "FlatGeobuf"}))
         self.assertFalse(self.handler.can_handle({"url": "http://example.com/y.jpg", "type": "image"}))
 
-    @patch("geonode.upload.handlers.remote.flatgeobuf.requests.head")
-    @patch("geonode.upload.handlers.remote.flatgeobuf.requests.get")
-    @patch("geonode.upload.handlers.common.remote.requests.get")
-    def test_is_valid_url_success(self, mock_base_get, mock_get, mock_head):
-        mock_head.return_value.headers = {"Accept-Ranges": "bytes"}
-        mock_head.return_value.status_code = 200
-        mock_base_get.return_value.status_code = 200
+    @patch("geonode.upload.handlers.remote.flatgeobuf.is_safe_url_with_redirects")
+    @patch("geonode.upload.handlers.remote.flatgeobuf.safe_request_url")
+    def test_is_valid_url_success(self, mock_safe_request_url, mock_is_safe_url):
+        mock_is_safe_url.return_value = (True, None)
+
+        mock_head_response = MagicMock()
+        mock_head_response.headers = {"Accept-Ranges": "bytes"}
+        mock_head_response.status_code = 200
+        mock_head_response.raise_for_status.return_value = None
+
+        mock_safe_request_url.return_value = mock_head_response
 
         self.assertTrue(self.handler.is_valid_url(self.valid_url))
 
-    @patch("geonode.upload.handlers.remote.flatgeobuf.requests.head")
-    @patch("geonode.upload.handlers.remote.flatgeobuf.requests.get")
-    @patch("geonode.upload.handlers.common.remote.requests.get")
-    def test_is_valid_url_no_range_support(self, mock_base_get, mock_get, mock_head):
-        mock_head.return_value.headers = {}
-        mock_get.return_value.status_code = 404
-        mock_base_get.return_value.status_code = 200
+    @patch("geonode.upload.handlers.remote.flatgeobuf.is_safe_url_with_redirects")
+    @patch("geonode.upload.handlers.remote.flatgeobuf.safe_request_url")
+    def test_is_valid_url_no_range_support(self, mock_safe_request_url, mock_is_safe_url):
+        mock_is_safe_url.return_value = (True, None)
+
+        mock_head_response = MagicMock()
+        mock_head_response.headers = {}
+        mock_head_response.status_code = 200
+        mock_head_response.raise_for_status.return_value = None
+
+        mock_get_response = MagicMock()
+        mock_get_response.status_code = 404
+        mock_get_response.close.return_value = None
+
+        mock_safe_request_url.side_effect = [mock_head_response, mock_get_response]
 
         with self.assertRaises(ImportException):
             self.handler.is_valid_url(self.valid_url)

geonode/upload/handlers/remote/tests/test_wms.py

@@ -18,7 +18,7 @@
 #########################################################################
 from collections import namedtuple
 from urllib.parse import ParseResult
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from mock import MagicMock, patch
 from geonode.upload.api.exceptions import ImportException
 from django.contrib.auth import get_user_model
@@ -113,6 +113,7 @@ def test_is_valid_should_raise_exception_if_the_url_is_invalid(self):
         self.assertIsNotNone(_exc)
         self.assertTrue("The provided url is not reachable")
 
+    @override_settings(SAFE_URL_TRUSTED_HOSTS=["geoserver:8080"])
     def test_is_valid_should_pass_with_valid_url(self):
         self.handler.is_valid_url(url=self.valid_payload_with_parse_false["url"])
 

geonode/upload/handlers/remote/tiles3d.py

@@ -18,7 +18,6 @@
 #########################################################################
 import logging
 
-import requests
 from geonode.layers.models import Dataset
 from geonode.upload.handlers.common.remote import BaseRemoteResourceHandler
 from geonode.upload.handlers.common.serializer import RemoteResourceSerializer
@@ -27,7 +26,7 @@
 from geonode.upload.handlers.tiles3d.exceptions import Invalid3DTilesException
 from geonode.base.enumerations import SOURCE_TYPE_REMOTE
 from geonode.base.models import ResourceBase
-from geonode.utils import is_safe_url_with_redirects
+from geonode.utils import is_safe_url_with_redirects, safe_request_url
 
 logger = logging.getLogger("importer")
 
@@ -60,9 +59,13 @@ def have_table(self):
 
     @staticmethod
     def is_valid_url(url, **kwargs):
-        BaseRemoteResourceHandler.is_valid_url(url)
+        # Reject disallowed URLs early, before running the remaining validity checks.
+        is_safe, _ = is_safe_url_with_redirects(url)
+        if not is_safe:
+            raise Invalid3DTilesException("The requested URL is not allowed.")
+        BaseRemoteResourceHandler.is_valid_url(url, **kwargs)
         try:
-            payload = requests.get(url, timeout=10).json()
+            payload = safe_request_url("GET", url, timeout=10).json()
             # required key described in the specification of 3dtiles
             # https://docs.ogc.org/cs/22-025r4/22-025r4.html#toc92
             is_valid = all(key in payload.keys() for key in ("asset", "geometricError", "root"))
@@ -92,12 +95,9 @@ def create_geonode_resource(
         _exec = orchestrator.get_execution_object(exec_id=execution_id)
 
         url = _exec.input_params.get("url")
-        is_safe, unsafe_url = is_safe_url_with_redirects(url)
-        if not is_safe:
-            raise Invalid3DTilesException("Invalid URL Provided")
 
         try:
-            js_file = requests.get(url, timeout=10).json()
+            js_file = safe_request_url("GET", url, timeout=10).json()
         except Exception as e:
             raise Invalid3DTilesException(e)
 

geonode/utils.py

@@ -1935,3 +1935,24 @@ def is_safe_url_with_redirects(url: str, max_redirects: int = 3):
             return False, current_url
 
     return False, current_url
+
+
+class SafeSession(requests.Session):
+    """requests Session that validates the target host of every request it
+    dispatches, including each redirect hop, before the socket is opened.
+    """
+
+    def send(self, request, **kwargs):
+        # request.url is the fully-resolved absolute URL of this hop.
+        if not is_safe_url(request.url):
+            # Keep the raised message generic: the URL may carry tokens or credentials.
+            logger.debug("Rejected request to disallowed host: %s", urlparse(request.url).hostname)
+            raise requests.exceptions.InvalidURL("The requested URL is not allowed.")
+        return super().send(request, **kwargs)
+
+
+def safe_request_url(method, url, max_redirects=3, **kwargs):
+    kwargs.setdefault("timeout", 10)
+    with SafeSession() as session:
+        session.max_redirects = max_redirects
+        return session.request(method, url, **kwargs)