Merge pull request #2471 from vitorpamplona/claude/debug-group-event-handler-DEcD1
Fix MLS group state divergence and outer-layer decryption issues
This commit is contained in:
Vitor Pamplona
+19
@@ -404,6 +404,15 @@ private suspend fun processMarmotWelcomeFlow(
|
||||
}
|
||||
}
|
||||
|
||||
is WelcomeResult.AlreadyJoined -> {
|
||||
// Benign replay of a gift-wrapped Welcome (kind:1059) we already
|
||||
// processed in a prior session — the relay is just re-delivering
|
||||
// it after app restart. Log at DEBUG, not WARN.
|
||||
Log.d("MarmotDbg") {
|
||||
"processMarmotWelcomeFlow: already joined group=${result.nostrGroupId.take(8)}… — treating Welcome as replay"
|
||||
}
|
||||
}
|
||||
|
||||
is WelcomeResult.Error -> {
|
||||
Log.w("MarmotDbg") { "processMarmotWelcomeFlow: ERROR ${result.message}" }
|
||||
}
|
||||
@@ -619,6 +628,16 @@ class GroupEventHandler(
|
||||
Log.d("MarmotDbg") { "GroupEventHandler.add: Duplicate kind:445 for group=${result.groupId.take(8)}…" }
|
||||
}
|
||||
|
||||
is GroupEventResult.UndecryptableOuterLayer -> {
|
||||
// Expected for commits + application messages from epochs
|
||||
// that predate our join — per MLS forward secrecy we
|
||||
// never held those keys. Not a bug, not a warning.
|
||||
Log.d("MarmotDbg") {
|
||||
"GroupEventHandler.add: undecryptable outer layer for group=${result.groupId.take(8)}… " +
|
||||
"(current + ${result.retainedEpochCount} retained epoch key(s) tried) — likely from before our join"
|
||||
}
|
||||
}
|
||||
|
||||
is GroupEventResult.Error -> {
|
||||
Log.w("MarmotDbg") { "GroupEventHandler.add: ERROR ${result.message}" }
|
||||
}
|
||||
|
||||
+31
-6
@@ -115,6 +115,7 @@ class MarmotManager(
|
||||
|
||||
is GroupEventResult.CommitPending,
|
||||
is GroupEventResult.Duplicate,
|
||||
is GroupEventResult.UndecryptableOuterLayer,
|
||||
is GroupEventResult.Error,
|
||||
-> {}
|
||||
}
|
||||
@@ -178,10 +179,23 @@ class MarmotManager(
|
||||
"KeyPackage credential identity does not match memberPubKey"
|
||||
}
|
||||
|
||||
// Per RFC 9420 §12.4 (and MDK), the outbound kind:445 MUST be
|
||||
// outer-encrypted with the pre-commit (epoch-N) exporter secret so
|
||||
// that other existing members still at epoch N can decrypt and
|
||||
// process the commit. CommitResult.preCommitExporterSecret carries
|
||||
// that key; the local group state has already advanced to N+1 by
|
||||
// the time addMember returns, so we can't read it from the group
|
||||
// any more.
|
||||
val commitResult = groupManager.addMember(nostrGroupId, keyPackageBytes)
|
||||
val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes)
|
||||
// We've already applied this commit locally; prevent the relay-echoed
|
||||
// copy from being re-applied by the inbound pipeline.
|
||||
val commitEvent =
|
||||
outboundProcessor.buildCommitEvent(
|
||||
nostrGroupId = nostrGroupId,
|
||||
commitBytes = commitResult.framedCommitBytes,
|
||||
exporterKey = commitResult.preCommitExporterSecret,
|
||||
)
|
||||
// The published kind:445 will echo back from the relay — without this
|
||||
// dedup our own inbound pipeline would try to re-apply a commit whose
|
||||
// epoch we've already merged.
|
||||
inboundProcessor.markEventProcessed(commitEvent.signedEvent.id)
|
||||
|
||||
val welcomeDelivery =
|
||||
@@ -269,7 +283,12 @@ class MarmotManager(
|
||||
targetLeafIndex: Int,
|
||||
): OutboundGroupEvent {
|
||||
val commitResult = groupManager.removeMember(nostrGroupId, targetLeafIndex)
|
||||
val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes)
|
||||
val commitEvent =
|
||||
outboundProcessor.buildCommitEvent(
|
||||
nostrGroupId = nostrGroupId,
|
||||
commitBytes = commitResult.framedCommitBytes,
|
||||
exporterKey = commitResult.preCommitExporterSecret,
|
||||
)
|
||||
inboundProcessor.markEventProcessed(commitEvent.signedEvent.id)
|
||||
return commitEvent
|
||||
}
|
||||
@@ -282,8 +301,14 @@ class MarmotManager(
|
||||
nostrGroupId: HexKey,
|
||||
metadata: MarmotGroupData,
|
||||
): OutboundGroupEvent {
|
||||
val commitResult = groupManager.updateGroupExtensions(nostrGroupId, listOf(metadata.toExtension()))
|
||||
val commitEvent = outboundProcessor.buildCommitEvent(nostrGroupId, commitResult.framedCommitBytes)
|
||||
val commitResult =
|
||||
groupManager.updateGroupExtensions(nostrGroupId, listOf(metadata.toExtension()))
|
||||
val commitEvent =
|
||||
outboundProcessor.buildCommitEvent(
|
||||
nostrGroupId = nostrGroupId,
|
||||
commitBytes = commitResult.framedCommitBytes,
|
||||
exporterKey = commitResult.preCommitExporterSecret,
|
||||
)
|
||||
inboundProcessor.markEventProcessed(commitEvent.signedEvent.id)
|
||||
return commitEvent
|
||||
}
|
||||
|
||||
+116
-26
@@ -79,6 +79,21 @@ sealed class GroupEventResult {
|
||||
val groupId: HexKey,
|
||||
) : GroupEventResult()
|
||||
|
||||
/**
|
||||
* The outer ChaCha20-Poly1305 layer could not be decrypted with the
|
||||
* current-epoch exporter key or any retained prior-epoch key.
|
||||
*
|
||||
* This is the expected outcome whenever a member receives a kind:445
|
||||
* from an epoch before they joined (via Welcome). Per MLS forward
|
||||
* secrecy, the new member never held those keys, so the bytes are
|
||||
* unreadable to them — and that is by design, not an error. Callers
|
||||
* should surface this at DEBUG, not WARN.
|
||||
*/
|
||||
data class UndecryptableOuterLayer(
|
||||
val groupId: HexKey,
|
||||
val retainedEpochCount: Int,
|
||||
) : GroupEventResult()
|
||||
|
||||
/**
|
||||
* The event could not be processed.
|
||||
*/
|
||||
@@ -101,6 +116,20 @@ sealed class WelcomeResult {
|
||||
val needsKeyPackageRotation: Boolean,
|
||||
) : WelcomeResult()
|
||||
|
||||
/**
|
||||
* The Welcome was for a group we're already a member of — benign replay.
|
||||
*
|
||||
* Happens after an app restart: the gift-wrapped Welcome (kind:1059) is
|
||||
* still sitting on the relay and gets redelivered, but the KeyPackage
|
||||
* bundle it referenced was already consumed and marked. Rather than
|
||||
* logging a noisy "No matching KeyPackageBundle" error, we detect the
|
||||
* replay up front by checking `groupManager.isMember(hintNostrGroupId)`
|
||||
* and return this result. Callers should log at DEBUG.
|
||||
*/
|
||||
data class AlreadyJoined(
|
||||
val nostrGroupId: HexKey,
|
||||
) : WelcomeResult()
|
||||
|
||||
/**
|
||||
* The Welcome could not be processed.
|
||||
*/
|
||||
@@ -176,15 +205,25 @@ class MarmotInboundProcessor(
|
||||
val result =
|
||||
try {
|
||||
// Step 1: Outer ChaCha20-Poly1305 decryption
|
||||
val mlsBytes = decryptOuterLayer(groupId, groupEvent.encryptedContent())
|
||||
val mlsBytes = tryDecryptOuterLayer(groupId, groupEvent.encryptedContent())
|
||||
if (mlsBytes == null) {
|
||||
// Expected when this kind:445 was encrypted with an epoch
|
||||
// key that predates our join (classical MLS forward
|
||||
// secrecy), or when the sender's epoch has drifted. Not
|
||||
// an error — callers should log at DEBUG.
|
||||
GroupEventResult.UndecryptableOuterLayer(
|
||||
groupId,
|
||||
retainedEpochCount = groupManager.retainedExporterSecrets(groupId).size,
|
||||
)
|
||||
} else {
|
||||
// Step 2: Parse the MLS message
|
||||
val mlsMessage = MlsMessage.decodeTls(TlsReader(mlsBytes))
|
||||
|
||||
// Step 2: Parse the MLS message
|
||||
val mlsMessage = MlsMessage.decodeTls(TlsReader(mlsBytes))
|
||||
|
||||
when (mlsMessage.wireFormat) {
|
||||
WireFormat.PRIVATE_MESSAGE -> processPrivateMessage(groupId, mlsMessage, groupEvent)
|
||||
WireFormat.PUBLIC_MESSAGE -> processPublicMessage(groupId, mlsMessage, groupEvent)
|
||||
else -> GroupEventResult.Error(groupId, "Unexpected wire format: ${mlsMessage.wireFormat}")
|
||||
when (mlsMessage.wireFormat) {
|
||||
WireFormat.PRIVATE_MESSAGE -> processPrivateMessage(groupId, mlsMessage, groupEvent)
|
||||
WireFormat.PUBLIC_MESSAGE -> processPublicMessage(groupId, mlsMessage, groupEvent)
|
||||
else -> GroupEventResult.Error(groupId, "Unexpected wire format: ${mlsMessage.wireFormat}")
|
||||
}
|
||||
}
|
||||
} catch (e: Exception) {
|
||||
GroupEventResult.Error(groupId, "Failed to process GroupEvent: ${e.message}", e)
|
||||
@@ -245,6 +284,22 @@ class MarmotInboundProcessor(
|
||||
"MarmotInboundProcessor.processWelcome: welcomeBytes=${welcomeBytes.size}B looking up KeyPackage by ref=${keyPackageEventId.take(8)}…"
|
||||
}
|
||||
|
||||
// Short-circuit if we're already a member of the group the
|
||||
// Welcome is inviting us to. Happens every time the app
|
||||
// restarts: the gift-wrapped kind:1059 is still on the relay
|
||||
// and gets redelivered, but the KeyPackage bundle it
|
||||
// referenced was already consumed + marked during the first
|
||||
// processing. Without this check the fallthrough below would
|
||||
// log a noisy "No matching KeyPackageBundle" warning for what
|
||||
// is actually a benign replay.
|
||||
if (hintNostrGroupId != null && groupManager.isMember(hintNostrGroupId)) {
|
||||
com.vitorpamplona.quartz.utils.Log
|
||||
.d("MarmotDbg") {
|
||||
"MarmotInboundProcessor.processWelcome: already a member of group=${hintNostrGroupId.take(8)}… — treating Welcome as replay"
|
||||
}
|
||||
return WelcomeResult.AlreadyJoined(hintNostrGroupId)
|
||||
}
|
||||
|
||||
// Find the KeyPackageBundle that was consumed.
|
||||
//
|
||||
// The Welcome's "e" tag carries the *Nostr event id* of the
|
||||
@@ -446,7 +501,12 @@ class MarmotInboundProcessor(
|
||||
commitEvent: GroupEvent,
|
||||
): GroupEventResult =
|
||||
try {
|
||||
val mlsBytes = decryptOuterLayer(groupId, commitEvent.encryptedContent())
|
||||
val mlsBytes =
|
||||
tryDecryptOuterLayer(groupId, commitEvent.encryptedContent())
|
||||
?: return GroupEventResult.UndecryptableOuterLayer(
|
||||
groupId,
|
||||
retainedEpochCount = groupManager.retainedExporterSecrets(groupId).size,
|
||||
)
|
||||
val mlsMessage = MlsMessage.decodeTls(TlsReader(mlsBytes))
|
||||
|
||||
when (mlsMessage.wireFormat) {
|
||||
@@ -467,17 +527,44 @@ class MarmotInboundProcessor(
|
||||
WireFormat.PUBLIC_MESSAGE -> {
|
||||
val pubMsg = PublicMessage.decodeTls(TlsReader(mlsMessage.payload))
|
||||
val tag = pubMsg.confirmationTag
|
||||
if (tag == null) {
|
||||
GroupEventResult.Error(groupId, "PublicMessage commit missing confirmation_tag")
|
||||
} else {
|
||||
groupManager.processCommit(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = pubMsg.content,
|
||||
senderLeafIndex = pubMsg.sender.leafIndex,
|
||||
confirmationTag = tag,
|
||||
)
|
||||
val group = groupManager.getGroup(groupId)
|
||||
GroupEventResult.CommitProcessed(groupId, group?.epoch ?: 0)
|
||||
val currentEpoch = groupManager.getGroup(groupId)?.epoch
|
||||
when {
|
||||
tag == null -> {
|
||||
GroupEventResult.Error(groupId, "PublicMessage commit missing confirmation_tag")
|
||||
}
|
||||
|
||||
// Reject commits that are not for our current epoch.
|
||||
// Happens most commonly when our own already-applied
|
||||
// commit is echoed back from the relay after an app
|
||||
// restart (the in-memory dedup set is cleared), and
|
||||
// the outer layer decrypts via a retained epoch key.
|
||||
// Calling `processCommit` on a past-epoch commit
|
||||
// partially mutates tree / groupContext / epochSecrets
|
||||
// before throwing on the confirmation-tag check,
|
||||
// leaving the local state diverged from every other
|
||||
// member's — they then can't decrypt anything we
|
||||
// send next.
|
||||
currentEpoch != null && pubMsg.epoch < currentEpoch -> {
|
||||
GroupEventResult.Duplicate(groupId)
|
||||
}
|
||||
|
||||
currentEpoch != null && pubMsg.epoch > currentEpoch -> {
|
||||
GroupEventResult.Error(
|
||||
groupId,
|
||||
"Commit epoch ${pubMsg.epoch} is ahead of local epoch $currentEpoch; ignoring",
|
||||
)
|
||||
}
|
||||
|
||||
else -> {
|
||||
groupManager.processCommit(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = pubMsg.content,
|
||||
senderLeafIndex = pubMsg.sender.leafIndex,
|
||||
confirmationTag = tag,
|
||||
)
|
||||
val group = groupManager.getGroup(groupId)
|
||||
GroupEventResult.CommitProcessed(groupId, group?.epoch ?: 0)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -495,11 +582,17 @@ class MarmotInboundProcessor(
|
||||
*
|
||||
* After a commit advances the epoch, late-arriving messages encrypted
|
||||
* with the previous epoch's exporter key would fail without this fallback.
|
||||
*
|
||||
* Returns null when neither the current epoch key nor any retained key
|
||||
* decrypts. This happens normally for commits/application messages from
|
||||
* epochs that predate our join (we never held those keys), so callers
|
||||
* should treat null as an expected "nothing to do here" outcome and log
|
||||
* at DEBUG, not as an error.
|
||||
*/
|
||||
private fun decryptOuterLayer(
|
||||
private fun tryDecryptOuterLayer(
|
||||
groupId: HexKey,
|
||||
encryptedContent: String,
|
||||
): ByteArray {
|
||||
): ByteArray? {
|
||||
// Try current epoch key first
|
||||
try {
|
||||
val exporterKey = groupManager.exporterSecret(groupId)
|
||||
@@ -518,9 +611,6 @@ class MarmotInboundProcessor(
|
||||
}
|
||||
}
|
||||
|
||||
// All keys exhausted — throw to let callers produce an error result
|
||||
throw IllegalStateException(
|
||||
"Outer decryption failed with current and ${retainedKeys.size} retained epoch key(s)",
|
||||
)
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
+14
-5
@@ -121,20 +121,29 @@ class MarmotOutboundProcessor(
|
||||
/**
|
||||
* Build a GroupEvent carrying a Commit for publishing.
|
||||
*
|
||||
* Used after MlsGroupManager.commit() or addMember()/removeMember().
|
||||
* The commit bytes are already MLS-formatted.
|
||||
* Used after MlsGroupManager.stageAddMember()/stageRemoveMember()/etc.
|
||||
* The commit bytes are already MLS-formatted (PublicMessage envelope).
|
||||
*
|
||||
* @param nostrGroupId the Nostr group ID
|
||||
* @param commitBytes the raw MLS commit bytes from CommitResult
|
||||
* @param commitBytes the framed MLS commit bytes from [com.vitorpamplona.quartz.marmot.mls.group.StagedCommit.framedCommitBytes]
|
||||
* @param exporterKey optional explicit outer-encryption key. Callers
|
||||
* publishing a Commit MUST pass the **pre-commit** exporter secret
|
||||
* (from [com.vitorpamplona.quartz.marmot.mls.group.StagedCommit.preCommitExporterSecret])
|
||||
* so that other existing members at epoch N can decrypt and process
|
||||
* the commit. If null, falls back to the current epoch's exporter
|
||||
* secret — which is only correct when the commit has *not* been
|
||||
* applied locally yet (i.e. this call is made before
|
||||
* [com.vitorpamplona.quartz.marmot.mls.group.MlsGroup.mergeStagedCommit]).
|
||||
* @return the signed GroupEvent ready for relay publishing
|
||||
*/
|
||||
suspend fun buildCommitEvent(
|
||||
nostrGroupId: HexKey,
|
||||
commitBytes: ByteArray,
|
||||
exporterKey: ByteArray? = null,
|
||||
): OutboundGroupEvent {
|
||||
// Outer ChaCha20-Poly1305 encryption of the MLS commit
|
||||
val exporterKey = groupManager.exporterSecret(nostrGroupId)
|
||||
val encryptedContent = GroupEventEncryption.encrypt(commitBytes, exporterKey)
|
||||
val outerKey = exporterKey ?: groupManager.exporterSecret(nostrGroupId)
|
||||
val encryptedContent = GroupEventEncryption.encrypt(commitBytes, outerKey)
|
||||
|
||||
// Build the GroupEvent template
|
||||
val template =
|
||||
|
||||
@@ -403,6 +403,13 @@ class MlsGroup private constructor(
|
||||
// (i.e. no remaining member appears in the post-commit admin list).
|
||||
enforceNoAdminDepletion(proposals)
|
||||
|
||||
// Capture the pre-commit exporter secret BEFORE any mutation.
|
||||
// Publishers of the outbound kind:445 MUST outer-encrypt with this
|
||||
// key (epoch N) so that other existing members at epoch N can decrypt
|
||||
// and process the commit. See CommitResult.preCommitExporterSecret.
|
||||
val preCommitExporterSecret =
|
||||
exporterSecret("marmot", "group-event".encodeToByteArray(), 32)
|
||||
|
||||
val proposalOrRefs = proposals.map { ProposalOrRef.Inline(it.proposal) }
|
||||
|
||||
// Check if we need an UpdatePath (required unless only SelfRemove)
|
||||
@@ -563,6 +570,7 @@ class MlsGroup private constructor(
|
||||
welcomeBytes = welcomeBytes,
|
||||
groupInfoBytes = null,
|
||||
framedCommitBytes = framedCommitBytes,
|
||||
preCommitExporterSecret = preCommitExporterSecret,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -2071,7 +2079,9 @@ class MlsGroup private constructor(
|
||||
|
||||
/**
|
||||
* Add a member to the group by their KeyPackage.
|
||||
* Creates and applies a Commit with an Add proposal.
|
||||
* Creates and applies a Commit with an Add proposal. The resulting
|
||||
* [CommitResult.preCommitExporterSecret] is the key the outer kind:445
|
||||
* MUST be encrypted with (RFC 9420 §12.4 + MDK parity).
|
||||
*/
|
||||
fun addMember(keyPackageBytes: ByteArray): CommitResult {
|
||||
proposeAdd(keyPackageBytes)
|
||||
@@ -2080,7 +2090,8 @@ class MlsGroup private constructor(
|
||||
|
||||
/**
|
||||
* Remove a member from the group.
|
||||
* Creates and applies a Commit with a Remove proposal.
|
||||
* Creates and applies a Commit with a Remove proposal. See [addMember]
|
||||
* for the pre-commit exporter key contract on the returned [CommitResult].
|
||||
*/
|
||||
fun removeMember(targetLeafIndex: Int): CommitResult {
|
||||
proposeRemove(targetLeafIndex)
|
||||
|
||||
+37
-26
@@ -267,11 +267,9 @@ class MlsGroupManager(
|
||||
suspend fun commit(nostrGroupId: HexKey): CommitResult =
|
||||
mutex.withLock {
|
||||
val group = requireGroup(nostrGroupId)
|
||||
|
||||
// Retain current epoch secrets before transition
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
val result = group.commit()
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
result
|
||||
}
|
||||
@@ -292,10 +290,16 @@ class MlsGroupManager(
|
||||
) = mutex.withLock {
|
||||
val group = requireGroup(nostrGroupId)
|
||||
|
||||
// Retain current epoch secrets before transition
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
// Capture the outgoing epoch's secrets BEFORE advancing, but only
|
||||
// commit them to the retention window once processCommit succeeds —
|
||||
// otherwise a failed commit (e.g. "Duplicate encryption key" on an
|
||||
// add-me relay echo) would pollute the window with a duplicate of
|
||||
// the current epoch key, wasting the finite retention slots.
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
|
||||
group.processCommit(commitBytes, senderLeafIndex, confirmationTag)
|
||||
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
}
|
||||
|
||||
@@ -359,6 +363,12 @@ class MlsGroupManager(
|
||||
|
||||
/**
|
||||
* Add a member and create a Commit.
|
||||
*
|
||||
* The returned [CommitResult.preCommitExporterSecret] is the key the
|
||||
* outbound kind:445 MUST be outer-encrypted with (RFC 9420 §12.4 + MDK
|
||||
* parity). The local group state advances to the new epoch before this
|
||||
* function returns; publishers get one shot at the correct pre-commit
|
||||
* key via the [CommitResult] payload.
|
||||
*/
|
||||
suspend fun addMember(
|
||||
nostrGroupId: HexKey,
|
||||
@@ -366,14 +376,16 @@ class MlsGroupManager(
|
||||
): CommitResult =
|
||||
mutex.withLock {
|
||||
val group = requireGroup(nostrGroupId)
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
val result = group.addMember(keyPackageBytes)
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
result
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a member and create a Commit.
|
||||
* Remove a member and create a Commit. See [addMember] for the
|
||||
* pre-commit exporter key contract on the returned [CommitResult].
|
||||
*/
|
||||
suspend fun removeMember(
|
||||
nostrGroupId: HexKey,
|
||||
@@ -381,8 +393,9 @@ class MlsGroupManager(
|
||||
): CommitResult =
|
||||
mutex.withLock {
|
||||
val group = requireGroup(nostrGroupId)
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
val result = group.removeMember(targetLeafIndex)
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
result
|
||||
}
|
||||
@@ -391,31 +404,22 @@ class MlsGroupManager(
|
||||
* Rotate the signing key within a group and commit.
|
||||
*
|
||||
* Per MIP-00, members SHOULD self-update within 24 hours of joining.
|
||||
* This creates an Update proposal with a fresh signing key and commits it.
|
||||
*
|
||||
* @param nostrGroupId hex-encoded Nostr group ID
|
||||
* @return the [CommitResult] to publish
|
||||
* See [addMember] for the pre-commit exporter key contract.
|
||||
*/
|
||||
suspend fun rotateSigningKey(nostrGroupId: HexKey): CommitResult =
|
||||
mutex.withLock {
|
||||
val group = requireGroup(nostrGroupId)
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
group.proposeSigningKeyRotation()
|
||||
val result = group.commit()
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
result
|
||||
}
|
||||
|
||||
/**
|
||||
* Update group extensions (e.g., MIP-01 metadata) via a GroupContextExtensions proposal.
|
||||
* Creates the proposal, commits it, and persists the new state.
|
||||
*
|
||||
* Authorization (MIP-01): extension updates require admin privileges once
|
||||
* the group has at least one admin. During bootstrap — before any
|
||||
* `admin_pubkeys` are configured — any member may seed the initial
|
||||
* extension set (e.g. the creator installing the first
|
||||
* [com.vitorpamplona.quartz.marmot.mip01Groups.MarmotGroupData] with
|
||||
* themselves as admin).
|
||||
* See [addMember] for the pre-commit exporter key contract.
|
||||
*/
|
||||
suspend fun updateGroupExtensions(
|
||||
nostrGroupId: HexKey,
|
||||
@@ -428,9 +432,10 @@ class MlsGroupManager(
|
||||
check(!adminsConfigured || group.isLocalAdmin()) {
|
||||
"MIP-01: only admins may update group extensions"
|
||||
}
|
||||
retainEpochSecrets(nostrGroupId, group)
|
||||
val retainedBefore = group.retainedSecrets()
|
||||
group.proposeGroupContextExtensions(extensions)
|
||||
val result = group.commit()
|
||||
pushRetainedEpoch(nostrGroupId, retainedBefore)
|
||||
persistGroup(nostrGroupId)
|
||||
result
|
||||
}
|
||||
@@ -564,12 +569,18 @@ class MlsGroupManager(
|
||||
}
|
||||
}
|
||||
|
||||
private fun retainEpochSecrets(
|
||||
/**
|
||||
* Push a previously-captured [RetainedEpochSecrets] into the bounded
|
||||
* retention window. Call after the epoch advance has been applied
|
||||
* successfully so that failed commits don't pollute the window with
|
||||
* duplicate current-epoch keys.
|
||||
*/
|
||||
private fun pushRetainedEpoch(
|
||||
nostrGroupId: HexKey,
|
||||
group: MlsGroup,
|
||||
retainedBefore: RetainedEpochSecrets,
|
||||
) {
|
||||
val retained = retainedEpochs.getOrPut(nostrGroupId) { mutableListOf() }
|
||||
retained.add(group.retainedSecrets())
|
||||
retained.add(retainedBefore)
|
||||
|
||||
// Trim to retention window (keep only the most recent N-1 epochs)
|
||||
while (retained.size > EPOCH_RETENTION_WINDOW) {
|
||||
|
||||
@@ -107,6 +107,18 @@ data class CommitResult(
|
||||
* Wire format: MlsMessage(version=mls10, wireFormat=mls_public_message, payload=PublicMessage(...)).
|
||||
*/
|
||||
val framedCommitBytes: ByteArray = commitBytes,
|
||||
/**
|
||||
* `MLS-Exporter("marmot", "group-event", 32)` evaluated at the **pre-commit**
|
||||
* epoch (N) — the key the group had when this commit was computed, before
|
||||
* local state advanced to N+1. Publishers of the kind:445 MUST ChaCha20-wrap
|
||||
* the commit with this key (RFC 9420 §12.4 and MDK parity). Using the
|
||||
* post-commit (N+1) key makes the commit unreadable to existing members
|
||||
* still at epoch N — the exact scenario that caused Eden to stall at
|
||||
* epoch 1 and never decrypt any of David's subsequent messages.
|
||||
*
|
||||
* Empty by default for the test-only entry points that don't need it.
|
||||
*/
|
||||
val preCommitExporterSecret: ByteArray = ByteArray(0),
|
||||
) {
|
||||
override fun equals(other: Any?): Boolean {
|
||||
if (this === other) return true
|
||||
|
||||
+41
-34
@@ -47,9 +47,6 @@ class SecretTree(
|
||||
private val encryptionSecret: ByteArray,
|
||||
private val leafCount: Int,
|
||||
) {
|
||||
/** Cached tree node secrets, computed lazily */
|
||||
private val treeSecrets = mutableMapOf<Int, ByteArray>()
|
||||
|
||||
/** Per-sender ratchet state: (handshake generation, handshake secret, app generation, app secret) */
|
||||
private val senderState = mutableMapOf<Int, SenderRatchetState>()
|
||||
|
||||
@@ -72,11 +69,6 @@ class SecretTree(
|
||||
const val MAX_CONSUMED_GENERATIONS_PER_SENDER = 1000
|
||||
}
|
||||
|
||||
init {
|
||||
// Seed the root
|
||||
treeSecrets[BinaryTree.root(leafCount)] = encryptionSecret
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the next (key, nonce) for application messages from a given sender.
|
||||
*/
|
||||
@@ -252,36 +244,51 @@ class SecretTree(
|
||||
}
|
||||
|
||||
/**
|
||||
* Derive the leaf secret from the encryption secret using the tree structure.
|
||||
* Derive the leaf secret from the encryption secret by walking DOWN the
|
||||
* binary tree from the root to the target leaf (RFC 9420 §9).
|
||||
*
|
||||
* At each step we pick left or right based on which subtree contains the
|
||||
* target. In an MLS left-balanced tree the left-subtree node indices are
|
||||
* always strictly less than the current node, and the right-subtree
|
||||
* indices strictly greater — so the direction is simply
|
||||
* `targetNode < currentNode`.
|
||||
*
|
||||
* This also correctly handles non-power-of-2 leaf counts (3, 5, 6, 7, 9
|
||||
* …) where the rightmost leaves live beneath "virtual" intermediate
|
||||
* nodes that are beyond `nodeCount`. Walking down still succeeds because
|
||||
* [BinaryTree.left] / [BinaryTree.right] give the correct descendants
|
||||
* even for virtual nodes, and the target leaf is reachable via a chain
|
||||
* of left/right steps that mixes real and virtual intermediates.
|
||||
*
|
||||
* The previous implementation derived the target's secret from
|
||||
* `BinaryTree.parent(target)` which, for leaves on the right edge of a
|
||||
* non-full tree, returned a high ancestor (often the root) that is NOT
|
||||
* the target's direct parent. Its `left()` and `right()` then pointed
|
||||
* at different nodes entirely, the target stayed un-cached, and the
|
||||
* follow-up `return treeSecrets[nodeIndex]!!` either threw NPE (on
|
||||
* JVM) or — when repeatedly re-entered — recursed into
|
||||
* [getNodeSecret] until the stack was exhausted (on ART).
|
||||
*/
|
||||
private fun getLeafSecret(leafIndex: Int): ByteArray {
|
||||
val nodeIndex = BinaryTree.leafToNode(leafIndex)
|
||||
return getNodeSecret(nodeIndex)
|
||||
}
|
||||
val targetNode = BinaryTree.leafToNode(leafIndex)
|
||||
val rootIdx = BinaryTree.root(leafCount)
|
||||
var currentSecret = encryptionSecret
|
||||
var currentNode = rootIdx
|
||||
|
||||
/**
|
||||
* Recursively derive a node's secret from its parent in the secret tree.
|
||||
*/
|
||||
private fun getNodeSecret(nodeIndex: Int): ByteArray {
|
||||
treeSecrets[nodeIndex]?.let { return it }
|
||||
while (currentNode != targetNode) {
|
||||
val goLeft = targetNode < currentNode
|
||||
val label = if (goLeft) "left" else "right"
|
||||
currentSecret =
|
||||
MlsCryptoProvider.expandWithLabel(
|
||||
currentSecret,
|
||||
"tree",
|
||||
label.encodeToByteArray(),
|
||||
MlsCryptoProvider.HASH_OUTPUT_LENGTH,
|
||||
)
|
||||
currentNode = if (goLeft) BinaryTree.left(currentNode) else BinaryTree.right(currentNode)
|
||||
}
|
||||
|
||||
val parentIdx = BinaryTree.parent(nodeIndex, BinaryTree.nodeCount(leafCount))
|
||||
val parentSecret = getNodeSecret(parentIdx)
|
||||
|
||||
// Derive left and right children secrets
|
||||
val leftIdx = BinaryTree.left(parentIdx)
|
||||
val rightIdx = BinaryTree.right(parentIdx)
|
||||
|
||||
val leftSecret = MlsCryptoProvider.expandWithLabel(parentSecret, "tree", "left".encodeToByteArray(), MlsCryptoProvider.HASH_OUTPUT_LENGTH)
|
||||
val rightSecret = MlsCryptoProvider.expandWithLabel(parentSecret, "tree", "right".encodeToByteArray(), MlsCryptoProvider.HASH_OUTPUT_LENGTH)
|
||||
|
||||
treeSecrets[leftIdx] = leftSecret
|
||||
treeSecrets[rightIdx] = rightSecret
|
||||
|
||||
// Clear parent secret for forward secrecy
|
||||
treeSecrets.remove(parentIdx)
|
||||
|
||||
return treeSecrets[nodeIndex]!!
|
||||
return currentSecret
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+445
@@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEvent
|
||||
import com.vitorpamplona.quartz.marmot.mip03GroupMessages.GroupEventEncryption
|
||||
import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupManager
|
||||
import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupStateStore
|
||||
import com.vitorpamplona.quartz.nip01Core.core.toHexKey
|
||||
import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair
|
||||
import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal
|
||||
import kotlinx.coroutines.runBlocking
|
||||
@@ -427,6 +428,450 @@ class MarmotPipelineTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCreatorDoesNotDoubleApplyOwnCommitAfterRestart() {
|
||||
// Root cause: after David restarts, his own add-member kind:445 that
|
||||
// got echoed back from the relay is no longer marked as "already
|
||||
// processed" (the dedup set lives in memory and isn't persisted).
|
||||
// The inbound path then outer-decrypts it via a retained epoch key
|
||||
// and feeds the commit into group.processCommit — which is NOT
|
||||
// atomic, so it partially advances David's local state before failing
|
||||
// with "Confirmation tag verification failed" (the tag was computed
|
||||
// at the original epoch; David is already two epochs past).
|
||||
//
|
||||
// After this partial apply, David's epoch / tree / exporter_secret
|
||||
// drift forward by one epoch while Eden and Fred stay put. Any
|
||||
// subsequent message David sends uses a key nobody else has, and
|
||||
// they log "Outer decryption failed with current and N retained
|
||||
// epoch key(s)".
|
||||
runBlocking {
|
||||
val davidStore = TestGroupStateStore()
|
||||
val davidMgr = MlsGroupManager(davidStore)
|
||||
val edenMgr = MlsGroupManager(TestGroupStateStore())
|
||||
val fredMgr = MlsGroupManager(TestGroupStateStore())
|
||||
val davidId = ByteArray(32) { 0xD1.toByte() }
|
||||
val edenId = ByteArray(32) { 0xE2.toByte() }
|
||||
val fredId = ByteArray(32) { 0xF3.toByte() }
|
||||
|
||||
davidMgr.createGroup(groupId, davidId)
|
||||
davidMgr.updateGroupExtensions(
|
||||
groupId,
|
||||
listOf(
|
||||
com.vitorpamplona.quartz.marmot.mip01Groups
|
||||
.MarmotGroupData(
|
||||
nostrGroupId = groupId,
|
||||
adminPubkeys = listOf(davidId.toHexKey()),
|
||||
).toExtension(),
|
||||
),
|
||||
)
|
||||
val davidGroup = davidMgr.getGroup(groupId)!!
|
||||
val edenBundle = davidGroup.createKeyPackage(edenId, ByteArray(0))
|
||||
val fredBundle = davidGroup.createKeyPackage(fredId, ByteArray(0))
|
||||
|
||||
val addEden = davidMgr.addMember(groupId, edenBundle.keyPackage.toTlsBytes())
|
||||
edenMgr.processWelcome(addEden.welcomeBytes!!, edenBundle)
|
||||
val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes())
|
||||
fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle)
|
||||
edenMgr.processCommit(
|
||||
groupId,
|
||||
addFred.commitBytes,
|
||||
davidMgr.getGroup(groupId)!!.leafIndex,
|
||||
ByteArray(0),
|
||||
)
|
||||
|
||||
val preRestartEpoch = davidMgr.getGroup(groupId)!!.epoch
|
||||
val preRestartExporter = davidMgr.exporterSecret(groupId)
|
||||
|
||||
// Simulate David's app restart.
|
||||
val davidMgr2 = MlsGroupManager(davidStore)
|
||||
davidMgr2.restoreAll()
|
||||
|
||||
// Now replay David's own echoed add-Eden commit that's still on
|
||||
// the relay. After restart, the inbound dedup set is empty so
|
||||
// the inbound pipeline does the full outer-decrypt via retained
|
||||
// keys + processCommit on an already-applied commit.
|
||||
val inbound =
|
||||
com.vitorpamplona.quartz.marmot
|
||||
.MarmotInboundProcessor(
|
||||
groupManager = davidMgr2,
|
||||
keyPackageRotationManager =
|
||||
com.vitorpamplona.quartz.marmot.mip00KeyPackages
|
||||
.KeyPackageRotationManager(),
|
||||
)
|
||||
val outbound = MarmotOutboundProcessor(davidMgr2)
|
||||
|
||||
// Re-encrypt the add-Eden framed commit with the pre-commit key
|
||||
// so it looks exactly like what the relay would re-deliver.
|
||||
val echoed =
|
||||
outbound.buildCommitEvent(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = addEden.framedCommitBytes,
|
||||
exporterKey = addEden.preCommitExporterSecret,
|
||||
)
|
||||
val result = inbound.processGroupEvent(echoed.signedEvent)
|
||||
assertIs<GroupEventResult.Duplicate>(
|
||||
result,
|
||||
"echoed already-applied commit must be treated as a no-op Duplicate",
|
||||
)
|
||||
|
||||
val postEchoEpoch = davidMgr2.getGroup(groupId)!!.epoch
|
||||
val postEchoExporter = davidMgr2.exporterSecret(groupId)
|
||||
|
||||
assertEquals(
|
||||
preRestartEpoch,
|
||||
postEchoEpoch,
|
||||
"processing our own already-applied commit echo must NOT advance the local epoch",
|
||||
)
|
||||
kotlin.test.assertContentEquals(
|
||||
preRestartExporter,
|
||||
postEchoExporter,
|
||||
"processing our own already-applied commit echo must NOT change the exporter secret",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCreatorCanSendMessageAfterRestart() {
|
||||
// Reproduces production symptom: David creates a group, adds Eden and
|
||||
// Fred, sends messages — all fine. After David restarts the app
|
||||
// (simulated here by save-state + fresh MlsGroupManager + restoreAll),
|
||||
// his outbound kind:445 outer ChaCha20 layer suddenly uses a
|
||||
// different key than Eden/Fred, and they fail with
|
||||
// "Outer decryption failed with current and N retained epoch key(s)".
|
||||
runBlocking {
|
||||
val davidStore = TestGroupStateStore()
|
||||
val davidMgr = MlsGroupManager(davidStore)
|
||||
val edenMgr = MlsGroupManager(TestGroupStateStore())
|
||||
val fredMgr = MlsGroupManager(TestGroupStateStore())
|
||||
|
||||
val davidId = ByteArray(32) { 0xD1.toByte() }
|
||||
val edenId = ByteArray(32) { 0xE2.toByte() }
|
||||
val fredId = ByteArray(32) { 0xF3.toByte() }
|
||||
|
||||
davidMgr.createGroup(groupId, davidId)
|
||||
davidMgr.updateGroupExtensions(
|
||||
nostrGroupId = groupId,
|
||||
extensions =
|
||||
listOf(
|
||||
com.vitorpamplona.quartz.marmot.mip01Groups
|
||||
.MarmotGroupData(
|
||||
nostrGroupId = groupId,
|
||||
adminPubkeys = listOf(davidId.toHexKey()),
|
||||
).toExtension(),
|
||||
),
|
||||
)
|
||||
val davidGroup = davidMgr.getGroup(groupId)!!
|
||||
val edenBundle = davidGroup.createKeyPackage(edenId, ByteArray(0))
|
||||
val fredBundle = davidGroup.createKeyPackage(fredId, ByteArray(0))
|
||||
|
||||
// David adds Eden.
|
||||
val addEden = davidMgr.addMember(groupId, edenBundle.keyPackage.toTlsBytes())
|
||||
edenMgr.processWelcome(addEden.welcomeBytes!!, edenBundle)
|
||||
|
||||
// David adds Fred; Eden processes Alice's add-Fred commit.
|
||||
val addFred = davidMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes())
|
||||
fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle)
|
||||
edenMgr.processCommit(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = addFred.commitBytes,
|
||||
senderLeafIndex = davidMgr.getGroup(groupId)!!.leafIndex,
|
||||
confirmationTag = ByteArray(0),
|
||||
)
|
||||
|
||||
// Pre-restart sanity: all three share the same outer exporter key.
|
||||
val preRestartDavidKey = davidMgr.exporterSecret(groupId)
|
||||
kotlin.test.assertContentEquals(
|
||||
preRestartDavidKey,
|
||||
edenMgr.exporterSecret(groupId),
|
||||
"Eden's exporter key should match David's before restart",
|
||||
)
|
||||
kotlin.test.assertContentEquals(
|
||||
preRestartDavidKey,
|
||||
fredMgr.exporterSecret(groupId),
|
||||
"Fred's exporter key should match David's before restart",
|
||||
)
|
||||
|
||||
// Simulate David's app restart: drop his in-memory manager and
|
||||
// rebuild it from the persisted store.
|
||||
val davidMgr2 = MlsGroupManager(davidStore)
|
||||
davidMgr2.restoreAll()
|
||||
|
||||
val postRestartDavidKey = davidMgr2.exporterSecret(groupId)
|
||||
kotlin.test.assertContentEquals(
|
||||
preRestartDavidKey,
|
||||
postRestartDavidKey,
|
||||
"David's exporter key must survive a save+restore round-trip; " +
|
||||
"otherwise Eden/Fred can't decrypt his next outer layer.",
|
||||
)
|
||||
|
||||
// David sends a message post-restart; Eden and Fred must decrypt.
|
||||
val outbound = MarmotOutboundProcessor(davidMgr2)
|
||||
val msg = "hi after restart"
|
||||
val postRestartEvent =
|
||||
outbound.buildGroupEventFromBytes(groupId, msg.encodeToByteArray())
|
||||
|
||||
val edenKey = edenMgr.exporterSecret(groupId)
|
||||
val mlsBytesEden = GroupEventEncryption.decrypt(postRestartEvent.signedEvent.content, edenKey)
|
||||
val edenDecrypted = edenMgr.decrypt(groupId, mlsBytesEden)
|
||||
assertEquals(msg, edenDecrypted.content.decodeToString())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testFredEncryptsAfterJoiningThreeMemberGroup() {
|
||||
// Reproduces the production StackOverflowError: the last-joined member
|
||||
// (Fred at leafIndex=2 in a 3-member group) attempts to encrypt his
|
||||
// first message and the SecretTree.getNodeSecret recursion never
|
||||
// terminates.
|
||||
runBlocking {
|
||||
val aliceMgr = createGroupManager()
|
||||
val bobMgr = createGroupManager()
|
||||
val fredMgr = createGroupManager()
|
||||
|
||||
val aliceIdBytes = ByteArray(32) { 0xA1.toByte() }
|
||||
aliceMgr.createGroup(groupId, aliceIdBytes)
|
||||
aliceMgr.updateGroupExtensions(
|
||||
nostrGroupId = groupId,
|
||||
extensions =
|
||||
listOf(
|
||||
com.vitorpamplona.quartz.marmot.mip01Groups
|
||||
.MarmotGroupData(
|
||||
nostrGroupId = groupId,
|
||||
adminPubkeys = listOf(aliceIdBytes.toHexKey()),
|
||||
).toExtension(),
|
||||
),
|
||||
)
|
||||
val aliceGroup = aliceMgr.getGroup(groupId)!!
|
||||
val bobBundle = aliceGroup.createKeyPackage(ByteArray(32) { 0xB2.toByte() }, ByteArray(0))
|
||||
val fredBundle = aliceGroup.createKeyPackage(ByteArray(32) { 0xF3.toByte() }, ByteArray(0))
|
||||
|
||||
// Alice adds Bob.
|
||||
val addBob = aliceMgr.addMember(groupId, bobBundle.keyPackage.toTlsBytes())
|
||||
bobMgr.processWelcome(addBob.welcomeBytes!!, bobBundle)
|
||||
|
||||
// Alice adds Fred (he joins at leafIndex=2, in a 3-leaf tree).
|
||||
val addFred = aliceMgr.addMember(groupId, fredBundle.keyPackage.toTlsBytes())
|
||||
fredMgr.processWelcome(addFred.welcomeBytes!!, fredBundle)
|
||||
// Bob (existing member at the pre-add-Fred epoch) receives and
|
||||
// applies Alice's add-Fred commit to reach the same epoch as
|
||||
// Alice + Fred. Without this, Bob can't decrypt Fred's subsequent
|
||||
// application messages.
|
||||
bobMgr.processCommit(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = addFred.commitBytes,
|
||||
senderLeafIndex = aliceMgr.getGroup(groupId)!!.leafIndex,
|
||||
confirmationTag = ByteArray(0),
|
||||
)
|
||||
|
||||
// Sanity: Fred is at leafIndex=2, leafCount=3.
|
||||
assertEquals(2, fredMgr.getGroup(groupId)!!.leafIndex)
|
||||
|
||||
// This is where production crashed with StackOverflowError.
|
||||
val fredMsg = "hi from fred"
|
||||
val fredCiphertext = fredMgr.encrypt(groupId, fredMsg.encodeToByteArray())
|
||||
|
||||
// And Alice & Bob must be able to decrypt it.
|
||||
val aliceDecrypted = aliceMgr.decrypt(groupId, fredCiphertext)
|
||||
assertEquals(fredMsg, aliceDecrypted.content.decodeToString())
|
||||
val bobDecrypted = bobMgr.decrypt(groupId, fredCiphertext)
|
||||
assertEquals(fredMsg, bobDecrypted.content.decodeToString())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testAddMemberExposesPreCommitExporterSecret() {
|
||||
// Regression: before the fix, MarmotManager.addMember outer-encrypted
|
||||
// the kind:445 commit with the POST-commit (epoch N+1) exporter key,
|
||||
// which meant existing members at epoch N couldn't decrypt it. The
|
||||
// fix captures the pre-commit key inside MlsGroup.commit() and ships
|
||||
// it back via CommitResult.preCommitExporterSecret so MarmotManager
|
||||
// can pass it to the outbound builder.
|
||||
runBlocking {
|
||||
val manager = createGroupManager()
|
||||
manager.createGroup(groupId, "alice".encodeToByteArray())
|
||||
|
||||
val preEpochExporter = manager.exporterSecret(groupId)
|
||||
val preEpoch = manager.getGroup(groupId)!!.epoch
|
||||
|
||||
val bobBundle =
|
||||
manager
|
||||
.getGroup(groupId)!!
|
||||
.createKeyPackage("bob".encodeToByteArray(), ByteArray(0))
|
||||
val result = manager.addMember(groupId, bobBundle.keyPackage.toTlsBytes())
|
||||
|
||||
// Local state advanced to N+1.
|
||||
assertEquals(preEpoch + 1, manager.getGroup(groupId)!!.epoch)
|
||||
// But the returned pre-commit exporter secret matches what the
|
||||
// group held BEFORE advancing — the key existing members still at
|
||||
// epoch N are holding.
|
||||
kotlin.test.assertContentEquals(
|
||||
preEpochExporter,
|
||||
result.preCommitExporterSecret,
|
||||
)
|
||||
// The post-commit exporter (what groupManager.exporterSecret now
|
||||
// returns) MUST differ — otherwise we haven't actually rotated.
|
||||
kotlin.test.assertFalse(
|
||||
manager.exporterSecret(groupId).contentEquals(preEpochExporter),
|
||||
"post-commit exporter key must differ from the pre-commit one",
|
||||
)
|
||||
assertNotNull(result.welcomeBytes)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testCreatorCanAddTwoMembersAndAllDecryptFollowingMessage() {
|
||||
// End-to-end regression for the David/Eden/Fred scenario: a group
|
||||
// creator (Alice) adds two members (Bob then Carol) in sequence and
|
||||
// then publishes an application message. Both members MUST be able to
|
||||
// decrypt the message. Before the stage/merge fix, the add-Carol
|
||||
// commit was outer-encrypted with the post-commit (epoch 2) key,
|
||||
// leaving Bob stuck at epoch 1 and unable to decrypt any of Alice's
|
||||
// subsequent application messages.
|
||||
runBlocking {
|
||||
val aliceMgr = createGroupManager()
|
||||
val bobMgr = createGroupManager()
|
||||
val carolMgr = createGroupManager()
|
||||
|
||||
// Use 32-byte identities so they fit the MarmotGroupData
|
||||
// admin_pubkeys 32-byte slots.
|
||||
val aliceIdBytes = ByteArray(32) { 0xA1.toByte() }
|
||||
val bobIdBytes = ByteArray(32) { 0xB2.toByte() }
|
||||
val carolIdBytes = ByteArray(32) { 0xC3.toByte() }
|
||||
aliceMgr.createGroup(groupId, aliceIdBytes)
|
||||
// Install the Marmot group data extension so the Welcome carries
|
||||
// the NostrGroupData (MlsGroupManager.processWelcome requires it).
|
||||
aliceMgr.updateGroupExtensions(
|
||||
nostrGroupId = groupId,
|
||||
extensions =
|
||||
listOf(
|
||||
com.vitorpamplona.quartz.marmot.mip01Groups
|
||||
.MarmotGroupData(
|
||||
nostrGroupId = groupId,
|
||||
adminPubkeys = listOf(aliceIdBytes.toHexKey()),
|
||||
).toExtension(),
|
||||
),
|
||||
)
|
||||
val aliceGroup = aliceMgr.getGroup(groupId)!!
|
||||
|
||||
// KeyPackages are just MLS artifacts carrying identity + init key;
|
||||
// createKeyPackage on any group instance produces a usable bundle.
|
||||
val bobBundle = aliceGroup.createKeyPackage(bobIdBytes, ByteArray(0))
|
||||
val carolBundle = aliceGroup.createKeyPackage(carolIdBytes, ByteArray(0))
|
||||
|
||||
val outbound = MarmotOutboundProcessor(aliceMgr)
|
||||
|
||||
// --- Step 1: Alice adds Bob. CommitResult carries the pre-commit
|
||||
// exporter secret so buildCommitEvent can outer-encrypt with
|
||||
// the epoch-N (pre-Bob) key that Bob-less Alice held. ---
|
||||
val addBobResult = aliceMgr.addMember(groupId, bobBundle.keyPackage.toTlsBytes())
|
||||
val addBobCommitEvent =
|
||||
outbound.buildCommitEvent(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = addBobResult.framedCommitBytes,
|
||||
exporterKey = addBobResult.preCommitExporterSecret,
|
||||
)
|
||||
|
||||
// Bob joins via Welcome (emulated: we hand him the Welcome bytes).
|
||||
bobMgr.processWelcome(addBobResult.welcomeBytes!!, bobBundle)
|
||||
|
||||
val epochAfterAddBob = aliceMgr.getGroup(groupId)!!.epoch
|
||||
assertEquals(epochAfterAddBob, bobMgr.getGroup(groupId)!!.epoch)
|
||||
|
||||
// --- Step 2: Alice adds Carol. addCarolResult.preCommitExporterSecret
|
||||
// is the epoch-N (2-member) key that Bob still holds. ---
|
||||
val addCarolResult = aliceMgr.addMember(groupId, carolBundle.keyPackage.toTlsBytes())
|
||||
val addCarolCommitEvent =
|
||||
outbound.buildCommitEvent(
|
||||
nostrGroupId = groupId,
|
||||
commitBytes = addCarolResult.framedCommitBytes,
|
||||
exporterKey = addCarolResult.preCommitExporterSecret,
|
||||
)
|
||||
|
||||
// Carol joins via Welcome at the new epoch.
|
||||
carolMgr.processWelcome(addCarolResult.welcomeBytes!!, carolBundle)
|
||||
|
||||
// *** The key assertion ***: Bob (still at epoch 1) receives the
|
||||
// add-Carol kind:445 and must be able to:
|
||||
// (a) outer-decrypt using his current (epoch 1) exporter key,
|
||||
// (b) parse the MlsMessage → PublicMessage → COMMIT,
|
||||
// (c) apply the commit and advance to epoch 2.
|
||||
val bobExporterAtE1 = bobMgr.exporterSecret(groupId)
|
||||
kotlin.test.assertContentEquals(
|
||||
bobExporterAtE1,
|
||||
addCarolResult.preCommitExporterSecret,
|
||||
"Bob's current exporter (pre-add-Carol) must match the " +
|
||||
"pre-commit key used to outer-encrypt the add-Carol commit; " +
|
||||
"otherwise Bob cannot decrypt and will be stuck on the old epoch.",
|
||||
)
|
||||
|
||||
val decryptedMlsBytes =
|
||||
GroupEventEncryption.decrypt(
|
||||
addCarolCommitEvent.signedEvent.content,
|
||||
bobExporterAtE1,
|
||||
)
|
||||
val mlsMessage =
|
||||
com.vitorpamplona.quartz.marmot.mls.framing.MlsMessage
|
||||
.decodeTls(
|
||||
com.vitorpamplona.quartz.marmot.mls.codec
|
||||
.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
|
||||
assertEquals(epochAfterAddBob + 1, epochAfterAddCarol)
|
||||
assertEquals(epochAfterAddCarol, bobMgr.getGroup(groupId)!!.epoch)
|
||||
assertEquals(epochAfterAddCarol, carolMgr.getGroup(groupId)!!.epoch)
|
||||
|
||||
// --- Step 3: Alice sends an application message. Both members must decrypt. ---
|
||||
val msg = "Hi from Alice"
|
||||
val appEvent =
|
||||
outbound.buildGroupEventFromBytes(groupId, msg.encodeToByteArray())
|
||||
|
||||
val bobKey = bobMgr.exporterSecret(groupId)
|
||||
val carolKey = carolMgr.exporterSecret(groupId)
|
||||
val aliceKey = aliceMgr.exporterSecret(groupId)
|
||||
kotlin.test.assertContentEquals(aliceKey, bobKey, "Bob must share Alice's epoch-2 key")
|
||||
kotlin.test.assertContentEquals(aliceKey, carolKey, "Carol must share Alice's epoch-2 key")
|
||||
|
||||
val bobMlsBytes = GroupEventEncryption.decrypt(appEvent.signedEvent.content, bobKey)
|
||||
val bobDecrypted = bobMgr.decrypt(groupId, bobMlsBytes)
|
||||
assertEquals(msg, bobDecrypted.content.decodeToString())
|
||||
|
||||
val carolMlsBytes = GroupEventEncryption.decrypt(appEvent.signedEvent.content, carolKey)
|
||||
val carolDecrypted = carolMgr.decrypt(groupId, carolMlsBytes)
|
||||
assertEquals(msg, carolDecrypted.content.decodeToString())
|
||||
|
||||
// --- Alice also receives her own echo (as the app does via the
|
||||
// relay round-trip); this exercises the secretTree-based decrypt
|
||||
// path with myLeafIndex != sender or when sentKeys misses.
|
||||
val aliceMlsBytes = GroupEventEncryption.decrypt(appEvent.signedEvent.content, aliceKey)
|
||||
val aliceDecrypted = aliceMgr.decrypt(groupId, aliceMlsBytes)
|
||||
assertEquals(msg, aliceDecrypted.content.decodeToString())
|
||||
|
||||
// Send a second and third message to stress the secretTree ratchet
|
||||
// and ensure getNodeSecret stays terminating across generations.
|
||||
for (i in 1..3) {
|
||||
val m = "msg-$i"
|
||||
val ev = outbound.buildGroupEventFromBytes(groupId, m.encodeToByteArray())
|
||||
val keyNow = aliceMgr.exporterSecret(groupId)
|
||||
val bytes = GroupEventEncryption.decrypt(ev.signedEvent.content, keyNow)
|
||||
val dec = bobMgr.decrypt(groupId, bytes)
|
||||
assertEquals(m, dec.content.decodeToString())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun testFullRoundtripEncryptDecrypt() {
|
||||
runBlocking {
|
||||
|
||||
+5
-4
@@ -225,9 +225,11 @@ class MlsGroupEdgeCaseTest {
|
||||
// 6. Multiple epochs of encrypt/decrypt
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
// BUG: processCommit key derivation diverges — commit_secret decryption from
|
||||
// UpdatePath does not correctly derive matching epoch secrets between commit()
|
||||
// and processCommit(). See MlsGroupLifecycleTest.testThreeMemberGroup_SequentialAdditions.
|
||||
// BUG: same empty-commit (no proposals, pure UpdatePath) divergence as
|
||||
// MlsGroupLifecycleTest.testEmptyCommit_AdvancesEpoch. The
|
||||
// SecretTree.getNodeSecret non-full-tree fix doesn't address this — after
|
||||
// 5 empty commits Alice and Bob's ratchet secrets diverge and AEAD
|
||||
// decryption fails with "Tag mismatch". Tracked separately.
|
||||
@Ignore
|
||||
@Test
|
||||
fun testMultipleEpochTransitions_EncryptDecryptStillWorks() {
|
||||
@@ -242,7 +244,6 @@ class MlsGroupEdgeCaseTest {
|
||||
bob.processCommit(commitResult.commitBytes, alice.leafIndex, ByteArray(0))
|
||||
}
|
||||
|
||||
assertEquals(7L, alice.epoch) // epoch 0 + addMember(1) + 5 commits = 6... wait
|
||||
// epoch 0 (create) -> epoch 1 (add bob) -> 5 empty commits = epoch 6
|
||||
assertEquals(6L, alice.epoch)
|
||||
assertEquals(alice.epoch, bob.epoch)
|
||||
|
||||
+5
-7
@@ -176,7 +176,6 @@ class MlsGroupLifecycleTest {
|
||||
// because the commit_secret decryption from the UpdatePath does not
|
||||
// correctly walk the ratchet tree to find the common ancestor's path secret.
|
||||
// This causes AEAD decryption failures on cross-member messages.
|
||||
@Ignore
|
||||
@Test
|
||||
fun testThreeMemberGroup_SequentialAdditions() {
|
||||
// Alice creates the group
|
||||
@@ -243,7 +242,6 @@ class MlsGroupLifecycleTest {
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
@Ignore
|
||||
@Test
|
||||
fun testExternalJoin_ZaraJoinsViaGroupInfo() {
|
||||
val alice = MlsGroup.create("alice".encodeToByteArray())
|
||||
@@ -266,7 +264,6 @@ class MlsGroupLifecycleTest {
|
||||
}
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
@Ignore
|
||||
@Test
|
||||
fun testExternalJoin_ExporterSecretsAgree() {
|
||||
val alice = MlsGroup.create("alice".encodeToByteArray())
|
||||
@@ -312,7 +309,6 @@ class MlsGroupLifecycleTest {
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
@Ignore
|
||||
@Test
|
||||
fun testSigningKeyRotation_EpochAdvances() {
|
||||
val alice = MlsGroup.create("alice".encodeToByteArray())
|
||||
@@ -342,7 +338,6 @@ class MlsGroupLifecycleTest {
|
||||
}
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
@Ignore
|
||||
@Test
|
||||
fun testEncryptDecryptAfterSigningKeyRotation() {
|
||||
val alice = MlsGroup.create("alice".encodeToByteArray())
|
||||
@@ -412,7 +407,6 @@ class MlsGroupLifecycleTest {
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
@Ignore
|
||||
@Test
|
||||
fun testPskProposal_EpochAdvancesWithPsk() {
|
||||
val alice = MlsGroup.create("alice".encodeToByteArray())
|
||||
@@ -472,7 +466,11 @@ class MlsGroupLifecycleTest {
|
||||
// 12. Empty commit (no proposals, just UpdatePath for forward secrecy)
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
// BUG: processCommit key derivation diverges — see testThreeMemberGroup_SequentialAdditions
|
||||
// BUG: empty-commit (no proposals, pure UpdatePath) diverges Alice's and
|
||||
// Bob's exporter secrets after processCommit. Unrelated to the SecretTree
|
||||
// non-full-tree fix — the other @Ignored "processCommit diverges" tests in
|
||||
// this file and MlsGroupEdgeCaseTest now pass, but this one still fails.
|
||||
// Tracked separately.
|
||||
@Ignore
|
||||
@Test
|
||||
fun testEmptyCommit_AdvancesEpoch() {
|
||||
|
||||
Reference in New Issue
Block a user