From 1569b456715bb8056e939d73ec3a78eb21bff5ea Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 20 Apr 2026 17:06:51 +0000 Subject: [PATCH] fix: align Marmot implementation with MIP specs for interop with MDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings Amethyst's Marmot code into wire-level interoperability with the reference MDK (Rust) implementation by resolving four spec violations and tightening the epoch lookback window: - MIP-01 (NostrGroupData extension): switch to QUIC-style VarInt length prefixes (RFC 9000 §16) instead of fixed uint16. MIP-01 mandates this encoding (the one produced by the Rust `tls_codec` crate v0.4+), so Amethyst's previous uint16 framing could not round-trip with any MDK peer. admin_pubkeys, relays (outer + each inner), name, description, image_*, and disappearing_message_secs now all use VarInt prefixes. - MIP-01: enforce "admin_pubkeys MUST NOT contain duplicates" in the MarmotGroupData constructor. - MIP-05 (Push Notifications): token plaintext padded size was 220 (→ 280 encrypted); spec MUSTs are exactly 1024 bytes plaintext (1084 encrypted). A server expecting MIP-05 tokens would reject Amethyst's frames outright. - MIP-05: HKDF IKM was sha256(shared_point); spec requires the raw 32-byte ECDH x-coordinate. The extra sha256 produced a different PRK, so token ciphertext authenticated only within Amethyst. Removed the hash; ECDH already returns the x-only compact form via pubKeyTweakMulCompact. - MIP-03: bump EPOCH_RETENTION_WINDOW from 2 to 5 to match MDK's DEFAULT_EPOCH_LOOKBACK, so late-arriving GroupEvents whose ChaCha20 outer key was derived from a prior epoch's exporter secret can still be decrypted after a Commit advances the group. Hand-crafted MarmotGroupData test blobs were updated to emit VarInt length prefixes. --- .../marmot/mip01Groups/MarmotGroupData.kt | 82 +++++++++++-------- .../mip05PushNotifications/TokenEncryption.kt | 37 ++++----- .../mip05PushNotifications/tags/TokenTag.kt | 9 +- .../marmot/mls/group/MlsGroupManager.kt | 13 ++- .../quartz/marmot/MarmotMipComplianceTest.kt | 24 +++--- 5 files changed, 92 insertions(+), 73 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip01Groups/MarmotGroupData.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip01Groups/MarmotGroupData.kt index e326bbcf3..534bebd8c 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip01Groups/MarmotGroupData.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip01Groups/MarmotGroupData.kt @@ -100,6 +100,9 @@ data class MarmotGroupData( require(disappearingMessageSecs == null || disappearingMessageSecs > 0UL) { "disappearing_message_secs must be > 0 when set (MIP-01)" } + require(adminPubkeys.size == adminPubkeys.toSet().size) { + "MarmotGroupData.admin_pubkeys MUST NOT contain duplicates (MIP-01)" + } } /** Whether the given pubkey is an admin of this group */ @@ -111,33 +114,41 @@ data class MarmotGroupData( /** * Encode this MarmotGroupData to TLS wire format bytes. * Mirrors the [decodeTls] format. + * + * Per MIP-01, all variable-length vectors use QUIC-style variable-length integer + * (VarInt) length prefixes, as implemented by the Rust `tls_codec` crate (v0.4+): + * - lengths 0..63 → 1 byte (high bits 00) + * - lengths 64..16383 → 2 bytes (high bits 01) + * - lengths 16384+ → 4 bytes (high bits 10) */ fun encodeTls(): ByteArray { val writer = TlsWriter() writer.putUint16(version) writer.putBytes(nostrGroupId.hexToByteArray()) - writer.putOpaque2(name.encodeToByteArray()) - writer.putOpaque2(description.encodeToByteArray()) + writer.putOpaqueVarInt(name.encodeToByteArray()) + writer.putOpaqueVarInt(description.encodeToByteArray()) - // Admin pubkeys: concatenated 32-byte keys within a length-prefixed block + // admin_pubkeys: Vec<[u8;32]> — outer VarInt covers total bytes, each 32-byte + // key is fixed-size with no inner length prefix. val adminBytes = ByteArray(adminPubkeys.size * 32) adminPubkeys.forEachIndexed { index, key -> key.hexToByteArray().copyInto(adminBytes, index * 32) } - writer.putOpaque2(adminBytes) + writer.putOpaqueVarInt(adminBytes) - // Relays: length-prefixed block of length-prefixed UTF-8 strings + // relays: Vec> — outer VarInt covers total bytes, each inner relay + // string is VarInt-length-prefixed UTF-8. val relayWriter = TlsWriter() for (relay in relays) { - relayWriter.putOpaque2(relay.encodeToByteArray()) + relayWriter.putOpaqueVarInt(relay.encodeToByteArray()) } - writer.putOpaque2(relayWriter.toByteArray()) + writer.putOpaqueVarInt(relayWriter.toByteArray()) - // Optional image fields - writer.putOpaque2(imageHash?.hexToByteArray() ?: ByteArray(0)) - writer.putOpaque2(imageKey ?: ByteArray(0)) - writer.putOpaque2(imageNonce ?: ByteArray(0)) - writer.putOpaque2(imageUploadKey ?: ByteArray(0)) + // Optional image fields — empty Vec encodes as a single zero byte (VarInt(0)). + writer.putOpaqueVarInt(imageHash?.hexToByteArray() ?: ByteArray(0)) + writer.putOpaqueVarInt(imageKey ?: ByteArray(0)) + 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 = @@ -150,7 +161,7 @@ data class MarmotGroupData( } out } ?: ByteArray(0) - writer.putOpaque2(disappearingBytes) + writer.putOpaqueVarInt(disappearingBytes) return writer.toByteArray() } @@ -182,19 +193,22 @@ data class MarmotGroupData( /** * Decode MarmotGroupData from TLS wire format bytes. * + * Per MIP-01, all variable-length vectors use QUIC-style VarInt length prefixes + * (`tls_codec` v0.4+). The TLS comment syntax below uses `` to denote VarInt. + * * Wire format (v3): * ``` * uint16 version // rejected if 0 or unsupported * opaque nostr_group_id[32] - * opaque name<0..2^16-1> - * opaque description<0..2^16-1> - * opaque admin_pubkeys<0..2^16-1> // concatenated 32-byte keys - * RelayUrl relays<0..2^16-1> // length-prefixed UTF-8 strings - * opaque image_hash<0..32> - * opaque image_key<0..32> - * opaque image_nonce<0..12> - * opaque image_upload_key<0..32> - * opaque disappearing_message_secs<0..8> // v3+: 0 bytes or 8-byte uint64 (reject 0) + * opaque name + * opaque description + * opaque admin_pubkeys // concatenated 32-byte keys + * RelayUrl relays // VarInt-length-prefixed UTF-8 strings + * opaque image_hash + * opaque image_key + * opaque image_nonce + * opaque image_upload_key + * opaque disappearing_message_secs // v3+: 0 bytes or 8-byte uint64 (reject 0) * ``` * * Unknown trailing bytes from future versions are silently ignored for @@ -209,14 +223,14 @@ data class MarmotGroupData( val nostrGroupIdBytes = reader.readBytes(32) val nostrGroupId = nostrGroupIdBytes.toHexKey() - val nameBytes = reader.readOpaque2() + val nameBytes = reader.readOpaqueVarInt() val name = nameBytes.decodeToString() - val descriptionBytes = reader.readOpaque2() + val descriptionBytes = reader.readOpaqueVarInt() val description = descriptionBytes.decodeToString() - // Admin pubkeys: concatenated 32-byte keys within a length-prefixed block - val adminBlock = reader.readOpaque2() + // Admin pubkeys: concatenated 32-byte keys within a VarInt-prefixed block + val adminBlock = reader.readOpaqueVarInt() val adminPubkeys = mutableListOf() var i = 0 while (i + 32 <= adminBlock.size) { @@ -224,23 +238,23 @@ data class MarmotGroupData( i += 32 } - // Relays: length-prefixed block of length-prefixed UTF-8 strings - val relaysBlock = reader.readOpaque2() + // Relays: VarInt-prefixed block of VarInt-prefixed UTF-8 strings + val relaysBlock = reader.readOpaqueVarInt() val relays = mutableListOf() val relayReader = TlsReader(relaysBlock) while (relayReader.hasRemaining) { - val relayBytes = relayReader.readOpaque2() + val relayBytes = relayReader.readOpaqueVarInt() relays.add(relayBytes.decodeToString()) } // Optional fields — read if remaining - val imageHash = if (reader.hasRemaining) reader.readOpaque2().takeIf { it.isNotEmpty() }?.toHexKey() else null - val imageKey = if (reader.hasRemaining) reader.readOpaque2().takeIf { it.isNotEmpty() } else null - val imageNonce = if (reader.hasRemaining) reader.readOpaque2().takeIf { it.isNotEmpty() } else null - val imageUploadKey = if (reader.hasRemaining) reader.readOpaque2().takeIf { it.isNotEmpty() } else null + val imageHash = if (reader.hasRemaining) reader.readOpaqueVarInt().takeIf { it.isNotEmpty() }?.toHexKey() else null + val imageKey = if (reader.hasRemaining) reader.readOpaqueVarInt().takeIf { it.isNotEmpty() } else null + val imageNonce = if (reader.hasRemaining) reader.readOpaqueVarInt().takeIf { it.isNotEmpty() } else null + val imageUploadKey = if (reader.hasRemaining) reader.readOpaqueVarInt().takeIf { it.isNotEmpty() } else null // v3+: disappearing_message_secs - val disappearingBytes = if (reader.hasRemaining) reader.readOpaque2() else ByteArray(0) + val disappearingBytes = if (reader.hasRemaining) reader.readOpaqueVarInt() else ByteArray(0) val disappearingMessageSecs: ULong? = when (disappearingBytes.size) { 0 -> { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/TokenEncryption.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/TokenEncryption.kt index fcc9284e8..9fdd5c6eb 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/TokenEncryption.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/TokenEncryption.kt @@ -25,30 +25,29 @@ import com.vitorpamplona.quartz.nip44Encryption.crypto.ChaCha20Poly1305 import com.vitorpamplona.quartz.utils.RandomInstance import com.vitorpamplona.quartz.utils.Secp256k1Instance import com.vitorpamplona.quartz.utils.mac.MacInstance -import com.vitorpamplona.quartz.utils.sha256.sha256 import kotlin.io.encoding.Base64 import kotlin.io.encoding.ExperimentalEncodingApi /** * Handles EncryptedToken creation and decryption for Marmot push notifications (MIP-05). * - * EncryptedToken format (280 bytes total): - * ephemeral_pubkey(32) || nonce(12) || ciphertext(236 = 220 plaintext + 16 tag) + * EncryptedToken format (MUST be exactly 1084 bytes per MIP-05): + * ephemeral_pubkey(32) || nonce(12) || ciphertext(1040 = 1024 plaintext + 16 tag) * - * Token payload (220 bytes, padded): - * platform(1) || token_length(2 BE) || device_token(N) || random_padding(220-3-N) + * Token plaintext (MUST be exactly 1024 bytes per MIP-05): + * platform(1) || token_length(2 BE) || device_token(N) || random_padding(1024-3-N) * - * Key derivation: - * 1. ECDH: shared_point = ephemeral_privkey * server_pubkey - * 2. shared_x = sha256(shared_point) (x-coordinate as shared secret) - * 3. PRK = HKDF-Extract(salt="mip05-v1", IKM=shared_x) - * 4. encryption_key = HKDF-Expand(PRK, info="mip05-token-encryption", 32) - * 5. Encrypt padded payload with ChaCha20-Poly1305(key, nonce, payload, aad="") + * Key derivation (MIP-05 §"Key Derivation"): + * 1. ECDH: shared_x = secp256k1_ecdh(ephemeral_privkey, server_pubkey) — raw 32-byte x + * 2. PRK = HKDF-Extract(salt="mip05-v1", IKM=shared_x) + * 3. encryption_key = HKDF-Expand(PRK, info="mip05-token-encryption", 32) + * 4. Encrypt padded plaintext with ChaCha20-Poly1305(key, nonce, plaintext, aad="") * * Platform values: 0x01 = APNs, 0x02 = FCM */ object TokenEncryption { - private const val PADDED_PAYLOAD_SIZE = 220 + /** Token plaintext MUST be exactly 1024 bytes per MIP-05. */ + private const val PADDED_PAYLOAD_SIZE = 1024 private const val NONCE_SIZE = 12 private const val PUBKEY_SIZE = 32 private const val HEADER_SIZE = 3 // platform(1) + token_length(2) @@ -97,9 +96,9 @@ object TokenEncryption { // Extract the 32-byte x-only public key by dropping the SEC1 prefix byte val ephemeralPubKey = compressedPubKey.copyOfRange(1, 33) - // ECDH: shared_x = sha256(ephemeral_privkey * server_pubkey) - val sharedPoint = Secp256k1Instance.pubKeyTweakMulCompact(serverPubKey, ephemeralPrivKey) - val sharedX = sha256(sharedPoint) + // ECDH: shared_x = secp256k1_ecdh(ephemeral_privkey, server_pubkey) — raw 32-byte x + // per MIP-05. Do NOT hash; HKDF-Extract will mix the salt. + val sharedX = Secp256k1Instance.pubKeyTweakMulCompact(serverPubKey, ephemeralPrivKey) // HKDF-Extract then Expand to get encryption key val encryptionKey = hkdfDeriveKey(sharedX) @@ -108,7 +107,7 @@ object TokenEncryption { val nonce = RandomInstance.bytes(NONCE_SIZE) val ciphertextWithTag = ChaCha20Poly1305.encrypt(payload, EMPTY_AAD, nonce, encryptionKey) - // Assemble: ephemeral_pubkey(32) || nonce(12) || ciphertext+tag(236) + // Assemble: ephemeral_pubkey(32) || nonce(12) || ciphertext+tag(1040) val result = ByteArray(TokenTag.ENCRYPTED_TOKEN_SIZE) ephemeralPubKey.copyInto(result, 0) nonce.copyInto(result, PUBKEY_SIZE) @@ -140,9 +139,9 @@ object TokenEncryption { val nonce = data.copyOfRange(PUBKEY_SIZE, PUBKEY_SIZE + NONCE_SIZE) val ciphertextWithTag = data.copyOfRange(PUBKEY_SIZE + NONCE_SIZE, data.size) - // ECDH: shared_x = sha256(server_privkey * ephemeral_pubkey) - val sharedPoint = Secp256k1Instance.pubKeyTweakMulCompact(ephemeralPubKey, serverPrivKey) - val sharedX = sha256(sharedPoint) + // ECDH: shared_x = secp256k1_ecdh(server_privkey, ephemeral_pubkey) — raw 32-byte x + // per MIP-05. + val sharedX = Secp256k1Instance.pubKeyTweakMulCompact(ephemeralPubKey, serverPrivKey) // Derive encryption key val encryptionKey = hkdfDeriveKey(sharedX) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/tags/TokenTag.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/tags/TokenTag.kt index 6eca9b6f8..5edbadf60 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/tags/TokenTag.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mip05PushNotifications/tags/TokenTag.kt @@ -38,7 +38,7 @@ import com.vitorpamplona.quartz.utils.ensure */ @Immutable data class TokenTagData( - /** Base64-encoded EncryptedToken (280 bytes when decoded) */ + /** Base64-encoded EncryptedToken (1084 bytes when decoded per MIP-05) */ val encryptedToken: String, /** Hex-encoded notification server public key */ val serverPubKey: HexKey, @@ -52,8 +52,11 @@ class TokenTag { companion object { const val TAG_NAME = "token" - /** Expected decoded size of an EncryptedToken */ - const val ENCRYPTED_TOKEN_SIZE = 280 + /** + * Expected decoded size of an EncryptedToken per MIP-05: + * ephemeral_pubkey(32) || nonce(12) || ciphertext(1024 + 16 tag) = 1084 bytes. + */ + const val ENCRYPTED_TOKEN_SIZE = 1084 fun parse(tag: Array): TokenTagData? { ensure(tag.has(3) && tag[0] == TAG_NAME) { return null } 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 422fd0ad2..bdfec1984 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 @@ -79,8 +79,11 @@ import kotlinx.coroutines.sync.withLock * ## Cross-Implementation Notes * * This manager uses ciphersuite 0x0001 (MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519). - * The epoch secret retention window is [EPOCH_RETENTION_WINDOW] = 2, meaning secrets - * for the current and previous epoch are kept for late-message decryption. + * The epoch secret retention window is [EPOCH_RETENTION_WINDOW] = 5, matching the + * `DEFAULT_EPOCH_LOOKBACK` used by the MDK reference implementation so that late- + * arriving MIP-03 GroupEvents (and MLS application messages) whose outer ChaCha20- + * Poly1305 key was derived from a prior epoch's exporter secret can still be + * decrypted after a Commit advances the group. * * Thread safety: All suspending mutation methods are guarded by a [Mutex] * to prevent concurrent state corruption. Non-suspending read methods @@ -662,9 +665,11 @@ class MlsGroupManager( /** * Number of past epochs to retain for late-arriving message decryption. - * MLS forward secrecy guarantees mean we want to limit this window. + * Matches MDK's `DEFAULT_EPOCH_LOOKBACK` so a message encrypted under + * the prior N epochs' exporter secrets can still be decrypted after a + * Commit advances the group. Capped for forward-secrecy reasons. */ - const val EPOCH_RETENTION_WINDOW = 2 + const val EPOCH_RETENTION_WINDOW = 5 /** Size of reuse_guard in PrivateMessage (RFC 9420 §6.3.1) */ private const val REUSE_GUARD_LENGTH = 4 diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipComplianceTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipComplianceTest.kt index f5c7be6c0..a323e7f5b 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipComplianceTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotMipComplianceTest.kt @@ -115,27 +115,24 @@ class MarmotMipComplianceTest { @Test fun marmotGroupData_rejectsZeroDisappearingSecsOnDecode() { - // Hand-crafted TLS blob: version=3, group_id=32x0, empty opaque2 for - // name/description/admins/relays/images, then disappearing_message_secs - // = 8 bytes of zero (invalid). + // Hand-crafted TLS blob (MIP-01 QUIC VarInt length prefixes): + // uint16 version=3 | opaque group_id[32] | 8x empty VarInt(0) fields + // (name..image_upload_key) | disappearing_message_secs = VarInt(8) + 8 + // zero bytes (invalid per MIP-01). val header = ByteArray(2 + 32) { - // version + groupId when (it) { 0 -> 0 - 1 -> 3 - - // version=3 else -> 0 } } - // 8x opaque2 fields of length 0, each encoded as two zero bytes: + // 8 empty VarInt-prefixed opaque fields, each a single 0x00 byte: // name, description, admin_pubkeys, relays, image_hash, image_key, // image_nonce, image_upload_key - val zeroFields = ByteArray(8 * 2) // all zeros - // disappearing_message_secs opaque2 with 8 zero bytes - val disappearingField = ByteArray(2 + 8).also { it[1] = 8 } + val zeroFields = ByteArray(8) // all 0x00 + // disappearing_message_secs: VarInt(8) = 0x08, then 8 zero bytes + val disappearingField = ByteArray(1 + 8).also { it[0] = 0x08 } val blob = header + zeroFields + disappearingField // decodeTls catches any exception and returns null @@ -150,8 +147,9 @@ class MarmotMipComplianceTest { it[0] = 0 it[1] = 99 } - val zeroFields = ByteArray(8 * 2) // name..image_upload_key - val disappearingField = ByteArray(2) // zero-length + val zeroFields = ByteArray(8) // 8x VarInt(0) for name..image_upload_key + val disappearingField = ByteArray(1) // VarInt(0) — zero-length field + val blob = header + zeroFields + disappearingField assertNull(MarmotGroupData.decodeTls(blob))