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

MSC1219: storing megolm keys serverside #1538

Merged
merged 36 commits into from
Oct 29, 2019
Merged
Changes from 2 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
1b81970
initial commit of e2e backup proposal
uhoreg Aug 18, 2018
6e8ba1f
add more details
uhoreg Aug 24, 2018
8777232
various clarifications
uhoreg Sep 6, 2018
846e9e8
add clarifications
uhoreg Sep 6, 2018
72df5fe
add details on recovery key format, and some cleanups/fixes
uhoreg Oct 10, 2018
de51203
change "string or object" to just "object"
uhoreg Oct 11, 2018
b45416e
change version from string to integer, plus other minor improvements
uhoreg Oct 20, 2018
9d51d1e
expand the background
uhoreg Oct 20, 2018
c8eac3e
add details on how the encryption is done
uhoreg Oct 24, 2018
dc0dd18
note that version is optional for GET, and say what to do when no key…
uhoreg Oct 25, 2018
7b4b4a2
fix some English and some minor additions
uhoreg Oct 30, 2018
982abc1
add some examples
uhoreg Oct 30, 2018
7713a0f
snake-case for consistency
uhoreg Oct 30, 2018
3918ed3
distinguish between retrieving an empty backup and a nonexistent backup
uhoreg Oct 30, 2018
2dce235
wording fixes
uhoreg Nov 14, 2018
b45cf44
providing an alternative to key sharing is currently a non-goal
uhoreg Nov 14, 2018
c9b38cb
Key backup: add `PUT /room_keys/version/{version}` to allow matrix cl…
manuroe Feb 6, 2019
e02b345
Revert "Key backup: add `PUT /room_keys/version/{version}` to allow m…
manuroe Feb 6, 2019
2099308
Key backup: add `PUT /room_keys/version/{version}` to allow matrix cl…
manuroe Feb 6, 2019
d43b595
Key backup: Fix PR remarks on `PUT /room_keys/version/{version}`
manuroe Feb 7, 2019
e7f7926
add algorithm and version to the example
uhoreg Feb 8, 2019
ed945d6
Key backup: Expose the number of keys stored in the backup
manuroe Feb 6, 2019
82ff866
Key backup: Add `hash` to represent stored keys
manuroe Feb 6, 2019
7cde319
Key backup: Explain `hash` better
manuroe Feb 7, 2019
0051c6a
Key backup: Return {hash, count} for key upload requests
manuroe Feb 7, 2019
87824c1
Update proposals/1219-storing-megolm-keys-serverside.md
richvdh Mar 15, 2019
1c4262e
Apply suggestions from code review
richvdh Mar 15, 2019
825757f
add information about verifying backup by entering key
uhoreg Jul 31, 2019
80adbaf
switch to MSC1946 for storing recovery key
uhoreg Jul 31, 2019
7ed5367
clarifications, fix formatting
uhoreg Aug 10, 2019
cf953c4
clarifications, change "hash" to "etag"
uhoreg Sep 9, 2019
8123c4e
additional clarification
uhoreg Sep 10, 2019
54e73e4
Apply suggestions from code review
uhoreg Oct 4, 2019
576177b
make version optional in versions update
uhoreg Oct 9, 2019
5799c43
add HTTP status codes for errors and move key format to the right spot
uhoreg Oct 10, 2019
a6977f1
Update proposals/1219-storing-megolm-keys-serverside.md
uhoreg Oct 28, 2019
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
234 changes: 234 additions & 0 deletions proposals/1219-storing-megolm-keys-serverside.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
Storing megolm keys serverside
==============================

Background
----------

We *optionally* let clients store a copy of their megolm inbound session keys
on the HS so that they can recover history if all devices are lost without an
explicit key export; fix UISIs; support clients with limited local storage for
keys.
uhoreg marked this conversation as resolved.
Show resolved Hide resolved

See also:

* https://github.com/matrix-org/matrix-doc/issues/1219
* https://github.com/vector-im/riot-web/issues/3661
* https://github.com/vector-im/riot-web/issues/5675
* https://docs.google.com/document/d/1MOoIA9qEKIhUQ3UmKZG-loqA8e0BzgWKKlKRUGMynVc/edit#
(old version of proposal)

