fix: tighten Marmot MIP-00/01/02/05 + RFC 9420 PublicMessage framing

Follow-up to the earlier MIP-spec alignment commit, addressing the
remaining audit items:

- MIP-00 (KeyPackageUtils.isValid): now performs every strict tag-level
  check that MDK does — d tag is exactly 64 hex chars, mls_protocol_version
  is exactly "1.0", mls_ciphersuite is exactly "0x0001", mls_extensions
  contains both 0xf2ee + 0x000a, mls_proposals contains 0x000a, encoding
  is "base64", content non-empty, i tag present. Adds
  `isCryptographicallyValid` for deep checks: i tag matches the computed
  KeyPackageRef (RFC 9420 §5.2), Basic credential identity equals the
  event's pubkey, KeyPackage signature verifies.

- MIP-01 (MarmotGroupData): encoder now omits the v3-only
  `disappearing_message_secs` field when version < 3, so v2 output
  matches MDK's `tls_codec` 0.4 byte-for-byte. Adds byte-level fixture
  tests pinned to the Rust reference (encode + decode in both directions).

- MIP-02 (WelcomeGiftWrap): replaces the redundant sign-then-rumorize
  step with `RumorAssembler.assembleRumor`, producing a true unsigned
  rumor as NIP-59 requires (no wasted secp256k1 signature).

- MIP-05 kind 449 (TokenRemovalEvent.build): no longer accepts a tag
  initializer. Per the MIP-05 spec the token-removal event MUST have
  empty tags; allowing arbitrary caller-supplied tags risks metadata
  leakage and rejection by strict validators (e.g. the MDK reference).

- RFC 9420 §6 PublicMessage framing: encoder/decoder now branch on
  `contentType`. Application messages keep the `opaque<V>` length prefix;
  Proposal/Commit bodies are emitted/parsed as their typed structs (no
  outer length prefix), per the MLS RFC. Previously encoder used
  `putBytes` raw and decoder used `readOpaqueVarInt`, so neither could
  roundtrip and Proposal/Commit PublicMessages from other MLS clients
  would mis-parse.

- KeyPackageUtilsTest fixtures: switch tiny "0"/"1"/"lr" slot ids to
  realistic 64-char hex slot ids, since `isValid` now enforces the
  MIP-00 d-tag format.
This commit is contained in:
Claude
2026-04-20 18:02:12 +00:00
parent 1569b45671
commit 77c3634745
7 changed files with 296 additions and 48 deletions
@@ -21,12 +21,19 @@
package com.vitorpamplona.quartz.marmot.mip00KeyPackages
import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.EncodingTag
import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.MlsCiphersuiteTag
import com.vitorpamplona.quartz.marmot.mip00KeyPackages.tags.MlsProtocolVersionTag
import com.vitorpamplona.quartz.marmot.mls.codec.TlsReader
import com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider
import com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage
import com.vitorpamplona.quartz.marmot.mls.tree.Credential
import com.vitorpamplona.quartz.nip01Core.core.Event
import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import com.vitorpamplona.quartz.nip01Core.signers.EventTemplate
import kotlin.io.encoding.Base64
import kotlin.io.encoding.ExperimentalEncodingApi
/**
* Utility functions for KeyPackage lifecycle management (MIP-00).
@@ -88,12 +95,91 @@ object KeyPackageUtils {
/**
* Validates a KeyPackage event has required fields and proper encoding.
*
* Performs the same strict tag-level MIP-00 checks used by MDK so that
* malformed or adversarial events are rejected at parse time:
* - `d` tag is exactly 64 lowercase hex characters (32-byte slot ID)
* - `mls_protocol_version` is exactly "1.0"
* - `mls_ciphersuite` is exactly "0x0001"
* - `mls_extensions` contains both "0xf2ee" (NostrGroupData) and
* "0x000a" (LastResort)
* - `mls_proposals` contains "0x000a" (SelfRemove)
* - `encoding` is "base64" and content is non-empty
* - `i` (keyPackageRef) tag is non-empty
*
* For deep cryptographic checks (KeyPackageRef hash match, credential
* identity == event.pubkey) call [isCryptographicallyValid].
*/
fun isValid(event: KeyPackageEvent): Boolean =
event.encoding() == EncodingTag.BASE64 &&
event.content.isNotEmpty() &&
!event.keyPackageRef().isNullOrEmpty() &&
!event.mlsCiphersuite().isNullOrEmpty()
fun isValid(event: KeyPackageEvent): Boolean {
// d tag: exactly 64 hex chars per MIP-00
val dTag = event.dTag()
if (dTag.length != 64 || !dTag.all { it.isHexChar() }) return false
// mls_protocol_version == "1.0"
if (event.mlsProtocolVersion() != MlsProtocolVersionTag.CURRENT_VERSION) return false
// mls_ciphersuite == "0x0001"
if (event.mlsCiphersuite() != MlsCiphersuiteTag.DEFAULT_CIPHERSUITE) return false
// mls_extensions MUST include both 0xf2ee and 0x000a
val extensions = event.mlsExtensions()?.map { it.lowercase() }?.toSet() ?: return false
if (!extensions.contains("0xf2ee") || !extensions.contains("0x000a")) return false
// mls_proposals MUST include 0x000a (SelfRemove)
val proposals = event.mlsProposals()?.map { it.lowercase() }?.toSet() ?: return false
if (!proposals.contains("0x000a")) return false
// encoding MUST be base64 and content non-empty
if (event.encoding() != EncodingTag.BASE64) return false
if (event.content.isEmpty()) return false
// i (KeyPackageRef) tag MUST be present
if (event.keyPackageRef().isNullOrEmpty()) return false
return true
}
/**
* Deep MIP-00 validation: decodes the KeyPackage content and verifies:
* - `i` tag matches the computed `KeyPackageRef` (RFC 9420 §5.2)
* - Credential identity (BasicCredential) equals the event's `pubkey`
* (32-byte x-only Nostr pubkey)
* - KeyPackage signature over KeyPackageTBS is valid
*
* Returns true only if [isValid] also holds and every cryptographic check
* passes. Requires [isValid] to be true as a precondition — it is called
* internally.
*/
@OptIn(ExperimentalEncodingApi::class)
fun isCryptographicallyValid(event: KeyPackageEvent): Boolean {
if (!isValid(event)) return false
val iTag = event.keyPackageRef() ?: return false
val keyPackage =
try {
val bytes = Base64.decode(event.content)
MlsKeyPackage.decodeTls(TlsReader(bytes))
} catch (_: Throwable) {
return false
}
// i tag MUST equal the computed KeyPackageRef
if (keyPackage.reference().toHexKey() != iTag.lowercase()) return false
// Credential identity MUST equal the event's pubkey (32-byte x-only).
// MIP-00 requires BasicCredential with the raw 32-byte Nostr pubkey.
val credential = keyPackage.leafNode.credential
if (credential !is Credential.Basic) return false
if (credential.identity.size != 32) return false
if (credential.identity.toHexKey().lowercase() != event.pubKey.lowercase()) return false
// KeyPackage signature MUST verify against the LeafNode's signatureKey.
if (!keyPackage.verifySignature()) return false
return true
}
private fun Char.isHexChar(): Boolean = this in '0'..'9' || this in 'a'..'f' || this in 'A'..'F'
/**
* Builds a rotated KeyPackage for the same d-tag slot.
@@ -150,18 +150,22 @@ data class MarmotGroupData(
writer.putOpaqueVarInt(imageNonce ?: ByteArray(0))
writer.putOpaqueVarInt(imageUploadKey ?: ByteArray(0))
// v3+: disappearing_message_secs (0 bytes = none, 8 bytes big-endian uint64 = secs)
val disappearingBytes =
disappearingMessageSecs?.let { secs ->
val out = ByteArray(8)
var v = secs.toLong()
for (i in 7 downTo 0) {
out[i] = (v and 0xFF).toByte()
v = v ushr 8
}
out
} ?: ByteArray(0)
writer.putOpaqueVarInt(disappearingBytes)
// v3+: disappearing_message_secs (0 bytes = none, 8 bytes big-endian uint64 = secs).
// Only emitted for version ≥ 3; v1/v2 have no such field, so omitting it keeps
// the wire format byte-for-byte compatible with older implementations (MDK v2).
if (version >= 3) {
val disappearingBytes =
disappearingMessageSecs?.let { secs ->
val out = ByteArray(8)
var v = secs.toLong()
for (i in 7 downTo 0) {
out[i] = (v and 0xFF).toByte()
v = v ushr 8
}
out
} ?: ByteArray(0)
writer.putOpaqueVarInt(disappearingBytes)
}
return writer.toByteArray()
}
@@ -24,6 +24,7 @@ import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner
import com.vitorpamplona.quartz.nip59Giftwrap.rumors.Rumor
import com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler
import com.vitorpamplona.quartz.nip59Giftwrap.seals.SealedRumorEvent
import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent
import com.vitorpamplona.quartz.utils.TimeUtils
@@ -60,7 +61,9 @@ object WelcomeGiftWrap {
nostrGroupId: HexKey? = null,
createdAt: Long = TimeUtils.now(),
): GiftWrapEvent {
// Step 1: Build the WelcomeEvent template and sign it
// Step 1: Build the WelcomeEvent template directly as an unsigned rumor.
// Per NIP-59 rumors MUST have an empty sig field, so we skip the outer
// signature entirely and let the SealedRumorEvent carry authorship.
val welcomeTemplate =
WelcomeEvent.build(
welcomeBase64 = welcomeBase64,
@@ -69,10 +72,14 @@ object WelcomeGiftWrap {
nostrGroupId = nostrGroupId,
createdAt = createdAt,
)
val welcomeEvent: WelcomeEvent = signer.sign(welcomeTemplate)
val welcomeRumor: WelcomeEvent =
RumorAssembler.assembleRumor(
pubKey = signer.pubKey,
ev = welcomeTemplate,
)
// Step 2: Create a Rumor from the signed event and seal it (kind:13)
val rumor = Rumor.create(welcomeEvent)
// Step 2: Create a Rumor from the unsigned event and seal it (kind:13)
val rumor = Rumor.create(welcomeRumor)
val sealedRumor =
SealedRumorEvent.create(
rumor = rumor,
@@ -23,7 +23,6 @@ package com.vitorpamplona.quartz.marmot.mip05PushNotifications
import androidx.compose.runtime.Immutable
import com.vitorpamplona.quartz.nip01Core.core.Event
import com.vitorpamplona.quartz.nip01Core.core.HexKey
import com.vitorpamplona.quartz.nip01Core.core.TagArrayBuilder
import com.vitorpamplona.quartz.nip01Core.signers.eventTemplate
import com.vitorpamplona.quartz.utils.TimeUtils
@@ -33,8 +32,10 @@ import com.vitorpamplona.quartz.utils.TimeUtils
* Unsigned application message sent inside a GroupEvent (kind:445) when a device
* leaves a group or wants to disable push notifications.
*
* Has no tags — the MLS leaf index is implicit from the MLS sender identity.
* Receiving clients MUST remove the token for the identified leaf.
* Per MIP-05 this event MUST have **no tags**. The MLS leaf index is implicit
* from the MLS sender identity; receiving clients MUST remove the token for
* the identified leaf. Adding extra tags could leak metadata or be rejected
* by strict MIP-05 validators (e.g. the MDK reference).
*
* MUST remain unsigned (no sig field) per MIP-03 security requirements.
*/
@@ -50,11 +51,6 @@ class TokenRemovalEvent(
companion object {
const val KIND = 449
fun build(
createdAt: Long = TimeUtils.now(),
initializer: TagArrayBuilder<TokenRemovalEvent>.() -> Unit = {},
) = eventTemplate(KIND, "", createdAt) {
initializer()
}
fun build(createdAt: Long = TimeUtils.now()) = eventTemplate<TokenRemovalEvent>(KIND, "", createdAt)
}
}
@@ -23,6 +23,8 @@ package com.vitorpamplona.quartz.marmot.mls.framing
import com.vitorpamplona.quartz.marmot.mls.codec.TlsReader
import com.vitorpamplona.quartz.marmot.mls.codec.TlsSerializable
import com.vitorpamplona.quartz.marmot.mls.codec.TlsWriter
import com.vitorpamplona.quartz.marmot.mls.messages.Commit
import com.vitorpamplona.quartz.marmot.mls.messages.Proposal
/**
* MLS MLSMessage (RFC 9420 Section 6).
@@ -161,7 +163,17 @@ data class PublicMessage(
encodeSender(writer, sender)
writer.putOpaqueVarInt(authenticatedData)
writer.putUint8(contentType.value)
writer.putBytes(content)
// RFC 9420 §6 FramedContent.content is a type-dependent body:
// case application: opaque application_data<V>
// case proposal: Proposal proposal (struct, no outer length prefix)
// case commit: Commit commit (struct, no outer length prefix)
// `content` holds the already-serialized body for proposal/commit, and
// the raw application bytes for application.
when (contentType) {
ContentType.APPLICATION -> writer.putOpaqueVarInt(content)
ContentType.PROPOSAL, ContentType.COMMIT -> writer.putBytes(content)
}
// FramedContentAuthData
writer.putOpaqueVarInt(signature)
@@ -191,9 +203,30 @@ data class PublicMessage(
val authenticatedData = reader.readOpaqueVarInt()
val contentType = ContentType.fromValue(reader.readUint8())
// Content is variable based on content_type, read remaining content
// For now, read as opaque
val content = reader.readOpaqueVarInt()
// RFC 9420 §6 FramedContent.content body varies by content_type.
// For PROPOSAL/COMMIT we decode the inner struct to advance the reader
// and then re-serialize back to bytes so the invariant
// "content holds the serialized body" holds for all variants.
val content: ByteArray =
when (contentType) {
ContentType.APPLICATION -> {
reader.readOpaqueVarInt()
}
ContentType.PROPOSAL -> {
val proposal = Proposal.decodeTls(reader)
val w = TlsWriter()
proposal.encodeTls(w)
w.toByteArray()
}
ContentType.COMMIT -> {
val commit = Commit.decodeTls(reader)
val w = TlsWriter()
commit.encodeTls(w)
w.toByteArray()
}
}
val signature = reader.readOpaqueVarInt()
val confirmationTag =
@@ -37,8 +37,20 @@ class KeyPackageUtilsTest {
private val testPubKey = "a".repeat(64)
private val testRef = "b".repeat(64)
/**
* Per MIP-00 the `d` tag MUST be 64 lowercase hex characters
* (a random 32-byte slot ID). The strict [KeyPackageUtils.isValid] enforces
* this on the parse side, so test fixtures need realistic 64-char hex
* slot IDs even when we only care about the selection logic.
*/
private fun slot(label: Int): String = "0".repeat(63) + label.toString(16)
private val slot0 = slot(0)
private val slot1 = slot(1)
private val slotLastResort = "f".repeat(64)
private fun makeKeyPackageEvent(
dTag: String = "0",
dTag: String = slot0,
createdAt: Long = 1000,
encoding: String = "base64",
ciphersuite: String = "0x0001",
@@ -105,8 +117,8 @@ class KeyPackageUtilsTest {
@Test
fun testSelectBest_PrefersNewest() {
val old = makeKeyPackageEvent(dTag = "0", createdAt = 1000)
val newer = makeKeyPackageEvent(dTag = "1", createdAt = 2000)
val old = makeKeyPackageEvent(dTag = slot0, createdAt = 1000)
val newer = makeKeyPackageEvent(dTag = slot1, createdAt = 2000)
val best = KeyPackageUtils.selectBest(listOf(old, newer))
assertNotNull(best)
@@ -115,33 +127,33 @@ class KeyPackageUtilsTest {
@Test
fun testSelectBest_PrefersNonLastResort() {
val lastResort = makeKeyPackageEvent(dTag = "lr", createdAt = 3000)
val regular = makeKeyPackageEvent(dTag = "0", createdAt = 1000)
val lastResort = makeKeyPackageEvent(dTag = slotLastResort, createdAt = 3000)
val regular = makeKeyPackageEvent(dTag = slot0, createdAt = 1000)
// Even though lastResort is newer, regular is preferred
val best = KeyPackageUtils.selectBest(listOf(lastResort, regular), lastResortDTag = "lr")
val best = KeyPackageUtils.selectBest(listOf(lastResort, regular), lastResortDTag = slotLastResort)
assertNotNull(best)
assertEquals("0", best.dTag())
assertEquals(slot0, best.dTag())
}
@Test
fun testSelectBest_FallsBackToLastResort() {
val lastResort = makeKeyPackageEvent(dTag = "lr", createdAt = 3000)
val lastResort = makeKeyPackageEvent(dTag = slotLastResort, createdAt = 3000)
// Only last-resort available
val best = KeyPackageUtils.selectBest(listOf(lastResort), lastResortDTag = "lr")
val best = KeyPackageUtils.selectBest(listOf(lastResort), lastResortDTag = slotLastResort)
assertNotNull(best)
assertEquals("lr", best.dTag())
assertEquals(slotLastResort, best.dTag())
}
@Test
fun testSelectBest_FiltersOutInvalid() {
val invalid = makeKeyPackageEvent(dTag = "0", encoding = "raw")
val valid = makeKeyPackageEvent(dTag = "1", createdAt = 500)
val invalid = makeKeyPackageEvent(dTag = slot0, encoding = "raw")
val valid = makeKeyPackageEvent(dTag = slot1, createdAt = 500)
val best = KeyPackageUtils.selectBest(listOf(invalid, valid))
assertNotNull(best)
assertEquals("1", best.dTag())
assertEquals(slot1, best.dTag())
}
@Test
@@ -159,7 +171,7 @@ class KeyPackageUtilsTest {
val template =
KeyPackageUtils.buildRotation(
newKeyPackageBase64 = "bmV3IGtleXBhY2thZ2U=",
dTagSlot = "0",
dTagSlot = slot0,
newKeyPackageRef = testRef,
relays = emptyList(),
)
@@ -155,6 +155,116 @@ class MarmotMipComplianceTest {
assertNull(MarmotGroupData.decodeTls(blob))
}
@Test
fun marmotGroupData_rejectsDuplicateAdminPubkeys() {
// MIP-01: admin_pubkeys MUST NOT contain duplicate keys.
assertFailsWith<IllegalArgumentException> {
MarmotGroupData(
nostrGroupId = groupId32,
adminPubkeys = listOf(adminPubkey, adminPubkey),
)
}
}
// --- MIP-01 byte-level interop fixtures (MDK reference) ----------------
//
// These fixtures were produced by serializing the identical struct via the
// Rust `tls_codec` 0.4 crate used by MDK (see commit message for the
// generator). They pin Amethyst's v2 encoder output byte-for-byte against
// the MDK reference, so any future regression in VarInt framing surfaces
// immediately.
//
// Fixtures are v2 (no `disappearing_message_secs` field) because MDK's
// current `CURRENT_VERSION = 2` reference has no v3 support yet.
private fun mdkFixtureA(): ByteArray =
(
// version=2 + 32 bytes of group_id (all zero)
"0002" +
"00".repeat(32) +
// name=empty, description=empty (VarInt(0) = single 0x00)
"0000" +
// admin_pubkeys: VarInt(32) = 0x20, then one 32-byte key of 0xAA
"20" + "aa".repeat(32) +
// relays: outer VarInt(21) = 0x15; inner VarInt(20) = 0x14 +
// "wss://relay.example/" (20 bytes)
"15" + "14" + "7773733a2f2f72656c61792e6578616d706c652f" +
// image_hash, image_key, image_nonce, image_upload_key — all empty
"00000000"
).hexToByteArray()
private fun mdkFixtureB(): ByteArray =
(
"0002" +
"11".repeat(32) +
// name: VarInt(8)=0x08 + "Amethyst"
"08" + "416d657468797374" +
// description: VarInt(10)=0x0a + "Test group"
"0a" + "546573742067726f7570" +
// admin_pubkeys: outer VarInt(64) — 64 = 0x40, two-byte VarInt
// prefix "40 40" (high bits 01, value 0x0040) + 2×32 bytes
"4040" + "bb".repeat(32) + "cc".repeat(32) +
// relays outer VarInt(44) = 0x2c, then two inner relays each
// VarInt(21) + 21-byte URL
"2c" +
"15" + "7773733a2f2f72656c6179312e6578616d706c652f" +
"15" + "7773733a2f2f72656c6179322e6578616d706c652f" +
// image_* all empty
"00000000"
).hexToByteArray()
@Test
fun marmotGroupData_encodesFixtureAByteForByteVsMdk() {
// Encode an Amethyst MarmotGroupData with the same inputs and assert the
// bytes match MDK's tls_codec 0.4 output exactly.
val data =
MarmotGroupData(
version = 2,
nostrGroupId = "00".repeat(32),
name = "",
description = "",
adminPubkeys = listOf("aa".repeat(32)),
relays = listOf("wss://relay.example/"),
)
assertContentEquals(mdkFixtureA(), data.encodeTls())
}
@Test
fun marmotGroupData_encodesFixtureBByteForByteVsMdk() {
val data =
MarmotGroupData(
version = 2,
nostrGroupId = "11".repeat(32),
name = "Amethyst",
description = "Test group",
adminPubkeys = listOf("bb".repeat(32), "cc".repeat(32)),
relays = listOf("wss://relay1.example/", "wss://relay2.example/"),
)
assertContentEquals(mdkFixtureB(), data.encodeTls())
}
@Test
fun marmotGroupData_decodesMdkFixtureA() {
val decoded = assertNotNull(MarmotGroupData.decodeTls(mdkFixtureA()))
assertEquals(2, decoded.version)
assertEquals("00".repeat(32), decoded.nostrGroupId)
assertEquals("", decoded.name)
assertEquals("", decoded.description)
assertEquals(listOf("aa".repeat(32)), decoded.adminPubkeys)
assertEquals(listOf("wss://relay.example/"), decoded.relays)
assertNull(decoded.disappearingMessageSecs)
}
@Test
fun marmotGroupData_decodesMdkFixtureB() {
val decoded = assertNotNull(MarmotGroupData.decodeTls(mdkFixtureB()))
assertEquals(2, decoded.version)
assertEquals("Amethyst", decoded.name)
assertEquals("Test group", decoded.description)
assertEquals(listOf("bb".repeat(32), "cc".repeat(32)), decoded.adminPubkeys)
assertEquals(listOf("wss://relay1.example/", "wss://relay2.example/"), decoded.relays)
}
@Test
fun mip01ImageCrypto_deriveImageKeyIs32BytesAndDeterministic() {
val seed = "11".repeat(32).hexToByteArray()