diff --git a/CHANGES.md b/CHANGES.md index 93e2bf3be..18b4f25bd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -7,6 +7,8 @@ dependencies such as `chartlets ^0.2.0`. ### Fixes + +* Closed security vulnerability in xcube-server related to URL construction (#1203). * Adapted to xarray version 2026.04.0 (#1211) ## Changes in 1.13.1 diff --git a/examples/serve/demo/config.yml b/examples/serve/demo/config.yml index abc235c7e..118f18001 100644 --- a/examples/serve/demo/config.yml +++ b/examples/serve/demo/config.yml @@ -18,6 +18,12 @@ DatasetAttribution: #DatasetChunkCacheSize: 100M +## To improve security, you may want to set a list of allowed origin hosts. +## (See also https://github.com/xcube-dev/xcube/issues/1203) +allowed_origins: + - "localhost:8080" + # - the address of your server instance + ## You may want to specify a location of your server resources. #base_dir: s3:///// diff --git a/test/server/test_server.py b/test/server/test_server.py index 1736007ab..b9501ef7e 100644 --- a/test/server/test_server.py +++ b/test/server/test_server.py @@ -141,6 +141,11 @@ def test_config_schema_effectively_merged(self): "default": "0.0.0.0", "title": "Server address.", }, + "allowed_hosts": { + "type": "array", + "title": "List of hosts allowed for URL construction. " + "If not set, all hosts are allowed.", + }, "port": { "type": "integer", "title": "Server port.", diff --git a/test/server/webservers/test_tornado.py b/test/server/webservers/test_tornado.py index f37f71c30..56e4cd417 100644 --- a/test/server/webservers/test_tornado.py +++ b/test/server/webservers/test_tornado.py @@ -4,14 +4,15 @@ import unittest from collections.abc import Awaitable, Sequence -from test.server.mocks import mock_server -from typing import Any, Callable, Dict, Optional, Tuple, Type, Union +from typing import Any, Callable, Optional, Union import pytest import tornado.httputil import tornado.web from tornado import concurrent +from tornado.concurrent import Future +from test.server.mocks import mock_server from xcube.server.api import ( Api, ApiContextT, @@ -381,6 +382,70 @@ async def test_it(): asyncio.run(test_it()) + # noinspection PyMethodMayBeStatic + def test_allowed_hosts(self): + application = tornado.web.Application() + context = MockContext() + setattr(application, SERVER_CTX_ATTR_NAME, context) + + class TestHandler(ApiHandler): + def get(self): + pass + + async def make_not_validated_request(): + # noinspection PyTypeChecker + tr = tornado.httputil.HTTPServerRequest( + method="GET", + host="www.evil.empire", + uri="/datasets?details=x", + connection=MockConnection(), + ) + api_route = ApiRoute("test", "/test", TestHandler) + handler = TornadoRequestHandler(application, tr, api_route=api_route) + # noinspection PyAsyncCall + handler.prepare() # not automatically called in test + await handler.get() + + async def make_valid_request(): + # noinspection PyTypeChecker + tr = tornado.httputil.HTTPServerRequest( + method="GET", + host="localhost:8080", + uri="/datasets?details=x", + connection=MockConnection(), + ) + api_route = ApiRoute("test", "/test", TestHandler) + handler = TornadoRequestHandler(application, tr, api_route=api_route) + # noinspection PyAsyncCall + handler.prepare() # not automatically called in test + await handler.get() + + async def make_invalid_request(): + # noinspection PyTypeChecker + tr = tornado.httputil.HTTPServerRequest( + method="GET", + host="www.evil.empire", + uri="/datasets?details=x", + connection=MockConnection(), + ) + api_route = ApiRoute("test", "/test", TestHandler) + with pytest.raises(tornado.web.HTTPError, match=r".*400.*"): + handler = TornadoRequestHandler(application, tr, api_route=api_route) + # noinspection PyAsyncCall + handler.prepare() # not automatically called in test + + import asyncio + + context._config = {} + + self.assertFalse("allowed_hosts" in context._config) + asyncio.run(make_not_validated_request()) + + context._config["allowed_hosts"] = "localhost:8080" + + asyncio.run(make_valid_request()) + asyncio.run(make_invalid_request()) + # Helpers diff --git a/xcube/server/config.py b/xcube/server/config.py index f9c47dbee..d601226b0 100644 --- a/xcube/server/config.py +++ b/xcube/server/config.py @@ -22,6 +22,10 @@ address=JsonStringSchema( title="Server address.", default=DEFAULT_SERVER_ADDRESS ), + allowed_hosts=JsonArraySchema( + title="List of hosts allowed for URL construction." + " If not set, all hosts are allowed." + ), base_dir=JsonStringSchema( title="Base directory used to resolve relative local paths." " Can be a local filesystem path or an absolute URL.", diff --git a/xcube/server/webservers/tornado.py b/xcube/server/webservers/tornado.py index 2b0f91fe9..17906ebf5 100644 --- a/xcube/server/webservers/tornado.py +++ b/xcube/server/webservers/tornado.py @@ -291,6 +291,7 @@ def __init__( super().__init__(application, request) server_ctx = getattr(application, SERVER_CTX_ATTR_NAME, None) assert isinstance(server_ctx, Context) + self._server_ctx = server_ctx api_route: ApiRoute = kwargs.pop("api_route") ctx: Context = server_ctx.get_api_ctx(api_route.api_name) self._api_handler: ApiHandler = api_route.handler_cls( @@ -304,6 +305,10 @@ def __init__( **api_route.handler_kwargs, ) + @property + def server_ctx(self) -> Context: + return self._server_ctx + def set_default_headers(self): self.set_header("Server", f"xcube-server/{version}") # TODO: get from config @@ -330,6 +335,15 @@ def write_error(self, status_code: int, **kwargs: Any): error_info.update(reason=exc_val.reason) self.finish({"error": error_info}) + def prepare(self) -> Optional[Awaitable[None]]: + if ( + "allowed_hosts" in self.server_ctx.config + and self.request.host not in self.server_ctx.config["allowed_hosts"] + ): + raise tornado.web.HTTPError( + 400, log_message=f"Host {self.request.host} not allowed." + ) + async def head(self, *args, **kwargs): await self._call_method("head", *args, **kwargs)