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 a1cbdd8e2..3316be1d2 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 @@ -39,6 +39,7 @@ import com.vitorpamplona.quartz.marmot.mls.framing.encodeSender import com.vitorpamplona.quartz.marmot.mls.messages.Commit import com.vitorpamplona.quartz.marmot.mls.messages.CommitResult import com.vitorpamplona.quartz.marmot.mls.messages.EncryptedGroupSecrets +import com.vitorpamplona.quartz.marmot.mls.messages.ExternalJoinResult import com.vitorpamplona.quartz.marmot.mls.messages.GroupContext import com.vitorpamplona.quartz.marmot.mls.messages.GroupInfo import com.vitorpamplona.quartz.marmot.mls.messages.GroupSecrets @@ -1036,6 +1037,62 @@ class MlsGroup private constructor( // --- Commit Processing --- + /** + * Process a fully-framed `MlsMessage(PublicMessage(Commit))` produced by + * [CommitResult.framedCommitBytes]. Unpacks the wire envelope, verifies the + * RFC 9420 §6.2 membership_tag, and delegates to [processCommit] with the + * correct signature, confirmation_tag, and sender leaf index. This is the + * symmetric peer-side entry point for commits returned by [commit] / + * [addMember] / [removeMember] — tests and any caller that holds the + * framed bytes should prefer this over the low-level [processCommit]. + * + * Mirrors the production unwrap done by `MarmotInboundProcessor` for + * inbound kind:445 PublicMessage commits. + */ + fun processFramedCommit(framedCommitBytes: ByteArray) { + val mlsMessage = MlsMessage.decodeTls(TlsReader(framedCommitBytes)) + require(mlsMessage.wireFormat == WireFormat.PUBLIC_MESSAGE) { + "processFramedCommit expects a PublicMessage envelope, got ${mlsMessage.wireFormat}" + } + val pubMsg = PublicMessage.decodeTls(TlsReader(mlsMessage.payload)) + require(pubMsg.contentType == ContentType.COMMIT) { + "Framed commit must contain ContentType.COMMIT, got ${pubMsg.contentType}" + } + val tag = + requireNotNull(pubMsg.confirmationTag) { + "PublicMessage commit missing confirmation_tag" + } + // External commits don't carry a membership_tag (sender isn't yet a + // member). For member-sender commits, RFC 9420 §6.2 requires the tag. + if (pubMsg.sender.senderType == SenderType.MEMBER) { + require(verifyPublicMessageCommitMembershipTag(pubMsg)) { + "Invalid membership_tag on PublicMessage commit" + } + } + // RFC 9420 §6: `new_member_commit` senders carry no leaf_index field + // on the wire — the joiner's slot is determined by the receiver + // using the same algorithm the joiner used in [RatchetTree.addLeaf]: + // first blank leaf, else append past the current leaf_count. + val senderLeafIndex = + when (pubMsg.sender.senderType) { + SenderType.NEW_MEMBER_COMMIT -> { + (0 until tree.leafCount).firstOrNull { tree.getLeaf(it) == null } + ?: tree.leafCount + } + + else -> { + pubMsg.sender.leafIndex + } + } + processCommit( + commitBytes = pubMsg.content, + senderLeafIndex = senderLeafIndex, + confirmationTag = tag, + signature = pubMsg.signature, + wireFormat = WireFormat.PUBLIC_MESSAGE, + ) + } + /** * Process a received Commit message, advancing the epoch. * @@ -2513,13 +2570,15 @@ class MlsGroup private constructor( * @param groupInfoBytes TLS-serialized GroupInfo * @param identity the joiner's identity * @param signingKey optional Ed25519 signing key (generated if null) - * @return the new MlsGroup and the commit bytes to send to the group + * @return the new MlsGroup along with the raw inner commit bytes and a + * wire-ready PublicMessage envelope. Existing group members consume + * the framed bytes via [MlsGroup.processFramedCommit]. */ fun externalJoin( groupInfoBytes: ByteArray, identity: ByteArray, signingKey: ByteArray? = null, - ): Pair { + ): ExternalJoinResult { val groupInfo = GroupInfo.decodeTls(TlsReader(groupInfoBytes)) val groupContext = groupInfo.groupContext @@ -2600,37 +2659,23 @@ class MlsGroup private constructor( map } - // Build UpdatePath for our leaf + // Build UpdatePath for our leaf using the same staging pattern + // as regular commit() (RFC 9420 §7.6): stage public keys → apply → + // patch parent_hash → rebuild leaf → compute post-mutation context + // → HPKE-encrypt with that context. Encrypting against the + // PRE-mutation groupContext (the bug we're fixing here) makes + // existing members fail to open the path-secret AEAD because + // their decryption context bumps the epoch and folds in the new + // tree_hash. val leafSecret = MlsCryptoProvider.randomBytes(MlsCryptoProvider.HASH_OUTPUT_LENGTH) val pathSecrets = tree.derivePathSecrets(myLeafIndex, leafSecret) val copath = BinaryTree.copath(myLeafIndex, tree.leafCount) - val pathNodes = - pathSecrets.zip(copath).map { (pathKey, copathNode) -> - val resolution = tree.resolution(copathNode) - val encryptedSecrets = - resolution.mapNotNull { resNode -> - val node = tree.getNode(resNode) ?: return@mapNotNull null - val recipientPub = - when (node) { - is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Leaf -> { - node.leafNode.encryptionKey - } - is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent -> { - node.parentNode.encryptionKey - } - } - MlsCryptoProvider.encryptWithLabel( - recipientPub, - "UpdatePathNode", - groupContext.toTlsBytes(), - pathKey.pathSecret, - ) - } - UpdatePathNode(pathKey.publicKey, encryptedSecrets) - } - - tree.applyUpdatePath(myLeafIndex, pathNodes) + // Stage path-keys WITHOUT encrypted secrets so subsequent + // parent_hash / tree_hash computations reflect the new keys. + val stagedPathNodes = + pathSecrets.map { pathKey -> UpdatePathNode(pathKey.publicKey, emptyList()) } + tree.applyUpdatePath(myLeafIndex, stagedPathNodes) // Compute parent_hash chain + the leaf's parent_hash (RFC 9420 // §7.9.2) on the post-update tree, then patch parent nodes and @@ -2662,6 +2707,42 @@ class MlsGroup private constructor( ) tree.setLeaf(myLeafIndex, leafNode) + // Post-mutation path-encryption context: bump epoch, swap in the + // post-update tree_hash. Matches what existing members compute + // in processCommitInner when they HPKE-Open the path secret. + val pathEncContextBytes = + groupContext + .copy( + epoch = groupContext.epoch + 1, + treeHash = tree.treeHash(), + ).toTlsBytes() + + val pathNodes = + stagedPathNodes.zip(copath).zip(pathSecrets) { (staged, copathNode), pathKey -> + val resolution = tree.resolution(copathNode) + val encryptedSecrets = + resolution.mapNotNull { resNode -> + val node = tree.getNode(resNode) ?: return@mapNotNull null + val recipientPub = + when (node) { + is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Leaf -> { + node.leafNode.encryptionKey + } + + is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent -> { + node.parentNode.encryptionKey + } + } + MlsCryptoProvider.encryptWithLabel( + recipientPub, + "UpdatePathNode", + pathEncContextBytes, + pathKey.pathSecret, + ) + } + UpdatePathNode(staged.encryptionKey, encryptedSecrets) + } + val updatePath = UpdatePath(leafNode, pathNodes) // Build Commit @@ -2723,7 +2804,32 @@ class MlsGroup private constructor( interimTranscriptHash = interimTranscriptHash, ) - return Pair(group, commitBytes) + // Wrap the commit in a PublicMessage envelope so existing members + // can extract sender / signature / confirmation_tag via + // MlsGroup.processFramedCommit (the symmetric receive path). + // External commits (RFC 9420 §12.4.3) carry sender = + // new_member_commit and no membership_tag — the joiner is + // claiming a slot that doesn't yet exist in the group's + // membership_key MAC space. + val publicMessage = + PublicMessage( + groupId = groupContext.groupId, + epoch = groupContext.epoch, + sender = Sender(SenderType.NEW_MEMBER_COMMIT, 0), + authenticatedData = ByteArray(0), + contentType = ContentType.COMMIT, + content = commitBytes, + signature = ByteArray(0), + confirmationTag = confirmationTag, + membershipTag = null, + ) + val framedCommitBytes = MlsMessage.fromPublicMessage(publicMessage).toTlsBytes() + + return ExternalJoinResult( + group = group, + commitBytes = commitBytes, + framedCommitBytes = framedCommitBytes, + ) } /** 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 ddab3c19d..565db9b15 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 @@ -24,6 +24,7 @@ import com.vitorpamplona.quartz.marmot.mls.codec.TlsReader import com.vitorpamplona.quartz.marmot.mls.codec.TlsWriter import com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider import com.vitorpamplona.quartz.marmot.mls.messages.CommitResult +import com.vitorpamplona.quartz.marmot.mls.messages.ExternalJoinResult import com.vitorpamplona.quartz.marmot.mls.messages.KeyPackageBundle import com.vitorpamplona.quartz.marmot.mls.schedule.KeySchedule import com.vitorpamplona.quartz.marmot.mls.schedule.SecretTree @@ -246,12 +247,12 @@ class MlsGroupManager( groupInfoBytes: ByteArray, identity: ByteArray, signingKey: ByteArray? = null, - ): Pair = + ): ExternalJoinResult = mutex.withLock { - val (group, commitBytes) = MlsGroup.externalJoin(groupInfoBytes, identity, signingKey) - groups[nostrGroupId] = group + val result = MlsGroup.externalJoin(groupInfoBytes, identity, signingKey) + groups[nostrGroupId] = result.group persistGroup(nostrGroupId) - Pair(group, commitBytes) + result } // --- Epoch Transitions --- diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/messages/Commit.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/messages/Commit.kt index 624dd8fff..9931ec236 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/messages/Commit.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/messages/Commit.kt @@ -128,3 +128,27 @@ data class CommitResult( override fun hashCode(): Int = commitBytes.contentHashCode() } + +/** + * Result of [com.vitorpamplona.quartz.marmot.mls.group.MlsGroup.externalJoin]. + * + * [commitBytes] is the raw TLS-encoded [Commit] struct. For on-the-wire + * distribution to existing group members, callers MUST publish [framedCommitBytes] + * — a `MlsMessage(PublicMessage(FramedContent(commit)))` envelope with sender + * `new_member_commit` (RFC 9420 §12.4.3) — so that receivers can parse the + * confirmation_tag and process the commit via + * [com.vitorpamplona.quartz.marmot.mls.group.MlsGroup.processFramedCommit]. + */ +data class ExternalJoinResult( + val group: com.vitorpamplona.quartz.marmot.mls.group.MlsGroup, + val commitBytes: ByteArray, + val framedCommitBytes: ByteArray, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is ExternalJoinResult) return false + return commitBytes.contentEquals(other.commitBytes) + } + + override fun hashCode(): Int = commitBytes.contentHashCode() +} diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupEdgeCaseTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupEdgeCaseTest.kt index dd7add392..a9750d7b5 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupEdgeCaseTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupEdgeCaseTest.kt @@ -234,7 +234,7 @@ class MlsGroupEdgeCaseTest { // Advance through several epochs with empty commits for (i in 0 until 5) { val commitResult = alice.commit() - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) } // epoch 0 (create) -> epoch 1 (add bob) -> 5 empty commits = epoch 6 @@ -271,7 +271,7 @@ class MlsGroupEdgeCaseTest { for (i in 0 until 3) { val commitResult = alice.commit() - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) keys.add(alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32)) } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt index c067facda..bfd27bce6 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt @@ -170,11 +170,6 @@ class MlsGroupLifecycleTest { // 4. Three-member group: sequential additions // ----------------------------------------------------------------------- - // BUG: processCommit does not derive the same epoch secrets as commit(). - // After Bob.processCommit(Alice's commit), Bob's key schedule diverges - // because the commit_secret decryption from the UpdatePath does not - // correctly walk the ratchet tree to find the common ancestor's path secret. - // This causes AEAD decryption failures on cross-member messages. @Test fun testThreeMemberGroup_SequentialAdditions() { // Alice creates the group @@ -190,7 +185,7 @@ class MlsGroupLifecycleTest { // Alice adds Carol (Bob processes Alice's commit) val carolBundle = createStandaloneKeyPackage("carol") val addCarolResult = alice.addMember(carolBundle.keyPackage.toTlsBytes()) - bob.processCommit(addCarolResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(addCarolResult.framedCommitBytes) val carol = MlsGroup.processWelcome(addCarolResult.welcomeBytes!!, carolBundle) assertEquals(2L, alice.epoch) @@ -228,7 +223,7 @@ class MlsGroupLifecycleTest { val addCarolResult = bob.addMember(carolBundle.keyPackage.toTlsBytes()) // Alice processes Bob's commit - alice.processCommit(addCarolResult.commitBytes, bob.leafIndex, ByteArray(0)) + alice.processFramedCommit(addCarolResult.framedCommitBytes) assertEquals(2L, alice.epoch) assertEquals(2L, bob.epoch) @@ -240,18 +235,18 @@ class MlsGroupLifecycleTest { // 6. External join via GroupInfo // ----------------------------------------------------------------------- - // BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions @Test fun testExternalJoin_ZaraJoinsViaGroupInfo() { val alice = MlsGroup.create("alice".encodeToByteArray()) val groupInfoBytes = alice.groupInfo().toTlsBytes() // Zara joins externally - val (zara, commitBytes) = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray()) + val externalJoin = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray()) + val zara = externalJoin.group assertEquals(1L, zara.epoch) // Alice processes the external commit - alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0)) + alice.processFramedCommit(externalJoin.framedCommitBytes) assertEquals(1L, alice.epoch) assertEquals(2, alice.memberCount) @@ -262,14 +257,14 @@ class MlsGroupLifecycleTest { assertContentEquals(msg, dec.content) } - // BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions @Test fun testExternalJoin_ExporterSecretsAgree() { val alice = MlsGroup.create("alice".encodeToByteArray()) val groupInfoBytes = alice.groupInfo().toTlsBytes() - val (zara, commitBytes) = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray()) - alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0)) + val externalJoin = MlsGroup.externalJoin(groupInfoBytes, "zara".encodeToByteArray()) + val zara = externalJoin.group + alice.processFramedCommit(externalJoin.framedCommitBytes) val aliceKey = alice.exporterSecret("marmot", "group-event".encodeToByteArray(), 32) val zaraKey = zara.exporterSecret("marmot", "group-event".encodeToByteArray(), 32) @@ -307,7 +302,6 @@ class MlsGroupLifecycleTest { // 8. Signing key rotation (Update proposal) // ----------------------------------------------------------------------- - // BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions @Test fun testSigningKeyRotation_EpochAdvances() { val alice = MlsGroup.create("alice".encodeToByteArray()) @@ -322,7 +316,7 @@ class MlsGroupLifecycleTest { val commitResult = alice.commit() // Bob processes Alice's rotation commit - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) assertEquals(2L, alice.epoch) assertEquals(2L, bob.epoch) @@ -336,7 +330,6 @@ class MlsGroupLifecycleTest { assertFalse(keyBefore.contentEquals(aliceKey), "Exporter secret should change after rotation") } - // BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions @Test fun testEncryptDecryptAfterSigningKeyRotation() { val alice = MlsGroup.create("alice".encodeToByteArray()) @@ -347,7 +340,7 @@ class MlsGroupLifecycleTest { // Alice rotates her signing key alice.proposeSigningKeyRotation() val commitResult = alice.commit() - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) // Both directions should still work after rotation val msg1 = "After rotation from Alice".encodeToByteArray() @@ -405,7 +398,6 @@ class MlsGroupLifecycleTest { // 10. PSK proposal: register and use in commit // ----------------------------------------------------------------------- - // BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions @Test fun testPskProposal_EpochAdvancesWithPsk() { val alice = MlsGroup.create("alice".encodeToByteArray()) @@ -426,7 +418,7 @@ class MlsGroupLifecycleTest { val commitResult = alice.commit() // Bob processes the commit - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) assertEquals(epochBefore + 1, alice.epoch) assertEquals(alice.epoch, bob.epoch) @@ -457,7 +449,7 @@ class MlsGroupLifecycleTest { assertNotNull(alice.reInitPending, "ReInit should be pending after commit") // Bob processes and should also see reInit - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) assertNotNull(bob.reInitPending, "Bob should also see ReInit pending") } @@ -476,7 +468,7 @@ class MlsGroupLifecycleTest { // Alice commits with no proposals (purely for forward secrecy / UpdatePath) val commitResult = alice.commit() - bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0)) + bob.processFramedCommit(commitResult.framedCommitBytes) assertEquals(epochBefore + 1, alice.epoch) assertEquals(alice.epoch, bob.epoch) diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupTest.kt index 57ded76b7..e01677c71 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupTest.kt @@ -254,17 +254,18 @@ class MlsGroupTest { val groupInfoBytes = alice.groupInfo().toTlsBytes() // Zara joins via external commit (without a Welcome) - val (zara, commitBytes) = + val externalJoin = MlsGroup.externalJoin( groupInfoBytes, "zara".encodeToByteArray(), ) + val zara = externalJoin.group // Zara is now in the group at epoch 1 assertEquals(1L, zara.epoch) // Alice processes Zara's external commit - alice.processCommit(commitBytes, zara.leafIndex, ByteArray(0)) + alice.processFramedCommit(externalJoin.framedCommitBytes) assertEquals(1L, alice.epoch) assertEquals(2, alice.memberCount) }