Proposal
--------

This proposal creates new APIs to allow clients to back up room decryption keys
on the server. Decryption keys are encrypted (using public key crypto) before
being sent to the server along with some unencrypted metadata to allow the
server to manage the backups, overwriting backups with "better" versions of the
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
keys. The user is given a private recovery key to save for recovering the keys
from the backup.

Clients can create new versions of backups. Aside from the initial backup
creation, a client might start a new version of a backup when, for example, a
user loses a device, and wants to ensure that that device does not get any new
decryption keys.

Once one client has created a backup version, other clients can fetch the
public key for the backup from the server and add keys to the backup, if they
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
trust that the backup was not created by a malicious device.

uhoreg marked this conversation as resolved.
Show resolved Hide resolved
### Possible UX for interactive clients

On receipt of encryption keys (1st time):

1. client checks if there is an existing backup: `GET /room_keys/version`
1. if not, ask if the user wants to back up keys
1. if yes:
1. generate new key pair
2. create new backup version: `POST /room_keys/version`
3. display private key to user to save
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
2. if no, exit and remember decision (user can change their mind later)
3. while prompting, continue to poll `GET /room_keys/versions`, as
another device may have created a backup. If so, go to 1.2.
2. if yes, get public key, prompt user to verify a device that signed the
key¹, or enter recovery key (which can derive the backup key).
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
1. User can also decide to create a new backup, in which case, go to 1.1.
2. send key to backup: `PUT /room_keys/keys/${roomId}/${sessionId}?version=$v`
3. continue backing up keys as we receive them (may receive a
`M_WRONG_ROOM_KEYS_VERSION` error if a new backup version has been created:
see below)

On `M_WRONG_ROOM_KEYS_VERSION` error when trying to `PUT` keys:

1. get the current version
2. notify the user that there is a new backup version, and display relevant
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should be exposing multiple backup versions to the user: i.e. "Online backup started by device at ", to help protect the user against a malicious attacker starting a new backup to a) overwrite their actual keys or b) steal their keys. We could also let the user delete their online backups (after doing UI auth) to help them control their keys.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't really have an opinion on what other metadata we should be exposing to the user, but I think we can put it in a future MSC if we think it's worthwhile. Clients already check that a backup is trusted before storing keys in a backup, so I think the risk of an attacker creating a new backup to steal keys is low.

information
3. confirm with user that they want to use the backup (user may want use the
backup, to stop backing up keys, or to create a new backup)
4. verify the device that signed the backup key¹, or enter recovery key

¹: cross-signing (when that is completed) can be used to verify the device
that signed the key.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be very useful to spell out the sort of flows we expose via settings as per the original proposal: i.e. the ability to explicitly recover keys from a backup; change (rotate) the recovery key; delete the backup(s); and start a (new) backup. Particularly in terms of how much we should re-auth the user before each operation.

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 has been mentioned, but doesn't go into much detail. I don't think we need to go into too much detail, since I don't think we should be mandating too much UI-wise in the spec.

On receipt of undecryptable message:

1. ask user if they want to restore backup (ask whether to get individual key,
room keys, or all keys). (This can be done in the same place as asking if
the user wants to request keys from other devices.)
2. if yes, prompt for private key, and get keys: `GET /room_keys/keys`

Users can also set up or disable backups, or restore from backup via user
settings.

### API

#### Backup versions

##### `POST /room_keys/version`

Create a new backup version.

Body parameters:

- `algorithm` (string): Required. The algorithm used for storing backups.
Currently, only `m.megolm_backup.v1` is defined. (FIXME: change the algorithm
name to include the encryption method)
- `auth_data` (string or object): Required. algorithm-dependent data. For
`m.megolm_backup.v1`, this is a signedjson object with the following keys:
- `public_key` (string): the public key used to encrypt the backups
- `signatures` (object): signatures of the public key

Example:

```javascript
{
"algorithm": "m.megolm_backup.v1",
"auth_data": {
"public_key": "abcdefg",
"signatures": {
"something": {
"ed25519:something": "hijklmnop"
}
}
}
}
```

On success, returns a JSON object with keys:

- `version` (integer): the backup version

##### `GET /room_keys/version`

