diff --git a/synapse/api/constants.py b/synapse/api/constants.py index f2c7fa6a81..103729d880 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -356,7 +356,18 @@ class MemberKeys: SIGNATURE_ENTITY: Final = "sender" """The entity name under which the sender's member key signs an event. (The authorising member key of a restricted join signs under the member key string - itself, and a server co-signing linkage information signs under its name.)""" + itself.)""" + + LINKAGE_SIGNATURE_ENTITY: Final = "state_key" + """The entity name under which a server co-signs the linkage information of an + `m.room.member` event: the claim that the member key in `state_key` belongs to + `content.localpart` at `content.domain`. Naming the entity for the member it is + about, rather than for the server making the claim, is what lets a third party + tell an assertion about the target from one about the sender: an invite is sent + by one member key and co-signed by the *invitee's* server. The signature is made + with the server's own key, so the key ID is a real server key ID rather than the + member keys' constant `ed25519:1`, and names the published key to check it + against.""" KEY_ALGORITHM: Final = "ed25519" KEY_VERSION: Final = "1" diff --git a/synapse/crypto/event_signing.py b/synapse/crypto/event_signing.py index 401f8fc8c1..4e99395ae7 100644 --- a/synapse/crypto/event_signing.py +++ b/synapse/crypto/event_signing.py @@ -200,8 +200,10 @@ def add_hashes_and_signatures_from_signers( * A member key string (the authorising member of a restricted join, MSC-Q §2): the event must be a join naming that key in `join_authorised_via_users_server`, and the key must be that member key. - * Our own server name (`server_name`), co-signing linkage information: the event - must be an `m.room.member` event whose `content.domain` is us. + * `MemberKeys.LINKAGE_SIGNATURE_ENTITY` (`state_key`), co-signing the linkage + information of the member named by `state_key`: the event must be an + `m.room.member` event whose `content.domain` is us, and the key must be our + server signing key. Anything else is refused. To add one signature to an event that is already hashed (one received from another server), use @@ -322,9 +324,14 @@ def _check_member_key_room_signer( ) return - # Our server name: the co-signature on linkage information. - if signature_name != server_name: - raise refuse("not this server's name (%s)" % (server_name,)) + # The co-signature on linkage information. The entity is the constant + # `state_key`, naming the member the claim is about; the claim itself is + # `content.domain`, which must be us, since this server signs for nobody else. + if signature_name != MemberKeys.LINKAGE_SIGNATURE_ENTITY: + raise refuse( + "not a signature this event may carry (expected %s)" + % (MemberKeys.LINKAGE_SIGNATURE_ENTITY,) + ) if event_dict.get("type") != EventTypes.Member: raise refuse("servers only co-sign m.room.member events") if content.get(EventContentFields.MSC4430_DOMAIN) != server_name: diff --git a/synapse/handlers/member_keys.py b/synapse/handlers/member_keys.py index f159997313..2328b777da 100644 --- a/synapse/handlers/member_keys.py +++ b/synapse/handlers/member_keys.py @@ -544,10 +544,11 @@ class MemberKeyHandler: """Who signs an event we are building in a member key room: * the sender's member key, under the entity `sender`, if we hold it; - * our server signing key on an `m.room.member` event whose target is a - key we hold and whose linkage information is exactly what we vouch for - (our domain, and the user's localpart unless they are erased): that is - what lets other servers verify the member's link; + * our server signing key, under the entity `state_key`, on an + `m.room.member` event whose target is a key we hold and whose linkage + information is exactly what we vouch for (our domain, and the user's + localpart unless they are erased): that is what lets other servers + verify the member's link; * the authorising member's key, under its own name, on a restricted join that names a key we hold. @@ -583,7 +584,9 @@ class MemberKeyHandler: if target_user_id is not None and await self._is_our_linkage( target_user_id, content ): - signers.append((self.server_name, self._server_signing_key)) + signers.append( + (MemberKeys.LINKAGE_SIGNATURE_ENTITY, self._server_signing_key) + ) authorising_member = content.get(EventContentFields.AUTHORISING_USER) if ( diff --git a/tests/crypto/test_event_signing.py b/tests/crypto/test_event_signing.py index a661d0593c..1b7e265bd9 100644 --- a/tests/crypto/test_event_signing.py +++ b/tests/crypto/test_event_signing.py @@ -339,23 +339,34 @@ class MemberKeySigningTestCase(unittest.TestCase): ) def test_server_cosignature(self) -> None: + """The server co-signs linkage information under the entity `state_key`, + naming the member the claim is about rather than itself, with its own key + ID: a verifier reads `content.domain`, fetches that server's published key + of that ID, and checks the `state_key` signature.""" event = self._sign( self._member_event_dict(), (MemberKeys.SIGNATURE_ENTITY, self.member_signing_key), - (self.server_name, self.server_signing_key), + (MemberKeys.LINKAGE_SIGNATURE_ENTITY, self.server_signing_key), ) - # Both signatures are present: the second signer added to the first. + # Both signatures are present: the second signer added to the first. The + # co-signature carries the server's own key ID, not the member keys' + # constant `ed25519:1`, and the server's name appears nowhere. self.assertEqual( - set(self._signatures(event)), - {MemberKeys.SIGNATURE_ENTITY, self.server_name}, + {k: list(v) for k, v in self._signatures(event).items()}, + { + MemberKeys.SIGNATURE_ENTITY: [MemberKeys.KEY_ID], + MemberKeys.LINKAGE_SIGNATURE_ENTITY: ["ed25519:srv"], + }, ) redacted = self._redacted_pdu(event) verify_signed_json( redacted, MemberKeys.SIGNATURE_ENTITY, self.member_key.verify_key() ) verify_signed_json( - redacted, self.server_name, get_verify_key(self.server_signing_key) + redacted, + MemberKeys.LINKAGE_SIGNATURE_ENTITY, + get_verify_key(self.server_signing_key), ) def test_authorising_member_key_signature(self) -> None: @@ -436,7 +447,7 @@ class MemberKeySigningTestCase(unittest.TestCase): event = self._sign( self._member_event_dict(), (MemberKeys.SIGNATURE_ENTITY, self.member_signing_key), - (self.server_name, self.server_signing_key), + (MemberKeys.LINKAGE_SIGNATURE_ENTITY, self.server_signing_key), ) redacted = prune_event(event) @@ -451,7 +462,9 @@ class MemberKeySigningTestCase(unittest.TestCase): pdu, MemberKeys.SIGNATURE_ENTITY, self.member_key.verify_key() ) verify_signed_json( - pdu, self.server_name, get_verify_key(self.server_signing_key) + pdu, + MemberKeys.LINKAGE_SIGNATURE_ENTITY, + get_verify_key(self.server_signing_key), ) # ... and cover `domain`: changing it invalidates both. tampered = copy.deepcopy(pdu) @@ -462,7 +475,9 @@ class MemberKeySigningTestCase(unittest.TestCase): ) with self.assertRaises(SignatureVerifyException): verify_signed_json( - tampered, self.server_name, get_verify_key(self.server_signing_key) + tampered, + MemberKeys.LINKAGE_SIGNATURE_ENTITY, + get_verify_key(self.server_signing_key), ) # The event ID (the reference hash) covers `domain` too... @@ -558,21 +573,18 @@ class MemberKeySigningTestCase(unittest.TestCase): def test_refuses_server_cosignature_on_wrong_event(self) -> None: """A server co-signs only `m.room.member` events whose `content.domain` is itself: our co-signature is a statement about our own user.""" - # Another server's name, even if it matches `content.domain`. + linkage = MemberKeys.LINKAGE_SIGNATURE_ENTITY + # Another domain's linkage information. event_dict = self._member_event_dict() event_dict["content"]["domain"] = "other.example" self._assert_refused( - event_dict, ("other.example", self.server_signing_key), "not this server" - ) - # Our name, on another domain's linkage information. - self._assert_refused( - event_dict, (self.server_name, self.server_signing_key), "content.domain" + event_dict, (linkage, self.server_signing_key), "content.domain" ) # No linkage information at all. event_dict = self._member_event_dict() del event_dict["content"]["domain"] self._assert_refused( - event_dict, (self.server_name, self.server_signing_key), "content.domain" + event_dict, (linkage, self.server_signing_key), "content.domain" ) # Not a membership event. event_dict = self._member_event_dict() @@ -580,10 +592,21 @@ class MemberKeySigningTestCase(unittest.TestCase): del event_dict["state_key"] self._assert_refused( event_dict, - (self.server_name, self.server_signing_key), + (linkage, self.server_signing_key), "only co-sign m.room.member", ) + def test_refuses_signature_under_a_server_name(self) -> None: + """Linkage is co-signed under the constant `state_key`, so a signature + under a server's name — ours or anyone else's — is not a signature a + member key room event may carry.""" + for entity in (self.server_name, "other.example"): + self._assert_refused( + self._member_event_dict(), + (entity, self.server_signing_key), + "not a signature this event may carry", + ) + def test_refuses_non_member_key_room(self) -> None: for fn in ( lambda: add_hashes_and_signatures_from_signers( diff --git a/tests/handlers/test_member_keys.py b/tests/handlers/test_member_keys.py index 0fbbe182f4..a69ebb5310 100644 --- a/tests/handlers/test_member_keys.py +++ b/tests/handlers/test_member_keys.py @@ -209,7 +209,7 @@ class EventTranslationTestCase(unittest.HomeserverTestCase): ) self.assertEqual( self._signers(event_dict), - [MemberKeys.SIGNATURE_ENTITY, self.hs.hostname], + [MemberKeys.SIGNATURE_ENTITY, MemberKeys.LINKAGE_SIGNATURE_ENTITY], ) def test_erased_user_gets_no_localpart(self) -> None: @@ -232,7 +232,7 @@ class EventTranslationTestCase(unittest.HomeserverTestCase): ) self.assertEqual( self._signers(event_dict), - [MemberKeys.SIGNATURE_ENTITY, self.hs.hostname], + [MemberKeys.SIGNATURE_ENTITY, MemberKeys.LINKAGE_SIGNATURE_ENTITY], ) def test_remote_target_is_refused(self) -> None: @@ -305,7 +305,11 @@ class EventTranslationTestCase(unittest.HomeserverTestCase): ) self.assertEqual( self._signers(event_dict), - [MemberKeys.SIGNATURE_ENTITY, self.hs.hostname, alice_key], + [ + MemberKeys.SIGNATURE_ENTITY, + MemberKeys.LINKAGE_SIGNATURE_ENTITY, + alice_key, + ], ) def _key_for(self, user_id: str) -> str: @@ -461,7 +465,8 @@ class EventTranslationTestCase(unittest.HomeserverTestCase): EventContentFields.MSC4430_DOMAIN: self.hs.hostname, } self.assertEqual( - signers_for(ours), [MemberKeys.SIGNATURE_ENTITY, self.hs.hostname] + signers_for(ours), + [MemberKeys.SIGNATURE_ENTITY, MemberKeys.LINKAGE_SIGNATURE_ENTITY], ) for wrong in ( {**ours, EventContentFields.MSC4430_LOCALPART: "bob"}, @@ -476,7 +481,7 @@ class EventTranslationTestCase(unittest.HomeserverTestCase): self.assertEqual(signers_for(ours), [MemberKeys.SIGNATURE_ENTITY]) self.assertEqual( signers_for({EventContentFields.MSC4430_DOMAIN: self.hs.hostname}), - [MemberKeys.SIGNATURE_ENTITY, self.hs.hostname], + [MemberKeys.SIGNATURE_ENTITY, MemberKeys.LINKAGE_SIGNATURE_ENTITY], ) def test_remote_additional_creator_is_refused(self) -> None: diff --git a/tests/rest/client/test_member_key_rooms.py b/tests/rest/client/test_member_key_rooms.py index 4dd3f39f0e..cd539eb95d 100644 --- a/tests/rest/client/test_member_key_rooms.py +++ b/tests/rest/client/test_member_key_rooms.py @@ -149,7 +149,7 @@ class _MemberKeyRoomsBase(unittest.HomeserverTestCase): {entity: set(sigs) for entity, sigs in event.signatures.as_dict().items()}, { MemberKeys.SIGNATURE_ENTITY: {MemberKeys.KEY_ID}, - self.hs.hostname: { + MemberKeys.LINKAGE_SIGNATURE_ENTITY: { f"{self.hs.signing_key.alg}:{self.hs.signing_key.version}" }, }, @@ -366,7 +366,7 @@ class MemberKeyRoomsTestCase(_MemberKeyRoomsBase): leave.content[EventContentFields.MSC4430_DOMAIN], self.hs.hostname ) # Still ours, so still co-signed. - self.assertIn(self.hs.hostname, leave.signatures) + self.assertIn(MemberKeys.LINKAGE_SIGNATURE_ENTITY, leave.signatures) def test_additional_creators(self) -> None: """Additional creators are named by key in the create event, which is the @@ -610,7 +610,7 @@ class MemberKeyModerationTestCase(_MemberKeyRoomsBase): self.assertEqual( ban.content[EventContentFields.MSC4430_DOMAIN], self.hs.hostname ) - self.assertIn(self.hs.hostname, ban.signatures.as_dict()) + self.assertIn(MemberKeys.LINKAGE_SIGNATURE_ENTITY, ban.signatures.as_dict()) self._assert_no_user_ids_in_room(room_id) def test_moderation_through_the_state_api(self) -> None: