andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #1714 from kingosticks/feature/disable-http-csrf-config

Browse Source 
http: Add config option to disable CSRF protection (Fixes: #1713)
This commit is contained in:
Stein Magnus Jodal 2018-10-15 22:04:30 +02:00 committed by GitHub
commit 2390f3a891
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 94 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
docs/changelog.rst
View File

@ -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)
===================

16
docs/ext/http.rst
View File

@ -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.

1
mopidy/http/__init__.py
View File

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

1
mopidy/http/ext.conf
View File

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

38
mopidy/http/handlers.py
View File

@ -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()

3
tests/http/test_handlers.py
View File

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

43
tests/http/test_server.py
View File

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