From 36c73fd65412b7b5cebe30d10c58ab31b5394153 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 03:01:17 +0000 Subject: [PATCH] fix(marmot): a Commit must not rebuild our own leaf from defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Commit replaces the committer's leaf through the UpdatePath. It is the same member with new key material, so everything the leaf says about the member has to survive — but `buildLeafNode` was called without either `capabilities` or `leafExtensions`, so it fell through to the legacy defaults every time. That cost us every invitation we have ever sent to MDK. A current-profile leaf carries `marmot.member.account-identity-proof.v2` inside an `app_data_dictionary` LEAF extension, and no proposal can put a leaf extension back, so our very first Commit silently demoted the group creator out of the current profile. The rebuilt leaf also stopped advertising the `app_data_dictionary` extension (0x0006) and the `app_data_update` proposal (0x0008) that the group's own `required_capabilities` demands, which makes the resulting tree fail RFC 9420 §7.3 leaf validation for every receiver. MDK reported `PublicGroupError(LeafNodeValidation(UnsupportedExtensions))` and dropped the Welcome minted by that same commit — the invitee simply never saw an invite, with nothing logged on either side. The same omission was in `proposeSigningKeyRotation`, where an Update proposal replaces our leaf for forward secrecy, and in `externalJoin`, which had no way to express a current-profile joiner leaf at all. Verified against MDK 0.9.20 on the interop harness: before, the invitee's pending-invite list stayed empty; after, our group arrives with its routing, profile and admin policy intact. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016kCuA6tc4JQzHPCDd39GHq --- cli/tests/.gitignore | 1 + .../quartz/marmot/mls/group/MlsGroup.kt | 36 +++++- .../MdkKeyPackageRoundTripTest.kt | 80 ++++++++++++ .../group/CommitPreservesLeafIdentityTest.kt | 118 ++++++++++++++++++ 4 files changed, 234 insertions(+), 1 deletion(-) create mode 100644 quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/MdkKeyPackageRoundTripTest.kt create mode 100644 quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/group/CommitPreservesLeafIdentityTest.kt diff --git a/cli/tests/.gitignore b/cli/tests/.gitignore index a74c7de8b8..363484739c 100644 --- a/cli/tests/.gitignore +++ b/cli/tests/.gitignore @@ -10,3 +10,4 @@ git/state-git-nip34/ buzz/state-job-loop/ buzz/state-workflow-loop/ buzz/state-agent-exec/ +marmot/state-repro/ 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 79ca2a0c90..e48121953d 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 @@ -446,6 +446,9 @@ class MlsGroup private constructor( (currentLeaf?.credential as? Credential.Basic)?.identity ?: ByteArray(0) + // An Update replaces our leaf with fresh key material and nothing + // else. Capabilities and leaf extensions (the account identity proof + // among them) describe the member, not the keys, so they carry over. val newLeafNode = buildLeafNode( encryptionKey = newEncKp.publicKey, @@ -455,6 +458,8 @@ class MlsGroup private constructor( signingKey = newSigKp.privateKey, groupId = groupId, leafIndex = myLeafIndex, + capabilities = currentLeaf?.capabilities ?: marmotLeafCapabilities(), + leafExtensions = currentLeaf?.extensions ?: emptyList(), ) val proposal = Proposal.Update(newLeafNode) @@ -694,18 +699,35 @@ class MlsGroup private constructor( // signature we mint fails to verify. val effectiveSigningKey = pendingSigningKey ?: signingPrivateKey val newEncKp = X25519.generateKeyPair() + // RFC 9420 §7.1: an UpdatePath leaf REPLACES our leaf. It is + // the same member, so everything about that member that is not + // key material carries over — capabilities and the leaf + // extensions. Rebuilding from defaults instead is not a + // cosmetic loss: a current-profile leaf keeps its + // `account-identity-proof` in an `app_data_dictionary` LEAF + // extension, and that extension can never be re-added by a + // proposal, so dropping it here silently demotes us out of the + // current profile at our very first commit. It also drops the + // `app_data_dictionary` capability the group's own + // `required_capabilities` demands, which makes the resulting + // tree fail RFC 9420 §7.3 leaf validation for every receiver — + // openmls reports `LeafNodeValidation(UnsupportedExtensions)` + // and the Welcome we just minted is unjoinable. + val previousLeaf = tree.getLeaf(myLeafIndex) val newLeafNode = buildLeafNode( encryptionKey = newEncKp.publicKey, signatureKey = Ed25519.publicFromPrivate(effectiveSigningKey), identity = - (tree.getLeaf(myLeafIndex)?.credential as? Credential.Basic)?.identity + (previousLeaf?.credential as? Credential.Basic)?.identity ?: ByteArray(0), source = LeafNodeSource.COMMIT, signingKey = effectiveSigningKey, groupId = groupId, leafIndex = myLeafIndex, parentHash = leafParentHash, + capabilities = previousLeaf?.capabilities ?: marmotLeafCapabilities(), + leafExtensions = previousLeaf?.extensions ?: emptyList(), ) encryptionPrivateKey = newEncKp.privateKey tree.setLeaf(myLeafIndex, newLeafNode) @@ -3583,6 +3605,12 @@ class MlsGroup private constructor( * @param groupInfoBytes TLS-serialized GroupInfo * @param identity the joiner's identity * @param signingKey optional Ed25519 signing key (generated if null) + * @param capabilities the joiner leaf's capabilities. A current-profile + * join MUST pass [currentProfileLeafCapabilities]; the default only + * satisfies a legacy group's `required_capabilities`. + * @param leafExtensions the joiner leaf's extensions. A current-profile + * join MUST pass the `app_data_dictionary` carrying its account + * identity proof — leaf extensions cannot be added after the fact. * @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]. @@ -3591,6 +3619,8 @@ class MlsGroup private constructor( groupInfoBytes: ByteArray, identity: ByteArray, signingKey: ByteArray? = null, + capabilities: Capabilities = marmotLeafCapabilities(), + leafExtensions: List = emptyList(), ): ExternalJoinResult { val groupInfo = GroupInfo.decodeTls(TlsReader(groupInfoBytes)) val groupContext = groupInfo.groupContext @@ -3655,6 +3685,8 @@ class MlsGroup private constructor( signingKey = sigKp.privateKey, groupId = groupContext.groupId, leafIndex = tree.leafCount, + capabilities = capabilities, + leafExtensions = leafExtensions, ) val myLeafIndex = tree.addLeaf(placeholderLeaf) @@ -3717,6 +3749,8 @@ class MlsGroup private constructor( groupId = groupContext.groupId, leafIndex = myLeafIndex, parentHash = extLeafParentHash, + capabilities = capabilities, + leafExtensions = leafExtensions, ) tree.setLeaf(myLeafIndex, leafNode) diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/MdkKeyPackageRoundTripTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/MdkKeyPackageRoundTripTest.kt new file mode 100644 index 0000000000..37d972dd98 --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/MdkKeyPackageRoundTripTest.kt @@ -0,0 +1,80 @@ +/* + * 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.mip00KeyPackages + +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import kotlin.io.encoding.Base64 +import kotlin.io.encoding.ExperimentalEncodingApi +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals + +/** + * A KeyPackage we did not author has to survive decode -> re-encode byte for + * byte, because the KeyPackageRef that names the joining member in a Welcome is + * `RefHash("MLS 1.0 KeyPackage Reference", serialize(KeyPackage))` over exactly + * those bytes. The invitee looks its private bundle up by that hash. One byte + * of drift anywhere in the struct -- a dropped extension, a reordered + * dictionary entry, a differently-sized length prefix -- and the Welcome names + * a KeyPackage nobody has, so the invitee silently has nothing to join with. + * + * The fixture is a real KeyPackage published by MDK 0.9.20 (`wn`), captured off + * the interop harness relay. + */ +@OptIn(ExperimentalEncodingApi::class) +class MdkKeyPackageRoundTripTest { + private val mdkFramedKeyPackage = "AAEABQABAAEgGhmBTAXpFdSxyMphrHiFKeFO4cmy5AvBVb1NSULTC18gWC4G7Zsb/wLG17JOfUoWuNLG7MpbwUNz/pgGxFy9EUsgiXlykzgtwdj1WFEW8/pwDNMzuah4ukUFMiySi4Au1loAASCNeT1vOihOZ/jZIF1Lltoa/AY1+fTYX3P/uxDoiykUeQIAAQIAAQgABvLR8tLy1AQACgAIAgABAQAAAABqoLL8AAAAAGsPfwxAkgAGQI5AjAABGRgAAYABgAKAA4AEgAWABoAHgAiACYALgAwAAgEAgAlAaI15PW86KE5n+NkgXUuW2hr8BjX59Nhfc/+7EOiLKRR5AAAAAGqgwQxu5AAeyJtPRpsTzslSt1o2N+kwYHQG5wUPsGgedSFMdRqP4mZeNEn8hi5zImQORBhf7tf4hISvja/gd/prx4ecQEC1dvSgT+hvumSJOgjdmLt9Yq9YCUY1Ih45QZXAy3EzPWTRfr5URwifcj4sscM7k9g7MPoAJQ1P4apLTlSgt7ILBwAGBAMABABAQJUP1427jN2rhINxSQuF2pKugrrvb6Qzo7kUYCGL8cHNcf2QArnyBdPxgNX/IRYk/d0bdQcByU81gSGPeA42PA8=" + + @Test + fun reEncodesAnMdkKeyPackageByteForByte() { + val framed = Base64.decode(mdkFramedKeyPackage) + val keyPackage = KeyPackageUtils.decodeKeyPackage(framed) + + // The framed publication is 4 bytes of MLSMessage header plus the + // KeyPackage struct; compare against the payload, not the envelope. + val payload = framed.copyOfRange(4, framed.size) + assertContentEquals(payload, keyPackage.toTlsBytes()) + } + + @Test + fun reFramesToTheExactBytesMdkPublished() { + val framed = Base64.decode(mdkFramedKeyPackage) + val keyPackage = KeyPackageUtils.decodeKeyPackage(framed) + assertContentEquals(framed, KeyPackageUtils.frameKeyPackage(keyPackage)) + } + + /** + * The `i` tag MDK put on the publication is the KeyPackageRef MDK filed its + * own private bundle under. Recomputing it from the bytes we decoded is the + * end-to-end check: if these agree, a Welcome we address to this member + * names a KeyPackage the member can actually find. + */ + @Test + fun computesTheSameReferenceMdkAdvertised() { + val framed = Base64.decode(mdkFramedKeyPackage) + val keyPackage = KeyPackageUtils.decodeKeyPackage(framed) + assertEquals(MDK_ADVERTISED_REF, keyPackage.reference().toHexKey()) + } + + companion object { + private const val MDK_ADVERTISED_REF = "1e6606f8e154a0c148587c4334f801fbb7aaf5a806d8971a87961a58366844ef" + } +} diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/group/CommitPreservesLeafIdentityTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/group/CommitPreservesLeafIdentityTest.kt new file mode 100644 index 0000000000..101d0048eb --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/group/CommitPreservesLeafIdentityTest.kt @@ -0,0 +1,118 @@ +/* + * 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.appComponents.AppComponentIds +import com.vitorpamplona.quartz.marmot.appComponents.CurrentProfileGroupFactory +import com.vitorpamplona.quartz.marmot.appComponents.GroupProfileV1 +import com.vitorpamplona.quartz.marmot.mls.components.AppDataDictionary +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * A Commit replaces the committer's own leaf through the UpdatePath. That leaf + * is the SAME member with new key material, so everything the leaf says about + * the member has to survive: its `capabilities`, and its extensions. + * + * Rebuilding from defaults instead cost us every invitation. A current-profile + * leaf carries `marmot.member.account-identity-proof.v2` inside an + * `app_data_dictionary` LEAF extension, and no proposal can put a leaf + * extension back, so the first Commit silently demoted the creator out of the + * current profile. Worse, the rebuilt leaf stopped advertising the + * `app_data_dictionary` extension and `app_data_update` proposal that the + * group's own `required_capabilities` demands, so the resulting tree failed + * RFC 9420 leaf validation for every receiver: MDK's openmls reported + * `LeafNodeValidation(UnsupportedExtensions)` and dropped the Welcome we had + * just minted from that same commit. + */ +class CommitPreservesLeafIdentityTest { + /** `app_data_update`, the proposal a current-profile group requires. */ + private val appDataUpdateProposalType = 0x0008 + + private fun signer(seed: Byte) = NostrSignerInternal(KeyPair(ByteArray(32) { seed })) + + private suspend fun aCurrentProfileGroup(): MlsGroup = + CurrentProfileGroupFactory.createGroup( + signer = signer(0x11), + nostrGroupId = ByteArray(32) { 0x22 }, + relays = listOf("wss://relay.example.com"), + profile = GroupProfileV1("Interop", ""), + ) + + private fun MlsGroup.myLeaf() = members().firstOrNull { it.first == leafIndex }?.second + + @Test + fun theCommitterLeafKeepsItsCurrentProfileCapabilities() = + runBlocking { + val group = aCurrentProfileGroup() + val before = assertNotNull(group.myLeaf()).capabilities + + val invitee = CurrentProfileGroupFactory.createKeyPackage(signer(0x33)) + group.proposeAdd(invitee.keyPackage.toTlsBytes()) + group.commit() + + val after = assertNotNull(group.myLeaf()).capabilities + assertContains(after.extensions, AppDataDictionary.EXTENSION_TYPE) + assertContains(after.proposals, appDataUpdateProposalType) + assertTrue(before.extensions.all { it in after.extensions }) + assertTrue(before.proposals.all { it in after.proposals }) + } + + @Test + fun theCommitterLeafKeepsItsAccountIdentityProof() = + runBlocking { + val group = aCurrentProfileGroup() + + val invitee = CurrentProfileGroupFactory.createKeyPackage(signer(0x33)) + group.proposeAdd(invitee.keyPackage.toTlsBytes()) + group.commit() + + val leaf = assertNotNull(group.myLeaf()) + val dictionary = + assertNotNull( + leaf.extensions.firstOrNull { it.extensionType == AppDataDictionary.EXTENSION_TYPE }, + ) + val decoded = AppDataDictionary.decode(dictionary.extensionData) + assertNotNull(decoded[AppComponentIds.ACCOUNT_IDENTITY_PROOF_V2]) + } + + @Test + fun aSigningKeyRotationAlsoKeepsTheLeafIdentity() = + runBlocking { + val group = aCurrentProfileGroup() + val before = assertNotNull(group.myLeaf()) + + group.proposeSigningKeyRotation() + group.commit() + + val after = assertNotNull(group.myLeaf()) + assertContains(after.capabilities.extensions, AppDataDictionary.EXTENSION_TYPE) + assertTrue( + after.extensions.any { it.extensionType == AppDataDictionary.EXTENSION_TYPE }, + ) + assertTrue(before.extensions.size <= after.extensions.size) + } +}