Strip "join_authorised_via_users_server" from join events which do not need it. (#10933)

This fixes a "Event not signed by authorising server" error when
transition room member from join -> join, e.g. when updating a
display name or avatar URL for restricted rooms.
This commit is contained in:
Patrick Cloke 2021-09-30 11:13:59 -04:00 committed by Patrick Cloke
parent 3412f5c8d8
commit 32072dcdac
11 changed files with 46 additions and 25 deletions

1
changelog.d/10933.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in Synapse v1.40.0 where changing a user's display name or avatar in a restricted room would cause an authentication error.

View File

@ -217,6 +217,9 @@ class EventContentFields:
# For "marker" events # For "marker" events
MSC2716_MARKER_INSERTION = "org.matrix.msc2716.marker.insertion" MSC2716_MARKER_INSERTION = "org.matrix.msc2716.marker.insertion"
# The authorising user for joining a restricted room.
AUTHORISING_USER = "join_authorised_via_users_server"
class RoomTypes: class RoomTypes:
"""Understood values of the room_type field of m.room.create events.""" """Understood values of the room_type field of m.room.create events."""

View File

@ -115,11 +115,11 @@ def check(
is_invite_via_allow_rule = ( is_invite_via_allow_rule = (
event.type == EventTypes.Member event.type == EventTypes.Member
and event.membership == Membership.JOIN and event.membership == Membership.JOIN
and "join_authorised_via_users_server" in event.content and EventContentFields.AUTHORISING_USER in event.content
) )
if is_invite_via_allow_rule: if is_invite_via_allow_rule:
authoriser_domain = get_domain_from_id( authoriser_domain = get_domain_from_id(
event.content["join_authorised_via_users_server"] event.content[EventContentFields.AUTHORISING_USER]
) )
if not event.signatures.get(authoriser_domain): if not event.signatures.get(authoriser_domain):
raise AuthError(403, "Event not signed by authorising server") raise AuthError(403, "Event not signed by authorising server")
@ -381,7 +381,9 @@ def _is_membership_change_allowed(
# Note that if the caller is in the room or invited, then they do # Note that if the caller is in the room or invited, then they do
# not need to meet the allow rules. # not need to meet the allow rules.
if not caller_in_room and not caller_invited: if not caller_in_room and not caller_invited:
authorising_user = event.content.get("join_authorised_via_users_server") authorising_user = event.content.get(
EventContentFields.AUTHORISING_USER
)
if authorising_user is None: if authorising_user is None:
raise AuthError(403, "Join event is missing authorising user.") raise AuthError(403, "Join event is missing authorising user.")
@ -836,10 +838,10 @@ def auth_types_for_event(
auth_types.add(key) auth_types.add(key)
if room_version.msc3083_join_rules and membership == Membership.JOIN: if room_version.msc3083_join_rules and membership == Membership.JOIN:
if "join_authorised_via_users_server" in event.content: if EventContentFields.AUTHORISING_USER in event.content:
key = ( key = (
EventTypes.Member, EventTypes.Member,
event.content["join_authorised_via_users_server"], event.content[EventContentFields.AUTHORISING_USER],
) )
auth_types.add(key) auth_types.add(key)

View File

@ -105,7 +105,7 @@ def prune_event_dict(room_version: RoomVersion, event_dict: dict) -> dict:
if event_type == EventTypes.Member: if event_type == EventTypes.Member:
add_fields("membership") add_fields("membership")
if room_version.msc3375_redaction_rules: if room_version.msc3375_redaction_rules:
add_fields("join_authorised_via_users_server") add_fields(EventContentFields.AUTHORISING_USER)
elif event_type == EventTypes.Create: elif event_type == EventTypes.Create:
# MSC2176 rules state that create events cannot be redacted. # MSC2176 rules state that create events cannot be redacted.
if room_version.msc2176_redaction_rules: if room_version.msc2176_redaction_rules:

View File

@ -15,7 +15,7 @@
import logging import logging
from collections import namedtuple from collections import namedtuple
from synapse.api.constants import MAX_DEPTH, EventTypes, Membership from synapse.api.constants import MAX_DEPTH, EventContentFields, EventTypes, Membership
from synapse.api.errors import Codes, SynapseError from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import EventFormatVersions, RoomVersion from synapse.api.room_versions import EventFormatVersions, RoomVersion
from synapse.crypto.event_signing import check_event_content_hash from synapse.crypto.event_signing import check_event_content_hash
@ -184,10 +184,10 @@ async def _check_sigs_on_pdu(
room_version.msc3083_join_rules room_version.msc3083_join_rules
and pdu.type == EventTypes.Member and pdu.type == EventTypes.Member
and pdu.membership == Membership.JOIN and pdu.membership == Membership.JOIN
and "join_authorised_via_users_server" in pdu.content and EventContentFields.AUTHORISING_USER in pdu.content
): ):
authorising_server = get_domain_from_id( authorising_server = get_domain_from_id(
pdu.content["join_authorised_via_users_server"] pdu.content[EventContentFields.AUTHORISING_USER]
) )
try: try:
await keyring.verify_event_for_server( await keyring.verify_event_for_server(

View File

@ -37,7 +37,7 @@ from typing import (
import attr import attr
from prometheus_client import Counter from prometheus_client import Counter
from synapse.api.constants import EventTypes, Membership from synapse.api.constants import EventContentFields, EventTypes, Membership
from synapse.api.errors import ( from synapse.api.errors import (
CodeMessageException, CodeMessageException,
Codes, Codes,
@ -875,9 +875,9 @@ class FederationClient(FederationBase):
# If the join is being authorised via allow rules, we need to send # If the join is being authorised via allow rules, we need to send
# the /send_join back to the same server that was originally used # the /send_join back to the same server that was originally used
# with /make_join. # with /make_join.
if "join_authorised_via_users_server" in pdu.content: if EventContentFields.AUTHORISING_USER in pdu.content:
destinations = [ destinations = [
get_domain_from_id(pdu.content["join_authorised_via_users_server"]) get_domain_from_id(pdu.content[EventContentFields.AUTHORISING_USER])
] ]
return await self._try_destination_list( return await self._try_destination_list(

View File

@ -34,7 +34,7 @@ from twisted.internet import defer
from twisted.internet.abstract import isIPAddress from twisted.internet.abstract import isIPAddress
from twisted.python import failure from twisted.python import failure
from synapse.api.constants import EduTypes, EventTypes, Membership from synapse.api.constants import EduTypes, EventContentFields, EventTypes, Membership
from synapse.api.errors import ( from synapse.api.errors import (
AuthError, AuthError,
Codes, Codes,
@ -765,11 +765,11 @@ class FederationServer(FederationBase):
if ( if (
room_version.msc3083_join_rules room_version.msc3083_join_rules
and event.membership == Membership.JOIN and event.membership == Membership.JOIN
and "join_authorised_via_users_server" in event.content and EventContentFields.AUTHORISING_USER in event.content
): ):
# We can only authorise our own users. # We can only authorise our own users.
authorising_server = get_domain_from_id( authorising_server = get_domain_from_id(
event.content["join_authorised_via_users_server"] event.content[EventContentFields.AUTHORISING_USER]
) )
if authorising_server != self.server_name: if authorising_server != self.server_name:
raise SynapseError( raise SynapseError(

View File

@ -27,7 +27,12 @@ from unpaddedbase64 import decode_base64
from twisted.internet import defer from twisted.internet import defer
from synapse import event_auth from synapse import event_auth
from synapse.api.constants import EventTypes, Membership, RejectedReason from synapse.api.constants import (
EventContentFields,
EventTypes,
Membership,
RejectedReason,
)
from synapse.api.errors import ( from synapse.api.errors import (
AuthError, AuthError,
CodeMessageException, CodeMessageException,
@ -712,7 +717,7 @@ class FederationHandler(BaseHandler):
if include_auth_user_id: if include_auth_user_id:
event_content[ event_content[
"join_authorised_via_users_server" EventContentFields.AUTHORISING_USER
] = await self._event_auth_handler.get_user_which_could_invite( ] = await self._event_auth_handler.get_user_which_could_invite(
room_id, room_id,
state_ids, state_ids,

View File

@ -573,6 +573,14 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
errcode=Codes.BAD_JSON, errcode=Codes.BAD_JSON,
) )
# The event content should *not* include the authorising user as
# it won't be properly signed. Strip it out since it might come
# back from a client updating a display name / avatar.
#
# This only applies to restricted rooms, but there should be no reason
# for a client to include it. Unconditionally remove it.
content.pop(EventContentFields.AUTHORISING_USER, None)
effective_membership_state = action effective_membership_state = action
if action in ["kick", "unban"]: if action in ["kick", "unban"]:
effective_membership_state = "leave" effective_membership_state = "leave"
@ -939,7 +947,7 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
# be included in the event content in order to efficiently validate # be included in the event content in order to efficiently validate
# the event. # the event.
content[ content[
"join_authorised_via_users_server" EventContentFields.AUTHORISING_USER
] = await self.event_auth_handler.get_user_which_could_invite( ] = await self.event_auth_handler.get_user_which_could_invite(
room_id, room_id,
current_state_ids, current_state_ids,

View File

@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from synapse.api.constants import EventContentFields
from synapse.api.room_versions import RoomVersions from synapse.api.room_versions import RoomVersions
from synapse.events import make_event_from_dict from synapse.events import make_event_from_dict
from synapse.events.utils import ( from synapse.events.utils import (
@ -352,7 +353,7 @@ class PruneEventTestCase(unittest.TestCase):
"event_id": "$test:domain", "event_id": "$test:domain",
"content": { "content": {
"membership": "join", "membership": "join",
"join_authorised_via_users_server": "@user:domain", EventContentFields.AUTHORISING_USER: "@user:domain",
"other_key": "stripped", "other_key": "stripped",
}, },
}, },
@ -372,7 +373,7 @@ class PruneEventTestCase(unittest.TestCase):
"type": "m.room.member", "type": "m.room.member",
"content": { "content": {
"membership": "join", "membership": "join",
"join_authorised_via_users_server": "@user:domain", EventContentFields.AUTHORISING_USER: "@user:domain",
"other_key": "stripped", "other_key": "stripped",
}, },
}, },
@ -380,7 +381,7 @@ class PruneEventTestCase(unittest.TestCase):
"type": "m.room.member", "type": "m.room.member",
"content": { "content": {
"membership": "join", "membership": "join",
"join_authorised_via_users_server": "@user:domain", EventContentFields.AUTHORISING_USER: "@user:domain",
}, },
"signatures": {}, "signatures": {},
"unsigned": {}, "unsigned": {},

View File

@ -16,6 +16,7 @@ import unittest
from typing import Optional from typing import Optional
from synapse import event_auth from synapse import event_auth
from synapse.api.constants import EventContentFields
from synapse.api.errors import AuthError from synapse.api.errors import AuthError
from synapse.api.room_versions import RoomVersions from synapse.api.room_versions import RoomVersions
from synapse.events import EventBase, make_event_from_dict from synapse.events import EventBase, make_event_from_dict
@ -380,7 +381,7 @@ class EventAuthTestCase(unittest.TestCase):
authorised_join_event = _join_event( authorised_join_event = _join_event(
pleb, pleb,
additional_content={ additional_content={
"join_authorised_via_users_server": "@creator:example.com" EventContentFields.AUTHORISING_USER: "@creator:example.com"
}, },
) )
event_auth.check( event_auth.check(
@ -404,7 +405,7 @@ class EventAuthTestCase(unittest.TestCase):
_join_event( _join_event(
pleb, pleb,
additional_content={ additional_content={
"join_authorised_via_users_server": "@inviter:foo.test" EventContentFields.AUTHORISING_USER: "@inviter:foo.test"
}, },
), ),
pl_auth_events, pl_auth_events,
@ -431,7 +432,7 @@ class EventAuthTestCase(unittest.TestCase):
_join_event( _join_event(
pleb, pleb,
additional_content={ additional_content={
"join_authorised_via_users_server": "@other:example.com" EventContentFields.AUTHORISING_USER: "@other:example.com"
}, },
), ),
auth_events, auth_events,
@ -448,7 +449,7 @@ class EventAuthTestCase(unittest.TestCase):
"join", "join",
sender=creator, sender=creator,
additional_content={ additional_content={
"join_authorised_via_users_server": "@inviter:foo.test" EventContentFields.AUTHORISING_USER: "@inviter:foo.test"
}, },
), ),
auth_events, auth_events,