From 104bdc625da7cbaa7e0f7de7199d1834dc2fa8f5 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Thu, 14 May 2020 14:29:41 +0900 Subject: [PATCH 1/2] Add SignatureVerifier for request verification --- slack/signature/__init__.py | 1 + slack/signature/verifier.py | 47 ++++++++++++++++++++ slack/web/base_client.py | 12 ++++-- tests/signature/test_signature_verifier.py | 50 ++++++++++++++++++++++ 4 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 slack/signature/__init__.py create mode 100644 slack/signature/verifier.py create mode 100644 tests/signature/test_signature_verifier.py diff --git a/slack/signature/__init__.py b/slack/signature/__init__.py new file mode 100644 index 000000000..21b38c0fc --- /dev/null +++ b/slack/signature/__init__.py @@ -0,0 +1 @@ +from .verifier import SignatureVerifier # noqa diff --git a/slack/signature/verifier.py b/slack/signature/verifier.py new file mode 100644 index 000000000..04b5bff9e --- /dev/null +++ b/slack/signature/verifier.py @@ -0,0 +1,47 @@ +import hashlib +import hmac +from time import time +from typing import Dict + + +class Clock: + def now(self) -> float: + return time() + + +class SignatureVerifier: + def __init__(self, signing_secret: str, clock: Clock = Clock()): + """Slack request signature verifier + + Slack signs its requests using a secret that's unique to your app. + With the help of signing secrets, your app can more confidently verify + whether requests from us are authentic. + https://api.slack.com/authentication/verifying-requests-from-slack + """ + self.signing_secret = signing_secret + self.clock = clock + + def is_valid_request(self, body: str, headers: Dict[str, str],) -> bool: + """Verifies if the given signature is valid""" + normalized_headers = {k.lower(): v for k, v in headers.items()} + return self.is_valid( + body=body, + timestamp=normalized_headers.get("x-slack-request-timestamp", None), + signature=normalized_headers.get("x-slack-signature", None), + ) + + def is_valid(self, body: str, timestamp: str, signature: str,) -> bool: + """Verifies if the given signature is valid""" + if abs(self.clock.now() - int(timestamp)) > 60 * 5: + return False + + calculated_signature = self.generate_signature(timestamp=timestamp, body=body) + return hmac.compare_digest(calculated_signature, signature) + + def generate_signature(self, *, timestamp: str, body: str) -> str: + """Generates a signature""" + format_req = str.encode(f"v0:{timestamp}:{body}") + encoded_secret = str.encode(self.signing_secret) + request_hash = hmac.new(encoded_secret, format_req, hashlib.sha256).hexdigest() + calculated_signature = f"v0={request_hash}" + return calculated_signature diff --git a/slack/web/base_client.py b/slack/web/base_client.py index 2467c8a88..f3991d8b8 100644 --- a/slack/web/base_client.py +++ b/slack/web/base_client.py @@ -6,7 +6,6 @@ import hmac import io import json -import json as json_module import logging import mimetypes import os @@ -15,8 +14,8 @@ import uuid import warnings from http.client import HTTPResponse -from typing import BinaryIO, Dict, List, Union -from typing import Optional +from typing import BinaryIO, Dict, List +from typing import Optional, Union from urllib.error import HTTPError from urllib.parse import urlencode from urllib.parse import urljoin @@ -333,7 +332,7 @@ def _request_for_pagination(self, api_url, req_args) -> Dict[str, any]: return { "status_code": int(response["status"]), "headers": dict(response["headers"]), - "data": json_module.loads(response["body"]), + "data": json.loads(response["body"]), } def _urllib_api_call( @@ -613,6 +612,11 @@ def validate_slack_signature( Returns: True if signatures matches """ + warnings.warn( + "As this method is deprecated since slackclient 2.6.0, " + "use `from slack.signature import SignatureVerifier` instead", + DeprecationWarning, + ) format_req = str.encode(f"v0:{timestamp}:{data}") encoded_secret = str.encode(signing_secret) request_hash = hmac.new(encoded_secret, format_req, hashlib.sha256).hexdigest() diff --git a/tests/signature/test_signature_verifier.py b/tests/signature/test_signature_verifier.py new file mode 100644 index 000000000..d28760e6c --- /dev/null +++ b/tests/signature/test_signature_verifier.py @@ -0,0 +1,50 @@ +import unittest + +from slack.signature import SignatureVerifier + + +class MockClock: + def now(self) -> float: + return 1531420618 + + +class TestSignatureVerifier(unittest.TestCase): + def setUp(self): + pass + + def tearDown(self): + pass + + body = "token=xyzz0WbapA4vBCDEFasx0q6G&team_id=T1DC2JH3J&team_domain=testteamnow&channel_id=G8PSS9T3V&channel_name=foobar&user_id=U2CERLKJA&user_name=roadrunner&command=%2Fwebhook-collect&text=&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT1DC2JH3J%2F397700885554%2F96rGlfmibIGlgcZRskXaIFfN&trigger_id=398738663015.47445629121.803a0bc887a14d10d2c447fce8b6703c" + headers = { + "X-Slack-Request-Timestamp": "1531420618", + "X-Slack-Signature": "v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503", + } + + def test_generate_signature(self): + # https://api.slack.com/authentication/verifying-requests-from-slack + verifier = SignatureVerifier("8f742231b10e8888abcd99yyyzzz85a5") + timestamp = "1531420618" + signature = verifier.generate_signature(timestamp=timestamp, body=self.body) + self.assertEqual("v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503", signature) + + def test_is_valid_request(self): + verifier = SignatureVerifier( + signing_secret="8f742231b10e8888abcd99yyyzzz85a5", + clock=MockClock() + ) + self.assertTrue(verifier.is_valid_request(self.body, self.headers)) + + def test_is_valid_request_invalid_body(self): + verifier = SignatureVerifier( + signing_secret="8f742231b10e8888abcd99yyyzzz85a5", + clock=MockClock() + ) + modified_body = self.body + "------" + self.assertFalse(verifier.is_valid_request(modified_body, self.headers)) + + def test_is_valid_request_expiration(self): + verifier = SignatureVerifier( + signing_secret="8f742231b10e8888abcd99yyyzzz85a5" + ) + self.assertFalse(verifier.is_valid_request(self.body, self.headers)) From c352e8867a4ac2ee9222a5e7d8ece6eb42e3d020 Mon Sep 17 00:00:00 2001 From: Kazuhiro Sera Date: Fri, 15 May 2020 08:23:07 +0900 Subject: [PATCH 2/2] Add more tests and check for None values --- slack/signature/verifier.py | 17 ++++++- tests/signature/test_signature_verifier.py | 52 ++++++++++++++++++---- 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/slack/signature/verifier.py b/slack/signature/verifier.py index 04b5bff9e..6f8bcf681 100644 --- a/slack/signature/verifier.py +++ b/slack/signature/verifier.py @@ -1,7 +1,7 @@ import hashlib import hmac from time import time -from typing import Dict +from typing import Dict, Optional class Clock: @@ -23,6 +23,8 @@ def __init__(self, signing_secret: str, clock: Clock = Clock()): def is_valid_request(self, body: str, headers: Dict[str, str],) -> bool: """Verifies if the given signature is valid""" + if headers is None: + return False normalized_headers = {k.lower(): v for k, v in headers.items()} return self.is_valid( body=body, @@ -32,14 +34,25 @@ def is_valid_request(self, body: str, headers: Dict[str, str],) -> bool: def is_valid(self, body: str, timestamp: str, signature: str,) -> bool: """Verifies if the given signature is valid""" + if timestamp is None or signature is None: + return False + if abs(self.clock.now() - int(timestamp)) > 60 * 5: return False + if body is None: + body = "" + calculated_signature = self.generate_signature(timestamp=timestamp, body=body) + if calculated_signature is None: + return False return hmac.compare_digest(calculated_signature, signature) - def generate_signature(self, *, timestamp: str, body: str) -> str: + def generate_signature(self, *, timestamp: str, body: str) -> Optional[str]: """Generates a signature""" + if timestamp is None: + return None + format_req = str.encode(f"v0:{timestamp}:{body}") encoded_secret = str.encode(self.signing_secret) request_hash = hmac.new(encoded_secret, format_req, hashlib.sha256).hexdigest() diff --git a/tests/signature/test_signature_verifier.py b/tests/signature/test_signature_verifier.py index d28760e6c..8015733e6 100644 --- a/tests/signature/test_signature_verifier.py +++ b/tests/signature/test_signature_verifier.py @@ -15,36 +15,72 @@ def setUp(self): def tearDown(self): pass + # https://api.slack.com/authentication/verifying-requests-from-slack + signing_secret = "8f742231b10e8888abcd99yyyzzz85a5" + body = "token=xyzz0WbapA4vBCDEFasx0q6G&team_id=T1DC2JH3J&team_domain=testteamnow&channel_id=G8PSS9T3V&channel_name=foobar&user_id=U2CERLKJA&user_name=roadrunner&command=%2Fwebhook-collect&text=&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT1DC2JH3J%2F397700885554%2F96rGlfmibIGlgcZRskXaIFfN&trigger_id=398738663015.47445629121.803a0bc887a14d10d2c447fce8b6703c" + + timestamp = "1531420618" + valid_signature = "v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503" + headers = { - "X-Slack-Request-Timestamp": "1531420618", - "X-Slack-Signature": "v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503", + "X-Slack-Request-Timestamp": timestamp, + "X-Slack-Signature": valid_signature, } def test_generate_signature(self): - # https://api.slack.com/authentication/verifying-requests-from-slack verifier = SignatureVerifier("8f742231b10e8888abcd99yyyzzz85a5") timestamp = "1531420618" signature = verifier.generate_signature(timestamp=timestamp, body=self.body) - self.assertEqual("v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503", signature) + self.assertEqual(self.valid_signature, signature) def test_is_valid_request(self): verifier = SignatureVerifier( - signing_secret="8f742231b10e8888abcd99yyyzzz85a5", + signing_secret=self.signing_secret, clock=MockClock() ) self.assertTrue(verifier.is_valid_request(self.body, self.headers)) def test_is_valid_request_invalid_body(self): verifier = SignatureVerifier( - signing_secret="8f742231b10e8888abcd99yyyzzz85a5", - clock=MockClock() + signing_secret=self.signing_secret, + clock=MockClock(), ) modified_body = self.body + "------" self.assertFalse(verifier.is_valid_request(modified_body, self.headers)) def test_is_valid_request_expiration(self): verifier = SignatureVerifier( - signing_secret="8f742231b10e8888abcd99yyyzzz85a5" + signing_secret=self.signing_secret, ) self.assertFalse(verifier.is_valid_request(self.body, self.headers)) + + def test_is_valid_request_none(self): + verifier = SignatureVerifier( + signing_secret=self.signing_secret, + clock=MockClock(), + ) + self.assertFalse(verifier.is_valid_request(None, self.headers)) + self.assertFalse(verifier.is_valid_request(self.body, None)) + self.assertFalse(verifier.is_valid_request(None, None)) + + def test_is_valid(self): + verifier = SignatureVerifier( + signing_secret=self.signing_secret, + clock=MockClock(), + ) + self.assertTrue(verifier.is_valid(self.body, self.timestamp, self.valid_signature)) + self.assertTrue(verifier.is_valid(self.body, 1531420618, self.valid_signature)) + + def test_is_valid_none(self): + verifier = SignatureVerifier( + signing_secret=self.signing_secret, + clock=MockClock(), + ) + self.assertFalse(verifier.is_valid(None, self.timestamp, self.valid_signature)) + self.assertFalse(verifier.is_valid(self.body, None, self.valid_signature)) + self.assertFalse(verifier.is_valid(self.body, self.timestamp, None)) + self.assertFalse(verifier.is_valid(None, None, self.valid_signature)) + self.assertFalse(verifier.is_valid(None, self.timestamp, None)) + self.assertFalse(verifier.is_valid(self.body, None, None)) + self.assertFalse(verifier.is_valid(None, None, None))