mirror of
https://github.com/bookwyrm-social/bookwyrm.git
synced 2024-10-31 22:19:00 +00:00
Don't assume user id is key id minus fragment
Fixes #2801 Related to #2794 It is legitimate to use any url for the user's key id. We have been assuming this id is the user id plus a fragment (#key-id) but this is not always the case, notably in the case of GoToSocial it is at /key-id. This commit instead checks the remote user's information to see if the key id listed matches the key id of the message allegedly received from them. Whilst troubleshooting this it also became apparent that there is a mismatch between Bookwyrm users' keyId and the KeyId we claim to be using in signed requests (there is a forward slash missing). Since everything after the slash is a fragment, this usually slips through but we should be consistent so I updated that.
This commit is contained in:
parent
912d0a0149
commit
632e3844b9
3 changed files with 7 additions and 8 deletions
|
@ -4,7 +4,7 @@ import sys
|
|||
|
||||
from .base_activity import ActivityEncoder, Signature, naive_parse
|
||||
from .base_activity import Link, Mention, Hashtag
|
||||
from .base_activity import ActivitySerializerError, resolve_remote_id
|
||||
from .base_activity import ActivitySerializerError, resolve_remote_id, get_activitypub_data
|
||||
from .image import Document, Image
|
||||
from .note import Note, GeneratedNote, Article, Comment, Quotation
|
||||
from .note import Review, Rating
|
||||
|
|
|
@ -38,8 +38,9 @@ def make_signature(method, sender, destination, date, digest=None):
|
|||
message_to_sign = "\n".join(signature_headers)
|
||||
signer = pkcs1_15.new(RSA.import_key(sender.key_pair.private_key))
|
||||
signed_message = signer.sign(SHA256.new(message_to_sign.encode("utf8")))
|
||||
# TODO: this should be /#main-key to match our user key ids, even though that was probably a mistake in hindsight.
|
||||
signature = {
|
||||
"keyId": f"{sender.remote_id}#main-key",
|
||||
"keyId": f"{sender.remote_id}/#main-key",
|
||||
"algorithm": "rsa-sha256",
|
||||
"headers": headers,
|
||||
"signature": b64encode(signed_message).decode("utf8"),
|
||||
|
|
|
@ -130,15 +130,13 @@ def has_valid_signature(request, activity):
|
|||
"""verify incoming signature"""
|
||||
try:
|
||||
signature = Signature.parse(request)
|
||||
|
||||
key_actor = urldefrag(signature.key_id).url
|
||||
if key_actor != activity.get("actor"):
|
||||
raise ValueError("Wrong actor created signature.")
|
||||
|
||||
remote_user = activitypub.resolve_remote_id(key_actor, model=models.User)
|
||||
remote_user = activitypub.resolve_remote_id(activity.get("actor"), model=models.User)
|
||||
if not remote_user:
|
||||
return False
|
||||
|
||||
if signature.key_id != remote_user.key_pair.remote_id:
|
||||
raise ValueError("Wrong actor created signature.")
|
||||
|
||||
try:
|
||||
signature.verify(remote_user.key_pair.public_key, request)
|
||||
except ValueError:
|
||||
|
|
Loading…
Reference in a new issue