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

e2ee/device verification: clarifications #1830

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0872ec7
e2ee/device verification: move general error handling under framework
sumnerevans May 28, 2024
01a816d
e2ee/device verification/error handling: fix typo start -> request
sumnerevans May 28, 2024
8905029
m.key.verification.start: remove wording that it is typically to-device
sumnerevans May 28, 2024
b8808ca
Add changelog
sumnerevans May 30, 2024
ea9dc84
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
edc4f38
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
3fbb048
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
d6c6a03
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
2ea4e13
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
d66a805
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
55375f4
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
d10deb0
fixup! e2ee/device verification: move general error handling under fr…
sumnerevans Jun 7, 2024
b87ffa6
e2ee/device verification: normalize all links
sumnerevans Jun 7, 2024
a79d5a6
fixup! Add changelog
sumnerevans Jun 7, 2024
4282a1a
e2ee/device verification start: clarify required nature of transactio…
sumnerevans Jun 7, 2024
a9350fc
fixup! e2ee/device verification: normalize all links
sumnerevans Jun 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Clarify a few things in the Device Verification section.
Clarify a few things in the Device Verification section and improve links throughout the section.
283 changes: 157 additions & 126 deletions content/client-server-api/modules/end_to_end_encryption.md
sumnerevans marked this conversation as resolved.
Show resolved Hide resolved

Large diffs are not rendered by default.

22 changes: 13 additions & 9 deletions data/event-schemas/schema/m.key.verification.cancel.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,30 @@ properties:
* `m.user`: The user cancelled the verification.

* `m.timeout`: The verification process timed out. Verification processes
can define their own timeout parameters.
can define their own timeout parameters.

* `m.unknown_transaction`: The device does not know about the given transaction
ID.
ID.

