matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.79k stars 2.13k forks source link

Is it correct that Synapse validates cross-signing signatures? #12548

Open richvdh opened 2 years ago

richvdh commented 2 years ago

When you upload a cross-signing signature, Synapse attempts to validate that signature.

https://spec.matrix.org/v1.2/client-server-api/#cross-signing doesn't seem to require this (though it does document M_INVALID_SIGNATURE: For example, the self-signing or user-signing key had an incorrect signature), and it's not obvious it's the homeserver's job to do this validation.

It also means that any future attempt to add new signature algorithms will require updates to the homeserver.

richvdh commented 2 years ago

@neilalexander reports:

For some reason it seems to be stripping signatures that Dendrite would pass through and Element Web is totally happy with ... which suggests to me there's a disagreement between the signature verification between Element and Synapse which feels quite bad

neilalexander commented 2 years ago

To clarify a bit about what I've done here: I have three users, one on matrix.org, one on dendrite.matrix.org and one on dendrite.neilalexander.dev. All three users have performed cross-signing verification of each other.

If I log out of my dendrite.matrix.org account, log in again and then verify using my security passphrase:

uhoreg commented 2 years ago

The validation was initially added as a sanity check to prevent clients from accidentally uploading obviously bad data. If it turns out that it's causing more problems than it solves, then I don't have a problem removing it. It does add a bunch of complexity to the code.

It would be nice, though, to find out why Element is generating signatures that Synapse doesn't like.

hpaantee commented 1 year ago

I just wanted to say that this problem still exists. I use dendrite (v0.13) while a friend uses synapse (v1.82). While Element web shows me that all his sessions are trusted, he gets the following warning: Screenshot_20230510-093158

marekvospel commented 1 year ago

What's the current status of this? @uhoreg Do we want to remove cross-signing validation? If so and no one is working on that, I'd love to try to do it.

I've run into issues with this myself and would love to see this resolved.

marekvospel commented 1 year ago

By the way. Eventhough probably expected, It's not just Element, it seems to be a general problem with self signed device keys. (I've tested it only on Element, Element mobile and Cinny, but I expect it's going to be same with other clients)

clokep commented 1 year ago

@uhoreg and @richvdh agree that we should remove this validation.

@marekvospel If you'd like to work on this that would be great!

I think you'll want to start by looking at synapse/rest/client/keys.py, although I don't immediately see where the validation is occurring. Stop by #synapse-dev:matrix.org if you have any questions!

marekvospel commented 1 year ago

Synapse only validates keys and signatures, that are uploaded by local users. (at least in synapse/handlers/e2e_keys.py) That means this can't be caused by the validation. I've removed the validation, tested everything, but new sessions from my dendrite server stay unverified.