From 6a269ed11fc46d7b14edd1fc11caf655922bf1a6 Mon Sep 17 00:00:00 2001 From: Roach Date: Wed, 15 Aug 2018 10:40:29 -0700 Subject: [PATCH] Improve security by adding request signing (#34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added request signing * rearranged things to reflect the order of checks * Updated README.rst * Updated screenshot * Fixed test request signature creation * Fixed Flask exception handling test * Added hash matching for Python 2.7.6 and add 2.7.6 to tox environments * Making hash comparisons work in 2.7.6, 2.7.7 and 3.6.5… --- .travis.yml | 1 + README.rst | 6 +-- example/README.rst | 6 +-- example/example.py | 17 +++--- slackeventsapi/__init__.py | 6 +-- slackeventsapi/server.py | 80 ++++++++++++++++++++++++---- tests/conftest.py | 19 +++++-- tests/test_events.py | 16 ++++-- tests/test_server.py | 103 ++++++++++++++++++++++++++++++++++--- tox.ini | 2 +- 10 files changed, 215 insertions(+), 41 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7a873e0..670d35e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,7 @@ sudo: false dist: trusty language: python python: + - "2.7.6" - "2.7" - "3.3" - "3.4" diff --git a/README.rst b/README.rst index 1260619..6dd98ac 100644 --- a/README.rst +++ b/README.rst @@ -72,7 +72,7 @@ user has authorized your app. .. code:: python - SLACK_VERIFICATION_TOKEN = os.environ["SLACK_VERIFICATION_TOKEN"] + SLACK_SIGNING_SECRET = os.environ["SLACK_SIGNING_SECRET"] Create a Slack Event Adapter for receiving actions via the Events API ----------------------------------------------------------------------- @@ -83,7 +83,7 @@ Create a Slack Event Adapter for receiving actions via the Events API from slackeventsapi import SlackEventAdapter - slack_events_adapter = SlackEventAdapter(SLACK_VERIFICATION_TOKEN, endpoint="/slack/events") + slack_events_adapter = SlackEventAdapter(SLACK_SIGNING_SECRET, endpoint="/slack/events") # Create an event listener for "reaction_added" events and print the emoji name @@ -118,7 +118,7 @@ Create a Slack Event Adapter for receiving actions via the Events API # Bind the Events API route to your existing Flask app by passing the server # instance as the last param, or with `server=app`. - slack_events_adapter = SlackEventAdapter(SLACK_VERIFICATION_TOKEN, "/slack/events", app) + slack_events_adapter = SlackEventAdapter(SLACK_SIGNING_SECRET, "/slack/events", app) # Create an event listener for "reaction_added" events and print the emoji name diff --git a/example/README.rst b/example/README.rst index 54b7c26..62fb216 100644 --- a/example/README.rst +++ b/example/README.rst @@ -73,13 +73,13 @@ Copy your app's **Bot User OAuth Access Token** and add it to your python enviro Next, go back to your app's **Basic Information** page -.. image:: https://cloud.githubusercontent.com/assets/32463/24877833/950dd53c-1de5-11e7-984f-deb26e8b9482.png +.. image:: https://user-images.githubusercontent.com/32463/43932347-63b21eca-9bf8-11e8-8b30-0a848c263bb1.png -Add your app's **Verification Token** to your python environmental variables +Add your app's **Signing Secret** to your python environmental variables .. code:: - export SLACK_VERIFICATION_TOKEN=xxxxxxxxXxxXxxXxXXXxxXxxx + export SLACK_SIGNING_SECRET=xxxxxxxxXxxXxxXxXXXxxXxxx **🤖 Start ngrok** diff --git a/example/example.py b/example/example.py index d10c4af..59bf966 100644 --- a/example/example.py +++ b/example/example.py @@ -3,12 +3,12 @@ import os # Our app's Slack Event Adapter for receiving actions via the Events API -SLACK_VERIFICATION_TOKEN = os.environ["SLACK_VERIFICATION_TOKEN"] -slack_events_adapter = SlackEventAdapter(SLACK_VERIFICATION_TOKEN, "/slack/events") +slack_signing_secret = os.environ["SLACK_SIGNING_SECRET"] +slack_events_adapter = SlackEventAdapter(slack_signing_secret, "/slack/events") # Create a SlackClient for your bot to use for Web API requests -SLACK_BOT_TOKEN = os.environ["SLACK_BOT_TOKEN"] -CLIENT = SlackClient(SLACK_BOT_TOKEN) +slack_bot_token = os.environ["SLACK_BOT_TOKEN"] +slack_client = SlackClient(slack_bot_token) # Example responder to greetings @slack_events_adapter.on("message") @@ -18,7 +18,7 @@ def handle_message(event_data): if message.get("subtype") is None and "hi" in message.get('text'): channel = message["channel"] message = "Hello <@%s>! :tada:" % message["user"] - CLIENT.api_call("chat.postMessage", channel=channel, text=message) + slack_client.api_call("chat.postMessage", channel=channel, text=message) # Example reaction emoji echo @@ -28,7 +28,12 @@ def reaction_added(event_data): emoji = event["reaction"] channel = event["item"]["channel"] text = ":%s:" % emoji - CLIENT.api_call("chat.postMessage", channel=channel, text=text) + slack_client.api_call("chat.postMessage", channel=channel, text=text) + +# Error events +@slack_events_adapter.on("error") +def error_handler(err): + print("ERROR: " + str(err)) # Once we have our event listeners configured, we can start the # Flask server with the default `/events` endpoint on port 3000 diff --git a/slackeventsapi/__init__.py b/slackeventsapi/__init__.py index cae332e..601adb3 100644 --- a/slackeventsapi/__init__.py +++ b/slackeventsapi/__init__.py @@ -5,10 +5,10 @@ class SlackEventAdapter(EventEmitter): # Initialize the Slack event server # If no endpoint is provided, default to listening on '/slack/events' - def __init__(self, verification_token, endpoint="/slack/events", server=None): + def __init__(self, signing_secret, endpoint="/slack/events", server=None, **kwargs): EventEmitter.__init__(self) - self.verification_token = verification_token - self.server = SlackServer(verification_token, endpoint, self, server) + self.signing_secret = signing_secret + self.server = SlackServer(signing_secret, endpoint, self, server, **kwargs) def start(self, host='127.0.0.1', port=None, debug=False, **kwargs): """ diff --git a/slackeventsapi/server.py b/slackeventsapi/server.py index 1e24411..fd34df5 100644 --- a/slackeventsapi/server.py +++ b/slackeventsapi/server.py @@ -2,12 +2,15 @@ import json import platform import sys +import hmac +import hashlib +from time import time from .version import __version__ class SlackServer(Flask): - def __init__(self, verification_token, endpoint, emitter, server): - self.verification_token = verification_token + def __init__(self, signing_secret, endpoint, emitter, server): + self.signing_secret = signing_secret self.emitter = emitter self.endpoint = endpoint self.package_info = self.get_package_info() @@ -41,6 +44,44 @@ def get_package_info(self): return " ".join(ua_string) + def verify_signature(self, timestamp, signature): + # Verify the request signature of the request sent from Slack + # Generate a new hash using the app's signing secret and request data + + # Compare the generated hash and incoming request signature + # Python 2.7.6 doesn't support compare_digest + # It's recommended to use Python 2.7.7+ + # noqa See https://docs.python.org/2/whatsnew/2.7.html#pep-466-network-security-enhancements-for-python-2-7 + if hasattr(hmac, "compare_digest"): + req = str.encode('v0:' + str(timestamp) + ':') + request.data + request_hash = 'v0=' + hmac.new( + str.encode(self.signing_secret), + req, hashlib.sha256 + ).hexdigest() + # Compare byte strings for Python 2 + if (sys.version_info[0] == 2): + return hmac.compare_digest(bytes(request_hash), bytes(signature)) + else: + return hmac.compare_digest(request_hash, signature) + else: + # So, we'll compare the signatures explicitly + req = str.encode('v0:' + str(timestamp) + ':') + request.data + request_hash = 'v0=' + hmac.new( + str.encode(self.signing_secret), + req, hashlib.sha256 + ).hexdigest() + + if len(request_hash) != len(signature): + return False + result = 0 + if isinstance(request_hash, bytes) and isinstance(signature, bytes): + for x, y in zip(request_hash, signature): + result |= x ^ y + else: + for x, y in zip(request_hash, signature): + result |= ord(x) ^ ord(y) + return result == 0 + def bind_route(self, server): @server.route(self.endpoint, methods=['GET', 'POST']) def event(): @@ -48,21 +89,31 @@ def event(): if request.method == 'GET': return make_response("These are not the slackbots you're looking for.", 404) + # Each request comes with request timestamp and request signature + # emit an error if the timestamp is out of range + req_timestamp = request.headers.get('X-Slack-Request-Timestamp') + if abs(time() - int(req_timestamp)) > 60 * 5: + slack_exception = SlackEventAdapterException('Invalid request timestamp') + self.emitter.emit('error', slack_exception) + return make_response("", 403) + + # Verify the request signature using the app's signing secret + # emit an error if the signature can't be verified + req_signature = request.headers.get('X-Slack-Signature') + if not self.verify_signature(req_timestamp, req_signature): + slack_exception = SlackEventAdapterException('Invalid request signature') + self.emitter.emit('error', slack_exception) + return make_response("", 403) + # Parse the request payload into JSON event_data = json.loads(request.data.decode('utf-8')) - # Echo the URL verification challenge code + # Echo the URL verification challenge code back to Slack if "challenge" in event_data: return make_response( event_data.get("challenge"), 200, {"content_type": "application/json"} ) - # Verify the request token - request_token = event_data.get("token") - if self.verification_token != request_token: - self.emitter.emit('error', Exception('invalid verification token')) - return make_response("Request contains invalid Slack verification token", 403) - # Parse the Event payload and emit the event to the event listener if "event" in event_data: event_type = event_data["event"]["type"] @@ -70,3 +121,14 @@ def event(): response = make_response("", 200) response.headers['X-Slack-Powered-By'] = self.package_info return response + + +class SlackEventAdapterException(Exception): + """ + Base exception for all errors raised by the SlackClient library + """ + def __init__(self, msg=None): + if msg is None: + # default error message + msg = "An error occurred in the SlackEventsApiAdapter library" + super(SlackEventAdapterException, self).__init__(msg) diff --git a/tests/conftest.py b/tests/conftest.py index 8409c43..6345a84 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,19 @@ -import pytest import json +import hashlib +import hmac +import pytest from slackeventsapi import SlackEventAdapter +def create_signature(secret, timestamp, data): + req = str.encode('v0:' + str(timestamp) + ':') + str.encode(data) + request_signature= 'v0='+hmac.new( + str.encode(secret), + req, hashlib.sha256 + ).hexdigest() + return request_signature + + def load_event_fixture(event, as_string=True): filename = "tests/data/{}.json".format(event) with open(filename) as json_data: @@ -23,12 +34,14 @@ def pytest_namespace(): return { 'reaction_event_fixture': load_event_fixture('reaction_added'), 'url_challenge_fixture': load_event_fixture('url_challenge'), - 'bad_token_fixture': event_with_bad_token() + 'bad_token_fixture': event_with_bad_token(), + 'create_signature': create_signature } @pytest.fixture def app(): - adapter = SlackEventAdapter("vFO9LARnLI7GflLR8tGqHgdy") + adapter = SlackEventAdapter("SIGNING_SECRET") app = adapter.server + app.testing = True return app diff --git a/tests/test_events.py b/tests/test_events.py index 2c5fb0c..cf53163 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -1,21 +1,27 @@ +import time import pytest from slackeventsapi import SlackEventAdapter -ADAPTER = SlackEventAdapter('vFO9LARnLI7GflLR8tGqHgdy') - +ADAPTER = SlackEventAdapter('SIGNING_SECRET') def test_event_emission(client): # Events should trigger an event - data = pytest.reaction_event_fixture - @ADAPTER.on('reaction_added') def event_handler(event): assert event["reaction"] == 'grinning' + data = pytest.reaction_event_fixture + timestamp = int(time.time()) + signature = pytest.create_signature(ADAPTER.signing_secret, timestamp, data) + res = client.post( '/slack/events', data=data, - content_type='application/json' + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } ) assert res.status_code == 200 diff --git a/tests/test_server.py b/tests/test_server.py index d785449..b344e13 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2,20 +2,23 @@ from flask import Flask import pytest import sys +import hmac +import time from slackeventsapi import SlackEventAdapter +from slackeventsapi.server import SlackEventAdapterException from slackeventsapi.version import __version__ def test_existing_flask(): valid_flask = Flask(__name__) - valid_adapter = SlackEventAdapter("vFO9LARnLI7GflLR8tGqHgdy", "/slack/events", valid_flask) + valid_adapter = SlackEventAdapter("SIGNING_SECRET", "/slack/events", valid_flask) assert isinstance(valid_adapter, SlackEventAdapter) def test_server_not_flask(): with pytest.raises(TypeError) as e: invalid_flask = "I am not a Flask" - SlackEventAdapter("vFO9LARnLI7GflLR8tGqHgdy", "/slack/events", invalid_flask) + SlackEventAdapter("SIGNING_SECRET", "/slack/events", invalid_flask) assert e.value.args[0] == 'Server must be an instance of Flask' @@ -26,33 +29,110 @@ def test_event_endpoint_get(client): def test_url_challenge(client): + slack_adapter = SlackEventAdapter("SIGNING_SECRET") data = pytest.url_challenge_fixture + timestamp = int(time.time()) + signature = pytest.create_signature(slack_adapter.signing_secret, timestamp, data) + res = client.post( '/slack/events', data=data, - content_type='application/json') + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } + ) assert res.status_code == 200 assert bytes.decode(res.data) == "valid_challenge_token" -def test_valid_event_request(client): +def test_invalid_request_signature(client): + # Verify [package metadata header is set + slack_adapter = SlackEventAdapter("SIGNING_SECRET") + + data = pytest.reaction_event_fixture + timestamp = int(time.time()) + signature = "bad signature" + + with pytest.raises(SlackEventAdapterException) as excinfo: + res = client.post( + '/slack/events', + data=data, + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } + ) + + assert str(excinfo.value) == 'Invalid request signature' + + +def test_invalid_request_timestamp(client): + # Verify [package metadata header is set + slack_adapter = SlackEventAdapter("SIGNING_SECRET") + + data = pytest.reaction_event_fixture + timestamp = int(time.time()+1000) + signature = "bad timestamp" + + with pytest.raises(SlackEventAdapterException) as excinfo: + res = client.post( + '/slack/events', + data=data, + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } + ) + + assert str(excinfo.value) == 'Invalid request timestamp' + + +def test_compare_digest_fallback(client, monkeypatch): + # Verify [package metadata header is set + slack_adapter = SlackEventAdapter("SIGNING_SECRET") + + if hasattr(hmac, "compare_digest"): + monkeypatch.delattr(hmac, 'compare_digest') + data = pytest.reaction_event_fixture + timestamp = int(time.time()) + signature =pytest.create_signature(slack_adapter.signing_secret, timestamp, data) + res = client.post( '/slack/events', data=data, - content_type='application/json') + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } + ) + assert res.status_code == 200 def test_version_header(client): # Verify [package metadata header is set - package_info = SlackEventAdapter("token").server.package_info + slack_adapter = SlackEventAdapter("SIGNING_SECRET") + package_info = slack_adapter.server.package_info data = pytest.reaction_event_fixture + timestamp = int(time.time()) + signature = pytest.create_signature(slack_adapter.signing_secret, timestamp, data) + res = client.post( '/slack/events', data=data, - content_type='application/json') + content_type='application/json', + headers={ + 'X-Slack-Request-Timestamp': timestamp, + 'X-Slack-Signature': signature + } + ) assert res.status_code == 200 assert res.headers["X-Slack-Powered-By"] == package_info @@ -60,7 +140,14 @@ def test_version_header(client): def test_server_start(mocker): # Verify server started with correct params - slack_events_adapter = SlackEventAdapter("token") + slack_events_adapter = SlackEventAdapter("SIGNING_SECRET") mocker.spy(slack_events_adapter, 'server') slack_events_adapter.start(port=3000) slack_events_adapter.server.run.assert_called_once_with(debug=False, host='127.0.0.1', port=3000) + + +def test_default_exception_msg(mocker): + with pytest.raises(SlackEventAdapterException) as excinfo: + raise SlackEventAdapterException + + assert str(excinfo.value) == 'An error occurred in the SlackEventsApiAdapter library' diff --git a/tox.ini b/tox.ini index cedffa0..f012ea5 100644 --- a/tox.ini +++ b/tox.ini @@ -4,7 +4,7 @@ ; for quality analysis, use `tox -e flake8` or just `flake8 slackeventsapi` ; to build the docs, use `tox -e docs` envlist= - py{27,33,34,35,36}, + py{276,27,33,34,35,36}, flake8, docs