diff --git a/docs/changelog.rst b/docs/changelog.rst index eddbeed6..f858fe16 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -27,6 +27,12 @@ Feature release. - File: Fix extraneous encoding of path. (PR: :issue:`1611`) +- HTTP: Protect RPC and Websocket interfaces against CSRF by blocking requests + that originiate from servers other than those specified in the new config + value :confval:`http/allowed_origins`. An artifact of this is that all + JSON-RPC requests must now always set the header + ``Content-Type: application/json``. (PR:1668 :issue:`1659`) + - MPD: Added ``idle`` to the list of available commands. (Fixes: :issue:`1593`, PR: :issue:`1597`) diff --git a/docs/ext/http.rst b/docs/ext/http.rst index 730c05b8..4b521b97 100644 --- a/docs/ext/http.rst +++ b/docs/ext/http.rst @@ -98,3 +98,15 @@ See :ref:`config` for general help on configuring Mopidy. be published. Set to an empty string to disable Zeroconf for HTTP. + +.. confval:: http/allowed_origins + + A list of domains allowed to perform Cross-Origin Resource Sharing (CORS) + requests. This applies to both JSON-RPC and Websocket requests. Values + should be in the format ``hostname:port`` and separated by either a comma or + newline. + + Same-origin requests (i.e. requests from Mopidy's web server) are always + allowed and so you don't need an entry for those. However, if your requests + originate from a different web server, you will need to add an entry for + that server in this list. diff --git a/mopidy/http/__init__.py b/mopidy/http/__init__.py index 3fa4bcd6..4f6239b9 100644 --- a/mopidy/http/__init__.py +++ b/mopidy/http/__init__.py @@ -25,6 +25,7 @@ class Extension(ext.Extension): schema['port'] = config_lib.Port() schema['static_dir'] = config_lib.Path(optional=True) schema['zeroconf'] = config_lib.String(optional=True) + schema['allowed_origins'] = config_lib.List(optional=True) return schema def validate_environment(self): diff --git a/mopidy/http/ext.conf b/mopidy/http/ext.conf index d35229bc..5750e855 100644 --- a/mopidy/http/ext.conf +++ b/mopidy/http/ext.conf @@ -4,3 +4,4 @@ hostname = 127.0.0.1 port = 6680 static_dir = zeroconf = Mopidy HTTP server on $hostname +allowed_origins = diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index 6250163c..1a16d43b 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -11,6 +11,7 @@ import tornado.websocket import mopidy from mopidy import core, models +from mopidy.compat import urllib from mopidy.internal import encoding, jsonrpc @@ -19,12 +20,17 @@ logger = logging.getLogger(__name__) def make_mopidy_app_factory(apps, statics): def mopidy_app_factory(config, core): + allowed_origins = { + x.lower() for x in config['http']['allowed_origins'] if x + } return [ (r'/ws/?', WebSocketHandler, { 'core': core, + 'allowed_origins': allowed_origins, }), (r'/rpc', JsonRpcHandler, { 'core': core, + 'allowed_origins': allowed_origins, }), (r'/(.+)', StaticFileHandler, { 'path': os.path.join(os.path.dirname(__file__), 'data'), @@ -96,8 +102,9 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler): # One callback per client to keep time we hold up the loop short loop.add_callback(functools.partial(_send_broadcast, client, msg)) - def initialize(self, core): + def initialize(self, core, allowed_origins): self.jsonrpc = make_jsonrpc_wrapper(core) + self.allowed_origins = allowed_origins def open(self): self.set_nodelay(True) @@ -132,9 +139,7 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler): self.close() def check_origin(self, origin): - # Allow cross-origin WebSocket connections, like Tornado before 4.0 - # defaulted to. - return True + return check_origin(origin, self.request.headers, self.allowed_origins) def set_mopidy_headers(request_handler): @@ -143,16 +148,31 @@ def set_mopidy_headers(request_handler): 'X-Mopidy-Version', mopidy.__version__.encode('utf-8')) +def check_origin(origin, request_headers, allowed_origins): + if origin is None: + logger.debug('Origin was not set') + return False + allowed_origins.add(request_headers.get('Host')) + parsed_origin = urllib.parse.urlparse(origin).netloc.lower() + return parsed_origin and parsed_origin in allowed_origins + + class JsonRpcHandler(tornado.web.RequestHandler): - def initialize(self, core): + def initialize(self, core, allowed_origins): self.jsonrpc = make_jsonrpc_wrapper(core) + self.allowed_origins = allowed_origins def head(self): self.set_extra_headers() self.finish() def post(self): + content_type = self.request.headers.get('Content-Type', '') + if content_type != 'application/json': + self.set_status(415, 'Content-Type must be application/json') + return + data = self.request.body if not data: return @@ -177,6 +197,18 @@ class JsonRpcHandler(tornado.web.RequestHandler): self.set_header('Accept', 'application/json') self.set_header('Content-Type', 'application/json; utf-8') + def options(self): + origin = self.request.headers.get('Origin') + if not check_origin( + origin, self.request.headers, self.allowed_origins): + self.set_status(403, 'Access denied for origin %s' % origin) + return + + self.set_header('Access-Control-Allow-Origin', '%s' % origin) + self.set_header('Access-Control-Allow-Headers', 'Content-Type') + self.set_status(204) + self.finish() + class ClientListHandler(tornado.web.RequestHandler): diff --git a/tests/http/test_handlers.py b/tests/http/test_handlers.py index 3ad0c7b6..23472499 100644 --- a/tests/http/test_handlers.py +++ b/tests/http/test_handlers.py @@ -46,7 +46,9 @@ class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase): def get_app(self): self.core = mock.Mock() return tornado.web.Application([ - (r'/ws/?', handlers.WebSocketHandler, {'core': self.core}) + (r'/ws/?', handlers.WebSocketHandler, { + 'core': self.core, 'allowed_origins': [] + }) ]) def connection(self): diff --git a/tests/http/test_server.py b/tests/http/test_server.py index bb1d8cf0..aa721f29 100644 --- a/tests/http/test_server.py +++ b/tests/http/test_server.py @@ -20,6 +20,7 @@ class HttpServerTest(tornado.testing.AsyncHTTPTestCase): 'port': 6680, 'static_dir': None, 'zeroconf': '', + 'allowed_origins': [], } } @@ -128,7 +129,8 @@ class MopidyRPCHandlerTest(HttpServerTest): def test_should_return_rpc_error(self): cmd = tornado.escape.json_encode({'action': 'get_version'}) - response = self.fetch('/mopidy/rpc', method='POST', body=cmd) + response = self.fetch('/mopidy/rpc', method='POST', body=cmd, headers={ + 'Content-Type': 'application/json'}) self.assertEqual( {'jsonrpc': '2.0', 'id': None, 'error': @@ -139,7 +141,8 @@ class MopidyRPCHandlerTest(HttpServerTest): def test_should_return_parse_error(self): cmd = '{[[[]}' - response = self.fetch('/mopidy/rpc', method='POST', body=cmd) + response = self.fetch('/mopidy/rpc', method='POST', body=cmd, headers={ + 'Content-Type': 'application/json'}) self.assertEqual( {'jsonrpc': '2.0', 'id': None, 'error': @@ -154,7 +157,8 @@ class MopidyRPCHandlerTest(HttpServerTest): 'id': 1, }) - response = self.fetch('/mopidy/rpc', method='POST', body=cmd) + response = self.fetch('/mopidy/rpc', method='POST', body=cmd, headers={ + 'Content-Type': 'application/json'}) self.assertEqual( {'jsonrpc': '2.0', 'id': 1, 'result': mopidy.__version__}, @@ -168,6 +172,38 @@ class MopidyRPCHandlerTest(HttpServerTest): self.assertIn('Cache-Control', response.headers) self.assertIn('Content-Type', response.headers) + def test_should_require_correct_content_type(self): + cmd = tornado.escape.json_encode({ + 'method': 'core.get_version', + 'params': [], + 'jsonrpc': '2.0', + 'id': 1, + }) + + response = self.fetch('/mopidy/rpc', method='POST', body=cmd, headers={ + 'Content-Type': 'text/plain'}) + + self.assertEqual(response.code, 415) + self.assertEqual( + response.reason, 'Content-Type must be application/json') + + def test_different_origin_returns_access_denied(self): + response = self.fetch('/mopidy/rpc', method='OPTIONS', headers={ + 'Host': 'me:6680', 'Origin': 'http://evil:666'}) + + self.assertEqual(response.code, 403) + self.assertEqual( + response.reason, 'Access denied for origin http://evil:666') + + def test_same_origin_returns_cors_headers(self): + response = self.fetch('/mopidy/rpc', method='OPTIONS', headers={ + 'Host': 'me:6680', 'Origin': 'http://me:6680'}) + + self.assertEqual( + response.headers['Access-Control-Allow-Origin'], 'http://me:6680') + self.assertEqual( + response.headers['Access-Control-Allow-Headers'], 'Content-Type') + class HttpServerWithStaticFilesTest(tornado.testing.AsyncHTTPTestCase):