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 ca5f592c5e..cc7ef8e2f9 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 @@ -660,8 +660,18 @@ class MlsGroupManager( if (privMsg.epoch != retained.epoch) return null // Derive sender data key/nonce using ciphertext sample (RFC 9420 §6.3.1) + // RFC 9420 §6.3.2: ciphertext_sample is the first KDF.Nh bytes + // (32 for HKDF-SHA256), not AEAD.Nk (16). Using AEAD.Nk here made + // sender-data decryption silently fail for every retained-epoch + // message and turned the fallback path into a no-op — the + // symptom was interop Test 12 (offline catch-up): kind:9 + // messages encrypted under epoch N arriving after a 1→N+1 + // commit got rejected with "Message epoch X doesn't match + // current epoch Y" instead of being pulled through this path. + // Same fix already applied to MlsGroup.decrypt (line ~878); + // this branch was missed when that one was patched. val ciphertextSample = - privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.AEAD_KEY_LENGTH)) + privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.HASH_OUTPUT_LENGTH)) val senderDataKey = MlsCryptoProvider.expandWithLabel( retained.senderDataSecret, @@ -710,13 +720,23 @@ class MlsGroupManager( contentAad.putUint8(privMsg.contentType.value) contentAad.putOpaqueVarInt(privMsg.authenticatedData) - val plaintext = + val pmcBytes = MlsCryptoProvider.aeadDecrypt(kng.key, guardedNonce, contentAad.toByteArray(), privMsg.ciphertext) + // AEAD plaintext is a PrivateMessageContent struct (RFC 9420 + // §6.3.1): `applicationData || signature || padding`. The + // main decrypt path parses this and returns the inner + // applicationData; the retained-epoch branch was returning the + // raw struct, which made callers see a length-prefixed blob + // with signature + zero-padding glued onto the end. Extract the + // applicationData the same way. + val pmcReader = TlsReader(pmcBytes) + val applicationData = pmcReader.readOpaqueVarInt() + DecryptedMessage( senderLeafIndex = senderLeafIndex, contentType = privMsg.contentType, - content = plaintext, + content = applicationData, epoch = privMsg.epoch, ) } catch (_: Exception) { diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt index ebfa6775d5..9727705f29 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt @@ -158,6 +158,84 @@ class MarmotPipelineTest { } } + /** + * Regression: the interop harness's Test 12 (offline catch-up) drove + * kind:9 application messages encrypted under epoch N into an Amethyst + * receiver that had already processed a 1→N+1 commit (e.g. add-member). + * Those epoch-N messages ought to decrypt through the retained-epoch + * fallback in [MlsGroupManager.decrypt], but the fallback was silently + * returning null because `tryDecryptWithRetainedEpoch` derived the + * sender-data sample using `AEAD_KEY_LENGTH` (16) instead of + * `HASH_OUTPUT_LENGTH` (32) — the same bug that was fixed on the + * main decrypt path earlier but never propagated to the retained + * branch. This test reproduces the flow and asserts the retained + * path actually decrypts. + */ + @Test + fun testDecryptRetainedEpoch_ApplicationMessageAfterCommit() { + runBlocking { + val aliceMgr = createGroupManager() + val bobMgr = createGroupManager() + + // 32-byte identities so they fit the MarmotGroupData 32-byte + // admin-pubkey slots. + val aliceId = ByteArray(32) { 0xA1.toByte() } + val bobId = ByteArray(32) { 0xB2.toByte() } + val carolId = ByteArray(32) { 0xC3.toByte() } + + aliceMgr.createGroup(groupId, aliceId) + // Install the MarmotGroupData extension so Welcome messages carry + // the NostrGroupData (MlsGroupManager.processWelcome requires it). + aliceMgr.updateGroupExtensions( + nostrGroupId = groupId, + extensions = + listOf( + com.vitorpamplona.quartz.marmot.mip01Groups + .MarmotGroupData( + nostrGroupId = groupId, + adminPubkeys = listOf(aliceId.toHexKey()), + ).toExtension(), + ), + ) + + // Bob joins at epoch 2 (epoch 0 = create, epoch 1 = GCE above, + // epoch 2 = add Bob). + val aliceGroup = aliceMgr.getGroup(groupId)!! + val bobBundle = aliceGroup.createKeyPackage(bobId, ByteArray(0)) + val addBob = aliceMgr.addMember(groupId, bobBundle.keyPackage.toTlsBytes()) + bobMgr.processWelcome(addBob.welcomeBytes!!, bobBundle) + val bobJoinEpoch = aliceMgr.getGroup(groupId)!!.epoch + assertEquals(bobJoinEpoch, bobMgr.getGroup(groupId)!!.epoch, "bob must be at alice's epoch after Welcome") + + // Bob encrypts a message at his current epoch — this would be + // held by a lossy relay and only reach Alice after she's + // already processed the next commit. + val bobStaleMsg = bobMgr.encrypt(groupId, "bob's offline epoch-N message".encodeToByteArray()) + + // Alice adds Carol, advancing to epoch N+1. Bob's earlier + // message is now out-of-order relative to Alice's tree state. + val carolBundle = aliceGroup.createKeyPackage(carolId, ByteArray(0)) + aliceMgr.addMember(groupId, carolBundle.keyPackage.toTlsBytes()) + assertEquals( + bobJoinEpoch + 1, + aliceMgr.getGroup(groupId)!!.epoch, + "alice must have advanced one epoch past bob's encrypt time", + ) + + // Bob's stale message now arrives at Alice. The primary decrypt + // path throws "Message epoch X doesn't match current epoch X+1"; + // the retained-epoch fallback inside MlsGroupManager.decrypt + // must pick it up. + val decrypted = aliceMgr.decrypt(groupId, bobStaleMsg) + assertEquals( + "bob's offline epoch-N message", + decrypted.content.decodeToString(), + "Alice must decrypt Bob's pre-commit message via retained-epoch fallback", + ) + assertEquals(bobJoinEpoch, decrypted.epoch, "message was encrypted at bob's join epoch") + } + } + @Test fun testInboundRejectsNonMemberGroup() { runBlocking {