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

Remove the v1 API #338

Open
richvdh opened this issue Apr 7, 2021 · 20 comments
Open

Remove the v1 API #338

richvdh opened this issue Apr 7, 2021 · 20 comments

Comments

@richvdh
Copy link
Member

richvdh commented Apr 7, 2021

Blocked by matrix-org/synapse#9677

The deprecated v1 identity service API is a source of security issues and poor UX. We should set a date to remove it, and follow through with that.

@richvdh
Copy link
Member Author

richvdh commented Apr 7, 2021

Some notable places this is still used:

@turt2live
Copy link
Member

mxisd is more likely to be ma1sd: https://github.com/ma1uta/ma1sd

@callahad
Copy link
Contributor

Would be good to set a public deprecation timeline for this

@turt2live
Copy link
Member

related: matrix-org/matrix-spec-proposals#2713

@erikjohnston
Copy link
Member

@turt2live suggests that there is other work that might need to be done before we can remove the v1 APIs, but he'd need to go and spend some time figuring out what that is. (I hope that is a fair representation)

@BillCarsonFr
Copy link
Member

Notice that email invites are still using V1 api in order to sign the invitation.
Also we cannot just change the email template to use the v2 API, because v2 API requires consent for the sign-ed25519 endpoint, this will lead to a pretty bad invite flow.

This API is defined as helper for client that cannot do crypto, but still element clients are using it.
I tried to get more info on what clients are supposed to do but failed to find more info.

I guess that before removing this API we should:

  • Update documentation to explain the invitation signing process (for client to do it without server)
  • Update EW, EA, EI to do the signing themself from the invite link
  • Modify the email template to not send a sign URL (as it wont be usable as is because in v2 params are in request body and not in query) but instead send the token, private_key as well as the identity server url as regular parameters

@richvdh
Copy link
Member Author

richvdh commented Jul 1, 2022

I decided to try and pull some figures on where the v1 API is being used. Over the 7 days to 24 June:

@t3chguy
Copy link
Member

t3chguy commented Nov 24, 2022

/_matrix/identity/api/v1/sign-ed25519 is a strange one.

3PID (email) invites include a link like https://app.element.io/#/room/%21cCpRBTFuXwPmOCUFRL%3Amatrix.org?email=michaelt%40element.io&signurl=https%3A%2F%2Fvector.im%2F_matrix%2Fidentity%2Fapi%2Fv1%2Fsign-ed25519%3Ftoken%3DcBeQOefWJXQrcgHavWuqjMtiazBrYMCvLbpGvOSWFLkitbMbuFGyEmgBbcPtfHhHxiKKsdEbEXSJezCbTXowCuFaQIKV%26private_key%3DRkzs-2BDCjkOOnS74LwLRO_Uw&room_name=&room_avatar_url=&inviter_name=Michael%20Telatynski&guest_access_token=&guest_user_id=&room_type=

Which passes a signurl using the aforementioned API, but encodes the params in query params rather than a POST body which the v2 API docs state the API demands. This means a simple v1->v2 port isn't so simple as now clients would need to receive the parameters outside of the URL and include them in a POST body themselves, with no backwards compatibility.

The API description being

To aid clients who may not be able to perform crypto themselves, the identity server offers some crypto functionality to help in accepting invitations. This is less secure than the client doing it itself, but may be useful where this isn’t possible.

when in reality clients can't really do the signing themselves without parsing the signurl in brittle ways given the URL params don't match any known documentation means that clients are forced to blindly just call a given URL in undocumented ways.

@richvdh
Copy link
Member Author

richvdh commented Nov 25, 2022

For links, the endpoint in question used to be specced at https://matrix.org/docs/spec/identity_service/r0.3.0.html#deprecated-post-matrix-identity-api-v1-sign-ed25519; note that a GET variant was never part of the spec.

@t3chguy (and @BillCarsonFr, since I think you said the same thing): I'm failing to grok how this "signurl" is used in the 3pid flow. How is the URL generated? How is it used? I can't find it mentioned at https://spec.matrix.org/v1.5/client-server-api/#server-behaviour-7.

@t3chguy
Copy link
Member

t3chguy commented Nov 25, 2022

It is being used as POST but with no body and all params being query params, so still doesn't match that spec

