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

Do not require UIA when first uploading cross-signing keys #1828

Merged
merged 4 commits into from
Jun 6, 2024

Conversation

zecakeh
Copy link
Contributor

@zecakeh zecakeh commented May 27, 2024

As per MSC3967.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from a team as a code owner May 27, 2024 10:35
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Comment on lines +31 to +36
- there is no existing cross-signing master key uploaded to the homeserver, OR
- there is an existing cross-signing master key and it exactly matches the
cross-signing master key provided in the request body. If there are any additional
keys provided in the request (self-signing key, user-signing key) they MUST also
match the existing keys stored on the server. In other words, the request contains
no new keys.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this section from the MSC, but it seems to ignore the case where a client uploads the master key and the other keys (self-signing and user-signing) separately.

My understanding according to the paragraph below is that setting the other keys if they were not set before, or re-uploading the same keys, should not require UIA, even if the master key is not part of the request (i.e. it was already uploaded before).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MSC (and this wording in particular) seems to imply that you have to upload the MSK on each request, otherwise you have to do UIA. I'm not sure that's what was intended, but it's probably ok, and we can't really just change it without another MSC.

data/api/client-server/cross_signing.yaml Outdated Show resolved Hide resolved
data/api/client-server/cross_signing.yaml Outdated Show resolved Hide resolved
Comment on lines +31 to +36
- there is no existing cross-signing master key uploaded to the homeserver, OR
- there is an existing cross-signing master key and it exactly matches the
cross-signing master key provided in the request body. If there are any additional
keys provided in the request (self-signing key, user-signing key) they MUST also
match the existing keys stored on the server. In other words, the request contains
no new keys.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MSC (and this wording in particular) seems to imply that you have to upload the MSK on each request, otherwise you have to do UIA. I'm not sure that's what was intended, but it's probably ok, and we can't really just change it without another MSC.

data/api/client-server/cross_signing.yaml Outdated Show resolved Hide resolved
@richvdh
Copy link
Member

richvdh commented May 28, 2024

LGTM other than a couple of nits.

Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
Signed-off-by: Kévin Commaille <zecakeh@tedomum.fr>
@zecakeh zecakeh requested a review from richvdh June 5, 2024 09:32
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@richvdh richvdh merged commit 1e303b3 into matrix-org:main Jun 6, 2024
12 checks passed
@zecakeh zecakeh deleted the msc3967 branch June 6, 2024 10:07
@zecakeh zecakeh mentioned this pull request Jun 7, 2024
53 tasks
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

Successfully merging this pull request may close these issues.

2 participants