Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
the zoom information box, persisted states, and the progress bar, as well as updated
dependencies such as `chartlets ^0.2.0`.

### Fixes

* Closed security vulnerability related to URL construction (#1203).
Comment thread
thomasstorm marked this conversation as resolved.
Outdated

## Changes in 1.13.1

### Enhancements
Expand Down
5 changes: 5 additions & 0 deletions examples/serve/demo/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ DatasetAttribution:

#DatasetChunkCacheSize: 100M

## You may want to set a list of allowed origin hosts, see https://github.com/xcube-dev/xcube/issues/1203
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
allowed_origins:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomasstorm this should be called allowed_hosts, right?

- "localhost:8080"
# - the address of your server instance

## You may want to specify a location of your server resources.
#base_dir: s3://<bucket>/<path-to-your>/<resources>/

Expand Down
1 change: 1 addition & 0 deletions test/server/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def test_config_schema_effectively_merged(self):
"default": "0.0.0.0",
"title": "Server address.",
},
"allowed_origins": {"type": "array"},
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
"port": {
"type": "integer",
"title": "Server port.",
Expand Down
70 changes: 68 additions & 2 deletions test/server/webservers/test_tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -381,6 +382,71 @@ 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

asyncio.run(make_not_validated_request())
Comment thread
thomasstorm marked this conversation as resolved.

context._config = {
"allowed_origins": [
"localhost:8080",
]
}

asyncio.run(make_valid_request())
asyncio.run(make_invalid_request())


# Helpers

Expand Down
1 change: 1 addition & 0 deletions xcube/server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
address=JsonStringSchema(
title="Server address.", default=DEFAULT_SERVER_ADDRESS
),
allowed_origins=JsonArraySchema(),
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
base_dir=JsonStringSchema(
title="Base directory used to resolve relative local paths."
" Can be a local filesystem path or an absolute URL.",
Expand Down
10 changes: 10 additions & 0 deletions xcube/server/webservers/tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
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(
Expand Down Expand Up @@ -330,6 +331,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_origins" in self.server_ctx.config
and self.request.host not in self.server_ctx.config["allowed_origins"]
Comment thread
thomasstorm marked this conversation as resolved.
Outdated
):
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)

Expand Down
Loading