Unable to backfill due to invalid event signature from key ed25519:key1

[anoadragon453]

@jboi:jboi.nl is unable to backfill from other servers in #twim:matrix.org as their unable to validate the signature of an event from conduit.rs. From jboi.nl:

synapse_1  | 2020-09-17 15:39:54,041 - synapse.federation.federation_base - 124 - WARNING - GET-251 - Signature check failed for $TAIwqUN7hV5jZ1z3KGzr8jWYFztX642mRsA6FvlqIzw: 403: event id $TAIwqUN7hV5jZ1z3KGzr8jWYFztX642mRsA6FvlqIzw: unable to verify signature for sender conduit.rs: 401: Invalid signature for server conduit.rs with key ed25519:key1: Unable to verify signature for conduit.rs: <class 'nacl.exceptions.BadSignatureError'> Signature was forged or corrupt
synapse_1  | 2020-09-17 15:39:54,658 - synapse.federation.federation_base - 124 - WARNING - GET-251 - Signature check failed for $f236gZlt9KplGfOW7EVOU3xJyit9HsO1hl0SgqpyqZ0: 403: event id $f236gZlt9KplGfOW7EVOU3xJyit9HsO1hl0SgqpyqZ0: unable to verify signature for sender conduit.rs: 401: Invalid signature for server conduit.rs with key ed25519:key1: Unable to verify signature for conduit.rs: <class 'nacl.exceptions.BadSignatureError'> Signature was forged or corrupt

@jboi:jboi.nl successfully joined the room today at 13:43 BST, but receive this exception repeatedly in their logs upon trying to scroll up in the room.

Timo, conduit's developer, says:

Does synapse add/manipulate the origin field in any way before signing it? And is the signing for backfilled events and for incoming transactions different?

Wild guess: maybe that time your hs tried to contact another hs for backfilling and that server's event order didn't make the broken event part of the response?

@jboi:jboi.nl is running Synapse v1.19.2.

[anoadragon453]

The signature of these events are invalid. Synapse seems to not care when receiving these events via a transaction, but does care when we're backfilling. Thus we end up with existing servers in the room carrying on happily, while new joiners are affected.

When we backfill, we pull the requested events out of the database here:

synapse/synapse/handlers/federation.py

Line 1842 in d4a7829

events = await self.store.get_backfill_events(room_id, pdu_list, limit)

Filter them for redaction/erased user reasons, and then happily serve them up without checking signatures.

Testing this myself, I tried backfilling from a remote server (both of us on v1.19.2) and the remote server served the invalid events with a 200 OK.

The server that requested the backfill is the one that's validating the events. That validation is done here:

synapse/synapse/federation/federation_client.py

Lines 220 to 225 in c570f24

	# FIXME: We should handle signature failures more gracefully.
	pdus[:] = await make_deferred_yieldable(
	defer.gatherResults(
	self._check_sigs_and_hashes(room_version, pdus), consumeErrors=True,
	).addErrback(unwrapFirstError)
	)

In this case we're rejecting the whole backfill and trying another server, which gives us the same events, try another server, and so on...

After iterating through many servers, I eventually hit one that returned a 403. Synapse then backed out of the loop and awkwardly returned a 403 - Host not in room to the client.

My conclusion is that instead of bailing out of the backfill, we should do the same as if we were receiving a transaction from another server - reject or drop the invalid event but keep the others if they're OK.

[ShadowJonathan]

Does this in any way break the DAG structure of past rooms, if a single (critical) link within the room is not accepted?

Could that be remedied by servers constructing manual merges in dummy events (upon detection) to ensure DAG continuity? (For instance, reference the last event with a valid signature, following from the first event in the room, and then reference the latest event in the room, this effectively allows the DAG to be "semi-valid" by having a clean link to the first valid events in the room.

The thing to be decided from thereon out is how to handle the "invalid" events in the leftover DAG.

[richvdh]

this is a dup of #3121

[anoadragon453]

Also a dup of #8339

[ara4n]

it is indeed suspiciously similar to #8339...