Release v2.2.1

.gitignore

@@ -15,6 +15,7 @@ cover/
 coverage.xml
 dist/
 docs/_build/
+docs/.doctrees/
 mopidy.log*
 nosetests.xml
 tmp/

docs/changelog.rst

@@ -5,6 +5,23 @@ Changelog
 This changelog is used to track all major changes to Mopidy.
 
 
+v2.2.1 (2018-10-15)
+===================
+
+Bug fix release.
+
+- HTTP: Stop blocking connections where the network location part of the
+  ``Origin`` header is empty, such as WebSocket connections originating from
+  local files. (Fixes: :issue:`1711`, PR: :issue:`1712`)
+
+- HTTP: Add new config value :confval:`http/csrf_protection` which enables all
+  CSRF protections introduced in Mopidy 2.2.0. It is enabled by default and
+  should only be disabled by those users who are unable to set a
+  ``Content-Type: application/json`` request header or cannot utilise the
+  :confval:`http/allowed_origins` config value. (Fixes: :issue:`1713`, PR:
+  :issue:`1714`)
+
+
 v2.2.0 (2018-09-30)
 ===================
 

docs/ext/http.rst

@@ -102,7 +102,7 @@ See :ref:`config` for general help on configuring Mopidy.
 .. 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
+    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.
     
@@ -110,3 +110,17 @@ See :ref:`config` for general help on configuring Mopidy.
     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.
+
+.. confval:: http/csrf_protection
+
+    Enable the HTTP server's protection against Cross-Site Request Forgery
+    (CSRF) from both JSON-RPC and WebSocket requests.
+
+    Disabling this will remove the requirement to set a ``Content-Type: application/json``
+    header for JSON-RPC POST requests. It will also disable all same-origin
+    checks, effectively ignoring the :confval:`http/allowed_origins` config
+    since requests from any origin will be allowed. Lastly, all
+    ``Access-Control-Allow-*`` response headers will be suppressed.
+
+    This config should only be disabled if you understand the security implications
+    and require the HTTP server's old behaviour.

mopidy/__init__.py

@@ -14,4 +14,4 @@ if not (2, 7) <= sys.version_info < (3,):
 warnings.filterwarnings('ignore', 'could not open display')
 
 
-__version__ = '2.2.0'
+__version__ = '2.2.1'

mopidy/http/__init__.py

@@ -26,6 +26,7 @@ class Extension(ext.Extension):
         schema['static_dir'] = config_lib.Path(optional=True)
         schema['zeroconf'] = config_lib.String(optional=True)
         schema['allowed_origins'] = config_lib.List(optional=True)
+        schema['csrf_protection'] = config_lib.Boolean(optional=True)
         return schema
 
     def validate_environment(self):

mopidy/http/ext.conf

@@ -5,3 +5,4 @@ port = 6680
 static_dir =
 zeroconf = Mopidy HTTP server on $hostname
 allowed_origins =
+csrf_protection = true

mopidy/http/handlers.py

@@ -20,6 +20,9 @@ logger = logging.getLogger(__name__)
 
 def make_mopidy_app_factory(apps, statics):
     def mopidy_app_factory(config, core):
+        if not config['http']['csrf_protection']:
+            logger.warn(
+                'HTTP Cross-Site Request Forgery protection is disabled')
         allowed_origins = {
             x.lower() for x in config['http']['allowed_origins'] if x
         }
@@ -27,10 +30,12 @@ def make_mopidy_app_factory(apps, statics):
             (r'/ws/?', WebSocketHandler, {
                 'core': core,
                 'allowed_origins': allowed_origins,
+                'csrf_protection': config['http']['csrf_protection'],
             }),
             (r'/rpc', JsonRpcHandler, {
                 'core': core,
                 'allowed_origins': allowed_origins,
+                'csrf_protection': config['http']['csrf_protection'],
             }),
             (r'/(.+)', StaticFileHandler, {
                 'path': os.path.join(os.path.dirname(__file__), 'data'),
@@ -102,9 +107,10 @@ 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, allowed_origins):
+    def initialize(self, core, allowed_origins, csrf_protection):
         self.jsonrpc = make_jsonrpc_wrapper(core)
         self.allowed_origins = allowed_origins
+        self.csrf_protection = csrf_protection
 
     def open(self):
         self.set_nodelay(True)
@@ -139,6 +145,8 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler):
             self.close()
 
     def check_origin(self, origin):
+        if not self.csrf_protection:
+            return True
         return check_origin(origin, self.request.headers, self.allowed_origins)
 
 
@@ -150,28 +158,37 @@ def set_mopidy_headers(request_handler):
 
 def check_origin(origin, request_headers, allowed_origins):
     if origin is None:
-        logger.debug('Origin was not set')
+        logger.warn('HTTP request denied for missing Origin header')
         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
+    # Some frameworks (e.g. Apache Cordova) use local files. Requests from
+    # these files don't really have a sensible Origin so the browser sets the
+    # header to something like 'file://' or 'null'. This results here in an
+    # empty parsed_origin which we choose to allow.
+    if parsed_origin and parsed_origin not in allowed_origins:
+        logger.warn('HTTP request denied for Origin "%s"', origin)
+        return False
+    return True
 
 
 class JsonRpcHandler(tornado.web.RequestHandler):
 
-    def initialize(self, core, allowed_origins):
+    def initialize(self, core, allowed_origins, csrf_protection):
         self.jsonrpc = make_jsonrpc_wrapper(core)
         self.allowed_origins = allowed_origins
+        self.csrf_protection = csrf_protection
 
     def head(self):
         self.set_extra_headers()
         self.finish()
 
     def post(self):
-        content_type = self.request.headers.get('Content-Type', '')
+        if self.csrf_protection:
-        if content_type != 'application/json':
+            content_type = self.request.headers.get('Content-Type', '')
-            self.set_status(415, 'Content-Type must be application/json')
+            if content_type != 'application/json':
-            return
+                self.set_status(415, 'Content-Type must be application/json')
+                return
 
         data = self.request.body
         if not data:
@@ -198,14 +215,16 @@ class JsonRpcHandler(tornado.web.RequestHandler):
         self.set_header('Content-Type', 'application/json; utf-8')
 
     def options(self):
-        origin = self.request.headers.get('Origin')
+        if self.csrf_protection:
-        if not check_origin(
+            origin = self.request.headers.get('Origin')
-                origin, self.request.headers, self.allowed_origins):
+            if not check_origin(origin, self.request.headers,
-            self.set_status(403, 'Access denied for origin %s' % origin)
+                                self.allowed_origins):
-            return
+                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_header('Access-Control-Allow-Origin', '%s' % origin)
-        self.set_header('Access-Control-Allow-Headers', 'Content-Type')
         self.set_status(204)
         self.finish()
 

tests/http/test_handlers.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
 import os
+import unittest
 
 import mock
 
@@ -47,7 +48,8 @@ class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase):
         self.core = mock.Mock()
         return tornado.web.Application([
             (r'/ws/?', handlers.WebSocketHandler, {
-                'core': self.core, 'allowed_origins': []
+                'core': self.core, 'allowed_origins': [],
+                'csrf_protection': True
             })
         ])
 
@@ -86,3 +88,42 @@ class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase):
         for client in handlers.WebSocketHandler.clients:
             client.ws_connection = None
         handlers.WebSocketHandler.broadcast('message')
+
+
+class CheckOriginTests(unittest.TestCase):
+
+    def setUp(self):
+        self.headers = {'Host': 'localhost:6680'}
+        self.allowed = set()
+
+    def test_missing_origin_blocked(self):
+        self.assertFalse(handlers.check_origin(
+            None, self.headers, self.allowed))
+
+    def test_empty_origin_allowed(self):
+        self.assertTrue(handlers.check_origin('', self.headers, self.allowed))
+
+    def test_chrome_file_origin_allowed(self):
+        self.assertTrue(handlers.check_origin(
+            'file://', self.headers, self.allowed))
+
+    def test_firefox_null_origin_allowed(self):
+        self.assertTrue(handlers.check_origin(
+            'null', self.headers, self.allowed))
+
+    def test_same_host_origin_allowed(self):
+        self.assertTrue(handlers.check_origin(
+            'http://localhost:6680', self.headers, self.allowed))
+
+    def test_different_host_origin_blocked(self):
+        self.assertFalse(handlers.check_origin(
+            'http://other:6680', self.headers, self.allowed))
+
+    def test_different_port_blocked(self):
+        self.assertFalse(handlers.check_origin(
+            'http://localhost:80', self.headers, self.allowed))
+
+    def test_extra_origin_allowed(self):
+        self.allowed.add('other:6680')
+        self.assertTrue(handlers.check_origin(
+            'http://other:6680', self.headers, self.allowed))

tests/http/test_server.py

@@ -21,6 +21,7 @@ class HttpServerTest(tornado.testing.AsyncHTTPTestCase):
                 'static_dir': None,
                 'zeroconf': '',
                 'allowed_origins': [],
+                'csrf_protection': True,
             }
         }
 
@@ -205,6 +206,48 @@ class MopidyRPCHandlerTest(HttpServerTest):
             response.headers['Access-Control-Allow-Headers'], 'Content-Type')
 
 
+class MopidyRPCHandlerNoCSRFProtectionTest(HttpServerTest):
+
+    def get_config(self):
+        config = super(MopidyRPCHandlerNoCSRFProtectionTest, self).get_config()
+        config['http']['csrf_protection'] = False
+        return config
+
+    def get_cmd(self):
+        return tornado.escape.json_encode({
+            'method': 'core.get_version',
+            'params': [],
+            'jsonrpc': '2.0',
+            'id': 1,
+        })
+
+    def test_should_ignore_incorrect_content_type(self):
+        response = self.fetch(
+            '/mopidy/rpc', method='POST', body=self.get_cmd(),
+            headers={'Content-Type': 'text/plain'})
+
+        self.assertEqual(response.code, 200)
+
+    def test_should_ignore_missing_content_type(self):
+        response = self.fetch(
+            '/mopidy/rpc', method='POST', body=self.get_cmd(), headers={})
+
+        self.assertEqual(response.code, 200)
+
+    def test_different_origin_returns_allowed(self):
+        response = self.fetch('/mopidy/rpc', method='OPTIONS', headers={
+            'Host': 'me:6680', 'Origin': 'http://evil:666'})
+
+        self.assertEqual(response.code, 204)
+
+    def test_should_not_return_cors_headers(self):
+        response = self.fetch('/mopidy/rpc', method='OPTIONS', headers={
+            'Host': 'me:6680', 'Origin': 'http://me:6680'})
+
+        self.assertNotIn('Access-Control-Allow-Origin', response.headers)
+        self.assertNotIn('Access-Control-Allow-Headers', response.headers)
+
+
 class HttpServerWithStaticFilesTest(tornado.testing.AsyncHTTPTestCase):
 
     def get_app(self):