feat(marmot): accept PrivateMessage commits (handshake ratchet + dispatch)

MDK-core (openmls) defaults group configuration to `MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY` — outgoing commits / proposals ship as PrivateMessage. Quartz only handled PrivateMessage for ContentType.APPLICATION; any handshake message came in via the application ratchet, consumed the first application generation on whatever sender sent an app message next, and then died with `Generation 0 already consumed` the moment the receiver saw the real PrivateMessage commit. That's why every A-side processing of B's rename / promote / remove / leave commit timed out. Fix: 1. SecretTree grows a parallel `handshakeKeyNonceForGeneration` path, using the per-sender `handshakeSecret` / `handshakeGeneration` the struct already tracked — with its own skipped-keys cache and replay-detection set so handshake and application generations never clash. 2. `MlsGroup.decrypt()` dispatches on the PrivateMessage's `content_type` BEFORE consuming any ratchet — COMMIT and PROPOSAL take the handshake path; APPLICATION stays on the existing application path. 3. COMMIT bodies decode as `Commit || signature<V> || confirmation_tag<V> || padding` (RFC 9420 §6.3.1) and route into `processCommit` inline, so the caller just sees the epoch advance — no new public API. PROPOSAL still rejects (standalone proposals aren't used by Marmot flows yet) with a clear message instead of a cryptic generation-consumed error. https://claude.ai/code/session_016kAxdp6ubB5CnF9URhCEzP
2026-04-22 03:52:23 +00:00
parent 94c5e0e833
commit e9df0155c1
3 changed files with 173 additions and 46 deletions
@@ -874,13 +874,25 @@ class MlsGroup private constructor(
            "Sender leaf is blank at index $senderLeafIndex (not a group member)"
        }

-        // Get the key/nonce for this sender+generation
-        // If we sent this message ourselves, use the cached key to avoid ratchet conflict
+        // Get the key/nonce for this sender+generation from the correct ratchet.
+        // PrivateMessage Application content uses the application ratchet (RFC 9420
+        // §6.3.2); PrivateMessage Commit / Proposal handshakes use a separate
+        // per-sender handshake ratchet. openmls / mdk default to AlwaysCiphertext
+        // outgoing, so every B→A commit quartz receives lands here with
+        // content_type == COMMIT.
        val kng =
            if (senderLeafIndex == myLeafIndex && sentKeys.containsKey(generation)) {
                sentKeys.remove(generation)!!
            } else {
-                secretTree.applicationKeyNonceForGeneration(senderLeafIndex, generation)
+                when (privMsg.contentType) {
+                    ContentType.APPLICATION -> {
+                        secretTree.applicationKeyNonceForGeneration(senderLeafIndex, generation)
+                    }
+
+                    ContentType.COMMIT, ContentType.PROPOSAL -> {
+                        secretTree.handshakeKeyNonceForGeneration(senderLeafIndex, generation)
+                    }
+                }
            }

        // Apply reuse_guard XOR to nonce (RFC 9420 §6.3.1)
