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

initial spec for cross-signing #2536

Merged
merged 10 commits into from
Dec 15, 2020
Merged

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented May 12, 2020

Spec PR for #1756

Does not have any mentions of SSSS, since that is not written up yet.

Rendered version of changes at

@uhoreg uhoreg marked this pull request as ready for review May 12, 2020 22:36
@Sorunome
Copy link
Contributor

Not sure which lines and thus don't know where to do line-comments on the PR, but in POST /_matrix/client/r0/keys/signatures/upload the section "Request format" says No parameters, while the text above lays out the map and the example request also contains said map.

@@ -233,7 +234,74 @@ paths:
"device_display_name": "Alice's mobile phone"
}
}

master_keys:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to only query the cross-signing keys? Is it sufficient to query only one device key of a user to them in the reply, or do you need to query all device keys of a user (empty list)?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, there's no way to query just the cross-signing keys. Querying device is sufficient. The user's ID just needs to appear in the query.

@uhoreg
Copy link
Member Author

uhoreg commented May 21, 2020

Not sure which lines and thus don't know where to do line-comments on the PR, but in POST /_matrix/client/r0/keys/signatures/upload the section "Request format" says No parameters, while the text above lays out the map and the example request also contains said map.

Yes, that seems to just be the way our documentation generator works. On fact, the whole body is a parameter, but since it doesn't have any listed properties, the documentation generator thinks there's nothing there.

@babolivier
Copy link
Contributor

As a comment that came out of matrix-org/synapse#7594, it's not super obvious that the USK is only used by its owner and isn't sent to other servers over federation (unlike the two other keys), maybe this could be detailed here?

@uhoreg uhoreg requested a review from a team June 12, 2020 15:39
@uhoreg
Copy link
Member Author

uhoreg commented Jun 12, 2020

As a comment that came out of matrix-org/synapse#7594, it's not super obvious that the USK is only used by its owner and isn't sent to other servers over federation (unlike the two other keys), maybe this could be detailed here?

Words (and diagram!) have been added that hopefully clarify this.

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.

generally looks great. a few bits and bobs.

changelogs/client_server/newsfragments/2536.feature Outdated Show resolved Hide resolved
specification/modules/end_to_end_encryption.rst Outdated Show resolved Hide resolved
specification/modules/end_to_end_encryption.rst Outdated Show resolved Hide resolved
specification/modules/end_to_end_encryption.rst Outdated Show resolved Hide resolved
specification/modules/end_to_end_encryption.rst Outdated Show resolved Hide resolved
api/client-server/cross_signing.yaml Outdated Show resolved Hide resolved
api/client-server/cross_signing.yaml Outdated Show resolved Hide resolved
security:
- accessToken: []
parameters:
- in: body
Copy link
Member

Choose a reason for hiding this comment

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

this isn't working in the rendered output, for some reason.

Copy link
Member Author

@uhoreg uhoreg Oct 16, 2020

Choose a reason for hiding this comment

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

It's because our generator wants the body to have a fixed set of properties, rather than being a mapping of IDs, so I don't think this can be fixed without making changes to the build system.

schema:
type: object
properties:
failures:
Copy link
Member

Choose a reason for hiding this comment

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

nor this

Copy link
Member Author

Choose a reason for hiding this comment

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

This looks fine to me

uhoreg and others added 2 commits June 19, 2020 21:40
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@uhoreg uhoreg requested a review from richvdh October 17, 2020 02:25
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.

lgtm!

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@uhoreg uhoreg merged commit f352de9 into matrix-org:master Dec 15, 2020
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.

4 participants