Get information about the current version.

On success, returns a JSON object with keys:

- `algorithm` (string): Required. Same as in the body parameters for `POST
/room_keys/version`.
- `auth_data` (string or object): Required. Same as in the body parameters for
`POST /room_keys/version`.
- `version` (integer): the backup version
uhoreg marked this conversation as resolved.
Show resolved Hide resolved

Error codes:

- `M_UNKNOWN`: No backup version has been created. FIXME: why not
`M_NOT_FOUND`?

#### Storing keys

##### `PUT /room_keys/keys/${roomId}/${sessionId}?version=$v`
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: It feels odd to mix path params, query params and a request body together. Could the version go in the path?

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably this was done for symmetry with the corresponding GET endpoint where version is optional. I see that you suggested doing the same there, and using a magic value to represent "latest", though that would mean that in this endpoint, we'd need to forbid using that magic value, which seems non-ideal. My own opinion is that the current version is less bad, but I don't feel very strongly about it.


Store the key for the given session in the given room, using the given backup
version.

If the server already has a backup in the backup version for the given session
and room, then it will keep the "better" one ...

Body parameters:

- `first_message_index` (integer): Required. The index of the first message
in the session that the key can decrypt.
- `forwarded_count` (integer): Required. The number of times this key has been
forwarded.
- `is_verified` (boolean): Whether the device backing up the key has verified
the device that the key is from.
- `session_data` (string or object): Algorithm-dependent data. For
`m.megolm_backup.v1`, this is an object with the following keys:
- `ciphertext` (string): the encrypted version of the session key. See below
for how the session key is encoded.
- `ephemeral` (string): the public ephemeral key that was used to encrypt the
session key.
- `mac` (string): the message authentication code for the ciphertext. FIXME:
more details

On success, returns ... ?
uhoreg marked this conversation as resolved.
Show resolved Hide resolved

Error codes:

- `M_WRONG_ROOM_KEYS_VERSION`: the version specified does not match the current
backup version

##### `PUT /room_keys/keys/${roomId}?version=$v`

Store several keys for the given room, using the given backup version.

Behaves the same way as if the keys were added individually using `PUT
/room_keys/keys/${roomId}/${sessionId}?version=$v`.

Body paremeters:
- `sessions` (object): an object where the keys are the session IDs, and the
values are objects of the same form as the body in `PUT
/room_keys/keys/${roomId}/${sessionId}?version=$v`.

Returns the same as `PUT
/room_keys/keys/${roomId}/${sessionId}?version=$v`

##### `PUT /room_keys/keys/?version=$v`

...

#### Retrieving keys

##### `GET /room_keys/keys/${roomId}/${sessionId}?version=$v`
Copy link
Member

Choose a reason for hiding this comment

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

I'd also put the version in the path here, with a magic value to represent "latest"

##### `GET /room_keys/keys/${roomId}?version=$v`
##### `GET /room_keys/keys/?version=$v`

#### Deleting keys

##### `DELETE /room_keys/keys/${roomId}/${sessionId}?version=$v`
##### `DELETE /room_keys/keys/${roomId}?version=$v`
##### `DELETE /room_keys/keys/?version=$v`

#### Key format

Session keys are encoded as a JSON object with the properties:

- `algorithm` (string): `m.megolm.v1.aes-sha2`
- `sender_key` (string): base64-encoded device curve25519 key
- `sender_claimed_keys` (object): object containing the identity keys for the
sending device
- `forwardingCurve25519KeyChain` (array): zero or more curve25519 keys for
devices who forwarded the session key
- `session_key` (string): base64-encoded session key

Tradeoffs
---------

Security Considerations
-----------------------

An attacker who gains access to a user's account can delete or corrupt their
uhoreg marked this conversation as resolved.
Show resolved Hide resolved
key backup. This proposal does not attempt to protect against that.

An attacker who gains access to a user's account can create a new backup
version using a key that they control. For this reason, clients SHOULD confirm
with users before sending keys to a new backup version.

Other Issues
------------

Since many clients will receive encryption keys at around the same time,
clients should randomly offset their requests ...
uhoreg marked this conversation as resolved.
Show resolved Hide resolved

Conclusion
----------