The link comes from the email templates https://github.com/search?q=repo%3Amatrix-org%2Fsydent+sign-ed25519&type=code
The client includes a third_party_signed in the /join/ request which is the object it got back from the sign-ed25519 API, the path & params to which it got via signurl in the link in the email template above

@richvdh
Copy link
Member Author

richvdh commented Nov 25, 2022

It is being used as POST but with no body and all params being query params, so still doesn't match that spec

Right. Yes.

The link comes from the email templates https://github.com/search?q=repo%3Amatrix-org%2Fsydent+sign-ed25519&type=code

Is that the link you meant? It doesn't seem to give anything useful here.

@t3chguy
Copy link
Member

t3chguy commented Nov 25, 2022

Yup - all the templates have v1 references to the API.

image

@richvdh
Copy link
Member Author

richvdh commented Nov 25, 2022

that link definitely doesn't show me those results, but thanks

@t3chguy
Copy link
Member

t3chguy commented Nov 25, 2022

Ah, might be because I'm on the Github Search Beta which makes the search quite usable

@turt2live
Copy link
Member

I'm failing to grok how this "signurl" is used in the 3pid flow. How is the URL generated? How is it used? I can't find it mentioned at https://spec.matrix.org/v1.5/client-server-api/#server-behaviour-7.

@richvdh worth noting that there's two flows an email invite can take, and only one of those flows is documented in the spec. The other is completely missing (I couldn't find an issue at first glance, but know this to be true)

@richvdh
Copy link
Member Author

richvdh commented Nov 25, 2022

@richvdh worth noting that there's two flows an email invite can take, and only one of those flows is documented in the spec. The other is completely missing (I couldn't find an issue at first glance, but know this to be true)

Opened matrix-org/matrix-spec#1359 to track this, then.

Re the signurl parameter: based on the evidence of https://github.com/matrix-org/sydent/blob/main/res/matrix-org/invite_template.eml and friends, it looks like:

  • The sign-ed25519 URI is a sydent URI generated by sydent, and as such we can consider it to be an implementation detail of sydent, and there is no need for it to be part of the spec.

    Following the example of similar endpoints in Synapse (such as the SSO callback), ideally it would not be on the /_matrix/... path and instead would be called something like /_sydent/get_stored_invite, to emphasise that it's not part of the spec. It would also ideally be documented in the Sydent docs.

    I have opened /_matrix/identity/api/v1/sign-ed25519 links in emails should use non-/_matrix namespace and have a better name #533 to track this.

  • The signurl parameter - indeed, the whole of the "single-click" link in the email, including the /#/room/<room_id> fragment, and the other pseudo-query-params - appears to be somewhat specific to Element, though since homeservers can change the target of the link via the unspecced org.matrix.web_client_location parameter, the whole thing seems somewhat dicey. I have opened Email invite flow uses unspecced org.matrix.web_client_location parameter matrix-spec#1360 to track this.

    It has to be said that a good start would be for the format of that link to be documented somewhere - either in the element-web docs, or the sydent docs. But in any case that's entirely orthogonal to the removal of /v1/.

@t3chguy
Copy link
Member

t3chguy commented Nov 28, 2022

and as such we can consider it to be an implementation detail of sydent, and there is no need for it to be part of the spec.

Yes, and no. Element Web code has to append a &mxid=<my_mxid> to the URL, presumably removing it would make it stop working, this behaviour means we'd need to spec that you need to do such for signurl somewhere.

@richvdh
Copy link
Member Author

richvdh commented Nov 28, 2022

Element Web code has to append a &mxid=<my_mxid> to the URL, presumably removing it would make it stop working

Ugh. But also, surely that would work just as well if the root of signurl was https://matrix.org/_sydent/get_stored_invite?... instead of https://matrix.org/_matrix/identity/api/v1/sign-ed25519?... ? In other words, the location of the endpoint is entirely internal to sydent, rather than being a matter for the spec, even if the parameters that should be passed to the endpoint have to be specced (which would be part of matrix-org/matrix-spec#1360).

@t3chguy
Copy link
Member

t3chguy commented Nov 28, 2022

Quite right - especially the ugh

@H-Shay
Copy link
Contributor

H-Shay commented Jun 3, 2023

Note that there is a config option to disable V1 bindings: #267 - although currently this is only applied to the bind endpoint:

if self.sydent.config.general.enable_v1_associations:
threepid_v1.putChild(b"bind", ThreePidBindServlet(sydent))

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

No branches or pull requests

7 participants