http: Add config option to control CSRF protection (Fixes: #1713)

Allows users to disable CSRF protection and revert to the HTTP server's
previous (less secure) behaviour. Users are advised to leave this config
value enabled if possible. However, if disabled this will:
  * Remove the requirement to set a ``Content-Type: application/json``
    header for JSON-RPC POST requests.
  * Disable all same-origin checks, effectively ignoring the ``allowed_origins``
    config since requests from any origin will be allowed.
  * Suppress all ``Access-Control-Allow-*`` response headers.

docs/changelog.rst

@@ -12,6 +12,13 @@ v2.2.1 (UNRELEASED)
   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/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)
 
 
@@ -166,19 +174,21 @@ def check_origin(origin, request_headers, allowed_origins):
 
 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 content_type != 'application/json':
-            self.set_status(415, 'Content-Type must be application/json')
-            return
+        if self.csrf_protection:
+            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:
@@ -205,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 not check_origin(
-                origin, self.request.headers, self.allowed_origins):
-            self.set_status(403, 'Access denied for origin %s' % origin)
-            return
+        if self.csrf_protection:
+            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_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

@@ -48,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
             })
         ])
 

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):