Skip to content

mark_key_package_synced deletes the record without purging the provider key #367

Description

@bahdotsh

Found while verifying ADR 0012 against the code for #365. The ADR now records this as a known gap; this issue is to decide what the behaviour should actually be.

What happens

MlsManager::mark_key_package_synced (crates/offline-protocol-mls/src/manager.rs:755-760) deletes the bundle record and nothing else:

pub fn mark_key_package_synced(&self, package_id: &str) -> Result<()> {
    let key_type = StorageKeyType::KeyPackage.as_str();
    self.storage.delete(key_type, package_id)?;
    ...
}

No purge_key_package_material. ADR 0012 states the rule this appears to break:

Deleting the bundle record alone leaves the private init key in the MLS provider forever, because only a peer's Welcome removes one. This was the pre-existing leak and it is the part most likely to be reintroduced.

and lists "deleting a key package record without purging its provider key" under "what would undo this".

Why it is ambiguous rather than obviously a bug

Retaining the provider key here is defensible. "Synced" means the package has been published, so a peer may still build a Welcome against it, and that Welcome must open — the key has to outlive the record. The two-stage expiry path (withdraw, then destroy past a 7-day grace) exists for exactly the case where the key must stay usable for a while.

The problem is what carries the expiry. Once the record is gone, nothing tracks the package, so a published-but-never-claimed key has no path to destruction at all, which is the stranding the rest of that section forbids.

Reach

No engine path calls it. It exists only on the FFI surface (mls_mark_key_package_synced, crates/offline-protocol-uniffi/src/lib.rs:5815) and is present in all three generated bindings, so an application can reach it.

Decide one of

  1. Keep the retention, add the tracking. Move the package to a published/withdrawn state instead of deleting the record, so expiry still owns it.
  2. Purge on sync, if a synced package is genuinely not expected to receive a Welcome (needs confirming — if wrong, this silently breaks establishment for anyone using it).
  3. Remove the FFI surface if publishing is not actually an application concern, and let expiry handle the whole lifecycle.

Option 1 looks right, but the call belongs to whoever owns the push path. Whatever is chosen, ADR 0012's "The one record-only delete that survives" section should be updated to match.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions