Merge pull request #2498 from vitorpamplona/claude/fix-marmot-mip-tests-F1mPy

Use processFramedCommit for RFC 9420 §6.1 signature verification
This commit is contained in:
Vitor Pamplona
2026-04-22 12:08:34 -04:00
committed by GitHub
4 changed files with 62 additions and 39 deletions
@@ -307,6 +307,32 @@ class MlsGroupManager(
persistGroup(nostrGroupId) persistGroup(nostrGroupId)
} }
/**
* Process a fully-framed `MlsMessage(PublicMessage(Commit))` — the wire shape
* returned in [CommitResult.framedCommitBytes]. Delegates to
* [MlsGroup.processFramedCommit] which extracts the sender leaf index,
* confirmation_tag, and FramedContentTBS signature from the envelope, then
* calls [MlsGroup.processCommit] with all fields properly populated.
*
* Intended for callers that hold the framed bytes directly (tests driving
* peer managers, CLI interop harnesses). Production inbound goes through
* `MarmotInboundProcessor`, which unwraps the outer ChaCha20 layer first
* and then calls [processCommit] with the already-decoded PublicMessage
* fields.
*/
suspend fun processFramedCommit(
nostrGroupId: HexKey,
framedCommitBytes: ByteArray,
) = mutex.withLock {
val group = requireGroup(nostrGroupId)
val retainedBefore = group.retainedSecrets()
group.processFramedCommit(framedCommitBytes)
pushRetainedEpoch(nostrGroupId, retainedBefore)
persistGroup(nostrGroupId)
}
// --- Message Encryption/Decryption --- // --- Message Encryption/Decryption ---
/** /**
@@ -60,8 +60,17 @@ class MarmotMipComplianceTest {
// ---------------------------------------------------------------------- MIP-01 // ---------------------------------------------------------------------- MIP-01
@Test @Test
fun marmotGroupData_defaultVersionIsThree() { fun marmotGroupData_currentVersionIsHeldAtTwoForMdkInterop() {
assertEquals(3, MarmotGroupData.CURRENT_VERSION) // MIP-01 spec targets v3, but mdk-core (whitenoise-rs' MLS engine)
// rejects v3 NostrGroupData payloads with `ExtensionFormatError`
// ("Trailing bytes in NostrGroupDataExtension"), violating the spec's
// forward-compat rule. Until mdk-core ships the fix, Amethyst holds
// CURRENT_VERSION at 2 for cross-client interop; v3 stays parseable
// (see disappearing_message_secs round-trip test below). Bump this
// assertion back to 3 in lockstep with the MarmotGroupData constant
// once mdk publishes the parser fix.
assertEquals(2, MarmotGroupData.CURRENT_VERSION)
assertTrue(3 in MarmotGroupData.SUPPORTED_VERSIONS, "v3 MUST remain parseable for forward compat")
} }
@Test @Test
@@ -89,8 +98,13 @@ class MarmotMipComplianceTest {
@Test @Test
fun marmotGroupData_roundTripWithDisappearingSecs() { fun marmotGroupData_roundTripWithDisappearingSecs() {
// disappearing_message_secs is a v3+ field. CURRENT_VERSION is held
// at 2 for MDK interop (see marmotGroupData_currentVersionIsHeldAtTwoForMdkInterop),
// so this test pins v3 explicitly to exercise the v3 encoder/decoder
// path end-to-end.
val original = val original =
MarmotGroupData( MarmotGroupData(
version = 3,
nostrGroupId = groupId32, nostrGroupId = groupId32,
adminPubkeys = listOf(adminPubkey), adminPubkeys = listOf(adminPubkey),
relays = listOf("wss://relay.example/"), relays = listOf("wss://relay.example/"),
@@ -202,8 +202,12 @@ class MarmotMipBehaviorTest {
val manager = createGroupManager() val manager = createGroupManager()
manager.createGroup(groupId, aliceId.hexToByteArray()) manager.createGroup(groupId, aliceId.hexToByteArray())
// disappearing_message_secs is a v3+ field. CURRENT_VERSION is held at 2 for
// MDK interop, so the encoder only emits the field for version ≥ 3 — pin
// version=3 here so the outbound processor actually sees the setting.
val configured = val configured =
MarmotGroupData( MarmotGroupData(
version = 3,
nostrGroupId = groupId, nostrGroupId = groupId,
adminPubkeys = listOf(aliceId), adminPubkeys = listOf(aliceId),
disappearingMessageSecs = 3600UL, disappearingMessageSecs = 3600UL,
@@ -472,12 +472,10 @@ class MarmotPipelineTest {
edenMgr.processWelcome(addEden.welcomeBytes!!, edenBundle) edenMgr.processWelcome(addEden.welcomeBytes!!, edenBundle)
val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes()) val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes())
fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle) fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle)
edenMgr.processCommit( // Feed the framed PublicMessage envelope so MlsGroup.processCommit
groupId, // can verify the RFC 9420 §6.1 FramedContentTBS signature (now
addFred.commitBytes, // mandatory on non-external commits).
davidMgr.getGroup(groupId)!!.leafIndex, edenMgr.processFramedCommit(groupId, addFred.framedCommitBytes)
ByteArray(0),
)
val preRestartEpoch = davidMgr.getGroup(groupId)!!.epoch val preRestartEpoch = davidMgr.getGroup(groupId)!!.epoch
val preRestartExporter = davidMgr.exporterSecret(groupId) val preRestartExporter = davidMgr.exporterSecret(groupId)
@@ -571,12 +569,10 @@ class MarmotPipelineTest {
// David adds Fred; Eden processes Alice's add-Fred commit. // David adds Fred; Eden processes Alice's add-Fred commit.
val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes()) val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes())
fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle) fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle)
edenMgr.processCommit( // Feed the framed PublicMessage envelope so MlsGroup.processCommit
nostrGroupId = groupId, // can verify the RFC 9420 §6.1 FramedContentTBS signature (now
commitBytes = addFred.commitBytes, // mandatory on non-external commits).
senderLeafIndex = davidMgr.getGroup(groupId)!!.leafIndex, edenMgr.processFramedCommit(groupId, addFred.framedCommitBytes)
confirmationTag = ByteArray(0),
)
// Pre-restart sanity: all three share the same outer exporter key. // Pre-restart sanity: all three share the same outer exporter key.
val preRestartDavidKey = davidMgr.exporterSecret(groupId) val preRestartDavidKey = davidMgr.exporterSecret(groupId)
@@ -655,13 +651,10 @@ class MarmotPipelineTest {
// Bob (existing member at the pre-add-Fred epoch) receives and // Bob (existing member at the pre-add-Fred epoch) receives and
// applies Alice's add-Fred commit to reach the same epoch as // applies Alice's add-Fred commit to reach the same epoch as
// Alice + Fred. Without this, Bob can't decrypt Fred's subsequent // Alice + Fred. Without this, Bob can't decrypt Fred's subsequent
// application messages. // application messages. Feed the framed PublicMessage envelope so
bobMgr.processCommit( // MlsGroup.processCommit can verify the mandatory RFC 9420 §6.1
nostrGroupId = groupId, // FramedContentTBS signature.
commitBytes = addFred.commitBytes, bobMgr.processFramedCommit(groupId, addFred.framedCommitBytes)
senderLeafIndex = aliceMgr.getGroup(groupId)!!.leafIndex,
confirmationTag = ByteArray(0),
)
// Sanity: Fred is at leafIndex=2, leafCount=3. // Sanity: Fred is at leafIndex=2, leafCount=3.
assertEquals(2, fredMgr.getGroup(groupId)!!.leafIndex) assertEquals(2, fredMgr.getGroup(groupId)!!.leafIndex)
@@ -809,24 +802,10 @@ class MarmotPipelineTest {
addCarolCommitEvent.signedEvent.content, addCarolCommitEvent.signedEvent.content,
bobExporterAtE1, bobExporterAtE1,
) )
val mlsMessage = // Feed the already-decoded MlsMessage(PublicMessage(Commit)) envelope
com.vitorpamplona.quartz.marmot.mls.framing.MlsMessage // through processFramedCommit so MlsGroup can verify the RFC 9420 §6.1
.decodeTls( // FramedContentTBS signature from the sender's PublicMessage.
com.vitorpamplona.quartz.marmot.mls.codec bobMgr.processFramedCommit(groupId, decryptedMlsBytes)
.TlsReader(decryptedMlsBytes),
)
val publicMessage =
com.vitorpamplona.quartz.marmot.mls.framing.PublicMessage
.decodeTls(
com.vitorpamplona.quartz.marmot.mls.codec
.TlsReader(mlsMessage.payload),
)
bobMgr.processCommit(
nostrGroupId = groupId,
commitBytes = publicMessage.content,
senderLeafIndex = publicMessage.sender.leafIndex,
confirmationTag = publicMessage.confirmationTag!!,
)
val epochAfterAddCarol = aliceMgr.getGroup(groupId)!!.epoch val epochAfterAddCarol = aliceMgr.getGroup(groupId)!!.epoch
assertEquals(epochAfterAddBob + 1, epochAfterAddCarol) assertEquals(epochAfterAddBob + 1, epochAfterAddCarol)