diff --git a/cli/tests/marmot/tests-create.sh b/cli/tests/marmot/tests-create.sh index 218851be04..b868380ef9 100644 --- a/cli/tests/marmot/tests-create.sh +++ b/cli/tests/marmot/tests-create.sh @@ -10,7 +10,9 @@ test_01_keypackage_discovery() { # B finds A's KP local raw ev raw=$(wn_b --json keys check "$A_NPUB" 2>>"$LOG_FILE" || true) - ev=$(printf '%s' "$raw" | jq -r '.result.event_id // .event_id // empty') + # MDK 0.9.x reports the found KeyPackage under result.key_package; the two + # older shapes are kept so this still reads a pre-0.9 daemon. + ev=$(printf '%s' "$raw" | jq -r '.result.key_package.key_package_event_id // .result.event_id // .event_id // empty') if [[ -z "$ev" || "$ev" == "null" ]]; then record_result "$id (B->A)" fail "wn couldn't find A's KP"; return fi 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 f61b8db7d5..9b16da3617 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 @@ -418,12 +418,11 @@ class MarmotManager( keyPackageEventId: HexKey, relays: List, ): Pair { - // Verify that the KeyPackage credential matches the expected member pubkey - val kp = - com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage.decodeTls( - com.vitorpamplona.quartz.marmot.mls.codec - .TlsReader(keyPackageBytes), - ) + // Verify that the KeyPackage credential matches the expected member + // pubkey. Accepts either framing — a peer's published KeyPackage is an + // MLSMessage, and bare bytes still arrive from our own pre-fix + // publications sitting on relays. + val kp = KeyPackageUtils.decodeKeyPackage(keyPackageBytes) val credential = kp.leafNode.credential require(credential is Credential.Basic) { "KeyPackage must use BasicCredential" @@ -439,7 +438,9 @@ class MarmotManager( // that key. val publication = commitAndPublish(nostrGroupId, relays) { - groupManager.stageAddMember(nostrGroupId, keyPackageBytes) + // The BARE KeyPackage, not the bytes as published. Transport + // framing is the Marmot layer's business; MLS takes the struct. + groupManager.stageAddMember(nostrGroupId, kp.toTlsBytes()) } // The Welcome is a SEPARATE, retryable per-invitee delivery obligation @@ -836,24 +837,32 @@ class MarmotManager( keyPackageRotationManager.generateKeyPackage(identity, dTag) } - val keyPackageBytes = bundle.keyPackage.toTlsBytes() - val keyPackageBase64 = Base64.encode(keyPackageBytes) + // Framed as an MLSMessage, never bare: `foundation/key-packages.md` + // says a transport publication IS the framed message. The ref stays + // over the INNER KeyPackage, which is what RFC 9420 MakeKeyPackageRef + // hashes — framing the ref too would make our `i` tag disagree with + // every other implementation's. + val keyPackageBase64 = Base64.encode(KeyPackageUtils.frameKeyPackage(bundle.keyPackage)) val keyPackageRef = bundle.keyPackage.reference().toHexKey() val template = if (currentProfile) { + // Every id-list tag is derived from the KeyPackage itself. + // They duplicate metadata already inside it, so writing them by + // hand is a second source of truth — and a receiver that + // compares them (MDK does, exactly) rejects the KeyPackage the + // moment the two disagree. + // // Deliberately no `relays` tag and no `encoding` tag. // `transports/nostr.md`: a KeyPackage is fetched from the - // account's own inbox relay set, so repeating them here would - // be a second, drifting source of truth; and the binding - // forbids an `encoding` tag outright, because a receiver that - // switched decoders on one could be steered into a different - // parse of the same bytes. - KeyPackageEvent.buildCurrentProfile( - keyPackageBase64 = keyPackageBase64, + // account's own NIP-65 write set, so repeating relays here + // would be another drifting duplicate; and the binding forbids + // an `encoding` tag outright, because a receiver that switched + // decoders on one could be steered into a different parse of + // the same bytes. + KeyPackageEvent.buildCurrentProfileFrom( + keyPackage = bundle.keyPackage, dTagSlot = dTag, - keyPackageRef = keyPackageRef, - appComponentIds = emptyList(), ) } else { KeyPackageEvent.build( @@ -884,8 +893,7 @@ class MarmotManager( val identity = signer.pubKey.hexToByteArray() return pendingSlots.map { slot -> val bundle = keyPackageRotationManager.rotateSlot(identity, slot) - val keyPackageBytes = bundle.keyPackage.toTlsBytes() - val keyPackageBase64 = Base64.encode(keyPackageBytes) + val keyPackageBase64 = Base64.encode(KeyPackageUtils.frameKeyPackage(bundle.keyPackage)) val keyPackageRef = bundle.keyPackage.reference().toHexKey() val template = diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageEvent.kt index 76b4238187..5686cd69d1 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageEvent.kt @@ -24,13 +24,19 @@ import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.AppComponentsTag import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.EncodingTag import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.MlsProposalsTag +import com.vitorpamplona.quartz.marmot.mls.components.AppDataDictionary +import com.vitorpamplona.quartz.marmot.mls.components.ComponentsList +import com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage import com.vitorpamplona.quartz.nip01Core.core.BaseAddressableEvent import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.TagArrayBuilder +import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import com.vitorpamplona.quartz.nip01Core.signers.eventTemplate import com.vitorpamplona.quartz.nip01Core.tags.dTag.dTag import com.vitorpamplona.quartz.utils.TimeUtils +import kotlin.io.encoding.Base64 +import kotlin.io.encoding.ExperimentalEncodingApi /** * Marmot KeyPackage Event (MIP-00) — kind 30443. @@ -125,6 +131,8 @@ class KeyPackageEvent( keyPackageRef: HexKey, appComponentIds: List, ciphersuite: String = "0x0001", + mlsExtensionIds: List = listOf(CURRENT_PROFILE_EXTENSION), + mlsProposalIds: List = listOf(APP_DATA_UPDATE_PROPOSAL, MlsProposalsTag.SELF_REMOVE), clientName: String? = null, createdAt: Long = TimeUtils.now(), initializer: TagArrayBuilder.() -> Unit = {}, @@ -132,8 +140,8 @@ class KeyPackageEvent( dTag(dTagSlot) mlsProtocolVersion() mlsCiphersuite(ciphersuite) - mlsExtensions(listOf(CURRENT_PROFILE_EXTENSION)) - mlsProposals(listOf(APP_DATA_UPDATE_PROPOSAL, MlsProposalsTag.SELF_REMOVE)) + mlsExtensions(mlsExtensionIds.distinct().sorted()) + mlsProposals(mlsProposalIds.distinct().sorted()) appComponents( (appComponentIds + AppComponentsTag.ACCOUNT_IDENTITY_PROOF_V2).distinct().sorted(), ) @@ -142,6 +150,65 @@ class KeyPackageEvent( initializer() } + /** + * Build the event from the KeyPackage itself, deriving every id-list + * tag from the bytes it advertises. + * + * The tags duplicate metadata that is already inside the KeyPackage, so + * writing them by hand is writing a second source of truth — and MDK + * rejects a KeyPackage whose `mls_extensions` tag "does not exactly + * match decoded KeyPackage metadata". Adding one leaf capability and + * forgetting the tag is enough to make every one of our KeyPackages + * unusable, which is exactly what happened. + * + * `app_components` lists the Marmot registry ids only. The + * `app_components` component itself (`0x0001`) and the other upstream + * MLS-extensions component ids live below `0x8000` and are not app + * components being advertised. + */ + @OptIn(ExperimentalEncodingApi::class) + fun buildCurrentProfileFrom( + keyPackage: MlsKeyPackage, + dTagSlot: String, + clientName: String? = null, + createdAt: Long = TimeUtils.now(), + initializer: TagArrayBuilder.() -> Unit = {}, + ) = buildCurrentProfile( + keyPackageBase64 = Base64.encode(KeyPackageUtils.frameKeyPackage(keyPackage)), + dTagSlot = dTagSlot, + keyPackageRef = keyPackage.reference().toHexKey(), + appComponentIds = advertisedAppComponents(keyPackage).map(::idHex), + ciphersuite = idHex(keyPackage.cipherSuite), + mlsExtensionIds = + keyPackage.leafNode.capabilities.extensions + .map(::idHex), + mlsProposalIds = + keyPackage.leafNode.capabilities.proposals + .map(::idHex), + clientName = clientName, + createdAt = createdAt, + initializer = initializer, + ) + + /** `0x`-prefixed lowercase hex of a 16-bit id, zero-padded to four digits. */ + fun idHex(id: Int): String { + val hex = id.toString(16) + return "0x" + "0".repeat(4 - hex.length) + hex + } + + /** Marmot app-component ids the leaf advertises, from its `app_components` component. */ + private fun advertisedAppComponents(keyPackage: MlsKeyPackage): List = + try { + val dictionary = AppDataDictionary.fromExtensionsOrEmpty(keyPackage.leafNode.extensions) + val list = dictionary[ComponentsList.APP_COMPONENTS_ID] ?: return emptyList() + ComponentsList.decode(list).filter { it >= MARMOT_COMPONENT_RANGE_START } + } catch (_: Exception) { + emptyList() + } + + /** Marmot's own component registry starts here; lower ids are upstream MLS-extensions ones. */ + private const val MARMOT_COMPONENT_RANGE_START = 0x8000 + /** MIP-era builder, kept for legacy groups already on disk. */ fun build( keyPackageBase64: String, diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageUtils.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageUtils.kt index e5617a900f..a5fd89b6ee 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageUtils.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageUtils.kt @@ -33,6 +33,8 @@ import com.vitorpamplona.quartz.marmot.mls.codec.TlsReader import com.vitorpamplona.quartz.marmot.mls.components.AppDataDictionary import com.vitorpamplona.quartz.marmot.mls.components.ComponentsList import com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider +import com.vitorpamplona.quartz.marmot.mls.framing.MlsMessage +import com.vitorpamplona.quartz.marmot.mls.framing.WireFormat import com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage import com.vitorpamplona.quartz.marmot.mls.tree.Credential import com.vitorpamplona.quartz.nip01Core.core.Event @@ -59,6 +61,52 @@ object KeyPackageUtils { */ const val MAX_LIFETIME_SECONDS = 7_261_200L + /** + * Frame a KeyPackage for publication. + * + * `foundation/key-packages.md` is explicit: "a transport publication is + * unambiguously the framed `MLSMessage`, not a bare `KeyPackage` struct." + * The envelope is only four bytes — `ProtocolVersion` then + * `WireFormat = mls_key_package` — but omitting it is not a cosmetic + * difference. A reader that expects the envelope reads a bare KeyPackage's + * leading `0x0001 0x0001` as version 1, wire format 1 (`mls_public_message`) + * and then parses the rest as a PublicMessage, desyncing a few fields in + * and failing on whatever byte it lands on. That is exactly how MDK + * rejected every KeyPackage we published, with a decode error naming a + * value that appears nowhere in the structure. + */ + fun frameKeyPackage(keyPackage: MlsKeyPackage): ByteArray = + MlsMessage( + wireFormat = WireFormat.KEY_PACKAGE, + payload = keyPackage.toTlsBytes(), + ).toTlsBytes() + + /** + * Decode published KeyPackage bytes, framed or bare. + * + * Framed is what the spec requires and what we now publish. The bare form + * is accepted because every KeyPackage this client published before the fix + * is bare, and those are still sitting on relays inside their publication + * lifetime; refusing them would make our own users un-invitable by each + * other until every one of them rotated. + * + * The two are told apart by the envelope rather than by trial and error: a + * framed message starts with `ProtocolVersion = 1` and + * `WireFormat = mls_key_package`, and a bare KeyPackage's second field is + * its ciphersuite, which is never 5 for any suite Marmot uses. + */ + fun decodeKeyPackage(bytes: ByteArray): MlsKeyPackage { + if (bytes.size >= 4) { + val version = ((bytes[0].toInt() and 0xFF) shl 8) or (bytes[1].toInt() and 0xFF) + val wireFormat = ((bytes[2].toInt() and 0xFF) shl 8) or (bytes[3].toInt() and 0xFF) + if (version == MlsMessage.MLS_VERSION_10 && wireFormat == WireFormat.KEY_PACKAGE.value) { + val message = MlsMessage.decodeTls(TlsReader(bytes)) + return MlsKeyPackage.decodeTls(TlsReader(message.payload)) + } + } + return MlsKeyPackage.decodeTls(TlsReader(bytes)) + } + /** Legacy non-addressable KeyPackage kind (pre-migration) */ const val LEGACY_KIND = 443 @@ -190,8 +238,7 @@ object KeyPackageUtils { val iTag = event.keyPackageRef() ?: return false val keyPackage = try { - val bytes = Base64.decode(event.content) - MlsKeyPackage.decodeTls(TlsReader(bytes)) + decodeKeyPackage(Base64.decode(event.content)) } catch (_: Throwable) { return false } diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageFramingTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageFramingTest.kt new file mode 100644 index 0000000000..26619fe6f9 --- /dev/null +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mip00KeyPackages/KeyPackageFramingTest.kt @@ -0,0 +1,104 @@ +/* + * 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.marmot.mls.framing.MlsMessage +import com.vitorpamplona.quartz.marmot.mls.framing.WireFormat +import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * `foundation/key-packages.md`: "a transport publication is unambiguously the + * framed `MLSMessage`, not a bare `KeyPackage` struct." + * + * We published bare bytes, and it cost us every invitation. A reader expecting + * the envelope reads a bare KeyPackage's leading `0x0001 0x0001` as + * version 1 / wire format 1 (`mls_public_message`), parses on as a + * PublicMessage, and dies several fields later on a byte that means nothing — + * MDK reported `UnknownValue(112)`, a number that appears nowhere in a + * KeyPackage. A four-byte envelope is not a detail when its absence is + * indistinguishable from a different message type. + */ +class KeyPackageFramingTest { + private fun aKeyPackage() = + MlsGroup + .create(identity = ByteArray(32) { 0x0a }) + .createKeyPackage(identity = ByteArray(32) { 0x0b }, signingKey = ByteArray(32) { 1 }) + .keyPackage + + @Test + fun framesAsAnMlsMessageWithTheKeyPackageWireFormat() { + val kp = aKeyPackage() + val framed = KeyPackageUtils.frameKeyPackage(kp) + + assertEquals(MlsMessage.MLS_VERSION_10, ((framed[0].toInt() and 0xFF) shl 8) or (framed[1].toInt() and 0xFF)) + assertEquals(WireFormat.KEY_PACKAGE.value, ((framed[2].toInt() and 0xFF) shl 8) or (framed[3].toInt() and 0xFF)) + assertEquals(kp.toTlsBytes().size + 4, framed.size) + } + + @Test + fun decodesItsOwnFraming() { + val kp = aKeyPackage() + val decoded = KeyPackageUtils.decodeKeyPackage(KeyPackageUtils.frameKeyPackage(kp)) + assertContentEquals(kp.toTlsBytes(), decoded.toTlsBytes()) + } + + /** + * Bare bytes still decode. Every KeyPackage this client published before + * the fix is bare and still inside its publication lifetime; refusing them + * would leave our own users unable to invite each other until all of them + * rotated. + */ + @Test + fun stillDecodesTheBareLegacyForm() { + val kp = aKeyPackage() + val decoded = KeyPackageUtils.decodeKeyPackage(kp.toTlsBytes()) + assertContentEquals(kp.toTlsBytes(), decoded.toTlsBytes()) + } + + /** + * The two forms are told apart by the envelope, not by trial and error: a + * bare KeyPackage's second field is its ciphersuite, which is never 5. + */ + @Test + fun tellsTheFormsApartByTheEnvelopeNotByGuessing() { + val kp = aKeyPackage() + val bare = kp.toTlsBytes() + assertEquals(MlsMessage.MLS_VERSION_10, ((bare[0].toInt() and 0xFF) shl 8) or (bare[1].toInt() and 0xFF)) + val suite = ((bare[2].toInt() and 0xFF) shl 8) or (bare[3].toInt() and 0xFF) + assertTrue(suite != WireFormat.KEY_PACKAGE.value, "a real ciphersuite must not collide with the wire format") + } + + /** + * `KeyPackageRef` is computed over the INNER KeyPackage, never the + * envelope — framing it too would make our `i` tag disagree with every + * other implementation's. + */ + @Test + fun theReferenceIsOverTheInnerKeyPackage() { + val kp = aKeyPackage() + val framed = KeyPackageUtils.frameKeyPackage(kp) + assertContentEquals(kp.reference(), KeyPackageUtils.decodeKeyPackage(framed).reference()) + } +}