Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tulir opened this issue Sep 21, 2020 · 4 comments
Open

Comments

@tulir
Copy link

tulir commented Sep 21, 2020

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
Copy link
Member

jevolk commented Sep 22, 2020

Construct stopped making use of GET /_matrix/key/v2/server/{keyId} earlier this year. There may be some more to research starting from matrix-org/synapse#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:

  • Synapse fails to return all keys as prescribed by that spec when the keyId path parameter is missing. This means multiple queries have to be made when a server accumulates a history of rotations, or if multiple keys are in active use.

  • When Synapse does choose to populate old_verify_keys it doesn't include a nested copy of the original verify_keys, it just signs the old key with the new key's signature. Ideally it should cover the old key and the old key's signature in the same structure as it was. I say "ideally" because I can't think of any new security issues stemming from just that. I say "new" because in reality we're accepting all of this data on the basis of the TLS certificate system anyway, and all of these signatures are a fool's exercise.

  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
Copy link
Member

jevolk commented Sep 22, 2020

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

key get <server_name> [notary_server]

@grinapo
Copy link

grinapo commented Sep 22, 2020

sidenote: reported to conduit.

@grinapo
Copy link

grinapo commented Sep 22, 2020

And to dendrite as well: matrix-org/dendrite#1435

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants