Faster joins: fix rejected events becoming un-rejected during resync #13413

richvdh · 2022-07-28T14:16:23Z

Make sure that we re-check the auth rules during state resync, otherwise rejected events get un-rejected.

Reading `EventContext.state_group` on a rejected event isn't valid (its value is actually a dummy state group which *includes* the rejected event).

richvdh · 2022-07-28T14:29:14Z

synapse/storage/databases/main/state.py

@@ -419,13 +419,15 @@ def _update_state_for_partial_state_event_txn(
        # anything that was rejected should have the same state as its
        # predecessor.
        if context.rejected:
-            assert context.state_group == context.state_group_before_event


I wrote this in the git commit, but to reiterate, this was completely bogus. The value of EventContext._state_group on a rejected event is actually a dummy state group which includes the rejected event (since the state group is created before we know if the event will be rejected or not).

EventContext.state_group is a getter which has an assertion to check you aren't shooting yourself in the foot, but the right thing to do here is not to access it for rejected events.

squahtx · 2022-07-29T13:29:41Z

synapse/handlers/federation_event.py

+
+            # since the state at this event has changed, we should now re-evaluate
+            # whether it should have been rejected. We must already have all of the
+            # auth events (from last time we went round this path), so there is no
+            # need to pass the origin.
+            await self._check_event_auth(None, event, context)
+


Can we now reject previously unrejected events? If so, we might want to update the rejections table or add a todo to do so.

yeah; I'm working on that currently.

(this is #13006)

squahtx

lgtm!

#13404 removed an import of `Optional` which was still needed due to #13413 added more usages.

…atrix-org#13413) Make sure that we re-check the auth rules during state resync, otherwise rejected events get un-rejected.

matrix-org#13404 removed an import of `Optional` which was still needed due to matrix-org#13413 added more usages.

@andrewdoh

Synapse 1.65.0 (2022-08-16) =========================== No significant changes since 1.65.0rc2. Synapse 1.65.0rc2 (2022-08-11) ============================== Internal Changes ---------------- - Revert 'Remove the unspecced `room_id` field in the `/hierarchy` response. ([\matrix-org#13365](matrix-org#13365))' to give more time for clients to update. ([\matrix-org#13501](matrix-org#13501)) Synapse 1.65.0rc1 (2022-08-09) ============================== Features -------- - Add support for stable prefixes for [MSC2285 (private read receipts)](matrix-org/matrix-spec-proposals#2285). ([\matrix-org#13273](matrix-org#13273)) - Add new unstable error codes `ORG.MATRIX.MSC3848.ALREADY_JOINED`, `ORG.MATRIX.MSC3848.NOT_JOINED`, and `ORG.MATRIX.MSC3848.INSUFFICIENT_POWER` described in [MSC3848](matrix-org/matrix-spec-proposals#3848). ([\matrix-org#13343](matrix-org#13343)) - Use stable prefixes for [MSC3827](matrix-org/matrix-spec-proposals#3827). ([\matrix-org#13370](matrix-org#13370)) - Add a new module API method to translate a room alias into a room ID. ([\matrix-org#13428](matrix-org#13428)) - Add a new module API method to create a room. ([\matrix-org#13429](matrix-org#13429)) - Add remote join capability to the module API's `update_room_membership` method (in a backwards compatible manner). ([\matrix-org#13441](matrix-org#13441)) Bugfixes -------- - Update the version of the LDAP3 auth provider module included in the `matrixdotorg/synapse` DockerHub images and the Debian packages hosted on packages.matrix.org to 0.2.2. This version fixes a regression in the module. ([\matrix-org#13470](matrix-org#13470)) - Fix a bug introduced in Synapse v1.41.0 where the `/hierarchy` API returned non-standard information (a `room_id` field under each entry in `children_state`). ([\matrix-org#13365](matrix-org#13365)) - Fix a bug introduced in Synapse 0.24.0 that would respond with the wrong error status code to `/joined_members` requests when the requester is not a current member of the room. Contributed by @andrewdoh. ([\matrix-org#13374](matrix-org#13374)) - Fix bug in handling of typing events for appservices. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13392](matrix-org#13392)) - Fix a bug introduced in Synapse 1.57.0 where rooms listed in `exclude_rooms_from_sync` in the configuration file would not be properly excluded from incremental syncs. ([\matrix-org#13408](matrix-org#13408)) - Fix a bug in the experimental faster-room-joins support which could cause it to get stuck in an infinite loop. ([\matrix-org#13353](matrix-org#13353)) - Faster room joins: fix a bug which caused rejected events to become un-rejected during state syncing. ([\matrix-org#13413](matrix-org#13413)) - Faster room joins: fix error when running out of servers to sync partial state with, so that Synapse raises the intended error instead. ([\matrix-org#13432](matrix-org#13432)) Updates to the Docker image --------------------------- - Make Docker images build on armv7 by installing cryptography dependencies in the 'requirements' stage. Contributed by Jasper Spaans. ([\matrix-org#13372](matrix-org#13372)) Improved Documentation ---------------------- - Update the 'registration tokens' page to acknowledge that the relevant MSC was merged into version 1.2 of the Matrix specification. Contributed by @moan0s. ([\matrix-org#11897](matrix-org#11897)) - Document which HTTP resources support gzip compression. ([\matrix-org#13221](matrix-org#13221)) - Add steps describing how to elevate an existing user to administrator by manipulating the database. ([\matrix-org#13230](matrix-org#13230)) - Fix wrong headline for `url_preview_accept_language` in documentation. ([\matrix-org#13437](matrix-org#13437)) - Remove redundant 'Contents' section from the Configuration Manual. Contributed by @dklimpel. ([\matrix-org#13438](matrix-org#13438)) - Update documentation for config setting `macaroon_secret_key`. ([\matrix-org#13443](matrix-org#13443)) - Update outdated information on `sso_mapping_providers` documentation. ([\matrix-org#13449](matrix-org#13449)) - Fix example code in module documentation of `password_auth_provider_callbacks`. ([\matrix-org#13450](matrix-org#13450)) - Make the configuration for the cache clearer. ([\matrix-org#13481](matrix-org#13481)) Internal Changes ---------------- - Extend the release script to automatically push a new SyTest branch, rather than having that be a manual process. ([\matrix-org#12978](matrix-org#12978)) - Make minor clarifications to the error messages given when we fail to join a room via any server. ([\matrix-org#13160](matrix-org#13160)) - Enable Complement CI tests in the 'latest deps' test run. ([\matrix-org#13213](matrix-org#13213)) - Fix long-standing bugged logic which was never hit in `get_pdu` asking every remote destination even after it finds an event. ([\matrix-org#13346](matrix-org#13346)) - Faster room joins: avoid blocking when pulling events with partially missing prev events. ([\matrix-org#13355](matrix-org#13355)) - Instrument `/messages` for understandable traces in Jaeger. ([\matrix-org#13368](matrix-org#13368)) - Remove an unused argument to `get_relations_for_event`. ([\matrix-org#13383](matrix-org#13383)) - Add a `merge-back` command to the release script, which automates merging the correct branches after a release. ([\matrix-org#13393](matrix-org#13393)) - Adding missing type hints to tests. ([\matrix-org#13397](matrix-org#13397)) - Faster Room Joins: don't leave a stuck room partial state flag if the join fails. ([\matrix-org#13403](matrix-org#13403)) - Refactor `_resolve_state_at_missing_prevs` to compute an `EventContext` instead. ([\matrix-org#13404](matrix-org#13404), [\matrix-org#13431](matrix-org#13431)) - Faster Room Joins: prevent Synapse from answering federated join requests for a room which it has not fully joined yet. ([\matrix-org#13416](matrix-org#13416)) - Re-enable running Complement tests against Synapse with workers. ([\matrix-org#13420](matrix-org#13420)) - Prevent unnecessary lookups to any external `get_event` cache. Contributed by Nick @ Beeper (@Fizzadar). ([\matrix-org#13435](matrix-org#13435)) - Add some tracing to give more insight into local room joins. ([\matrix-org#13439](matrix-org#13439)) - Rename class `RateLimitConfig` to `RatelimitSettings` and `FederationRateLimitConfig` to `FederationRatelimitSettings`. ([\matrix-org#13442](matrix-org#13442)) - Add some comments about how event push actions are stored. ([\matrix-org#13445](matrix-org#13445), [\matrix-org#13455](matrix-org#13455)) - Improve rebuild speed for the "synapse-workers" docker image. ([\matrix-org#13447](matrix-org#13447)) - Fix `@tag_args` being off-by-one with the arguments when tagging a span (tracing). ([\matrix-org#13452](matrix-org#13452)) - Update type of `EventContext.rejected`. ([\matrix-org#13460](matrix-org#13460)) - Use literals in place of `HTTPStatus` constants in tests. ([\matrix-org#13463](matrix-org#13463), [\matrix-org#13469](matrix-org#13469)) - Correct a misnamed argument in state res v2 internals. ([\matrix-org#13467](matrix-org#13467))

richvdh added 3 commits July 28, 2022 15:06

Faster joins: check event auth again during resync

7d1bd7f

Make sure that we re-check the auth rules during state resync, otherwise rejected events get un-rejected.

Avoid reading the state group of rejected events

a3cbbb2

Reading `EventContext.state_group` on a rejected event isn't valid (its value is actually a dummy state group which *includes* the rejected event).

changelog

5f448b8

richvdh requested a review from a team as a code owner July 28, 2022 14:16

richvdh commented Jul 28, 2022

View reviewed changes

squahtx reviewed Jul 29, 2022

View reviewed changes

squahtx approved these changes Jul 29, 2022

View reviewed changes

richvdh merged commit 23768cc into develop Aug 1, 2022

richvdh deleted the rav/reauth_during_resync branch August 1, 2022 10:20

clokep mentioned this pull request Aug 1, 2022

Fix missing import in federation_event handler. #13431

Merged

clokep added a commit that referenced this pull request Aug 1, 2022

Fix missing import in federation_event handler. (#13431)

f8e7a94

#13404 removed an import of `Optional` which was still needed due to #13413 added more usages.

azmeuk pushed a commit to azmeuk/synapse that referenced this pull request Aug 8, 2022

Fix missing import in federation_event handler. (matrix-org#13431)

ca13fb1

matrix-org#13404 removed an import of `Optional` which was still needed due to matrix-org#13413 added more usages.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Faster joins: fix rejected events becoming un-rejected during resync #13413

Faster joins: fix rejected events becoming un-rejected during resync #13413

richvdh commented Jul 28, 2022

richvdh Jul 28, 2022

squahtx Jul 29, 2022

richvdh Jul 29, 2022

richvdh Jul 29, 2022

squahtx left a comment

Faster joins: fix rejected events becoming un-rejected during resync #13413

Faster joins: fix rejected events becoming un-rejected during resync #13413

Conversation

richvdh commented Jul 28, 2022

richvdh Jul 28, 2022

Choose a reason for hiding this comment

squahtx Jul 29, 2022

Choose a reason for hiding this comment

richvdh Jul 29, 2022

Choose a reason for hiding this comment

richvdh Jul 29, 2022

Choose a reason for hiding this comment

squahtx left a comment

Choose a reason for hiding this comment