From af84b25726326842d3e6a4b9f9de5e670945d070 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Wed, 25 Jul 2018 07:57:35 -0400 Subject: [PATCH] prevent cross site request forgery in websockets w/ the CSRF token now that we have the CSRF middleware, we have a reliable token available to us which we can use to verify individual ws_receive payloads; this is _simpler_ than making sure you've properly configured trusted origins, and it's also more secure than Origin header checks see: https://github.com/ansible/tower/issues/2661 --- awx/main/conf.py | 11 ---- awx/main/consumers.py | 61 ++++++++----------- awx/main/tests/unit/test_consumers.py | 33 ---------- awx/settings/defaults.py | 5 -- .../src/shared/socket/socket.service.js | 6 +- 5 files changed, 28 insertions(+), 88 deletions(-) delete mode 100644 awx/main/tests/unit/test_consumers.py diff --git a/awx/main/conf.py b/awx/main/conf.py index 660c1803a9..69435953c7 100644 --- a/awx/main/conf.py +++ b/awx/main/conf.py @@ -101,17 +101,6 @@ register( category_slug='system', ) -register( - 'WEBSOCKET_ORIGIN_WHITELIST', - field_class=fields.StringListField, - label=_('Websocket Origin Whitelist'), - help_text=_("If Tower is behind a reverse proxy/load balancer, use this setting " - "to whitelist hostnames which represent trusted Origin hostnames from which " - "Tower should allow websocket connections."), - category=_('System'), - category_slug='system', -) - def _load_default_license_from_file(): try: diff --git a/awx/main/consumers.py b/awx/main/consumers.py index 66b2d0c4dd..eb2afe9ab0 100644 --- a/awx/main/consumers.py +++ b/awx/main/consumers.py @@ -3,15 +3,13 @@ import logging from channels import Group from channels.auth import channel_session_user_from_http, channel_session_user -from channels.exceptions import DenyConnection -from six.moves.urllib.parse import urlparse -from django.utils.http import is_same_domain -from django.conf import settings +from django.http.cookie import parse_cookie from django.core.serializers.json import DjangoJSONEncoder logger = logging.getLogger('awx.main.consumers') +XRF_KEY = '_auth_user_xrf' def discard_groups(message): @@ -20,47 +18,22 @@ def discard_groups(message): Group(group).discard(message.reply_channel) -def origin_is_valid(message, trusted_values): - origin = dict(message.content.get('headers', {})).get('origin', '') - for trusted in trusted_values: - try: - client = urlparse(origin) - trusted = urlparse(trusted) - except (AttributeError, ValueError): - # if we can't parse a hostname, consider it invalid and try the - # next one - pass - else: - # if we _can_ parse the origin header, verify that it's trusted - if ( - trusted.scheme == client.scheme and - is_same_domain(client.netloc, trusted.netloc) - ): - # the provided Origin matches at least _one_ whitelisted host, - # return True - return True - logger.error(( - "ws:// origin header mismatch {} not in {}; consider adding {} to " - "settings.WEBSOCKET_ORIGIN_WHITELIST if it's a trusted host." - ).format(origin, trusted_values, origin)) - return False - - - @channel_session_user_from_http def ws_connect(message): - if not origin_is_valid( - message, - [settings.TOWER_URL_BASE] + settings.WEBSOCKET_ORIGIN_WHITELIST - ): - raise DenyConnection() - + headers = dict(message.content.get('headers', '')) message.reply_channel.send({"accept": True}) message.content['method'] = 'FAKE' if message.user.is_authenticated(): message.reply_channel.send( {"text": json.dumps({"accept": True, "user": message.user.id})} ) + # store the valid CSRF token from the cookie so we can compare it later + # on ws_receive + cookie_token = parse_cookie( + headers.get('cookie') + ).get('csrftoken') + if cookie_token: + message.channel_session[XRF_KEY] = cookie_token else: logger.error("Request user is not authenticated to use websocket.") message.reply_channel.send({"close": True}) @@ -79,6 +52,20 @@ def ws_receive(message): raw_data = message.content['text'] data = json.loads(raw_data) + xrftoken = data.get('xrftoken') + if ( + not xrftoken or + XRF_KEY not in message.channel_session or + xrftoken != message.channel_session[XRF_KEY] + ): + logger.error( + "access denied to channel, XRF mismatch for {}".format(user.username) + ) + message.reply_channel.send({ + "text": json.dumps({"error": "access denied to channel"}) + }) + return + if 'groups' in data: discard_groups(message) groups = data['groups'] diff --git a/awx/main/tests/unit/test_consumers.py b/awx/main/tests/unit/test_consumers.py deleted file mode 100644 index a0840856ff..0000000000 --- a/awx/main/tests/unit/test_consumers.py +++ /dev/null @@ -1,33 +0,0 @@ -from collections import namedtuple - -import pytest - -from awx.main.consumers import origin_is_valid - - -def _msg(origin): - return namedtuple('message', ('content',))({ - 'headers': [ - ('origin', origin) - ] - }) - - -@pytest.mark.parametrize('origin, trusted, valid', [ - ('https://tower.example.org', ['https://tower.example.org'], True), # exact match - ('https://tower.example.org/', ['https://tower.example.org'], True), # trailing slash match - ('https://tower.example.org', ['https://.example.org'], True), # wildcard match - ('https://proxy.tower.example.org', ['https://.tower.example.org'], True), # complex wildcard match - ('', ['https://tower.example.org'], False), # origin header empty - (None, ['https://tower.example.org'], False), # origin header unset - ('https://[\">[', ['https://tower.example.org'], False), # origin header garbage - ('file:///bad.html', ['https://tower.example.org'], False), # file:// origin blocked - ('http://tower.example.org', ['https://tower.example.org'], False), # http != https - ('https://tower.example.org:443', ['https://tower.example.org:8043'], False), # port mismatch - ('https://evil.example.com', ['https://tower.example.org'], False), # domain mismatch - ('https://tower.example.org', [], False), # no trusted hosts - ('https://a', ['https://a', 'https://b'], True), # multiple with a match - ('https://evil', ['https://a', 'https://b'], False), # multiple no match -]) -def test_trusted_origin(origin, trusted, valid): - assert origin_is_valid(_msg(origin), trusted) is valid diff --git a/awx/settings/defaults.py b/awx/settings/defaults.py index 370b8375f3..818713431e 100644 --- a/awx/settings/defaults.py +++ b/awx/settings/defaults.py @@ -166,11 +166,6 @@ REMOTE_HOST_HEADERS = ['REMOTE_ADDR', 'REMOTE_HOST'] # REMOTE_HOST_HEADERS will be trusted unconditionally') PROXY_IP_WHITELIST = [] -# If Tower is behind a reverse proxy/load balancer, use this setting -# to whitelist hostnames which represent trusted Origin hostnames from which -# Tower should allow websocket connections. -WEBSOCKET_ORIGIN_WHITELIST = [] - # Note: This setting may be overridden by database settings. STDOUT_MAX_BYTES_DISPLAY = 1048576 diff --git a/awx/ui/client/src/shared/socket/socket.service.js b/awx/ui/client/src/shared/socket/socket.service.js index afae6fb36f..5ccc4ab460 100644 --- a/awx/ui/client/src/shared/socket/socket.service.js +++ b/awx/ui/client/src/shared/socket/socket.service.js @@ -5,8 +5,8 @@ *************************************************/ import ReconnectingWebSocket from 'reconnectingwebsocket'; export default -['$rootScope', '$location', '$log','$state', '$q', 'i18n', 'GetBasePath', 'Rest', - function ($rootScope, $location, $log, $state, $q, i18n, GetBasePath, Rest) { +['$rootScope', '$location', '$log','$state', '$q', 'i18n', 'GetBasePath', 'Rest', '$cookies', + function ($rootScope, $location, $log, $state, $q, i18n, GetBasePath, Rest, $cookies) { var needsResubscribing = false, socketPromise = $q.defer(), needsRefreshAfterBlur; @@ -166,6 +166,7 @@ export default // look like {"groups":{"jobs": ["status_changed", "summary"]}. // This is used by all socket-enabled $states state.data.socket.groups.control = ['limit_reached_' + $rootScope.current_user.id]; + state.data.socket.xrftoken = $cookies.get('csrftoken'); this.emit(JSON.stringify(state.data.socket)); this.setLast(state); }, @@ -174,6 +175,7 @@ export default // on a socket-enabled page, and sends an empty groups object // to the API: {"groups": {}}. // This is used for all pages that are socket-disabled + state.data.socket.xrftoken = $cookies.get('csrftoken'); if(this.requiresNewSubscribe(state)){ this.emit(JSON.stringify(state.data.socket) || JSON.stringify({"groups": {}})); }