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

Rotating the current device keys will break encryption and cause UTDs #2374

Open
Tracked by #245
BillCarsonFr opened this issue Mar 25, 2024 · 4 comments
Open
Tracked by #245

Comments

@BillCarsonFr
Copy link
Member

BillCarsonFr commented Mar 25, 2024

When a new session (device) is created, the device keys for that session (device) are uploaded:

  • Ed25519 fingerprint key
  • Curve25519 identity key

These keys are then published to the homeserver, which is done by using the device_keys property of the /keys/upload endpoint.

As per implementation guide:

Theoretically we should rotate the Curve25519 identity key from time to time, but we haven't implemented this yet.

Currently matrix-sdk-crypto will reject updates of device keys if the signing key (fingerprint key) has changed.

There are some reports of clients re-uploading a new set of Ed25519/Curve25519 keys. This will break encryption for devices that already downloaded the previous set of keys, because the new keys will be ignored, and the old one will still be used as a target for Olm messages.

As a consequence future encrypted to-device messages for that device will encrypt for the old keys and will fail to decrypt with: Olm event doesn't contain a ciphertext for our key.

Future devices will be able to decrypt because they will only have download the new keys.

The problem is that Synapse lets you update a new set of the device keys without complaining, and we know that it will break encryption.

What we can do

  • Modify the spec and synapse to do some sanity check on /keys/upload and rejects updates of the Ed25519 fingerprint key
  • Could we get some synapse stats on such failures
  • Given that no clients supports rotating the Curve25519 key, could we get rid of that in the spec?
@BillCarsonFr BillCarsonFr changed the title Rotating the current device this will break encryption and causing UTDs Rotating the current device keys this will break encryption and causing UTDs Mar 25, 2024
@BillCarsonFr BillCarsonFr changed the title Rotating the current device keys this will break encryption and causing UTDs Rotating the current device keys this will break encryption and cause UTDs Mar 25, 2024
@BillCarsonFr BillCarsonFr changed the title Rotating the current device keys this will break encryption and cause UTDs Rotating the current device keys will break encryption and cause UTDs Mar 25, 2024
@richvdh
Copy link
Member

richvdh commented Apr 26, 2024

As per spec:

Theoretically we should rotate the Curve25519 identity key from time to time, but we haven't implemented this yet.

@BillCarsonFr: where is this quote from, please?

@richvdh
Copy link
Member

richvdh commented Apr 26, 2024

Future devices will be able to decrypt because they will only have download the new keys.

@BillCarsonFr: should this be "encrypt"?

@BillCarsonFr
Copy link
Member Author

As per spec:

Theoretically we should rotate the Curve25519 identity key from time to time, but we haven't implemented this yet.

@BillCarsonFr: where is this quote from, please?

https://matrix.org/docs/matrix-concepts/end-to-end-encryption/

@dkasak
Copy link
Member

dkasak commented May 31, 2024

I'm all for ripping Curve25519 key rotation out of the spec and consider it nonsense. The only reason why we've historically opted for a pair of keys instead of just a single one was that we didn't know about or want to implement XEdDSA or equivalent, to be able to sign things with the Curve25519 key. Having a single key makes everything vastly simpler to reason about and also obviously cannot support key rotation without the device's identity changing.

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

No branches or pull requests

3 participants