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

Add some assertions about outliers #10773

Merged
merged 5 commits into from
Sep 8, 2021
Merged

Add some assertions about outliers #10773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb60923
Assert that events in `on_receive_pdu` are not outliers
richvdh Sep 7, 2021
f3ba242
Assert that events in _process_received_pdu are not outliers
richvdh Sep 7, 2021
e941b0f
add an assertion in _auth_and_persist_event
richvdh Sep 7, 2021
21be8ae
add an assertion in `check_event_auth`
richvdh Sep 7, 2021
05ed6bc
changelog
richvdh Sep 7, 2021
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/10773.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Clean up some of the federation event authentication code for clarity.
148 changes: 77 additions & 71 deletions synapse/handlers/federation_event.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
pdu: received PDU
"""

# We should never see any outliers here.
assert not pdu.internal_metadata.outlier

room_id = pdu.room_id
event_id = pdu.event_id

Expand Down Expand Up @@ -232,77 +235,71 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
)
return None

# Check that the event passes auth based on the state at the event. This is
# done for events that are to be added to the timeline (non-outliers).
#
# Get missing pdus if necessary:
# - Fetching any missing prev events to fill in gaps in the graph
# - Fetching state if we have a hole in the graph
if not pdu.internal_metadata.is_outlier():
Copy link
Member Author

Choose a reason for hiding this comment

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

this became a tautology after https://github.com/matrix-org/synapse/pull/10614/files#diff-0bc92da3d703202f5b9be2d3f845e375f5b1a6bc6ba61705a8af9be1121f5e42L301, for the record.

prevs = set(pdu.prev_event_ids())
seen = await self._store.have_events_in_timeline(prevs)
missing_prevs = prevs - seen
# Try to fetch any missing prev events to fill in gaps in the graph
prevs = set(pdu.prev_event_ids())
seen = await self._store.have_events_in_timeline(prevs)
missing_prevs = prevs - seen

if missing_prevs:
# We only backfill backwards to the min depth.
min_depth = await self.get_min_depth_for_context(pdu.room_id)
logger.debug("min_depth: %d", min_depth)
if missing_prevs:
# We only backfill backwards to the min depth.
min_depth = await self.get_min_depth_for_context(pdu.room_id)
logger.debug("min_depth: %d", min_depth)

if min_depth is not None and pdu.depth > min_depth:
# If we're missing stuff, ensure we only fetch stuff one
# at a time.
if min_depth is not None and pdu.depth > min_depth:
# If we're missing stuff, ensure we only fetch stuff one
# at a time.
logger.info(
"Acquiring room lock to fetch %d missing prev_events: %s",
len(missing_prevs),
shortstr(missing_prevs),
)
with (await self._room_pdu_linearizer.queue(pdu.room_id)):
logger.info(
"Acquiring room lock to fetch %d missing prev_events: %s",
"Acquired room lock to fetch %d missing prev_events",
len(missing_prevs),
shortstr(missing_prevs),
)
with (await self._room_pdu_linearizer.queue(pdu.room_id)):
logger.info(
"Acquired room lock to fetch %d missing prev_events",
len(missing_prevs),

try:
await self._get_missing_events_for_pdu(
origin, pdu, prevs, min_depth
)
except Exception as e:
raise Exception(
"Error fetching missing prev_events for %s: %s"
% (event_id, e)
) from e

try:
await self._get_missing_events_for_pdu(
origin, pdu, prevs, min_depth
)
except Exception as e:
raise Exception(
"Error fetching missing prev_events for %s: %s"
% (event_id, e)
) from e

# Update the set of things we've seen after trying to
# fetch the missing stuff
seen = await self._store.have_events_in_timeline(prevs)
missing_prevs = prevs - seen

if not missing_prevs:
logger.info("Found all missing prev_events")

if missing_prevs:
# since this event was pushed to us, it is possible for it to
# become the only forward-extremity in the room, and we would then
# trust its state to be the state for the whole room. This is very
# bad. Further, if the event was pushed to us, there is no excuse
# for us not to have all the prev_events. (XXX: apart from
# min_depth?)
#
# We therefore reject any such events.
logger.warning(
"Rejecting: failed to fetch %d prev events: %s",
len(missing_prevs),
shortstr(missing_prevs),
)
raise FederationError(
"ERROR",
403,
(
"Your server isn't divulging details about prev_events "
"referenced in this event."
),
affected=pdu.event_id,
)
# Update the set of things we've seen after trying to
# fetch the missing stuff
seen = await self._store.have_events_in_timeline(prevs)
missing_prevs = prevs - seen

if not missing_prevs:
logger.info("Found all missing prev_events")

if missing_prevs:
# since this event was pushed to us, it is possible for it to
# become the only forward-extremity in the room, and we would then
# trust its state to be the state for the whole room. This is very
# bad. Further, if the event was pushed to us, there is no excuse
# for us not to have all the prev_events. (XXX: apart from
# min_depth?)
#
# We therefore reject any such events.
logger.warning(
"Rejecting: failed to fetch %d prev events: %s",
len(missing_prevs),
shortstr(missing_prevs),
)
raise FederationError(
"ERROR",
403,
(
"Your server isn't divulging details about prev_events "
"referenced in this event."
),
affected=pdu.event_id,
)

await self._process_received_pdu(origin, pdu, state=None)

Expand Down Expand Up @@ -885,8 +882,13 @@ async def _process_received_pdu(
state: Optional[Iterable[EventBase]],
backfilled: bool = False,
) -> None:
"""Called when we have a new pdu. We need to do auth checks and put it
through the StateHandler.
"""Called when we have a new non-outlier event.

This is called when we have a new event to add to the room DAG - either directly
via a /send request, retrieved via get_missing_events after a /send request, or
backfilled after a client request.

We need to do auth checks and put it through the StateHandler.

Args:
origin: server sending the event
Expand All @@ -901,6 +903,7 @@ async def _process_received_pdu(
notification to clients, and validation of device keys.)
"""
logger.debug("Processing event: %s", event)
assert not event.internal_metadata.outlier

try:
context = await self._state_handler.compute_event_context(
Expand Down Expand Up @@ -1234,11 +1237,13 @@ async def _auth_and_persist_event(
Possibly incomplete, and possibly including events that are not yet
persisted, or authed, or in the right room.

Only populated where we may not already have persisted these events -
for example, when populating outliers.
Only populated when populating outliers.

backfilled: True if the event was backfilled.
"""
# claimed_auth_event_map should be given iff the event is an outlier
assert bool(claimed_auth_event_map) == event.internal_metadata.outlier

context = await self._check_event_auth(
origin,
event,
Expand Down Expand Up @@ -1277,15 +1282,16 @@ async def _check_event_auth(
Possibly incomplete, and possibly including events that are not yet
persisted, or authed, or in the right room.

Only populated where we may not already have persisted these events -
for example, when populating outliers, or the state for a backwards
extremity.
Only populated when populating outliers.

backfilled: True if the event was backfilled.

Returns:
The updated context object.
"""
# claimed_auth_event_map should be given iff the event is an outlier
assert bool(claimed_auth_event_map) == event.internal_metadata.outlier

room_version = await self._store.get_room_version_id(event.room_id)
room_version_obj = KNOWN_ROOM_VERSIONS[room_version]

Expand Down