@@ -893,48 +905,84 @@ class MlsGroup private constructor(
        val contentAad = buildPrivateContentAAD(privMsg.groupId, privMsg.epoch, privMsg.contentType, privMsg.authenticatedData)
        val pmcPlaintext = MlsCryptoProvider.aeadDecrypt(kng.key, guardedNonce, contentAad, privMsg.ciphertext)

-        // Parse PrivateMessageContent (RFC 9420 §6.3.1) and verify the
-        // sender's FramedContentTBS signature before returning bytes.
-        require(privMsg.contentType == ContentType.APPLICATION) {
-            // Commit/Proposal-via-PrivateMessage decoding is not implemented.
-            "decrypt() only supports application-content PrivateMessages"
-        }
+        // Parse PrivateMessageContent (RFC 9420 §6.3.1). The layout depends on
+        // content_type — application payloads carry `opaque application_data<V>`
+        // whereas commit / proposal payloads carry the struct directly (no
+        // outer length prefix).
        val pmcReader = TlsReader(pmcPlaintext)
-        val applicationData = pmcReader.readOpaqueVarInt()
-        val signature = pmcReader.readOpaqueVarInt()
-        // Remaining bytes are padding; all must be zero per §6.3.1.
-        while (pmcReader.hasRemaining) {
-            require(pmcReader.readBytes(1)[0] == 0.toByte()) {
-                "PrivateMessageContent padding must be zero"
+        when (privMsg.contentType) {
+            ContentType.APPLICATION -> {
+                val applicationData = pmcReader.readOpaqueVarInt()
+                val signature = pmcReader.readOpaqueVarInt()
+                while (pmcReader.hasRemaining) {
+                    require(pmcReader.readBytes(1)[0] == 0.toByte()) {
+                        "PrivateMessageContent padding must be zero"
+                    }
+                }
+
+                val senderLeaf =
+                    requireNotNull(tree.getLeaf(senderLeafIndex)) {
+                        "Sender leaf is blank at index $senderLeafIndex"
+                    }
+                require(
+                    MlsCryptoProvider.verifyWithLabel(
+                        senderLeaf.signatureKey,
+                        "FramedContentTBS",
+                        buildApplicationFramedContentTbs(
+                            groupId = privMsg.groupId,
+                            epoch = privMsg.epoch,
+                            senderLeafIndex = senderLeafIndex,
+                            authenticatedData = privMsg.authenticatedData,
+                            applicationData = applicationData,
+                            groupContext = groupContext,
+                        ),
+                        signature,
+                    ),
+                ) { "FramedContentTBS signature verification failed" }
+
+                return DecryptedMessage(
+                    senderLeafIndex = senderLeafIndex,
+                    contentType = privMsg.contentType,
+                    content = applicationData,
+                    epoch = privMsg.epoch,
+                )
+            }
+
+            ContentType.COMMIT -> {
+                // PrivateMessageContent for a Commit: the Commit struct
+                // (no length prefix) followed by signature<V> and
+                // confirmation_tag<V>, then zero padding. The caller drives
+                // processCommit with (commitBytes, signature, confirmationTag)
+                // — all available here once we re-serialize the parsed Commit
+                // back to bytes (openmls does the same round-trip).
+                val commit = Commit.decodeTls(pmcReader)
+                val commitWriter = TlsWriter()
+                commit.encodeTls(commitWriter)
+                val commitBytes = commitWriter.toByteArray()
+                val signature = pmcReader.readOpaqueVarInt()
+                val confirmationTag = pmcReader.readOpaqueVarInt()
+                while (pmcReader.hasRemaining) {
+                    require(pmcReader.readBytes(1)[0] == 0.toByte()) {
+                        "PrivateMessageContent padding must be zero"
+                    }
+                }
+                // Apply the commit inline — after this returns, the caller
+                // only needs to know the epoch advanced. The signature rides
+                // into the transcript-hash computation inside processCommit.
+                processCommit(commitBytes, senderLeafIndex, confirmationTag, signature)
+
+                return DecryptedMessage(
+                    senderLeafIndex = senderLeafIndex,
+                    contentType = privMsg.contentType,
+                    content = commitBytes,
+                    epoch = privMsg.epoch,
+                )
+            }
+
+            ContentType.PROPOSAL -> {
+                throw IllegalStateException("Standalone PrivateMessage proposals not yet supported")
            }
        }
-
-        val senderLeaf =
-            requireNotNull(tree.getLeaf(senderLeafIndex)) {
-                "Sender leaf is blank at index $senderLeafIndex"
-            }
-        require(
-            MlsCryptoProvider.verifyWithLabel(
-                senderLeaf.signatureKey,
-                "FramedContentTBS",
-                buildApplicationFramedContentTbs(
-                    groupId = privMsg.groupId,
-                    epoch = privMsg.epoch,
-                    senderLeafIndex = senderLeafIndex,
-                    authenticatedData = privMsg.authenticatedData,
-                    applicationData = applicationData,
-                    groupContext = groupContext,
-                ),
-                signature,
-            ),
-        ) { "FramedContentTBS signature verification failed" }
-
-        return DecryptedMessage(
-            senderLeafIndex = senderLeafIndex,
-            contentType = privMsg.contentType,
-            content = applicationData,
-            epoch = privMsg.epoch,
-        )
    }

    /**
@@ -50,17 +50,21 @@ class SecretTree(
    /** Per-sender ratchet state: (handshake generation, handshake secret, app generation, app secret) */
    private val senderState = mutableMapOf<Int, SenderRatchetState>()

-    /** Consumed (sender, generation) pairs for replay detection (RFC 9420 Section 9.1) */
+    /** Consumed (sender, generation) pairs for replay detection on the APPLICATION ratchet. */
    private val consumedGenerations = mutableMapOf<Int, MutableSet<Int>>()

+    /** Same replay tracker, but for the HANDSHAKE ratchet (commits / proposals). */
+    private val consumedHandshakeGenerations = mutableMapOf<Int, MutableSet<Int>>()
+
    /**
-     * Cache of key/nonce pairs for skipped generations.
+     * Cache of key/nonce pairs for skipped APPLICATION generations.
     * Key: (leafIndex, generation) -> derived KeyNonceGeneration.
-     * When fast-forwarding a ratchet, intermediate generations are saved here
-     * so that out-of-order messages arriving later can still be decrypted.
     */
    private val skippedKeys = mutableMapOf<Pair<Int, Int>, KeyNonceGeneration>()

+    /** Same cache for the HANDSHAKE ratchet. */
+    private val handshakeSkippedKeys = mutableMapOf<Pair<Int, Int>, KeyNonceGeneration>()
+
    private companion object {
        /** Maximum number of skipped key entries to retain (prevents unbounded memory growth). */
        const val MAX_SKIPPED_KEYS = 1000
@@ -186,6 +190,70 @@ class SecretTree(
        return result
    }

+    /**
+     * Handshake-ratchet counterpart to [applicationKeyNonceForGeneration].
+     *
+     * PrivateMessage commits / proposals (RFC 9420 §6.3.2) are encrypted
+     * with a separate handshake ratchet per sender leaf — NOT the
+     * application ratchet. openmls / mdk use this path by default
+     * (`MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY`), so quartz must also
+     * ratchet-forward the handshake chain when decrypting an inbound
+     * PrivateMessage commit.
+     */
+    fun handshakeKeyNonceForGeneration(
+        leafIndex: Int,
+        generation: Int,
+    ): KeyNonceGeneration {
+        val cachedKey = handshakeSkippedKeys.remove(Pair(leafIndex, generation))
+        if (cachedKey != null) {
+            val senderConsumed = consumedHandshakeGenerations.getOrPut(leafIndex) { mutableSetOf() }
+            require(generation !in senderConsumed) {
+                "Replay detected: handshake generation $generation from sender $leafIndex already consumed"
+            }
+            senderConsumed.add(generation)
+            return cachedKey
+        }
+
+        val state = getOrInitSender(leafIndex)
+
+        require(generation >= state.handshakeGeneration) {
+            "Handshake generation $generation already consumed (current: ${state.handshakeGeneration})"
+        }
+
+        val senderConsumed = consumedHandshakeGenerations.getOrPut(leafIndex) { mutableSetOf() }
+        require(generation !in senderConsumed) {
+            "Replay detected: handshake generation $generation from sender $leafIndex already consumed"
+        }
+        senderConsumed.add(generation)
+
+        if (senderConsumed.size > MAX_CONSUMED_GENERATIONS_PER_SENDER) {
+            val minGeneration = state.handshakeGeneration
+            senderConsumed.removeAll { it < minGeneration }
+        }
+
+        var secret = state.handshakeSecret
+        var gen = state.handshakeGeneration
+        while (gen < generation) {
+            val intermediateKng = deriveKeyNonce(secret, gen)
+            val cacheKey = Pair(leafIndex, gen)
+            if (handshakeSkippedKeys.size < MAX_SKIPPED_KEYS) {
+                handshakeSkippedKeys[cacheKey] = intermediateKng
+            }
+            secret = MlsCryptoProvider.expandWithLabel(secret, "secret", generationContext(gen), MlsCryptoProvider.HASH_OUTPUT_LENGTH)
+            gen++
+        }
+
+        val result = deriveKeyNonce(secret, generation)
+        val nextSecret = MlsCryptoProvider.expandWithLabel(secret, "secret", generationContext(generation), MlsCryptoProvider.HASH_OUTPUT_LENGTH)
+        senderState[leafIndex] =
+            state.copy(
+                handshakeSecret = nextSecret,
+                handshakeGeneration = generation + 1,
+            )
+
+        return result
+    }
+
    /**
     * Encode a generation counter as a 4-byte big-endian uint32 for DeriveTreeSecret context.
     */
@@ -0,0 +1,11 @@
+--- a/../../../.cargo/git/checkouts/mdk-7d5a3a2420b194f5/8a8d06c/crates/mdk-core/src/groups.rs
+++ b/../../../.cargo/git/checkouts/mdk-7d5a3a2420b194f5/8a8d06c/crates/mdk-core/src/groups.rs
+@@ -1215,7 +1215,7 @@
+         );
+         let group_config = MlsGroupCreateConfig::builder()
+             .ciphersuite(self.ciphersuite)
+-            .wire_format_policy(MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY)
+            .wire_format_policy(MIXED_PLAINTEXT_WIRE_FORMAT_POLICY)
+             .use_ratchet_tree_extension(true)
+             .capabilities(capabilities)
+             .with_group_context_extensions(extensions)