diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index b5b8c94de..635ae97ea 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -178,21 +178,39 @@ class MarmotManager( "KeyPackage credential identity does not match memberPubKey" } - val commitResult = groupManager.addMember(nostrGroupId, keyPackageBytes) - val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes) - // We've already applied this commit locally; prevent the relay-echoed - // copy from being re-applied by the inbound pipeline. + // Stage-then-merge (MDK / RFC 9420 §12.4): build the kind:445 with the + // PRE-commit epoch's exporter secret so that other existing members + // can outer-decrypt and process it. Only advance our own epoch once + // the framed commit has been handed to the outbound path. + val staged = groupManager.stageAddMember(nostrGroupId, keyPackageBytes) + val commitEvent = + outboundProcessor.buildCommitEvent( + nostrGroupId = nostrGroupId, + commitBytes = staged.framedCommitBytes, + exporterKey = staged.preCommitExporterSecret, + ) + // The published kind:445 will echo back from the relay — without this + // dedup our own inbound pipeline would try to re-apply a commit whose + // epoch we've already merged below. inboundProcessor.markEventProcessed(commitEvent.signedEvent.id) + val welcomeBytes = + staged.welcomeBytes + ?: throw IllegalStateException( + "stageAddMember did not produce a Welcome for $memberPubKey", + ) val welcomeDelivery = - welcomeSender.wrapWelcome( - commitResult = commitResult, + welcomeSender.wrapWelcomeBytes( + welcomeBytes = welcomeBytes, recipientPubKey = memberPubKey, keyPackageEventId = keyPackageEventId, relays = relays, nostrGroupId = nostrGroupId, ) + // Advance local epoch now that the commit + welcome have been built. + groupManager.mergeStagedCommit(nostrGroupId, staged) + return Pair(commitEvent, welcomeDelivery) } @@ -268,9 +286,15 @@ class MarmotManager( nostrGroupId: HexKey, targetLeafIndex: Int, ): OutboundGroupEvent { - val commitResult = groupManager.removeMember(nostrGroupId, targetLeafIndex) - val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes) + val staged = groupManager.stageRemoveMember(nostrGroupId, targetLeafIndex) + val commitEvent = + outboundProcessor.buildCommitEvent( + nostrGroupId = nostrGroupId, + commitBytes = staged.framedCommitBytes, + exporterKey = staged.preCommitExporterSecret, + ) inboundProcessor.markEventProcessed(commitEvent.signedEvent.id) + groupManager.mergeStagedCommit(nostrGroupId, staged) return commitEvent } @@ -282,9 +306,16 @@ class MarmotManager( nostrGroupId: HexKey, metadata: MarmotGroupData, ): OutboundGroupEvent { - val commitResult = groupManager.updateGroupExtensions(nostrGroupId, listOf(metadata.toExtension())) - val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes) + val staged = + groupManager.stageUpdateGroupExtensions(nostrGroupId, listOf(metadata.toExtension())) + val commitEvent = + outboundProcessor.buildCommitEvent( + nostrGroupId = nostrGroupId, + commitBytes = staged.framedCommitBytes, + exporterKey = staged.preCommitExporterSecret, + ) inboundProcessor.markEventProcessed(commitEvent.signedEvent.id) + groupManager.mergeStagedCommit(nostrGroupId, staged) return commitEvent } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotOutboundProcessor.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotOutboundProcessor.kt index df3cecc28..3f0e276cb 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotOutboundProcessor.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/MarmotOutboundProcessor.kt @@ -121,20 +121,29 @@ class MarmotOutboundProcessor( /** * Build a GroupEvent carrying a Commit for publishing. * - * Used after MlsGroupManager.commit() or addMember()/removeMember(). - * The commit bytes are already MLS-formatted. + * Used after MlsGroupManager.stageAddMember()/stageRemoveMember()/etc. + * The commit bytes are already MLS-formatted (PublicMessage envelope). * * @param nostrGroupId the Nostr group ID - * @param commitBytes the raw MLS commit bytes from CommitResult + * @param commitBytes the framed MLS commit bytes from [com.vitorpamplona.quartz.marmot.mls.group.StagedCommit.framedCommitBytes] + * @param exporterKey optional explicit outer-encryption key. Callers + * publishing a Commit MUST pass the **pre-commit** exporter secret + * (from [com.vitorpamplona.quartz.marmot.mls.group.StagedCommit.preCommitExporterSecret]) + * so that other existing members at epoch N can decrypt and process + * the commit. If null, falls back to the current epoch's exporter + * secret — which is only correct when the commit has *not* been + * applied locally yet (i.e. this call is made before + * [com.vitorpamplona.quartz.marmot.mls.group.MlsGroup.mergeStagedCommit]). * @return the signed GroupEvent ready for relay publishing */ suspend fun buildCommitEvent( nostrGroupId: HexKey, commitBytes: ByteArray, + exporterKey: ByteArray? = null, ): OutboundGroupEvent { // Outer ChaCha20-Poly1305 encryption of the MLS commit - val exporterKey = groupManager.exporterSecret(nostrGroupId) - val encryptedContent = GroupEventEncryption.encrypt(commitBytes, exporterKey) + val outerKey = exporterKey ?: groupManager.exporterSecret(nostrGroupId) + val encryptedContent = GroupEventEncryption.encrypt(commitBytes, outerKey) // Build the GroupEvent template val template = diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt index 74abe20fc..9bd7f44a5 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt @@ -98,7 +98,7 @@ import com.vitorpamplona.quartz.utils.mac.MacInstance */ class MlsGroup private constructor( private var groupContext: GroupContext, - private val tree: RatchetTree, + private var tree: RatchetTree, private var myLeafIndex: Int, private var epochSecrets: EpochSecrets, private var secretTree: SecretTree, @@ -199,6 +199,125 @@ class MlsGroup private constructor( var reInitPending: Proposal.ReInit? = null private set + // --- Stage/Merge Infrastructure (RFC 9420 §12.4 / MDK parity) --- + + /** + * Capture a complete snapshot of every mutable field so this group can be + * restored atomically. Used by [stageCommit] to rewind after computing a + * Commit so the outer code can encrypt the kind:445 with the pre-commit + * exporter secret before advancing locally. + */ + private fun captureSnapshot(): MlsGroupSnapshot { + val treeWriter = TlsWriter() + tree.encodeTls(treeWriter) + return MlsGroupSnapshot( + groupContext = groupContext, + treeBytes = treeWriter.toByteArray(), + myLeafIndex = myLeafIndex, + epochSecrets = epochSecrets, + secretTree = secretTree, + initSecret = initSecret, + signingPrivateKey = signingPrivateKey, + encryptionPrivateKey = encryptionPrivateKey, + interimTranscriptHash = interimTranscriptHash, + pskStore = pskStore.toMap(), + pendingProposals = pendingProposals.toList(), + sentKeys = sentKeys.toMap(), + pendingSigningKey = pendingSigningKey, + pendingEncryptionKey = pendingEncryptionKey, + reInitPending = reInitPending, + ) + } + + /** + * Overwrite every mutable field of this group from a prior snapshot. + * See [captureSnapshot] for the use case. + */ + private fun restoreSnapshot(s: MlsGroupSnapshot) { + groupContext = s.groupContext + tree = RatchetTree.decodeTls(TlsReader(s.treeBytes)) + myLeafIndex = s.myLeafIndex + epochSecrets = s.epochSecrets + // secretTree is keyed on encryption_secret + leafCount; rebuilding from + // the snapshot's encryption_secret avoids relying on SecretTree being + // a pure data class, which it currently is, but keeps us robust. + secretTree = SecretTree(s.epochSecrets.encryptionSecret, tree.leafCount) + initSecret = s.initSecret + signingPrivateKey = s.signingPrivateKey + encryptionPrivateKey = s.encryptionPrivateKey + interimTranscriptHash = s.interimTranscriptHash + pskStore.clear() + pskStore.putAll(s.pskStore) + pendingProposals.clear() + pendingProposals.addAll(s.pendingProposals) + sentKeys.clear() + sentKeys.putAll(s.sentKeys) + pendingSigningKey = s.pendingSigningKey + pendingEncryptionKey = s.pendingEncryptionKey + reInitPending = s.reInitPending + } + + /** + * Build a Commit from the currently pending proposals **without** advancing + * the local epoch. The caller can then outer-encrypt the kind:445 using + * [StagedCommit.preCommitExporterSecret] — which is still the current + * epoch's key — and call [mergeStagedCommit] to advance locally only after + * the kind:445 has been handed to the transport. + * + * This mirrors OpenMLS / MDK's `create_commit` + `merge_pending_commit` + * pair and is the only correct way to encrypt a Commit: existing members + * still at epoch N need the pre-commit exporter secret to decrypt the + * outer layer, while the new member joining via Welcome does not need to + * process this Commit at all. + * + * On success, the group state is unchanged. + * On failure, the group state is unchanged. + */ + fun stageCommit(): StagedCommit { + val preCommitSnapshot = captureSnapshot() + val preCommitExporter = exporterSecret("marmot", "group-event".encodeToByteArray(), 32) + val preCommitEpoch = groupContext.epoch + + val committed: CommitResult = + try { + commit() + } catch (e: Exception) { + // commit() may have mutated fields before throwing — always roll back. + restoreSnapshot(preCommitSnapshot) + throw e + } + + val postCommitSnapshot = captureSnapshot() + val postCommitEpoch = groupContext.epoch + + // Rewind to pre-commit state so the outer code sees epoch N until it + // explicitly calls mergeStagedCommit. + restoreSnapshot(preCommitSnapshot) + + return StagedCommit( + preCommitExporterSecret = preCommitExporter, + preCommitEpoch = preCommitEpoch, + postCommitEpoch = postCommitEpoch, + commitBytes = committed.commitBytes, + framedCommitBytes = committed.framedCommitBytes, + welcomeBytes = committed.welcomeBytes, + groupInfoBytes = committed.groupInfoBytes, + postState = postCommitSnapshot, + ) + } + + /** + * Apply a previously [stageCommit]ed transition, advancing this group from + * epoch N to epoch N+1. Call once the Commit has been successfully handed + * to the transport. + */ + fun mergeStagedCommit(staged: StagedCommit) { + check(groupContext.epoch == staged.preCommitEpoch) { + "mergeStagedCommit: group at epoch ${groupContext.epoch} but staged commit is for epoch ${staged.preCommitEpoch}" + } + restoreSnapshot(staged.postState) + } + /** * Register a pre-shared key for use in PSK proposals. */ @@ -2072,21 +2191,53 @@ class MlsGroup private constructor( /** * Add a member to the group by their KeyPackage. * Creates and applies a Commit with an Add proposal. + * + * This advances the local epoch eagerly; the returned [CommitResult] is + * encrypted on the wire with the **post-commit** exporter key, which is + * only correct for tests / offline MLS scenarios. Production callers MUST + * use [stageAddMember] + [mergeStagedCommit] so the kind:445 can be + * wrapped with the pre-commit exporter key, matching the MDK reference + * and allowing other existing members to actually decrypt and process the + * commit. */ fun addMember(keyPackageBytes: ByteArray): CommitResult { proposeAdd(keyPackageBytes) return commit() } + /** + * Stage an Add-member Commit without advancing the local epoch. + * Returned [StagedCommit] carries the pre-commit exporter secret that the + * outer kind:445 layer MUST be encrypted with (RFC 9420 §12.4 + MDK). + * Call [mergeStagedCommit] after the commit has been handed to the + * transport to advance locally. + */ + fun stageAddMember(keyPackageBytes: ByteArray): StagedCommit { + proposeAdd(keyPackageBytes) + return stageCommit() + } + /** * Remove a member from the group. * Creates and applies a Commit with a Remove proposal. + * + * See [addMember] for why production callers should prefer + * [stageRemoveMember] instead. */ fun removeMember(targetLeafIndex: Int): CommitResult { proposeRemove(targetLeafIndex) return commit() } + /** + * Stage a Remove-member Commit without advancing the local epoch. + * See [stageAddMember] for the rationale. + */ + fun stageRemoveMember(targetLeafIndex: Int): StagedCommit { + proposeRemove(targetLeafIndex) + return stageCommit() + } + /** * Remove self from the group. * diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt index bdfec1984..bea9ca8ac 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt @@ -267,11 +267,9 @@ class MlsGroupManager( suspend fun commit(nostrGroupId: HexKey): CommitResult = mutex.withLock { val group = requireGroup(nostrGroupId) - - // Retain current epoch secrets before transition - retainEpochSecrets(nostrGroupId, group) - + val retainedBefore = group.retainedSecrets() val result = group.commit() + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) result } @@ -292,10 +290,16 @@ class MlsGroupManager( ) = mutex.withLock { val group = requireGroup(nostrGroupId) - // Retain current epoch secrets before transition - retainEpochSecrets(nostrGroupId, group) + // Capture the outgoing epoch's secrets BEFORE advancing, but only + // commit them to the retention window once processCommit succeeds — + // otherwise a failed commit (e.g. "Duplicate encryption key" on an + // add-me relay echo) would pollute the window with a duplicate of + // the current epoch key, wasting the finite retention slots. + val retainedBefore = group.retainedSecrets() group.processCommit(commitBytes, senderLeafIndex, confirmationTag) + + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) } @@ -355,10 +359,18 @@ class MlsGroupManager( null } - // --- Member Management --- + // --- Member Management (atomic stage+merge; retained for tests/offline use) --- /** - * Add a member and create a Commit. + * Add a member and create a Commit, advancing the local epoch immediately. + * + * **Production callers MUST use [stageAddMember] + [mergeStagedCommit] + * instead.** The commit bytes returned here are wrapped on the wire with + * the *post-commit* exporter key, which is wrong — other existing members + * at epoch N cannot decrypt it, so the commit is unprocessable by anyone + * except the new member (who doesn't need it because their Welcome already + * carries the post-commit state). This entry point is retained only for + * unit tests and scenarios where outer framing isn't used. */ suspend fun addMember( nostrGroupId: HexKey, @@ -366,14 +378,16 @@ class MlsGroupManager( ): CommitResult = mutex.withLock { val group = requireGroup(nostrGroupId) - retainEpochSecrets(nostrGroupId, group) + val retainedBefore = group.retainedSecrets() val result = group.addMember(keyPackageBytes) + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) result } /** - * Remove a member and create a Commit. + * Remove a member and create a Commit. See [addMember] for the production + * caveat — use [stageRemoveMember] + [mergeStagedCommit] on the wire. */ suspend fun removeMember( nostrGroupId: HexKey, @@ -381,27 +395,25 @@ class MlsGroupManager( ): CommitResult = mutex.withLock { val group = requireGroup(nostrGroupId) - retainEpochSecrets(nostrGroupId, group) + val retainedBefore = group.retainedSecrets() val result = group.removeMember(targetLeafIndex) + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) result } /** - * Rotate the signing key within a group and commit. - * - * Per MIP-00, members SHOULD self-update within 24 hours of joining. - * This creates an Update proposal with a fresh signing key and commits it. - * - * @param nostrGroupId hex-encoded Nostr group ID - * @return the [CommitResult] to publish + * Rotate the signing key within a group and commit. See [addMember] for + * the production caveat — use [stageRotateSigningKey] + [mergeStagedCommit] + * on the wire. */ suspend fun rotateSigningKey(nostrGroupId: HexKey): CommitResult = mutex.withLock { val group = requireGroup(nostrGroupId) - retainEpochSecrets(nostrGroupId, group) + val retainedBefore = group.retainedSecrets() group.proposeSigningKeyRotation() val result = group.commit() + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) result } @@ -410,12 +422,8 @@ class MlsGroupManager( * Update group extensions (e.g., MIP-01 metadata) via a GroupContextExtensions proposal. * Creates the proposal, commits it, and persists the new state. * - * Authorization (MIP-01): extension updates require admin privileges once - * the group has at least one admin. During bootstrap — before any - * `admin_pubkeys` are configured — any member may seed the initial - * extension set (e.g. the creator installing the first - * [com.vitorpamplona.quartz.marmot.mip01Groups.MarmotGroupData] with - * themselves as admin). + * See [addMember] for the production caveat — use [stageUpdateGroupExtensions] + * + [mergeStagedCommit] on the wire. */ suspend fun updateGroupExtensions( nostrGroupId: HexKey, @@ -428,13 +436,89 @@ class MlsGroupManager( check(!adminsConfigured || group.isLocalAdmin()) { "MIP-01: only admins may update group extensions" } - retainEpochSecrets(nostrGroupId, group) + val retainedBefore = group.retainedSecrets() group.proposeGroupContextExtensions(extensions) val result = group.commit() + pushRetainedEpoch(nostrGroupId, retainedBefore) persistGroup(nostrGroupId) result } + // --- Staged Member Management (production path; MDK parity) --- + + /** + * Stage an Add-member Commit without advancing the local epoch. + * The returned [StagedCommit.preCommitExporterSecret] is the epoch-N key + * that the outer kind:445 ChaCha20 layer MUST use so that existing + * members at epoch N can decrypt + process it. Call [mergeStagedCommit] + * after the kind:445 has been handed to the transport. + */ + suspend fun stageAddMember( + nostrGroupId: HexKey, + keyPackageBytes: ByteArray, + ): StagedCommit = + mutex.withLock { + requireGroup(nostrGroupId).stageAddMember(keyPackageBytes) + } + + /** + * Stage a Remove-member Commit. See [stageAddMember]. + */ + suspend fun stageRemoveMember( + nostrGroupId: HexKey, + targetLeafIndex: Int, + ): StagedCommit = + mutex.withLock { + requireGroup(nostrGroupId).stageRemoveMember(targetLeafIndex) + } + + /** + * Stage a signing-key rotation Commit. See [stageAddMember]. + */ + suspend fun stageRotateSigningKey(nostrGroupId: HexKey): StagedCommit = + mutex.withLock { + val group = requireGroup(nostrGroupId) + group.proposeSigningKeyRotation() + group.stageCommit() + } + + /** + * Stage a GroupContextExtensions Commit (MIP-01 metadata). See + * [stageAddMember]. + */ + suspend fun stageUpdateGroupExtensions( + nostrGroupId: HexKey, + extensions: List, + ): StagedCommit = + mutex.withLock { + val group = requireGroup(nostrGroupId) + val currentMarmot = group.currentMarmotData() + val adminsConfigured = currentMarmot != null && currentMarmot.adminPubkeys.isNotEmpty() + check(!adminsConfigured || group.isLocalAdmin()) { + "MIP-01: only admins may update group extensions" + } + group.proposeGroupContextExtensions(extensions) + group.stageCommit() + } + + /** + * Merge a previously [stageAddMember]/[stageRemoveMember]/etc. commit, + * advancing the local epoch to N+1. Retains the pre-commit epoch's + * secrets for late-message decryption, then persists. + */ + suspend fun mergeStagedCommit( + nostrGroupId: HexKey, + staged: StagedCommit, + ) = mutex.withLock { + val group = requireGroup(nostrGroupId) + // Retain the outgoing epoch's secrets BEFORE advancing (they live in + // the pre-commit snapshot, which is this group's current state). + val retainedBefore = group.retainedSecrets() + group.mergeStagedCommit(staged) + pushRetainedEpoch(nostrGroupId, retainedBefore) + persistGroup(nostrGroupId) + } + /** * Leave a group (self-remove). * Returns the SelfRemove proposal bytes to publish, then removes @@ -564,12 +648,18 @@ class MlsGroupManager( } } - private fun retainEpochSecrets( + /** + * Push a previously-captured [RetainedEpochSecrets] into the bounded + * retention window. Call after the epoch advance has been applied + * successfully so that failed commits don't pollute the window with + * duplicate current-epoch keys. + */ + private fun pushRetainedEpoch( nostrGroupId: HexKey, - group: MlsGroup, + retainedBefore: RetainedEpochSecrets, ) { val retained = retainedEpochs.getOrPut(nostrGroupId) { mutableListOf() } - retained.add(group.retainedSecrets()) + retained.add(retainedBefore) // Trim to retention window (keep only the most recent N-1 epochs) while (retained.size > EPOCH_RETENTION_WINDOW) { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/StagedCommit.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/StagedCommit.kt new file mode 100644 index 000000000..f8aa53f0e --- /dev/null +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/StagedCommit.kt @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.marmot.mls.group + +import com.vitorpamplona.quartz.marmot.mls.messages.GroupContext +import com.vitorpamplona.quartz.marmot.mls.messages.Proposal +import com.vitorpamplona.quartz.marmot.mls.schedule.EpochSecrets +import com.vitorpamplona.quartz.marmot.mls.schedule.KeyNonceGeneration +import com.vitorpamplona.quartz.marmot.mls.schedule.SecretTree + +/** + * Snapshot of every mutable field of [MlsGroup] at a single point in time. + * + * Used to capture state both before a commit (for rollback) and after a commit + * (for later merging via [MlsGroup.mergeStagedCommit]). + * + * Contains secret key material — treat as sensitive. Do not serialize or log. + */ +internal data class MlsGroupSnapshot( + val groupContext: GroupContext, + /** TLS-encoded [com.vitorpamplona.quartz.marmot.mls.tree.RatchetTree] bytes. */ + val treeBytes: ByteArray, + val myLeafIndex: Int, + val epochSecrets: EpochSecrets, + val secretTree: SecretTree, + val initSecret: ByteArray, + val signingPrivateKey: ByteArray, + val encryptionPrivateKey: ByteArray, + val interimTranscriptHash: ByteArray, + val pskStore: Map, + val pendingProposals: List, + val sentKeys: Map, + val pendingSigningKey: ByteArray?, + val pendingEncryptionKey: ByteArray?, + val reInitPending: Proposal.ReInit?, +) + +/** + * Result of staging (but not yet merging) a Commit. + * + * Mirrors the MDK / OpenMLS `add_members` → `merge_pending_commit` flow + * (RFC 9420 §12.4): first the Commit is *computed* from the current group + * state (which remains at epoch N) so that the outer kind:445 can be + * ChaCha20-encrypted with the **pre-commit (epoch N)** exporter secret — + * the key that other existing members still hold. Only after the Commit has + * been handed to the transport does the local group actually advance to + * epoch N+1 via [MlsGroup.mergeStagedCommit]. + * + * [preCommitExporterSecret] is the output of + * `MLS-Exporter("marmot", "group-event", 32)` at the pre-commit epoch — + * exactly the key existing members need to outer-decrypt and process the + * Commit on inbound. + * + * [commitBytes] is the raw `Commit` TLS struct. [framedCommitBytes] is the + * full `MlsMessage(PublicMessage(Commit))` envelope ready for + * ChaCha20-Poly1305 wrapping; publishers MUST use the framed bytes so that + * receivers can recover the sender's leaf index and confirmation tag. + */ +@ConsistentCopyVisibility +data class StagedCommit internal constructor( + /** Pre-commit (epoch N) MLS-Exporter output for outer kind:445 encryption. */ + val preCommitExporterSecret: ByteArray, + /** Pre-commit epoch (N). */ + val preCommitEpoch: Long, + /** Post-commit epoch (N+1). */ + val postCommitEpoch: Long, + /** Raw Commit (RFC 9420 §12.4) bytes, for tests / processCommit. */ + val commitBytes: ByteArray, + /** MlsMessage(PublicMessage(Commit)) envelope for on-the-wire distribution. */ + val framedCommitBytes: ByteArray, + /** Welcome message bytes for any members added by this Commit, else null. */ + val welcomeBytes: ByteArray?, + /** GroupInfo bytes for external joiners, else null. */ + val groupInfoBytes: ByteArray?, + /** Full post-commit snapshot applied by [MlsGroup.mergeStagedCommit]. */ + internal val postState: MlsGroupSnapshot, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is StagedCommit) return false + return commitBytes.contentEquals(other.commitBytes) && + framedCommitBytes.contentEquals(other.framedCommitBytes) && + preCommitEpoch == other.preCommitEpoch && + postCommitEpoch == other.postCommitEpoch + } + + override fun hashCode(): Int { + var result = commitBytes.contentHashCode() + result = 31 * result + framedCommitBytes.contentHashCode() + result = 31 * result + preCommitEpoch.hashCode() + result = 31 * result + postCommitEpoch.hashCode() + return result + } +} diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt index bd574472f..cdddf5da1 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEventEncryption import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupManager import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupStateStore +import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal import kotlinx.coroutines.runBlocking @@ -427,6 +428,180 @@ class MarmotPipelineTest { } } + @Test + fun testStagedAddMemberUsesPreCommitExporter() { + // Regression: before the stage/merge split, MarmotManager.addMember + // was outer-encrypting the kind:445 commit with the POST-commit + // epoch key. Existing members still at epoch N couldn't decrypt it, + // and the newly-added member (joining via Welcome at epoch N+1) + // *could* decrypt it but then failed to apply ("Duplicate encryption + // key: leaf X"). The fix exposes the pre-commit exporter secret via + // StagedCommit so callers can encrypt correctly. + runBlocking { + val manager = createGroupManager() + manager.createGroup(groupId, "alice".encodeToByteArray()) + + val preEpochExporter = manager.exporterSecret(groupId) + val preEpoch = manager.getGroup(groupId)!!.epoch + + val bobBundle = + manager + .getGroup(groupId)!! + .createKeyPackage("bob".encodeToByteArray(), ByteArray(0)) + val staged = manager.stageAddMember(groupId, bobBundle.keyPackage.toTlsBytes()) + + // Stage must NOT have advanced the local epoch yet. + assertEquals(preEpoch, manager.getGroup(groupId)!!.epoch) + // preCommitExporterSecret must equal the pre-stage exporter key. + kotlin.test.assertContentEquals( + preEpochExporter, + staged.preCommitExporterSecret, + ) + // The staged post-commit epoch is N+1. + assertEquals(preEpoch + 1, staged.postCommitEpoch) + assertNotNull(staged.welcomeBytes) + + // Merge advances to N+1. + manager.mergeStagedCommit(groupId, staged) + assertEquals(preEpoch + 1, manager.getGroup(groupId)!!.epoch) + } + } + + @Test + fun testCreatorCanAddTwoMembersAndAllDecryptFollowingMessage() { + // End-to-end regression for the David/Eden/Fred scenario: a group + // creator (Alice) adds two members (Bob then Carol) in sequence and + // then publishes an application message. Both members MUST be able to + // decrypt the message. Before the stage/merge fix, the add-Carol + // commit was outer-encrypted with the post-commit (epoch 2) key, + // leaving Bob stuck at epoch 1 and unable to decrypt any of Alice's + // subsequent application messages. + runBlocking { + val aliceMgr = createGroupManager() + val bobMgr = createGroupManager() + val carolMgr = createGroupManager() + + // Use 32-byte identities so they fit the MarmotGroupData + // admin_pubkeys 32-byte slots. + val aliceIdBytes = ByteArray(32) { 0xA1.toByte() } + val bobIdBytes = ByteArray(32) { 0xB2.toByte() } + val carolIdBytes = ByteArray(32) { 0xC3.toByte() } + aliceMgr.createGroup(groupId, aliceIdBytes) + // Install the Marmot group data extension so the Welcome carries + // the NostrGroupData (MlsGroupManager.processWelcome requires it). + aliceMgr.updateGroupExtensions( + nostrGroupId = groupId, + extensions = + listOf( + com.vitorpamplona.quartz.marmot.mip01Groups + .MarmotGroupData( + nostrGroupId = groupId, + adminPubkeys = listOf(aliceIdBytes.toHexKey()), + ).toExtension(), + ), + ) + val aliceGroup = aliceMgr.getGroup(groupId)!! + + // KeyPackages are just MLS artifacts carrying identity + init key; + // createKeyPackage on any group instance produces a usable bundle. + val bobBundle = aliceGroup.createKeyPackage(bobIdBytes, ByteArray(0)) + val carolBundle = aliceGroup.createKeyPackage(carolIdBytes, ByteArray(0)) + + val outbound = MarmotOutboundProcessor(aliceMgr) + + // --- Step 1: Alice adds Bob via stage/merge --- + val stagedBob = aliceMgr.stageAddMember(groupId, bobBundle.keyPackage.toTlsBytes()) + val addBobCommitEvent = + outbound.buildCommitEvent( + nostrGroupId = groupId, + commitBytes = stagedBob.framedCommitBytes, + exporterKey = stagedBob.preCommitExporterSecret, + ) + + // Bob joins via Welcome (emulated: we hand him the Welcome bytes). + bobMgr.processWelcome(stagedBob.welcomeBytes!!, bobBundle) + + aliceMgr.mergeStagedCommit(groupId, stagedBob) + val epochAfterAddBob = aliceMgr.getGroup(groupId)!!.epoch + assertEquals(epochAfterAddBob, bobMgr.getGroup(groupId)!!.epoch) + + // --- Step 2: Alice adds Carol via stage/merge --- + val stagedCarol = aliceMgr.stageAddMember(groupId, carolBundle.keyPackage.toTlsBytes()) + val addCarolCommitEvent = + outbound.buildCommitEvent( + nostrGroupId = groupId, + commitBytes = stagedCarol.framedCommitBytes, + exporterKey = stagedCarol.preCommitExporterSecret, + ) + + // Carol joins via Welcome at epoch 2. + carolMgr.processWelcome(stagedCarol.welcomeBytes!!, carolBundle) + + // *** The key assertion ***: Bob (still at epoch 1) receives the + // add-Carol kind:445 and must be able to: + // (a) outer-decrypt using his current (epoch 1) exporter key, + // (b) parse the MlsMessage → PublicMessage → COMMIT, + // (c) apply the commit and advance to epoch 2. + val bobExporterAtE1 = bobMgr.exporterSecret(groupId) + kotlin.test.assertContentEquals( + bobExporterAtE1, + stagedCarol.preCommitExporterSecret, + "Bob's epoch-1 exporter must match the pre-commit key used to " + + "outer-encrypt the add-Carol commit; otherwise Bob cannot " + + "decrypt and will be stuck on the old epoch.", + ) + + val decryptedMlsBytes = + GroupEventEncryption.decrypt( + addCarolCommitEvent.signedEvent.content, + bobExporterAtE1, + ) + val mlsMessage = + com.vitorpamplona.quartz.marmot.mls.framing.MlsMessage + .decodeTls( + com.vitorpamplona.quartz.marmot.mls.codec + .TlsReader(decryptedMlsBytes), + ) + val publicMessage = + com.vitorpamplona.quartz.marmot.mls.framing.PublicMessage + .decodeTls( + com.vitorpamplona.quartz.marmot.mls.codec + .TlsReader(mlsMessage.payload), + ) + bobMgr.processCommit( + nostrGroupId = groupId, + commitBytes = publicMessage.content, + senderLeafIndex = publicMessage.sender.leafIndex, + confirmationTag = publicMessage.confirmationTag!!, + ) + + aliceMgr.mergeStagedCommit(groupId, stagedCarol) + val epochAfterAddCarol = aliceMgr.getGroup(groupId)!!.epoch + assertEquals(epochAfterAddBob + 1, epochAfterAddCarol) + assertEquals(epochAfterAddCarol, bobMgr.getGroup(groupId)!!.epoch) + assertEquals(epochAfterAddCarol, carolMgr.getGroup(groupId)!!.epoch) + + // --- Step 3: Alice sends an application message. Both members must decrypt. --- + val msg = "Hi from Alice" + val appEvent = + outbound.buildGroupEventFromBytes(groupId, msg.encodeToByteArray()) + + val bobKey = bobMgr.exporterSecret(groupId) + val carolKey = carolMgr.exporterSecret(groupId) + val aliceKey = aliceMgr.exporterSecret(groupId) + kotlin.test.assertContentEquals(aliceKey, bobKey, "Bob must share Alice's epoch-2 key") + kotlin.test.assertContentEquals(aliceKey, carolKey, "Carol must share Alice's epoch-2 key") + + val bobMlsBytes = GroupEventEncryption.decrypt(appEvent.signedEvent.content, bobKey) + val bobDecrypted = bobMgr.decrypt(groupId, bobMlsBytes) + assertEquals(msg, bobDecrypted.content.decodeToString()) + + val carolMlsBytes = GroupEventEncryption.decrypt(appEvent.signedEvent.content, carolKey) + val carolDecrypted = carolMgr.decrypt(groupId, carolMlsBytes) + assertEquals(msg, carolDecrypted.content.decodeToString()) + } + } + @Test fun testFullRoundtripEncryptDecrypt() { runBlocking {