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

Sender Data: Populate missing SenderData when we receive device info via /keys/query #3753

Closed
andybalaam opened this issue Jul 23, 2024 · 3 comments · Fixed by #3849
Closed
Assignees

Comments

@andybalaam
Copy link
Contributor

Part of #3544

@richvdh
Copy link
Member

richvdh commented Aug 1, 2024

Referring to the flowchart at

/// ┌───────────────────────────────────────────────────────────────────┐
/// │ A (start - we have a to-device message containing a room key) │
/// └───────────────────────────────────────────────────────────────────┘
/// │
/// __________________________________▼______________________________
/// ╱ ╲
/// ╱ Does the to-device message contain the device_keys property from ╲yes
/// ╲ MSC4147? ╱ │
/// ╲_________________________________________________________________╱ │
/// │ no │
/// ▼ │
/// ┌───────────────────────────────────────────────────────────────────┐ │
/// │ B (there are no device keys in the to-device message) │ │
/// │ │ │
/// │ We need to find the device details. │ │
/// └───────────────────────────────────────────────────────────────────┘ │
/// │ │
/// __________________________________▼______________________________ │
/// ╱ ╲ │
/// ╱ Does the store contain a device whose curve key matches the ╲ ▼
/// ╲ sender of the to-device message? ╱yes
/// ╲_________________________________________________________________╱ │
/// │ no │
/// ▼ │
/// ╭───────────────────────────────────────────────────────────────────╮ │
/// │ C (we don't know the sending device) │ │
/// │ │ │
/// │ Give up: we have no sender info for this room key. │ │
/// ╰───────────────────────────────────────────────────────────────────╯ │
/// ┌─────────────────────────────────┘
/// ▼
/// ┌───────────────────────────────────────────────────────────────────┐
/// │ D (we have the device) │
/// └───────────────────────────────────────────────────────────────────┘
/// │
/// __________________________________▼______________________________
/// ╱ ╲
/// ╱ Is the session owned by the device? ╲yes
/// ╲___________________________________________________________________╱ │
/// │ no │
/// ▼ │
/// ╭───────────────────────────────────────────────────────────────────╮ │
/// │ E (the device does not own the session) │ │
/// │ │ │
/// │ Give up: something is wrong with the session. │ │
/// ╰───────────────────────────────────────────────────────────────────╯ │
/// ┌─────────────────────────────────┘
/// __________________________________▼______________________________
/// ╱ ╲
/// ╱ Is the device cross-signed by the sender? ╲yes
/// ╲___________________________________________________________________╱ │
/// │ no │
/// ▼ │
/// ┌───────────────────────────────────────────────────────────────────┐ │
/// │ F (we have device keys, but they are not signed by the sender) │ │
/// │ │ │
/// │ Store the device with the session, in case we can confirm it │ │
/// │ later. │ │
/// ╰───────────────────────────────────────────────────────────────────╯ │
/// ┌─────────────────────────────────┘
/// ▼
/// ┌───────────────────────────────────────────────────────────────────┐
/// │ G (device is cross-signed by the sender) │
/// └───────────────────────────────────────────────────────────────────┘
/// │
/// __________________________________▼______________________________
/// ╱ ╲
/// ╱ Does the cross-signing key match that used ╲yes
/// ╲ to sign the device? ╱ │
/// ╲_________________________________________________________________╱ │
/// │ no │
/// ▼ │
/// ╭───────────────────────────────────────────────────────────────────╮ │
/// │ Store the device with the session, in case we get the │ │
/// │ right cross-signing key later. │ │
/// ╰───────────────────────────────────────────────────────────────────╯ │
/// ┌─────────────────────────────────┘
/// ▼
/// ┌───────────────────────────────────────────────────────────────────┐
/// │ H (cross-signing key matches that used to sign the device!) │
/// │ │
/// │ Look up the user_id and master_key for the user sending the │
/// │ to-device message. │
/// │ │
/// │ Decide the master_key trust level based on whether we have │
/// │ verified this user. │
/// │ │
/// │ Store this information with the session. │
/// ╰───────────────────────────────────────────────────────────────────╯

As I understand things, we need to do the following when handling each device in a /keys/query response:

  1. Look for InboundGroupSessions from the device whose sender_data is UnknownDevice (ie, state C in the flow-chart). For such sessions, we now have the device, and can resume the flowchart at state D.

    • XXX: should we exclude sessions where SenderData::UnknownDevice::owner_check_failed is true (ie, state E)?

      I think it is possible for the owner check to pass after it has previously failed (notably, when the device has rotated its curve25519 key, and we receive the megolm key from an Olm session created by the new key before we receive a /keys/query response in which the curve25519 key is signed by the ed25519 key), so in theory we should re-check this.

      On the other hand, it's very much an unlikely edge-case, and since there could be a large number of sessions to process, it could be a DoS risk to reprocess all those sessions on each /keys/query response for the affected device.

      I'm mostly inclined to do whatever is easiest in terms of the database query.

  2. If, and only if, the device is now correctly cross-signed (ie, Device.is_cross_signed_by_owner is true, and we have the master cross-signing key for the owner), look for InboundGroupSessions from the device whose sender_data is DeviceInfo. We can resume the flowchart at H.

@richvdh
Copy link
Member

richvdh commented Aug 1, 2024

So, in short, I think we need a new method on cryptostore:

async fn get_inbound_group_sessions_for_device(
   &self, 
  device_key: Curve25519PublicKey, 
  sender_data_type: SenderDataType
) -> InboundGroupSessionStream;

enum SenderDataType {
   UnknownDevice,
   DeviceInfo,
   SenderKnown, // for completeness
}

// InboundGroupSessionStream is a struct that implements Stream<Item=Result<InboundGroupSession>>. Or something.

We'd need to add indices on (curve_key, sender_data_type) to both the sqlite and indexeddb table implementations to implement this.

@poljar
Copy link
Contributor

poljar commented Aug 2, 2024

I think that this makes sense, though not sure we ever figured out how to implement a streaming storage API.

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 a pull request may close this issue.

3 participants