fix(collab): fail-closed E2e seal on BOTH write paths (CRITICAL #168 + #170) - #172
Merged
Conversation
…es (CRITICAL #168) build_kb_node_update_request decided seal-vs-plaintext purely on content_key.is_some() with no E2e knowledge, so an E2e KB with a missing key (e.g. after restart, before a share/join response repopulated content_keys) — or a seal failure — shipped PLAINTEXT to the key-blind daemon, which stores + relays it. Silent confidentiality breach. Fix (fail closed): - build_kb_node_update_request takes `e2e: bool` and returns Option: None = REFUSE. Both the no-key arm and the Err(_) seal-failure arm return None when e2e — never the cleartext. - New `kb_collection_is_e2e(collection_state)` reads the AUTHORITATIVE signed derive_encryption (F1 anchor pin), not the relay-flippable unsigned flag — a downgrade can't trick us into plaintext. The editor stamps this onto CollabCommand::KbNodeUpdate.e2e at drain time (the editor thread is the authority; it holds the collection replica). - The handler lazily reloads the persisted content key from content_key_store on an E2e KB with no in-memory key (restart liveness), then refuses + REQUEUES with a loud warn! if still absent — the edit retries when the key arrives (owner: on reload; member: on approve). No silent plaintext, no lost edit; observable via tracing. Tests (principle #14, the attacker oracle): - build_kb_node_update_request_fails_closed_on_e2e_without_key — E2e + no key ⇒ None on BOTH the signed and unsigned paths; selective control: the same inputs on an UNENCRYPTED KB still ship (Some), proving the refusal is the e2e gate working, not a dead function; E2e + key ⇒ Some (seals). - kb_collection_is_e2e_reads_signed_mode_not_the_flippable_flag — plain ⇒ false; signed enable ⇒ true; a relay forging the unsigned flag back to None does NOT downgrade. Sibling leak in the share/re-share path (raw plaintext node snapshots) filed as #170. 117 collab_bridge tests green; clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ntext snapshots (CRITICAL #170) The ShareKb handler sent node_states as RAW PLAINTEXT (update_to_base64(state)). The editor re-shares durably-shared KBs on reconnect, so an already-E2e KB leaked its full plaintext node content to the key-blind daemon every reconnect — worse than #168 (full content, not a delta). Fix (fail closed, principle #8 extraction): - New pure `select_share_node_states(kb_id, e2e, node_states, op_sets)` decides the wire states. On an UNENCRYPTED KB it is byte-identical to before (base64 the plaintext). On an E2e KB it NEVER ships the plaintext snapshot: it sends the already-sealed op-set we hold for the node (idempotent — the daemon stores the same op-set kb/node_update produces) and SKIPS a node we have no op-set for (loud warn), rather than leak its plaintext. - ShareKb computes `e2e = kb_collection_is_e2e(&collection_state)` (the authoritative signed mode, #168 helper) and routes through the new helper. Test (principle #14): select_share_node_states_never_ships_plaintext_on_e2e — E2e ships the sealed op-set for a held node, SKIPS a bare node, and a plaintext canary appears in NO wire payload; selective control: the unencrypted path still ships every plaintext node. Residual (documented separately): content shared BEFORE encryption was enabled stays plaintext on the daemon — retroactive re-encryption-on-enable is a known limitation, not this leak. This fix stops the re-share from RE-leaking post-enable content. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…encryption-on-enable limitation (#171) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cuttlefisch
added
area:collab
…r-op client_id (#167/#168) Proves WHY #168's always-seal closes #167's deletion-fence gap for E2e KBs: a deletion sealed into the op-set rides a client-id-stamped outer op, so update_new_op_authors attributes it to the (stale) seal client_id and the ADR-023 fence rejects it — even though the inner op is a pure, otherwise-unattributable yrs delete. Documents the residual: a PLAINTEXT pure-delete remains unattributable (the unencrypted-path #167 gap). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #168 and #170. Two CRITICAL confidentiality bugs found in the impl-pass security review over merged 3b/3c — an E2e KB could ship plaintext to the key-blind daemon on both write paths. This makes both fail closed.
#168 —
kb/node_updatefail-openbuild_kb_node_update_requestdecided seal-vs-plaintext oncontent_key.is_some()with no E2e knowledge → a missing key (e.g. post-restart, before a share/join response repopulatedcontent_keys) or a seal failure shipped plaintext.e2e: boolparam +Optionreturn:None= refuse (never plaintext), for both the no-key andErr(_)arms.kb_collection_is_e2e()reads the signedderive_encryption(anchor-pinned, not the relay-flippable flag); the editor stamps it onCollabCommand::KbNodeUpdate.e2eat drain time.warn!if still absent — retried when the key arrives. Observable via tracing.#170 —
kb/share/ reconnect-re-share fail-openShareKbsentnode_statesas raw plaintext; reconnect re-shares durably-shared KBs → an already-E2e KB leaked its full plaintext content every reconnect.select_share_node_states(kb_id, e2e, node_states, op_sets): E2e ships the already-sealed op-set we hold (idempotent) or skips the node (loud warn) — never the plaintext snapshot. Unencrypted path byte-identical.Tests (principle #14 — attacker oracles)
build_kb_node_update_request_fails_closed_on_e2e_without_key— E2e+no-key ⇒None(signed + unsigned); selective control: unencrypted ⇒Some; E2e+key ⇒ seals.select_share_node_states_never_ships_plaintext_on_e2e— plaintext canary in NO E2e wire payload; selective control: unencrypted ships plaintext.kb_collection_is_e2e_reads_signed_mode_not_the_flippable_flag— downgrade can't fool the gate.cargo test -p maegreen.Scope / follow-ups (filed, accurate picture)
E2E_ENCRYPTION.md §7.8, tracked separately. Not an active new-content leak (those are fixed here).release:none— no version bump.🤖 Generated with Claude Code