matrix-construct / construct

This is The Construct
Other
368 stars 43 forks source link

Construct uses POST /query instead of GET /server for fetching server keys #164

Open tulir opened 4 years ago

tulir commented 4 years ago

When fetching the keys of other servers, Construct uses the POST /_matrix/key/v2/query endpoint instead of GET /_matrix/key/v2/server/{keyId}. The former endpoint is meant for querying the keys of multiple different server from one trusted key server, while the latter is meant for fetching the server's own keys.

The problem is that many work-in-progress server implementations (e.g. Dendrite and Conduit) don't implement the mass query endpoint yet, which means Construct is unable to fetch their keys. Construct also doesn't use trusted key servers, which means it can't get the keys via another server either.

jevolk commented 4 years ago

Construct stopped making use of GET /_matrix/key/v2/server/{keyId} earlier this year. There may be some more to research starting from https://github.com/matrix-org/synapse/issues/6596 which was around the time Construct encountered trouble and made the change. Some of the issues with old_verify_keys might be fixed now, but several compelling reasons to not use this endpoint appear to remain:

  1. Fact: the key/query endpoint and the key/server endpoint are functionally equivalent when the former is set to query the query server itself.

  2. Ceteris paribus: the key/query endpoint can do more using the same single codepath because the query can be any server, whereas the key/server endpoint is just a key/query endpoint with a fixed server string.

  3. The key/query endpoint returns multiple keys in their original structure including their own signatures. In other words, all of the results from this endpoint share the same consistent format and fields. In Contrast, the old_verify_keys used by the key/server endpoint is a different format, with different fields, without its own signature.

The problem is that many work-in-progress server implementations (e.g. Dendrite and Conduit) don't implement the mass query endpoint yet, which means Construct is unable to fetch their keys.

The server/{keyId} endpoint is redundant and offers no advantage to key/v2/query. In fact, several aforementioned disadvantages. It can and should be phased out from the spec. I strongly encourage Dendrite and Conduit to make their own development simpler by making exclusive use of only one endpoint for outbound requests, specifically, not the wrong one.

I would prefer to be accommodating here rather than incompatible, though I contend that neither of them would do the same. The point has to be written out explicitly just so it's not lost on anyone: the burden is on them to implement the spec. The burden of compatibility and the effort toward accommodation should not be a step backward for Construct, or anyone. They should step forward. The simplest, cheapest, mutually beneficial solution here is for them to partially implement the query endpoint so it responds to their own server name. That's pretty low effort, and it allows them to stop using the inferior endpoint so it can be feasible to deprecate it.

Construct also doesn't use trusted key servers, which means it can't get the keys via another server either.

You're stating a fact, yes. It doesn't trust the adversary for the keys which defend against the adversary. Fact. Stated. Taken.

jevolk commented 4 years ago

Also, keys can be obtained indirectly with the key get command

key get <server_name> [notary_server]
grinapo commented 4 years ago

sidenote: reported to conduit.

grinapo commented 4 years ago

And to dendrite as well: https://github.com/matrix-org/dendrite/issues/1435