Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Block clients from sending server ACLs that lock the local server out. #8708

Merged
merged 6 commits into from
Nov 3, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8708.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Block attempts by clients to sender server ACLs, or redactions of server ACLs, that would result in the local server being blocked from the room.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ files =
synapse/config,
synapse/event_auth.py,
synapse/events/builder.py,
synapse/events/validator.py,
synapse/events/spamcheck.py,
synapse/federation,
synapse/handlers/_base.py,
Expand Down
27 changes: 18 additions & 9 deletions synapse/events/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,26 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Union

from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import EventFormatVersions
from synapse.config.homeserver import HomeServerConfig
from synapse.events import EventBase
from synapse.events.builder import EventBuilder
from synapse.events.utils import validate_canonicaljson
from synapse.federation.federation_server import server_matches_acl_event
from synapse.types import EventID, RoomID, UserID


class EventValidator:
def validate_new(self, event, config):
def validate_new(self, event: EventBase, config: HomeServerConfig):
"""Validates the event has roughly the right format

Args:
event (FrozenEvent): The event to validate.
config (Config): The homeserver's configuration.
event The event to validate.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
config: The homeserver's configuration.
"""
self.validate_builder(event)

Expand Down Expand Up @@ -76,12 +82,18 @@ def validate_new(self, event, config):
if event.type == EventTypes.Retention:
self._validate_retention(event)

def _validate_retention(self, event):
if event.type == EventTypes.ServerACL:
if not server_matches_acl_event(config.server_name, event):
raise SynapseError(
400, "Can't create an ACL event that denies the local server"
)

def _validate_retention(self, event: EventBase):
"""Checks that an event that defines the retention policy for a room respects the
format enforced by the spec.

Args:
event (FrozenEvent): The event to validate.
event: The event to validate.
"""
if not event.is_state():
raise SynapseError(code=400, msg="must be a state event")
Expand Down Expand Up @@ -116,13 +128,10 @@ def _validate_retention(self, event):
errcode=Codes.BAD_JSON,
)

def validate_builder(self, event):
def validate_builder(self, event: Union[EventBase, EventBuilder]):
"""Validates that the builder/event has roughly the right format. Only
checks values that we expect a proto event to have, rather than all the
fields an event would have

Args:
event (EventBuilder|FrozenEvent)
"""

strings = ["room_id", "sender", "type"]
Expand Down
3 changes: 3 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,9 @@ async def persist_and_notify_client_event(
if original_event.room_id != event.room_id:
raise SynapseError(400, "Cannot redact event from a different room")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this should probably be a 403, alas. out of scope for this PR though.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is a 400; redacting a room's event using a different room ID is ill-formed, it is not "well-formed but forbidden".


if original_event.type == EventTypes.ServerACL:
raise AuthError(403, "Redacting server ACL events is not permitted")

prev_state_ids = await context.get_prev_state_ids()
auth_events_ids = self.auth.compute_auth_events(
event, prev_state_ids, for_verification=True
Expand Down
57 changes: 57 additions & 0 deletions tests/handlers/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,60 @@ def test_duplicated_txn_id_one_call(self):
# Check that we've deduplicated the events.
self.assertEqual(len(events), 2)
self.assertEqual(events[0].event_id, events[1].event_id)


class ServerAclValidationTestCase(unittest.HomeserverTestCase):
servlets = [
admin.register_servlets,
login.register_servlets,
room.register_servlets,
]

def prepare(self, reactor, clock, hs):
self.user_id = self.register_user("tester", "foobar")
self.access_token = self.login("tester", "foobar")
self.room_id = self.helper.create_room_as(self.user_id, tok=self.access_token)

def test_allow_server_acl(self):
"""Test that sending an ACL that blocks everyone but ourselves work.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
"""

self.helper.send_state(
self.room_id,
EventTypes.ServerACL,
body={"allow": [self.hs.hostname]},
tok=self.access_token,
expect_code=200,
)

def test_deny_server_acl_block_outselves(self):
"""Test that sending an ACL that blocks ourselves does not work.
"""
self.helper.send_state(
self.room_id,
EventTypes.ServerACL,
body={},
tok=self.access_token,
expect_code=400,
)

def test_deny_redact_server_acl(self):
"""Test that attempting to redact an ACL is blocked
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
"""

body = self.helper.send_state(
self.room_id,
EventTypes.ServerACL,
body={"allow": [self.hs.hostname]},
tok=self.access_token,
expect_code=200,
)
event_id = body["event_id"]

# Redaction of event should fail.
path = "/_matrix/client/r0/rooms/%s/redact/%s" % (self.room_id, event_id)
request, channel = self.make_request(
"POST", path, content={}, access_token=self.access_token
)
self.render(request)
self.assertEqual(int(channel.result["code"]), 403)