diff --git a/geonode/tests/test_utils.py b/geonode/tests/test_utils.py index 31432252769..a59d1691222 100644 --- a/geonode/tests/test_utils.py +++ b/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") diff --git a/geonode/upload/handlers/common/remote.py b/geonode/upload/handlers/common/remote.py index 084cd71b54a..c5789362f38 100755 --- a/geonode/upload/handlers/common/remote.py +++ b/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") diff --git a/geonode/upload/handlers/remote/cog.py b/geonode/upload/handlers/remote/cog.py index 191a2f0ce57..c9f512ae07b 100644 --- a/geonode/upload/handlers/remote/cog.py +++ b/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: diff --git a/geonode/upload/handlers/remote/flatgeobuf.py b/geonode/upload/handlers/remote/flatgeobuf.py index f820834dc05..4579d39420e 100644 --- a/geonode/upload/handlers/remote/flatgeobuf.py +++ b/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: diff --git a/geonode/upload/handlers/remote/tests/test_3dtiles.py b/geonode/upload/handlers/remote/tests/test_3dtiles.py index f5216ec9414..8623d507d99 100644 --- a/geonode/upload/handlers/remote/tests/test_3dtiles.py +++ b/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) diff --git a/geonode/upload/handlers/remote/tests/test_cog.py b/geonode/upload/handlers/remote/tests/test_cog.py index 87f8b086561..589e970389c 100644 --- a/geonode/upload/handlers/remote/tests/test_cog.py +++ b/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) diff --git a/geonode/upload/handlers/remote/tests/test_flatgeobuf.py b/geonode/upload/handlers/remote/tests/test_flatgeobuf.py index 0776f0fa368..50863a8ef30 100644 --- a/geonode/upload/handlers/remote/tests/test_flatgeobuf.py +++ b/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) diff --git a/geonode/upload/handlers/remote/tests/test_wms.py b/geonode/upload/handlers/remote/tests/test_wms.py index b21a243484b..277ee000616 100644 --- a/geonode/upload/handlers/remote/tests/test_wms.py +++ b/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"]) diff --git a/geonode/upload/handlers/remote/tiles3d.py b/geonode/upload/handlers/remote/tiles3d.py index 94d655a3716..293b2f9dfd9 100644 --- a/geonode/upload/handlers/remote/tiles3d.py +++ b/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) diff --git a/geonode/utils.py b/geonode/utils.py index a7f143c0b29..df699653070 100755 --- a/geonode/utils.py +++ b/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)