diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 00000000..892a9ba8 --- /dev/null +++ b/docs/README.md @@ -0,0 +1,13 @@ +# mantra docs + +Notes on the parts of this app whose behaviour is not recoverable by reading the +code alone — where the reasoning lives in a protocol, a failure mode that is +silent, or a decision that looked arbitrary and was not. + +| document | covers | +|---|---| +| [shared-key-ceremony.md](./shared-key-ceremony.md) | ChillDKG over NIP-17: the rounds, the approval gates, the chat transcript, participant ordering | +| [shared-key-derivation.md](./shared-key-derivation.md) | deriving further keys from the group's threshold key with FROST tweaks — why not BIP32, why no chain code, and the one rule that must not be broken | +| [marmot-membership.md](./marmot-membership.md) | how members join an MLS group, and the epoch race that makes a missing member look like a successful invite | + +Start with the ceremony if you are new to this area; the other two both assume it. diff --git a/docs/marmot-membership.md b/docs/marmot-membership.md new file mode 100644 index 00000000..d642c56f --- /dev/null +++ b/docs/marmot-membership.md @@ -0,0 +1,149 @@ +# Adding members to a Marmot group + +How members join an MLS group in this app, why the current shape has a silent +failure mode, and what to do about it. + +This is the part of Marmot most likely to waste a day: everything compiles, the +invite reports success, and a member simply never appears. The reason is never in +the invite code. + +## The two paths through `inviteMember` + +`MarmotOutboundDao.inviteMember` branches on `isOneMemberInitialGroupCreation`: + +**`true` — the group is only its creator.** +The Welcome goes out immediately via `deliveryWelcome`, which builds it and +inserts a `GiftWrapPayload`. No commit event is broadcast and no +`MarmotCommitResult` is stored. Correct, because there is nobody else in the group +who needs to learn anything. + +**`false` — the group already has members.** +A commit event (kind 445, ephemeral signer, h-tagged with `nostrGroupId`) is +broadcast so existing members advance their epoch. The Welcome is *not* sent. +Instead `commitResult.welcomeBytes` is stored on a `MarmotCommitResult`, and +`DatabaseNostrRepository` picks it back up when the relay acknowledges the commit +and only then calls `deliveryWelcome`. + +The deferral is deliberate: the invitee must not join an epoch the existing +members have not reached yet. + +## The flag is not reachable from callers + +`inviteMemberToChatRoom` hardcodes `isOneMemberInitialGroupCreation = false`, and +`ChatRepository.inviteMember` does not expose it at all. Only +`createMlsDirectMessageChatRoom` passes `true`, for the single peer of a DM. + +So every group invite — `SelectChatRoomTypeViewModel.inviteMembers` when a room is +created, and `DkgRitualViewModel.inviteAdmins` for the `#admins` room — takes the +deferred path, **including the first one, when the group is still just the +creator**. + +For that first invite this is wrong twice over: + +- the commit has no audience. Nobody else is a member, and nobody outside the + group can decrypt it. It is noise on the relay. +- the Welcome is then gated on a relay acknowledging that pointless commit. If the + ack never arrives, the first invitee never receives anything. + +That second point sharpened when `Relays.DefaultDMRelayList` became a single relay: +every Welcome now depends on one relay acking. + +## Why this fails silently + +`MarmotInboundManager` refuses anything from a future epoch outright, on both +wire formats: + +``` +PrivateMessage epoch N is ahead of local epoch M; ignoring +Commit epoch N is ahead of local epoch M; ignoring +``` + +There is no queue and no replay for either. A commit that arrives before its +recipient's Welcome is **dropped, not deferred**, and that member never advances. +`EPOCH_RETENTION_WINDOW` (5) retains *past* epochs so late messages can still be +decrypted; it does nothing for messages from ahead. + +Now consider inviting two admins back to back under the current behaviour: + +1. invite admin 1 → commit 1 broadcast immediately, Welcome 1 waits for ack 1 +2. invite admin 2 → commit 2 broadcast immediately, Welcome 2 waits for ack 2 + +Both commits are on the wire before either Welcome. If commit 2 reaches admin 1 +before Welcome 1 does — different transports, no ordering guarantee, one is a gift +wrap and the other a kind:445 — admin 1 drops it and is stuck an epoch behind. The +coordinator sees nothing wrong: both invites returned successfully. + +## Options + +### 1. Expose the flag, pass `true` for the first invite + +Smallest change. Removes the pointless commit and the ack dependency for the first +invitee. Leaves n−1 sequential commits, and leaves every other caller unfixed +unless they are each updated. + +### 2. Derive it in the DAO (recommended as the immediate fix) + +`inviteMemberToChatRoom` can decide for itself: + +```kotlin +isOneMemberInitialGroupCreation = mlsGroup.members().size == 1 +``` + +`MlsGroup.members()` already exists. No signature changes, and it fixes every +caller at once — group creation as well as the `#admins` room. + +The condition is right for any group size, not just DMs. "The group has nobody to +inform" is true exactly once, on the first invite, whether the group will end up +with 2 members or 30: + +| invite | `members().size` | branch | correct because | +|---------|------------------|------------------------------|---------------------------| +| admin 1 | 1 | immediate Welcome, no commit | nobody to inform | +| admin 2 | 2 | commit + deferred Welcome | admin 1 must advance | +| admin 3 | 3 | commit + deferred Welcome | admins 1–2 must advance | + +Commit 2 is encrypted with `commitResult.preCommitExporterSecret` — the epoch-1 +secret, which admin 1 received in their Welcome — so they can decrypt it and +advance. + +This also *narrows* the race above rather than widening it. Welcome 1 is sent +before commit 2 exists at all, so admin 1 is already at epoch 1 when commit 2 +arrives. + +It does not close the race. For n ≥ 3 the window between Welcome 1 and commit 2 +still exists, and losing it is still silent. + +### 3. Batch every add into one commit + +`MlsGroup.addMember` is `proposeAdd` + `commit()` in one call, but those are +separate functions and `pendingProposals` is a list. Staging every member with +`proposeAdd` and issuing a single `commit()` gives: + +- one commit, which nobody has to have already joined to process +- n Welcomes carrying identical state +- no intermediate epoch for anyone to miss, so the race has nothing to lose + +This is the right shape whenever the whole membership is known up front, which is +exactly the case for a room created from a completed key ceremony. + +The cost is bookkeeping. `MarmotCommitResult` assumes one peer per commit — +`peerKeyPackageEventId` is singular — so batching means changing that model and +the ack-triggered fan-out in `DatabaseNostrRepository` to deliver several Welcomes +from one acknowledgement. + +## Other things that bite + +**Invites are sequential and each advances the epoch.** The room must be re-read +from the database between invites; a snapshot taken before the previous invite +builds its commit on state the group has already left. Both `inviteMembers` and +`inviteAdmins` do this, and both say so in a comment, because it is not obvious +and the symptom is a conflicting commit rather than an error. + +**A member with no published key package cannot be added.** A Marmot invite needs +the invitee's `MarmotKeyPackage`. Both call sites look it up with a timeout and +collect the ones that failed. Today that only reaches the log — the +`TODO: Update status of participant Invitation.PENDING -> Invitation.SENT` at the +Welcome delivery site is the same gap seen from the other end. + +**`deliveryWelcome` uses `Relays.DefaultDMRelayList`, not the room's relays.** +There is a `TODO: Get localChatRoom relays...` on the ack-triggered call site. diff --git a/docs/shared-key-ceremony.md b/docs/shared-key-ceremony.md new file mode 100644 index 00000000..919131bf --- /dev/null +++ b/docs/shared-key-ceremony.md @@ -0,0 +1,148 @@ +# The shared key ceremony + +A group creates a `t`-of-`n` FROST key by running ChillDKG over its NIP-17 chat. +No trusted dealer, no single device ever holding the whole key. + +Implemented in `ChillDkgRitualManager`, on `fr.acinq.bitcoin.crypto.dkg.chill.ChillDKG`. + +## Shape + +The group is the participant set, the member who opens the ceremony coordinates +it, and every protocol message travels as a gift-wrapped rumor on the same NIP-17 +pipeline chat messages already use — so there is no second transport to operate. + +The coordinator is a participant too, and ChillDKG treats it as untrusted: it +relays and aggregates but cannot learn secrets or bias the key. Being the room's +creator buys it no authority, only work. + +| kind | from | carries | +|-------|-------------|------------------------------------------------------| +| 30310 | coordinator | proposal — "let's make a t-of-n key" | +| 30311 | participant | host public key | +| 30312 | participant | `pmsg1`, this device's contribution | +| 30313 | coordinator | `cmsg1`, everyone's contributions combined | +| 30314 | participant | CertEq signature confirming the combined result | +| 30315 | coordinator | `cmsg2`, the success certificate | +| 30316 | anyone | failure — abort and blame | + +Every step is a pure function of inputs the device has already stored, so there is +no long-lived in-memory session to lose. Each inbound message is persisted and +then the ritual is asked whether it can move; if the app dies mid-round it resumes +on the next message. `DkgSession` deliberately stores *inputs* — the randomness +and the received messages — rather than protocol state, which is what makes that +work. + +**A ceremony cannot finish until every member takes part.** That is unusual for a +chat feature and drives most of the UI: the progress ladder names who it is +waiting on rather than showing a count, because "2 of 3" does not tell anyone whose +door to knock on. + +## Nothing publishes without approval + +The ritual is driven by arriving messages, which originally meant a relay +delivering an event to a phone in someone's pocket was enough to enrol its owner +in a group's permanent signing quorum. `acceptProposal` published the host key on +arrival; rounds 1 and 2 followed automatically. + +It now publishes nothing on this device's behalf until its owner agrees, at three +separate gates: + +| step | publishes | why it is its own decision | +|------------|--------------------------|-------------------------------------------------------------------------------| +| `HOST_KEY` | the host public key | joins the ceremony and fixes `n`. Joining then going quiet holds it open for everyone | +| `ROUND_1` | the key contribution | the member's secret material starts shaping a key they must help sign with | +| `ROUND_2` | the CertEq signature | a real check: it is what stops a coordinator substituting a key the members never contributed to | + +The coordinator's two aggregations are **not** gated. They relay other members' +already-published messages and disclose nothing of the coordinator's own, so an +approval there would stall the whole group on one person's attention without +protecting anybody. + +The member who opens a ceremony is auto-approved for the host key alone — +starting one is already the act of agreeing to be in it — and is still asked for +rounds 1 and 2. + +Each gate returns rather than throwing. The ritual is not failing, it is waiting +on a person; everything received stays stored and it resumes on approval. + +> **`pendingApproval` must mirror the gates in `advance` exactly.** If they drift, +> the screen offers an approval that does nothing, or offers none while the ritual +> sits still. Neither produces an error. + +Approvals are recorded as three nullable timestamps on `DkgSession`, plus +`approvalRequestedThrough` so the chat line asking for each is written once. + +## Faults are values, not exceptions + +ChillDKG reports a faulty participant in the `ChilldkgFault` field of each result, +because a faulty participant is a normal outcome of a DKG rather than a bug. + +This ritual has one response to all of them — the key is unusable, the session +dies, the group is told — so `raiseIfFaulty` turns them into an exception carrying +the culprit and lets them join `advance`'s single failure path. The gain is the +failure text: "ChillDKG round 2 failed: a participant is faulty (participant 3)" +rather than whatever `e.message` happened to hold. On a failed DKG, which +participant to blame is the only actionable thing there is. + +## The transcript + +Every protocol message becomes a line in the group's chat naming the member whose +device sent it, plus the three that bracket them: started, complete, abandoned. + +These rows are **not anybody's words**: no `giftWrapPayloadId`, no event behind +them, nothing sent to say them. Each device writes its own from messages it +already received, so they cost no traffic and cannot disagree with the ritual they +describe. They render as system lines rather than bubbles — attributing "a shared +key ceremony started" to the coordinator would read as something they said. + +Wording describes what a step accomplishes, not what it is called. "sent their +contribution to the key" is useful in a group chat; "sent pmsg1" is not, and the +protocol names are on the shared-key screen for anyone who wants them. + +**Idempotency is by construction, not de-duplication.** `ChatMessage` has no key +to make a second insert a no-op — its id is autogenerated — while ritual messages +arrive repeatedly: relays redeliver, and `replayStoredMessages` feeds the whole +backlog through `record()` again on every resume. So every announce is guarded by +reading the row it is about to write over. `DkgParticipantMessage` absorbs the +redelivery itself, being keyed on `(sessionId, participantPublicKey, kind)`; a chat +row cannot. + +Request lines carry their step in the message type — one type per step, not one +type for all three. The type is the only thing a transcript keeps: a line drawn +days later has no session to ask what was being requested. Sharing one type left +every request wearing the same icon. + +Whether a request was answered is read from the transcript rather than the +session: approving is the only thing that causes the step to be published, and +publishing writes an authored line. That keeps a room that has run more than one +ceremony correct, since `ChatMessage` has no session id to disambiguate with. + +## Ordering + +ChillDKG has no session-params object to agree on out of band. Every step takes +the host public keys and the threshold and hashes them into the session identity, +so **a group that orders its participants differently on different devices does +not get a weaker key — it gets no key.** + +Each device derives that order independently by sorting the host keys bytewise, +and orders each round's messages by their sender's host key to match. Sorting is +the only ordering every device can arrive at without being told. Nothing in the +protocol checks this, so `ChillDkgRitualOrderingTest` does. + +## After it completes + +The group has a threshold public key; each device keeps its own share, restorable +from that member's wallet backup and nobody else's. + +From there the coordinator can create the `#admins` room — a Marmot group whose id +is derived from the shared key. See [shared-key-derivation.md](./shared-key-derivation.md) +for how, and [marmot-membership.md](./marmot-membership.md) for how members are +added to it. + +## Known gaps + +- No ceremony has been run on a physical device. +- A member invited to the `#admins` room whose Welcome never goes out is + indistinguishable, from the coordinator's side, from one who joined. +- The ritual's request chat lines are `3n + 4` per ceremony — 19 lines for five + members. Compact, but they dominate a transcript while a ceremony runs. diff --git a/docs/shared-key-derivation.md b/docs/shared-key-derivation.md new file mode 100644 index 00000000..00e1d318 --- /dev/null +++ b/docs/shared-key-derivation.md @@ -0,0 +1,152 @@ +# Deriving keys from a group's shared key + +`SharedKeyDerivation` turns a group's ChillDKG threshold key into further keys the +group can sign with, at paths that look like BIP32 but deliberately are not. + +## What it produces + +```kotlin +val derived = SharedKeyDerivation.derive(thresholdPublicKey) // default m/9420/0/0 +derived.publicKey // XonlyPublicKey — 32 bytes, the form nostr and Marmot use +derived.cache // TweakCache — required to sign +derived.hex // publicKey as hex +``` + +**The cache is not an optimisation.** A FROST signing session has to be created +with a cache carrying the same tweaks, or the partial signatures aggregate to +something that verifies against a different key. Code that takes only +`publicKey` and later tries to sign will fail in a way that is tedious to diagnose +from the outside, because the signature is valid — just not for the key you +expected. + +Everything is a pure function of the threshold key and the path, so every member's +device computes the same result with no agreement round and nothing to store. +Rederive rather than persist. + +## Why not BIP32 + +The paths read like BIP32 and are walked the same way, index by index. They are +not BIP32, and the difference matters. + +**A BIP32 node is a key *and* a chain code. ChillDKG produces no chain code.** +`ParticipantFinalizeResult` gives you `thresholdPublicKey`, `secretShare`, +`publicShares` and `recovery` — no chain code, because ChillDKG is not a BIP32 +ceremony. + +**Hardened derivation is impossible here, not merely unimplemented.** It is: + +``` +I = HMAC-SHA512(c_par, 0x00 || ser256(k_par) || ser32(i)) +``` + +which takes the parent *private* key. In a FROST group nobody holds that; it +exists only as shares. No member, and no quorum of members short of reconstructing +the secret, can perform it. So `m/44'/1237'/0'/0/0` — the NIP-06 nostr path — is +not derivable from a threshold key by anyone. + +**Non-hardened derivation is available, as an additive tweak.** + +``` +t = HMAC-SHA512(c_par, serP(K_par) || ser32(i))[0:32] +K' = K + t·G +``` + +which is exactly what `TweakCache.tweak` does. But note where the chain code +appears: only in *computing* `t`. A FROST tweak takes `t` as an input, so +**choosing the scalar directly removes the chain code from the problem entirely.** + +That is what this does: + +``` +t = SHA256("mantra/shared-key/tweak/v1" || parentXonlyKey || index-as-4-bytes) +``` + +Each scalar commits to the key being tweaked as well as the index, so steps cannot +be reordered or replayed at a different depth to reach the same key. +`listOf(0L)` and `listOf(0L, 0L, 0L)` do not collide — there is a test for it. + +### What avoiding BIP32 also avoids + +With x-only keys there is no single obvious `serP(K_par)`: BIP32 serialises +compressed 33-byte keys, BIP340 uses 32-byte x-only, and the parity byte has to +come from somewhere. Two devices picking different conventions would **silently +derive different keys** rather than fail. Choosing the tweak input ourselves makes +the domain separation explicit and removes that class of bug. + +Nothing is lost in exchange. No external tool can derive these children anyway — +none of them has the chain code, and nostr has no way to publish one. An npub is +bare bech32 over a 32-byte key with no chain code, depth or parent fingerprint; +NIP-06 uses BIP32 internally but discards everything except the leaf public key. + +## The security property this inherits + +Additive tweaking is what non-hardened BIP32 does, and it carries the same +weakness. Because `t` is publicly computable: + +``` +k' = k + t ⟹ k = k' − t +``` + +**Anyone who learns one derived private key recovers the group's threshold key** +and can sign as the group with no quorum at all — defeating the entire point of +the ceremony. In ordinary BIP32 this is why BIP44 hardens the first three levels: +a leaked leaf costs you one account, not the wallet. That defence is unavailable +here. + +The mitigating factor is that a derived private key does not normally exist: +reconstructing one needs `t` members to collude, at which point they already have +the parent. So the rule is narrow and absolute: + +> **Never reconstruct a derived key in the clear.** Any code path that could — an +> export, a "reveal private key" screen, a test helper, a debugging convenience — +> leaks the group key, not just the key it appears to expose. + +If you need many keys that cannot be linked back to one another, derivation is the +wrong tool: run a ceremony per key. Each output is then independent and no single +leak reaches the others. + +## Paths + +`derive` and `marmotGroupId` both take `path: List`, defaulting to +`MARMOT_ADMIN_GROUP_PATH` (`m/9420/0/0`). Any depth works. + +`9420` is arbitrary and has to stay put: the derived key *is* the `#admins` room's +id, so changing the path orphans every room already created — members would derive +a different id and stop finding the room at all. + +There is no string-path parser for input. Paths are written as lists at the call +site. If one is added it must reject `'` outright rather than accepting a hardened +path it cannot honour. + +## Recording the path + +MIP-01's group data is a fixed TLS schema — version, `nostrGroupId`, name, +description, `adminPubkeys`, relays, four image fields, `disappearingMessageSecs`. +There is no extension map, and inventing a field would emit bytes other Marmot +clients cannot decode. + +So the path rides in the description, which is the only free text MIP-01 offers: + +``` +Admins of Ubuntu Collective. + +Shared key path: m/9420/0/0 +``` + +`formatPath`, `parsePath` and `describe` round-trip this. The marker sits on its +own line and `parsePath` scans lines for it, so somebody rewriting the rest of the +description does not cost the group the record of how its key was derived. + +Worth storing even though the path is currently a constant: it is what rebuilds +the `TweakCache` a signing session needs, and recomputing from the constant only +holds while the constant never changes. A room that records the path it was made +under lets a later scheme coexist with rooms already created. + +`parsePath` refuses hardened indices — `m/9420'/0/0` returns null. A hardened path +cannot have been walked here, so acting on one would derive something other than +what the room claims. + +Consequence worth knowing: the path is visible to anyone in the group, in any +Marmot client, since description is user-facing text. The path is not a secret and +the key it derives from is not published, but the room does announce how it was +made.