From 05842b5c182557901de7b0cf8b484313c3f0e7e7 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 11:08:52 +0100 Subject: [PATCH 1/6] Pull out make_signature to separate into function. --- fedireads/broadcast.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/fedireads/broadcast.py b/fedireads/broadcast.py index 6501137eb..ba0aaf3a6 100644 --- a/fedireads/broadcast.py +++ b/fedireads/broadcast.py @@ -63,37 +63,38 @@ def broadcast_task(sender_id, activity, recipients): return errors -def sign_and_send(sender, activity, destination): - ''' crpyto whatever and http junk ''' +def make_signature(sender, destination, date): inbox_parts = urlparse(destination) - now = http_date() signature_headers = [ '(request-target): post %s' % inbox_parts.path, 'host: %s' % inbox_parts.netloc, - 'date: %s' % now + 'date: %s' % date, ] message_to_sign = '\n'.join(signature_headers) - - if not sender.private_key: - # this shouldn't happen. it would be bad if it happened. - raise ValueError('No private key found for sender') signer = pkcs1_15.new(RSA.import_key(sender.private_key)) signed_message = signer.sign(SHA256.new(message_to_sign.encode('utf8'))) - signature = { 'keyId': '%s#main-key' % sender.actor, 'algorithm': 'rsa-sha256', 'headers': '(request-target) host date', 'signature': b64encode(signed_message).decode('utf8'), } - signature = ','.join('%s="%s"' % (k, v) for (k, v) in signature.items()) + return ','.join('%s="%s"' % (k, v) for (k, v) in signature.items()) + +def sign_and_send(sender, activity, destination): + ''' crpyto whatever and http junk ''' + now = http_date() + + if not sender.private_key: + # this shouldn't happen. it would be bad if it happened. + raise ValueError('No private key found for sender') response = requests.post( destination, data=json.dumps(activity), headers={ 'Date': now, - 'Signature': signature, + 'Signature': make_signature(sender, destination, now), 'Content-Type': 'application/activity+json; charset=utf-8', }, ) From 5d4076d628468391c5570fb92b995bf13f4ab1bc Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 11:09:47 +0100 Subject: [PATCH 2/6] Use public key from db if available. --- fedireads/incoming.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 580d978f5..bbe680486 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -82,6 +82,23 @@ def shared_inbox(request): return HttpResponse() +def get_public_key(key_id): + # TODO Use the anchor - actors can have multiple keys? + key_actor = key_id.split('#', 1)[0] + try: + public_key = models.User.objects.get(actor=key_actor).public_key + except models.User.DoesNotExist: + response = requests.get( + key_id, + headers={'Accept': 'application/activity+json'} + ) + if not response.ok: + raise ValueError('Could not load public key') + actor = response.json() + public_key = actor['publicKey']['publicKeyPem'] + + return RSA.import_key(public_key) + def verify_signature(request): ''' verify rsa signature ''' signature_dict = {} @@ -97,15 +114,7 @@ def verify_signature(request): except KeyError: raise ValueError('Invalid auth header') - response = requests.get( - key_id, - headers={'Accept': 'application/activity+json'} - ) - if not response.ok: - raise ValueError('Could not load public key') - - actor = response.json() - key = RSA.import_key(actor['publicKey']['publicKeyPem']) + key = get_public_key(key_id) comparison_string = [] for signed_header_name in headers.split(' '): From 10efe4d1b4893647677a3457aa6dfd833a2b0d63 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 11:18:48 +0100 Subject: [PATCH 3/6] Add test for use of the wrong signature. --- fedireads/tests/test_signing.py | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 fedireads/tests/test_signing.py diff --git a/fedireads/tests/test_signing.py b/fedireads/tests/test_signing.py new file mode 100644 index 000000000..9650f5527 --- /dev/null +++ b/fedireads/tests/test_signing.py @@ -0,0 +1,43 @@ +from urllib.parse import urlsplit + +from django.test import TestCase, Client +from django.utils.http import http_date + +from fedireads.models import User +from fedireads.broadcast import make_signature +from fedireads.activitypub import get_follow_request +from fedireads.settings import DOMAIN + +class Signature(TestCase): + def setUp(self): + self.mouse = User.objects.create_user('mouse', 'mouse@example.com', '') + self.rat = User.objects.create_user('rat', 'rat@example.com', '') + self.cat = User.objects.create_user('cat', 'cat@example.com', '') + + def test_wrong_signature(self): + ''' All messages must be signed by the right actor. + + (cat cannot sign messages on behalf of mouse) + ''' + activity = get_follow_request( + self.mouse, + self.rat, + ) + + now = http_date() + signature = make_signature(self.cat, self.rat.inbox, now) + + c = Client() + response = c.post( + urlsplit(self.rat.inbox).path, + data=activity, + content_type='application/json', + **{ + 'HTTP_DATE': now, + 'HTTP_SIGNATURE': signature, + 'HTTP_CONTENT_TYPE': 'application/activity+json; charset=utf-8', + 'HTTP_HOST': DOMAIN, + } + ) + + assert response.status_code == 401 From 2db4da4061da0e06d18636afffc133850948c5a4 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 11:40:57 +0100 Subject: [PATCH 4/6] Check all signatures are signed by the right actor. --- fedireads/incoming.py | 30 +++++++++++++++++------------ fedireads/tests/test_signing.py | 34 +++++++++++++++++---------------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/fedireads/incoming.py b/fedireads/incoming.py index bbe680486..6827e8ee5 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -1,6 +1,8 @@ ''' handles all of the activity coming in to the server ''' import json from base64 import b64decode +from urllib.parse import urldefrag + from Crypto.Hash import SHA256 from Crypto.PublicKey import RSA from Crypto.Signature import pkcs1_15 @@ -46,7 +48,7 @@ def shared_inbox(request): return HttpResponseBadRequest() try: - verify_signature(request) + verify_signature(activity.get('actor'), request) except ValueError: return HttpResponse(status=401) @@ -82,24 +84,24 @@ def shared_inbox(request): return HttpResponse() -def get_public_key(key_id): - # TODO Use the anchor - actors can have multiple keys? - key_actor = key_id.split('#', 1)[0] +def get_public_key(key_actor): try: - public_key = models.User.objects.get(actor=key_actor).public_key + user = models.User.objects.get(actor=key_actor) + public_key = user.public_key + actor = user.actor except models.User.DoesNotExist: response = requests.get( - key_id, + key_actor, headers={'Accept': 'application/activity+json'} ) if not response.ok: raise ValueError('Could not load public key') - actor = response.json() - public_key = actor['publicKey']['publicKeyPem'] + user_data = response.json() + public_key = user_data['publicKey']['publicKeyPem'] return RSA.import_key(public_key) -def verify_signature(request): +def verify_signature(required_actor, request): ''' verify rsa signature ''' signature_dict = {} for pair in request.headers['Signature'].split(','): @@ -114,7 +116,13 @@ def verify_signature(request): except KeyError: raise ValueError('Invalid auth header') - key = get_public_key(key_id) + # TODO Use the fragment - actors can have multiple keys? + key_actor = urldefrag(key_id).url + + if key_actor != required_actor: + raise ValueError("Wrong actor created signature.") + + key = get_public_key(key_actor) comparison_string = [] for signed_header_name in headers.split(' '): @@ -134,8 +142,6 @@ def verify_signature(request): # raises a ValueError if it fails signer.verify(digest, signature) - return True - @app.task def handle_follow(activity): diff --git a/fedireads/tests/test_signing.py b/fedireads/tests/test_signing.py index 9650f5527..f4da6033c 100644 --- a/fedireads/tests/test_signing.py +++ b/fedireads/tests/test_signing.py @@ -14,23 +14,14 @@ class Signature(TestCase): self.rat = User.objects.create_user('rat', 'rat@example.com', '') self.cat = User.objects.create_user('cat', 'cat@example.com', '') - def test_wrong_signature(self): - ''' All messages must be signed by the right actor. - - (cat cannot sign messages on behalf of mouse) - ''' - activity = get_follow_request( - self.mouse, - self.rat, - ) - - now = http_date() - signature = make_signature(self.cat, self.rat.inbox, now) - + def send_follow(self, signature, now): c = Client() - response = c.post( + return c.post( urlsplit(self.rat.inbox).path, - data=activity, + data=get_follow_request( + self.mouse, + self.rat, + ), content_type='application/json', **{ 'HTTP_DATE': now, @@ -40,4 +31,15 @@ class Signature(TestCase): } ) - assert response.status_code == 401 + def test_correct_signature(self): + now = http_date() + signature = make_signature(self.mouse, self.rat.inbox, now) + return self.send_follow(signature, now).status_code == 200 + + def test_wrong_signature(self): + ''' Messages must be signed by the right actor. + (cat cannot sign messages on behalf of mouse) + ''' + now = http_date() + signature = make_signature(self.cat, self.rat.inbox, now) + assert self.send_follow(signature, now).status_code == 401 From 3236e95ea29c71b5b3411601b4393cb12a896ea9 Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 11:53:25 +0100 Subject: [PATCH 5/6] Fix typo in requirements.txt --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 8dd653e6f..262b6e28a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ celery==4.4.2 -coverage=5.1 +coverage==5.1 Django==3.0.3 django-model-utils==4.0.0 environs==7.2.0 From 4974fe4e5b461ab95547ca57bfe9267479e7d27d Mon Sep 17 00:00:00 2001 From: Adam Kelly Date: Wed, 13 May 2020 12:26:07 +0100 Subject: [PATCH 6/6] Add tests for remote actors (fetch keys via http.) --- fedireads/tests/test_signing.py | 55 ++++++++++++++++++++++++++++++--- requirements.txt | 1 + 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/fedireads/tests/test_signing.py b/fedireads/tests/test_signing.py index f4da6033c..6cb69f16f 100644 --- a/fedireads/tests/test_signing.py +++ b/fedireads/tests/test_signing.py @@ -1,5 +1,11 @@ +from collections import namedtuple from urllib.parse import urlsplit +from Crypto import Random +from Crypto.PublicKey import RSA + +import responses + from django.test import TestCase, Client from django.utils.http import http_date @@ -8,18 +14,31 @@ from fedireads.broadcast import make_signature from fedireads.activitypub import get_follow_request from fedireads.settings import DOMAIN +Sender = namedtuple('Sender', ('actor', 'private_key', 'public_key')) + class Signature(TestCase): def setUp(self): self.mouse = User.objects.create_user('mouse', 'mouse@example.com', '') self.rat = User.objects.create_user('rat', 'rat@example.com', '') self.cat = User.objects.create_user('cat', 'cat@example.com', '') - def send_follow(self, signature, now): + random_generator = Random.new().read + key = RSA.generate(1024, random_generator) + private_key = key.export_key().decode('utf8') + public_key = key.publickey().export_key().decode('utf8') + + self.fake_remote = Sender( + 'http://localhost/user/remote', + private_key, + public_key, + ) + + def send_follow(self, sender, signature, now): c = Client() return c.post( urlsplit(self.rat.inbox).path, data=get_follow_request( - self.mouse, + sender, self.rat, ), content_type='application/json', @@ -34,7 +53,7 @@ class Signature(TestCase): def test_correct_signature(self): now = http_date() signature = make_signature(self.mouse, self.rat.inbox, now) - return self.send_follow(signature, now).status_code == 200 + return self.send_follow(self.mouse, signature, now).status_code == 200 def test_wrong_signature(self): ''' Messages must be signed by the right actor. @@ -42,4 +61,32 @@ class Signature(TestCase): ''' now = http_date() signature = make_signature(self.cat, self.rat.inbox, now) - assert self.send_follow(signature, now).status_code == 401 + assert self.send_follow(self.mouse, signature, now).status_code == 401 + + @responses.activate + def test_remote_signer(self): + responses.add( + responses.GET, + self.fake_remote.actor, + json={'publicKey': { + 'publicKeyPem': self.fake_remote.public_key + }}, + status=200) + + now = http_date() + sender = self.fake_remote + signature = make_signature(sender, self.rat.inbox, now) + assert self.send_follow(sender, signature, now).status_code == 200 + + @responses.activate + def test_nonexistent_signer(self): + responses.add( + responses.GET, + self.fake_remote.actor, + json={'error': 'not found'}, + status=404) + + now = http_date() + sender = self.fake_remote + signature = make_signature(sender, self.rat.inbox, now) + assert self.send_follow(sender, signature, now).status_code == 401 diff --git a/requirements.txt b/requirements.txt index 262b6e28a..7c386d2fd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,3 +10,4 @@ pycryptodome==3.9.4 python-dateutil==2.8.1 redis==3.4.1 requests==2.22.0 +responses==0.10.14