-
Notifications
You must be signed in to change notification settings - Fork 379
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
MSC4147: Including device keys with Olm-encrypted events #4147
base: main
Are you sure you want to change the base?
Changes from 3 commits
ce31f30
4fa6b3f
62352ca
419bed3
33bb7c2
04acb6b
407e1d9
5bb2093
0deb58f
28d03f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,135 @@ | ||||||
# MSC4147: Including device keys with Olm-encrypted events | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
When a Megolm session is sent from one device to another via Olm, the recipient | ||||||
can | ||||||
[query](https://spec.matrix.org/unstable/client-server-api/#post_matrixclientv3keysquery) | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
the sender's device keys and check whether the device has been cross-signed in | ||||||
order to determine whether the sending device can be trusted. However, this | ||||||
does not work if the sending device has since logged out as the recipient will | ||||||
not be able to query the sender's device keys. For example, this can happen if | ||||||
the recipient is offline for a long time. | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
One way to solve this is to include a copy of the device keys in the | ||||||
Olm-encrypted message, along with the cross-signing signatures, so that the | ||||||
recipient does not have to try to query the sender's keys. | ||||||
|
||||||
## Proposal | ||||||
|
||||||
The plaintext payload of the [`m.olm.v1.curve25519-aes-sha2` encryption | ||||||
uhoreg marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
algorithm](https://spec.matrix.org/unstable/client-server-api/#molmv1curve25519-aes-sha2) | ||||||
is currently of the form: | ||||||
|
||||||
```json | ||||||
{ | ||||||
"type": "<type of the plaintext event>", | ||||||
"content": "<content for the plaintext event>", | ||||||
"sender": "<sender_user_id>", | ||||||
"recipient": "<recipient_user_id>", | ||||||
"recipient_keys": { | ||||||
"ed25519": "<our_ed25519_key>" | ||||||
}, | ||||||
"keys": { | ||||||
"ed25519": "<sender_ed25519_key>" | ||||||
} | ||||||
} | ||||||
``` | ||||||
|
||||||
We propose to add a new property: `device_keys`, which is a copy of what the | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did wonder if we could improve the name here; in particular, to reflect the fact that these are the keys for the sender device.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the payload, we have also have |
||||||
server would return in response to a | ||||||
[`/keys/query`](https://spec.matrix.org/unstable/client-server-api/#post_matrixclientv3keysquery) | ||||||
request, as the device keys for the sender's device. In other words, the | ||||||
plaintext payload will now look something like: | ||||||
|
||||||
```json | ||||||
{ | ||||||
"type": "<type of the plaintext event>", | ||||||
"content": "<content for the plaintext event>", | ||||||
"sender": "<sender_user_id>", | ||||||
"recipient": "<recipient_user_id>", | ||||||
"recipient_keys": { | ||||||
"ed25519": "<our_ed25519_key>" | ||||||
}, | ||||||
"keys": { | ||||||
"ed25519": "<sender_ed25519_key>" | ||||||
}, | ||||||
"device_keys": { | ||||||
"algorithms": ["<supported>", "<algoriithms>"], | ||||||
"user_id": "<user_id>", | ||||||
"device_id": "<device_id>", | ||||||
"keys": { | ||||||
"ed25519:<device_id>": "<our_ed25519_key>", | ||||||
"curve25519:<device_id>": "<our_curve25519_key>" | ||||||
uhoreg marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}, | ||||||
"signatures": { | ||||||
"<user_id>": { | ||||||
"ed25519:<device_id>": "<device_signature>", | ||||||
"ed25519:<ssk_id>": "<ssk_signature>", | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
``` | ||||||
|
||||||
If this property is present, the `keys`.`ed25519` property of the plaintext | ||||||
payload must be the same as the `device_keys`.`keys`.`ed25519:<DEVICEID>` | ||||||
property. | ||||||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
As the `keys` property is now redundant, it may be removed in a future version | ||||||
of Olm. | ||||||
uhoreg marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
## Potential issues | ||||||
|
||||||
Adding this property will increase the size of the event. This could be | ||||||
mitigated by only sending the `device_keys` in pre-key messages (Olm messages | ||||||
with `type: 0` in the `m.room.encrypted` event -- with the rationale that if | ||||||
the Olm message is a normal (non-pre-key) message, this means that the | ||||||
recipient has already decrypted a pre-key message that contains the | ||||||
information, and so does not need to be re-sent the information), or if the | ||||||
signatures change (for example, if the sender resets their cross-signing keys), | ||||||
or if the sender has not yet sent their `device_keys`. However, this requires | ||||||
additional bookkeeping, and it is not clear whether this extra complexity is | ||||||
worth the reduction in bandwidth. | ||||||
|
||||||
If the sender resets their cross-signing keys, then the self-signing signature | ||||||
in the `device_keys` is meaningless. The recipient will need to re-query the | ||||||
device keys, and will need to treat the sender as untrusted if it fails to do | ||||||
so. The sender could include the self-signing key, signed by the | ||||||
master-signing key, in the plaintext event, so that if the user only resets | ||||||
their self-signing key but retains their master-signing key, the recipient can | ||||||
still check the sender's device keys. However, this will further increase the | ||||||
size of the event, and it is not common for clients to reset the self-signing | ||||||
key without also resetting the master-signing key, so this is unlikely to give | ||||||
much benefit. | ||||||
|
||||||
## Alternatives | ||||||
|
||||||
The `device_keys` property could be added to the cleartext. That is, it could | ||||||
be added as a property to the `m.room.encrypted` event. This information is | ||||||
already public, as it is accessible from `/keys/query` (while the device is | ||||||
logged in), and does not need to be authenticated as it is protected by .the | ||||||
self-signing signature, so it does not seem to need to be encrypted. However, | ||||||
there seems to be little reason not to encrypt the information. | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
The `device_keys` property could be added to the cleartext by the sender's | ||||||
homeserver, rather than by the sending client. Possibly within an `unsigned` | ||||||
property, as that is where properties added by homeservers are customarily | ||||||
added. It is not clear what advantage there would be to having this | ||||||
information being added by the client. | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
## Security considerations | ||||||
|
||||||
If a device is logged out, there is no indication why it was logged out. For | ||||||
example, an attacker could steal a device and use it send a message. The user, | ||||||
upon realizing that the device has been stolen, could log out the device, but | ||||||
the message may still be sent, if the user does not notice the message and | ||||||
redact it. Thus the recipient device should still indicate that the message | ||||||
came from a deleted device. | ||||||
|
||||||
## Unstable prefix | ||||||
|
||||||
Until this MSC is accepted, the new property should be named | ||||||
`org.matrix.msc4147.device_keys`. | ||||||
|
||||||
## Dependencies | ||||||
|
||||||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementation requirements:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been implemented in the Rust
matrix-sdk-crypto
crate, both sending and using. Demonstration of the problem being solved: https://youtu.be/b1jJlT2ENT8?feature=shared&t=345There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few links to the implementation.
On the sender side, the field is added here.
On the receiver side, it is deserialized after decryption into a DecryptedOlmV1Event. It is then used in SenderDataFinder whose job it is to construct a SenderData which records our knowledge about the sending device for a given session.