From ee0aabda1d5f228f9e26ea7be477a6d3af51a3cd Mon Sep 17 00:00:00 2001 From: Lex Berezhny Date: Wed, 3 Jun 2020 13:28:32 -0400 Subject: [PATCH] backwards compatible allowed_origin, default browsers not allowed --- lbry/conf.py | 4 ++- lbry/extras/daemon/daemon.py | 7 ++-- lbry/extras/daemon/security.py | 5 +++ .../lbrynet_daemon/test_allowed_origin.py | 36 +++++++++++++++++++ 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 lbry/extras/daemon/security.py create mode 100644 tests/unit/lbrynet_daemon/test_allowed_origin.py diff --git a/lbry/conf.py b/lbry/conf.py index 9e08e842c..deb9222f2 100644 --- a/lbry/conf.py +++ b/lbry/conf.py @@ -625,7 +625,9 @@ class Config(CLIConfig): previous_names=['upload_log', 'upload_log', 'share_debug_info'] ) track_bandwidth = Toggle("Track bandwidth usage", True) - allowed_origin = String("Allowed origin header for api calls, use * to allow all", 'null') + allowed_origin = String( + "Allowed `Origin` header value for API request (sent by browser), use * to allow " + "all hosts; default is to only allow API requests with no `Origin` value.", "") # media server streaming_server = String('Host name and port to serve streaming media over range requests', diff --git a/lbry/extras/daemon/daemon.py b/lbry/extras/daemon/daemon.py index 15a82e38b..2c121ff8c 100644 --- a/lbry/extras/daemon/daemon.py +++ b/lbry/extras/daemon/daemon.py @@ -47,6 +47,7 @@ from lbry.extras.daemon.componentmanager import ComponentManager from lbry.extras.daemon.json_response_encoder import JSONResponseEncoder from lbry.extras.daemon import comment_client from lbry.extras.daemon.undecorated import undecorated +from lbry.extras.daemon.security import is_request_allowed from lbry.file_analysis import VideoFileAnalyzer from lbry.schema.claim import Claim from lbry.schema.url import URL @@ -566,10 +567,8 @@ class Daemon(metaclass=JSONRPCServerType): log.info("finished shutting down") async def handle_old_jsonrpc(self, request): - origin = request.headers.get('Origin', 'null') - origin = None if origin == 'null' else origin - if origin != self.conf.allowed_origin != '*': - log.warning("API request from origin '%s' is not allowed", origin) + if is_request_allowed(request, self.conf): + log.warning("API request from origin '%s' is not allowed", request.headers.get('Origin')) raise web.HTTPForbidden() data = await request.json() params = data.get('params', {}) diff --git a/lbry/extras/daemon/security.py b/lbry/extras/daemon/security.py new file mode 100644 index 000000000..18a8326ec --- /dev/null +++ b/lbry/extras/daemon/security.py @@ -0,0 +1,5 @@ +def is_request_allowed(request, conf) -> bool: + origin = request.headers.get('Origin', 'null') + if origin == 'null' or conf.allowed_origin in ('*', origin): + return True + return False diff --git a/tests/unit/lbrynet_daemon/test_allowed_origin.py b/tests/unit/lbrynet_daemon/test_allowed_origin.py new file mode 100644 index 000000000..7d679a3dc --- /dev/null +++ b/tests/unit/lbrynet_daemon/test_allowed_origin.py @@ -0,0 +1,36 @@ +import unittest + +from aiohttp.test_utils import make_mocked_request as request + +from lbry.testcase import AsyncioTestCase +from lbry.conf import Config +from lbry.extras.daemon.security import is_request_allowed as allowed + + +class TestAllowedOrigin(unittest.TestCase): + + def test_allowed_origin_default(self): + conf = Config() + # no Origin is always allowed + self.assertTrue(allowed(request('GET', '/'), conf)) + # some clients send Origin: null (eg, https://github.com/electron/electron/issues/7931) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'null'}), conf)) + # deny all other Origins + self.assertFalse(allowed(request('GET', '/', headers={'Origin': 'localhost'}), conf)) + self.assertFalse(allowed(request('GET', '/', headers={'Origin': 'hackers.com'}), conf)) + + def test_allowed_origin_star(self): + conf = Config(allowed_origin='*') + # everything is allowed + self.assertTrue(allowed(request('GET', '/'), conf)) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'null'}), conf)) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'localhost'}), conf)) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'hackers.com'}), conf)) + + def test_allowed_origin_specified(self): + conf = Config(allowed_origin='localhost') + # no origin and only localhost are allowed + self.assertTrue(allowed(request('GET', '/'), conf)) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'null'}), conf)) + self.assertTrue(allowed(request('GET', '/', headers={'Origin': 'localhost'}), conf)) + self.assertFalse(allowed(request('GET', '/', headers={'Origin': 'hackers.com'}), conf))