* `m.unknown_method`: The device does not know how to handle the requested
method. This should be sent for `m.key.verification.start` messages and
messages defined by individual verification processes.
* `m.unknown_method`: The device does not know how to handle the
requested method. This should be sent for
[`m.key.verification.start`](#mkeyverificationstart) messages and
messages defined by individual verification processes.

* `m.unexpected_message`: The device received an unexpected message. Typically
raised when one of the parties is handling the verification out of order.
* `m.unexpected_message`: The device received an unexpected message.
Typically raised when one of the parties is handling the
verification out of order.

* `m.key_mismatch`: The key was not verified.

* `m.user_mismatch`: The expected user did not match the user verified.

* `m.invalid_message`: The message received was invalid.

* `m.accepted`: A `m.key.verification.request` was accepted by a different
device. The device receiving this error can ignore the verification request.
* `m.accepted`: A
[`m.key.verification.request`](#mkeyverificationrequest) was
accepted by a different device. The device receiving this error can
ignore the verification request.

Clients should be careful to avoid error loops. For example, if a device sends
an incorrect message and the client returns `m.invalid_message` to which it
Expand Down
11 changes: 6 additions & 5 deletions data/event-schemas/schema/m.key.verification.m.relates_to.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
---
description: |-
Required when sent as an in-room message. Indicates the
`m.key.verification.request` that this message is related to. Note that for
encrypted messages, this property should be in the unencrypted portion of the
event.
[`m.key.verification.request`](#mkeyverificationrequest) that this message is
Copy link
Member

Choose a reason for hiding this comment

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

Given that this is specifically about in-room messages, this is linking to the wrong thing.

Suggested change
[`m.key.verification.request`](#mkeyverificationrequest) that this message is
[`m.key.verification.request`](#mroommessagemkeyverificationrequest) that this message is

related to. Note that for encrypted messages, this property should be in the
unencrypted portion of the event.
properties:
rel_type:
type: string
Expand All @@ -15,7 +15,8 @@ properties:
event_id:
type: string
description: |-
The event ID of the `m.key.verification.request` that this message is
related to.
The event ID of the
[`m.key.verification.request`](#mkeyverificationrequest) that this
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[`m.key.verification.request`](#mkeyverificationrequest) that this
[`m.key.verification.request`](#mroommessagemkeyverificationrequest) that this

message is related to.
type: object
title: VerificationRelatesTo
8 changes: 4 additions & 4 deletions data/event-schemas/schema/m.key.verification.ready.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ allOf:

description: |-
Accepts a key verification request. Sent in response to an
`m.key.verification.request` event.
[`m.key.verification.request`](#mkeyverificationrequest) event.
properties:
content:
properties:
Expand All @@ -16,14 +16,14 @@ properties:
type: string
description: |-
Required when sent as a to-device message. The transaction ID of the
verification request, as given in the `m.key.verification.request`
message.
verification request, as given in the
[`m.key.verification.request`](#mkeyverificationrequest) message.
methods:
type: array
description: |-
The verification methods supported by the sender, corresponding to
the verification methods indicated in the
`m.key.verification.request` message.
[`m.key.verification.request`](#mkeyverificationrequest) message.
items:
type: string
m.relates_to:
Expand Down
5 changes: 3 additions & 2 deletions data/event-schemas/schema/m.key.verification.request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ allOf:

description: |-
Requests a key verification using to-device messaging. When requesting a key
verification in a room, a `m.room.message` should be used, with
[`m.key.verification.request`](#mroommessagemkeyverificationrequest) as msgtype.
verification in a room, a [`m.room.message`](#mroommessage should be used,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
verification in a room, a [`m.room.message`](#mroommessage should be used,
verification in a room, a [`m.room.message`](#mroommessage) should be used,

with [`m.key.verification.request`](#mroommessagemkeyverificationrequest) as
msgtype.
properties:
content:
properties:
Expand Down
50 changes: 44 additions & 6 deletions data/event-schemas/schema/m.key.verification.start.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,20 @@ properties:
transaction_id:
type: string
description: |-
Required when sent as a to-device message. An opaque identifier for
the verification process. Must be unique with respect to the devices
involved. Must be the same as the `transaction_id` given in the
`m.key.verification.request` if this process is originating from a
request.
Required when sent as a to-device message unless the start event is
sent without a corresponding
[`m.key.verification.request`](#mkeyverificationrequest).

An opaque identifier for the verification process. Must be unique
with respect to the devices involved.

Must be the same as the `transaction_id` given in the
[`m.key.verification.request`](#mkeyverificationrequest) if this
process is originating from a request.

Note that sending a start event without a request is deprecated, and
clients should not send a start event without first sending a request
event, but clients should handle other clients doing so.
Comment on lines +30 to +32
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that sending a start event without a request is deprecated, and
clients should not send a start event without first sending a request
event, but clients should handle other clients doing so.
Note that sending a `start` event without a `request` is deprecated, and
clients should not send a `start` event without first sending a `request`
event, but clients should handle other clients doing so.

method:
type: string
description: |-
Expand All @@ -32,7 +41,36 @@ properties:
when the `method` chosen only verifies one user's key. This field will
never be present if the `method` verifies keys both ways.
m.relates_to:
$ref: m.key.verification.m.relates_to.yaml
description: |-
Required when sent as an in-room message unless the start event is
sent without a corresponding
[`m.key.verification.request`](#mkeyverificationrequest).

Indicates the
[`m.key.verification.request`](#mkeyverificationrequest) that this
message is related to. Note that for encrypted messages, this
property should be in the unencrypted portion of the event.
Comment on lines +45 to +52
Copy link
Member

Choose a reason for hiding this comment

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

again, these are linking to the wrong m.key.verification.request


Note that sending a start event without a request is deprecated, and
clients should not send a start event without first sending a request
event, but clients should handle other clients doing so.
Comment on lines +54 to +56
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that sending a start event without a request is deprecated, and
clients should not send a start event without first sending a request
event, but clients should handle other clients doing so.
Note that sending a `start` event without a `request` is deprecated, and
clients should not send a `start` event without first sending a `request`
event, but clients should handle other clients doing so.

properties:
rel_type:
type: string
enum:
- m.reference
description: |-
The relationship type. Currently, this can only be an
[`m.reference`](/client-server-api/#reference-relations)
relationship type.
event_id:
type: string
description: |-
The event ID of the
[`m.key.verification.request`](#mkeyverificationrequest) that
this message is related to.
type: object
title: VerificationRelatesTo
Comment on lines +57 to +73
Copy link
Member

Choose a reason for hiding this comment

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

it should be possible to inherit these, but override the description. Probably something like:

      m.relates_to:
        allOf:
         - $ref: m.key.verification.m.relates_to.yaml
         - description: |-
           ... blah

required:
- from_device
- method
Expand Down