From 969a5f1f7b7d4c7461ca11456de4e7e006d9c860 Mon Sep 17 00:00:00 2001 From: TAG-Epic Date: Sun, 23 Jul 2023 02:58:12 +0200 Subject: [PATCH 1/5] feat(endpoint/interactions): basic request verifier --- nextcore/endpoint/__init__.py | 24 ++++++ .../endpoint/interactions/request_verifier.py | 84 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 nextcore/endpoint/__init__.py create mode 100644 nextcore/endpoint/interactions/request_verifier.py diff --git a/nextcore/endpoint/__init__.py b/nextcore/endpoint/__init__.py new file mode 100644 index 000000000..4a5c5bf6b --- /dev/null +++ b/nextcore/endpoint/__init__.py @@ -0,0 +1,24 @@ +# The MIT License (MIT) +# Copyright (c) 2021-present tag-epic +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +"""Wrapper around setting up your own webhook server and getting Discord to make requests to it.""" + +# NOTE: This is called endpoint and not webhook because of potential confusion between this and channel webhooks. diff --git a/nextcore/endpoint/interactions/request_verifier.py b/nextcore/endpoint/interactions/request_verifier.py new file mode 100644 index 000000000..e576de776 --- /dev/null +++ b/nextcore/endpoint/interactions/request_verifier.py @@ -0,0 +1,84 @@ +# The MIT License (MIT) +# Copyright (c) 2021-present tag-epic +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +# OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +# DEALINGS IN THE SOFTWARE. + +from __future__ import annotations +from nacl.signing import VerifyKey +from nacl.exceptions import BadSignatureError +from typing import TYPE_CHECKING +from logging import getLogger + +if TYPE_CHECKING: + from typing import Final + +logger = getLogger(__name__) + +class RequestVerifier: + """Helper class to verify requests from Discord + + Parameters + ---------- + public_key: + The public key from the developer portal. This can be a hex :class:`str`, or the hex :class:`str` converted to bytes. + + This will be converted to :class:`bytes` under the hood. + """ + + __slots__: Final[tuple[str, ...]] = ("verify_key",) + + def __init__(self, public_key: str | bytes) -> None: + if isinstance(public_key, str): + public_key = bytes.fromhex(public_key) + + self.verify_key: Final[VerifyKey] = VerifyKey(public_key) + + def is_valid(self, signature: str | bytes, timestamp: str, body: str) -> bool: + """Check if a request was made by Discord + + Parameters + ---------- + signature: + The signature from the ``X-Signature-Ed25519`` header. This can either be the raw header (a hex encoded :class:`str`) or it converted to :class:`bytes`. + + Returns + ------- + bool + If the signature was made from Discord. + + If this is :data:`False` you should reject the request with a ``401`` status code. + """ + + if isinstance(signature, str): + try: + signature = bytes.fromhex(signature) + except ValueError: + logger.debug("Request did not pass check because of signature not being valid HEX") + return False + + payload = f"{timestamp}{body}".encode() + try: + self.verify_key.verify(payload, signature) + except BadSignatureError: + logger.debug("Request did not pass check because of verify_key.verify") + return False + except ValueError: + logger.debug("Request did not pass check because of ValueError. This is probably due to wrong length") + + return True From 1b97f67cdd1971f9b805e8b513cd602095fde8ab Mon Sep 17 00:00:00 2001 From: TAG-Epic Date: Sun, 23 Jul 2023 02:59:55 +0200 Subject: [PATCH 2/5] fix(endpoint/interactions): auth bypass --- nextcore/endpoint/interactions/request_verifier.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nextcore/endpoint/interactions/request_verifier.py b/nextcore/endpoint/interactions/request_verifier.py index e576de776..71e42c3ca 100644 --- a/nextcore/endpoint/interactions/request_verifier.py +++ b/nextcore/endpoint/interactions/request_verifier.py @@ -80,5 +80,6 @@ def is_valid(self, signature: str | bytes, timestamp: str, body: str) -> bool: return False except ValueError: logger.debug("Request did not pass check because of ValueError. This is probably due to wrong length") + return False return True From 3990879081efaad98324c787e1545eed721e60b0 Mon Sep 17 00:00:00 2001 From: TAG-Epic Date: Sun, 23 Jul 2023 03:38:15 +0200 Subject: [PATCH 3/5] test(endpoint/interactions): basic tests --- .../interactions/test_request_verifier.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/endpoint/interactions/test_request_verifier.py diff --git a/tests/endpoint/interactions/test_request_verifier.py b/tests/endpoint/interactions/test_request_verifier.py new file mode 100644 index 000000000..2976a781d --- /dev/null +++ b/tests/endpoint/interactions/test_request_verifier.py @@ -0,0 +1,86 @@ +from nextcore.endpoint.interactions.request_verifier import RequestVerifier +from nacl.signing import SigningKey +from nacl.utils import random +from datetime import datetime +from pytest import mark + +def test_works(): + timestamp = str(datetime.now()) + body = "Hello, world" + + secret = random() + signing_key = SigningKey(secret) + signed_message = signing_key.sign(f"{timestamp}{body}".encode()) + signature = signed_message.signature.hex() + + public_key = signing_key.verify_key.encode() + request_verifier = RequestVerifier(public_key) + + assert request_verifier.is_valid(signature, timestamp, body), "Was marked as invalid" + +def test_modified_body(): + timestamp = str(datetime.now()) + body = "Hello, world" + + secret = random() + signing_key = SigningKey(secret) + signed_message = signing_key.sign(f"{timestamp}{body}".encode()) + signature = signed_message.signature.hex() + + public_key = signing_key.verify_key.encode() + request_verifier = RequestVerifier(public_key) + + body = body.replace("world", "space") + + assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though body was modified" + +def test_modified_timestamp(): + timestamp = str(datetime.now()) + body = "Hello, world" + + secret = random() + signing_key = SigningKey(secret) + signed_message = signing_key.sign(f"{timestamp}{body}".encode()) + signature = signed_message.signature.hex() + + public_key = signing_key.verify_key.encode() + request_verifier = RequestVerifier(public_key) + + timestamp = "".join(reversed(timestamp)) + + assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though timestamp was modified" + + +def test_modified_signature(): + timestamp = str(datetime.now()) + body = "Hello, world" + + secret = random() + signing_key = SigningKey(secret) + signed_message = signing_key.sign(f"{timestamp}{body}".encode()) + signature = signed_message.signature.hex() + + public_key = signing_key.verify_key.encode() + request_verifier = RequestVerifier(public_key) + + signature = "0x" + "".join(reversed(signature.replace("0x", ""))) + + assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though signature was modified" + + +@mark.parametrize("signature", [ + (bytes([0x0])), + (bytes([0xFF])), + (bytes([0xFF]*50)), +]) +def test_malformed_signature(signature: bytes): + timestamp = str(datetime.now()) + body = "Hello, world" + + secret = random() + signing_key = SigningKey(secret) + + public_key = signing_key.verify_key.encode() + request_verifier = RequestVerifier(public_key) + + assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though signature is wrong" From 2a65503890c65b554a3c437064236bb5aba5cbf4 Mon Sep 17 00:00:00 2001 From: TAG-Epic Date: Sun, 23 Jul 2023 03:41:02 +0200 Subject: [PATCH 4/5] build: add pynacl dependency --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index b30d5e3ea..14f208d6e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ typing-extensions = "^4.1.1" # Same as above orjson = {version = "^3.6.8", optional = true} types-orjson = {version = "^3.6.2", optional = true} discord-typings = "^0.5.0" +pynacl = "^1.5.0" [tool.poetry.group.dev.dependencies] Sphinx = "^5.0.0" From 4a8917fb150edcb702422fece6972f336ed34950 Mon Sep 17 00:00:00 2001 From: TAG-Epic Date: Sun, 23 Jul 2023 03:43:07 +0200 Subject: [PATCH 5/5] style: lint --- .../endpoint/interactions/request_verifier.py | 11 ++-- .../interactions/test_request_verifier.py | 52 ++++++++++++------- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/nextcore/endpoint/interactions/request_verifier.py b/nextcore/endpoint/interactions/request_verifier.py index 71e42c3ca..f5d14d43d 100644 --- a/nextcore/endpoint/interactions/request_verifier.py +++ b/nextcore/endpoint/interactions/request_verifier.py @@ -20,16 +20,19 @@ # DEALINGS IN THE SOFTWARE. from __future__ import annotations -from nacl.signing import VerifyKey -from nacl.exceptions import BadSignatureError -from typing import TYPE_CHECKING + from logging import getLogger +from typing import TYPE_CHECKING + +from nacl.exceptions import BadSignatureError +from nacl.signing import VerifyKey if TYPE_CHECKING: from typing import Final logger = getLogger(__name__) + class RequestVerifier: """Helper class to verify requests from Discord @@ -64,7 +67,7 @@ def is_valid(self, signature: str | bytes, timestamp: str, body: str) -> bool: If this is :data:`False` you should reject the request with a ``401`` status code. """ - + if isinstance(signature, str): try: signature = bytes.fromhex(signature) diff --git a/tests/endpoint/interactions/test_request_verifier.py b/tests/endpoint/interactions/test_request_verifier.py index 2976a781d..213b763aa 100644 --- a/tests/endpoint/interactions/test_request_verifier.py +++ b/tests/endpoint/interactions/test_request_verifier.py @@ -1,9 +1,12 @@ -from nextcore.endpoint.interactions.request_verifier import RequestVerifier +from datetime import datetime + from nacl.signing import SigningKey from nacl.utils import random -from datetime import datetime from pytest import mark +from nextcore.endpoint.interactions.request_verifier import RequestVerifier + + def test_works(): timestamp = str(datetime.now()) body = "Hello, world" @@ -12,12 +15,13 @@ def test_works(): signing_key = SigningKey(secret) signed_message = signing_key.sign(f"{timestamp}{body}".encode()) signature = signed_message.signature.hex() - + public_key = signing_key.verify_key.encode() request_verifier = RequestVerifier(public_key) assert request_verifier.is_valid(signature, timestamp, body), "Was marked as invalid" + def test_modified_body(): timestamp = str(datetime.now()) body = "Hello, world" @@ -26,13 +30,16 @@ def test_modified_body(): signing_key = SigningKey(secret) signed_message = signing_key.sign(f"{timestamp}{body}".encode()) signature = signed_message.signature.hex() - + public_key = signing_key.verify_key.encode() request_verifier = RequestVerifier(public_key) body = body.replace("world", "space") - assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though body was modified" + assert not request_verifier.is_valid( + signature, timestamp, body + ), "Was marked as valid even though body was modified" + def test_modified_timestamp(): timestamp = str(datetime.now()) @@ -42,13 +49,15 @@ def test_modified_timestamp(): signing_key = SigningKey(secret) signed_message = signing_key.sign(f"{timestamp}{body}".encode()) signature = signed_message.signature.hex() - + public_key = signing_key.verify_key.encode() request_verifier = RequestVerifier(public_key) - + timestamp = "".join(reversed(timestamp)) - - assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though timestamp was modified" + + assert not request_verifier.is_valid( + signature, timestamp, body + ), "Was marked as valid even though timestamp was modified" def test_modified_signature(): @@ -59,28 +68,35 @@ def test_modified_signature(): signing_key = SigningKey(secret) signed_message = signing_key.sign(f"{timestamp}{body}".encode()) signature = signed_message.signature.hex() - + public_key = signing_key.verify_key.encode() request_verifier = RequestVerifier(public_key) signature = "0x" + "".join(reversed(signature.replace("0x", ""))) - assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though signature was modified" + assert not request_verifier.is_valid( + signature, timestamp, body + ), "Was marked as valid even though signature was modified" -@mark.parametrize("signature", [ - (bytes([0x0])), - (bytes([0xFF])), - (bytes([0xFF]*50)), -]) +@mark.parametrize( + "signature", + [ + (bytes([0x0])), + (bytes([0xFF])), + (bytes([0xFF] * 50)), + ], +) def test_malformed_signature(signature: bytes): timestamp = str(datetime.now()) body = "Hello, world" secret = random() signing_key = SigningKey(secret) - + public_key = signing_key.verify_key.encode() request_verifier = RequestVerifier(public_key) - assert not request_verifier.is_valid(signature, timestamp, body), "Was marked as valid even though signature is wrong" + assert not request_verifier.is_valid( + signature, timestamp, body + ), "Was marked as valid even though signature is wrong"