fix(marmot): receive standalone SelfRemove proposals (test 15)
Two bugs that conspired to break interop test 15 once the test 14 OOM
was fixed:
1. **No receive path for standalone PublicMessage proposals.**
wn/openmls publishes a non-admin's `SelfRemove` as a kind:445 carrying
a `PublicMessage(content_type=PROPOSAL)` envelope and waits for an
admin to fold it into the next commit. Quartz's `MarmotInboundProcessor`
answered every such event with `Error("Standalone proposals not yet
supported")` and dropped it. The admin's subsequent commit then
failed with `Commit references unknown proposal (ref not found in
pending proposals)` because nobody had staged the SelfRemove.
Add `MlsGroup.receivePublicMessageProposal(pubMsg)` that:
- rejects mismatched epoch / group_id / sender,
- reconstructs the FramedContentTBS exactly as the proposer did and
verifies the leaf signature,
- verifies the membership_tag against the current epoch's
membership_key (same threat model as inbound PublicMessage commits),
- decodes the inner Proposal (only `SelfRemove` is accepted today —
other types come bundled in commits' `proposals` lists),
- stages the proposal in `pendingProposals` so a later commit can
resolve its `ProposalRef`.
`processPublicMessage` now routes `ContentType.PROPOSAL` through
that helper and returns a new `GroupEventResult.ProposalStaged`
variant (also surfaced as `MarmotIngestResult.ProposalStaged`),
replacing the old hard-error path.
2. **Wrong `ProposalRef` hash input.** RFC 9420 §5.2 specifies that a
`ProposalRef` hashes the **encoded `AuthenticatedContent`** that
delivered the proposal, not the bare `Proposal` struct. Quartz was
hashing `proposal.toTlsBytes()` only — fine for our local-only
flows where commits inline rather than reference our own pending
proposals, but fatal once we needed to match wn's reference to an
inbound proposal.
Extend `PendingProposal` with an optional `authenticatedContentBytes`
field. The standalone-proposal receive path captures the full
`wire_format || FramedContent || FramedContentAuthData` envelope at
stage time. The reference-resolution code in `processCommitInner`
prefers those bytes when present and falls back to the bare-proposal
hash for locally-proposed entries (which never get referenced
today).
Marmot interop score: 14/16 → 15/16 (test 9 — amy's kind:7 reaction
triggers a `SecretReuseError` on B's wn — is unrelated to this path
and remains for follow-up).
https://claude.ai/code/session_013VYkpz8P1mPh9Ejxy9anhJ
This commit is contained in:
+15
@@ -61,6 +61,17 @@ sealed class MarmotIngestResult {
|
|||||||
val inner: GroupEventResult.CommitProcessed,
|
val inner: GroupEventResult.CommitProcessed,
|
||||||
) : MarmotIngestResult()
|
) : MarmotIngestResult()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A kind:445 carried a standalone Proposal (currently only `SelfRemove`)
|
||||||
|
* which was staged in the group's pending pool. The group epoch did
|
||||||
|
* not advance — a later Commit referencing this proposal will pick
|
||||||
|
* it up.
|
||||||
|
*/
|
||||||
|
data class ProposalStaged(
|
||||||
|
val groupId: HexKey,
|
||||||
|
val senderLeafIndex: Int,
|
||||||
|
) : MarmotIngestResult()
|
||||||
|
|
||||||
/** A kind:445 whose outer layer we couldn't decrypt (pre-join epoch, etc). Debug-only. */
|
/** A kind:445 whose outer layer we couldn't decrypt (pre-join epoch, etc). Debug-only. */
|
||||||
data class UndecryptableOuter(
|
data class UndecryptableOuter(
|
||||||
val groupId: HexKey,
|
val groupId: HexKey,
|
||||||
@@ -138,6 +149,10 @@ private suspend fun MarmotManager.ingestGroupEvent(ge: GroupEvent): MarmotIngest
|
|||||||
MarmotIngestResult.Commit(result)
|
MarmotIngestResult.Commit(result)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
is GroupEventResult.ProposalStaged -> {
|
||||||
|
MarmotIngestResult.ProposalStaged(result.groupId, result.senderLeafIndex)
|
||||||
|
}
|
||||||
|
|
||||||
is GroupEventResult.Duplicate,
|
is GroupEventResult.Duplicate,
|
||||||
is GroupEventResult.CommitPending,
|
is GroupEventResult.CommitPending,
|
||||||
-> {
|
-> {
|
||||||
|
|||||||
+4
@@ -113,6 +113,10 @@ class MarmotManager(
|
|||||||
subscriptionManager.updateGroupSince(result.groupId, groupEvent.createdAt)
|
subscriptionManager.updateGroupSince(result.groupId, groupEvent.createdAt)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
is GroupEventResult.ProposalStaged -> {
|
||||||
|
subscriptionManager.updateGroupSince(result.groupId, groupEvent.createdAt)
|
||||||
|
}
|
||||||
|
|
||||||
is GroupEventResult.CommitPending,
|
is GroupEventResult.CommitPending,
|
||||||
is GroupEventResult.Duplicate,
|
is GroupEventResult.Duplicate,
|
||||||
is GroupEventResult.UndecryptableOuterLayer,
|
is GroupEventResult.UndecryptableOuterLayer,
|
||||||
|
|||||||
+30
-1
@@ -94,6 +94,17 @@ sealed class GroupEventResult {
|
|||||||
val retainedEpochCount: Int,
|
val retainedEpochCount: Int,
|
||||||
) : GroupEventResult()
|
) : GroupEventResult()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A standalone Proposal (currently only `SelfRemove`) was decoded,
|
||||||
|
* verified, and staged in the group's pending-proposals pool. The
|
||||||
|
* group epoch did not advance — the proposal is dormant until a
|
||||||
|
* subsequent Commit references it by hash.
|
||||||
|
*/
|
||||||
|
data class ProposalStaged(
|
||||||
|
val groupId: HexKey,
|
||||||
|
val senderLeafIndex: Int,
|
||||||
|
) : GroupEventResult()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The event could not be processed.
|
* The event could not be processed.
|
||||||
*/
|
*/
|
||||||
@@ -474,7 +485,25 @@ class MarmotInboundProcessor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
ContentType.PROPOSAL -> {
|
ContentType.PROPOSAL -> {
|
||||||
GroupEventResult.Error(groupId, "Standalone proposals not yet supported")
|
// wn/openmls publishes SelfRemove as a standalone PublicMessage
|
||||||
|
// proposal — admins fold it into their next commit. Stage it
|
||||||
|
// locally so a subsequent commit's `ProposalRef` can resolve;
|
||||||
|
// without this every other member silently dropped the
|
||||||
|
// proposal and the admin's commit then failed with "Commit
|
||||||
|
// references unknown proposal" (marmot-interop test 15).
|
||||||
|
val group =
|
||||||
|
groupManager.getGroup(groupId)
|
||||||
|
?: return GroupEventResult.Error(groupId, "Group not found")
|
||||||
|
try {
|
||||||
|
group.receivePublicMessageProposal(pubMsg)
|
||||||
|
GroupEventResult.ProposalStaged(groupId, pubMsg.sender.leafIndex)
|
||||||
|
} catch (e: Exception) {
|
||||||
|
GroupEventResult.Error(
|
||||||
|
groupId,
|
||||||
|
"Failed to stage standalone proposal: ${e.message}",
|
||||||
|
e,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ContentType.APPLICATION -> {
|
ContentType.APPLICATION -> {
|
||||||
|
|||||||
+136
-2
@@ -1260,8 +1260,18 @@ class MlsGroup private constructor(
|
|||||||
val refHash = proposalOrRef.proposalRef
|
val refHash = proposalOrRef.proposalRef
|
||||||
val resolved =
|
val resolved =
|
||||||
pendingProposals.find { pending ->
|
pendingProposals.find { pending ->
|
||||||
val proposalBytes = pending.proposal.toTlsBytes()
|
// RFC 9420 §5.2: a ProposalRef hashes the encoded
|
||||||
val hash = MlsCryptoProvider.refHash("MLS 1.0 Proposal Reference", proposalBytes)
|
// AuthenticatedContent that delivered the proposal,
|
||||||
|
// not just the bare Proposal struct. For inbound
|
||||||
|
// proposals (e.g. C's standalone SelfRemove) we
|
||||||
|
// captured those bytes at receive time. For local
|
||||||
|
// proposals (which we always inline in our own
|
||||||
|
// commits, never reference) we fall back to the
|
||||||
|
// bare-proposal hash — keeps existing flows
|
||||||
|
// working without needing an AC reconstruction
|
||||||
|
// round-trip on the send side.
|
||||||
|
val refValue = pending.authenticatedContentBytes ?: pending.proposal.toTlsBytes()
|
||||||
|
val hash = MlsCryptoProvider.refHash("MLS 1.0 Proposal Reference", refValue)
|
||||||
hash.contentEquals(refHash)
|
hash.contentEquals(refHash)
|
||||||
}
|
}
|
||||||
requireNotNull(resolved) {
|
requireNotNull(resolved) {
|
||||||
@@ -3083,11 +3093,135 @@ class MlsGroup private constructor(
|
|||||||
)
|
)
|
||||||
return MlsMessage.fromPublicMessage(publicMessage).toTlsBytes() to preCommitExporterSecret
|
return MlsMessage.fromPublicMessage(publicMessage).toTlsBytes() to preCommitExporterSecret
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Receive a standalone-proposal PublicMessage and stage it locally so a
|
||||||
|
* subsequent commit that references it (via `ProposalRef`) can resolve.
|
||||||
|
*
|
||||||
|
* Sent today by `wn`/openmls when a non-admin member self-removes — the
|
||||||
|
* member can't commit themselves out (admins reject `RequiredPathNotFound`),
|
||||||
|
* so they publish a `SelfRemove` proposal as a `PublicMessage` and wait
|
||||||
|
* for an admin to fold it into the next commit. Without this receiver
|
||||||
|
* side every other member silently drops the proposal, and the admin's
|
||||||
|
* subsequent commit then fails with "Commit references unknown proposal
|
||||||
|
* (ref not found in pending proposals)" — which is exactly the failure
|
||||||
|
* mode of marmot-interop test 15.
|
||||||
|
*
|
||||||
|
* Validation mirrors what every member-sender PublicMessage commit goes
|
||||||
|
* through: epoch + group_id match the current epoch, FramedContentTBS
|
||||||
|
* signature verifies against the sender's leaf signing key, and the
|
||||||
|
* membership_tag verifies against the current epoch's membership key.
|
||||||
|
* Anything missing or mismatched is a hard reject — the same threat
|
||||||
|
* model as PublicMessage commit reception.
|
||||||
|
*/
|
||||||
|
fun receivePublicMessageProposal(pubMsg: PublicMessage) {
|
||||||
|
require(pubMsg.contentType == ContentType.PROPOSAL) {
|
||||||
|
"Expected PublicMessage with content_type == PROPOSAL, got ${pubMsg.contentType}"
|
||||||
|
}
|
||||||
|
require(pubMsg.epoch == groupContext.epoch) {
|
||||||
|
"Proposal epoch ${pubMsg.epoch} doesn't match current epoch ${groupContext.epoch}"
|
||||||
|
}
|
||||||
|
require(pubMsg.groupId.contentEquals(groupContext.groupId)) {
|
||||||
|
"Proposal group_id doesn't match current group"
|
||||||
|
}
|
||||||
|
require(pubMsg.sender.senderType == SenderType.MEMBER) {
|
||||||
|
"Standalone proposals from non-members are not accepted"
|
||||||
|
}
|
||||||
|
val senderLeafIndex = pubMsg.sender.leafIndex
|
||||||
|
require(senderLeafIndex in 0 until tree.leafCount) {
|
||||||
|
"Sender leaf index $senderLeafIndex out of range"
|
||||||
|
}
|
||||||
|
val senderLeaf =
|
||||||
|
requireNotNull(tree.getLeaf(senderLeafIndex)) {
|
||||||
|
"Sender leaf is blank at index $senderLeafIndex"
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reconstruct FramedContentTBS exactly as the sender did in
|
||||||
|
// `buildSelfRemoveProposalMessage` so the signature and the
|
||||||
|
// membership_tag both verify against bit-identical bytes.
|
||||||
|
val tbsWriter = TlsWriter()
|
||||||
|
tbsWriter.putUint16(MlsMessage.MLS_VERSION_10)
|
||||||
|
tbsWriter.putUint16(WireFormat.PUBLIC_MESSAGE.value)
|
||||||
|
tbsWriter.putOpaqueVarInt(pubMsg.groupId)
|
||||||
|
tbsWriter.putUint64(pubMsg.epoch)
|
||||||
|
encodeSender(tbsWriter, pubMsg.sender)
|
||||||
|
tbsWriter.putOpaqueVarInt(pubMsg.authenticatedData)
|
||||||
|
tbsWriter.putUint8(ContentType.PROPOSAL.value)
|
||||||
|
tbsWriter.putBytes(pubMsg.content) // proposal struct, no length prefix
|
||||||
|
tbsWriter.putBytes(groupContext.toTlsBytes())
|
||||||
|
val tbs = tbsWriter.toByteArray()
|
||||||
|
|
||||||
|
require(MlsCryptoProvider.verifyWithLabel(senderLeaf.signatureKey, "FramedContentTBS", tbs, pubMsg.signature)) {
|
||||||
|
"Invalid FramedContentTBS signature on PublicMessage proposal from leaf $senderLeafIndex"
|
||||||
|
}
|
||||||
|
|
||||||
|
val membershipTag =
|
||||||
|
requireNotNull(pubMsg.membershipTag) {
|
||||||
|
"PublicMessage proposal from leaf $senderLeafIndex is missing membership_tag"
|
||||||
|
}
|
||||||
|
val tbmWriter = TlsWriter()
|
||||||
|
tbmWriter.putBytes(tbs)
|
||||||
|
tbmWriter.putOpaqueVarInt(pubMsg.signature)
|
||||||
|
require(verifyMembershipTag(tbmWriter.toByteArray(), membershipTag)) {
|
||||||
|
"Invalid membership_tag on PublicMessage proposal from leaf $senderLeafIndex"
|
||||||
|
}
|
||||||
|
|
||||||
|
val proposal = Proposal.decodeTls(TlsReader(pubMsg.content))
|
||||||
|
// SelfRemove is the only proposal type wn currently emits as a
|
||||||
|
// standalone PublicMessage. Other types come bundled inside a
|
||||||
|
// commit's `proposals` list. Defending against e.g. a standalone
|
||||||
|
// Add/Remove here would also work — they'd land in pendingProposals
|
||||||
|
// and be picked up by the next commit that references them — but we
|
||||||
|
// refuse anything other than SelfRemove for now to keep the receive
|
||||||
|
// side minimal until there's an interop reason to widen it.
|
||||||
|
require(proposal is Proposal.SelfRemove) {
|
||||||
|
"Only standalone SelfRemove proposals are accepted; got ${proposal::class.simpleName}"
|
||||||
|
}
|
||||||
|
|
||||||
|
// Capture the AuthenticatedContent bytes (RFC 9420 §6.1) so a
|
||||||
|
// subsequent commit's `ProposalRef` lookup can hash them per §5.2:
|
||||||
|
//
|
||||||
|
// AuthenticatedContent = wire_format || FramedContent || FramedContentAuthData
|
||||||
|
//
|
||||||
|
// FramedContentAuthData for a PROPOSAL is just `signature` (no
|
||||||
|
// confirmation_tag). The TBS prefix (version + GroupContext) is
|
||||||
|
// NOT part of the AuthenticatedContent — those go into the
|
||||||
|
// signature input only.
|
||||||
|
val acWriter = TlsWriter()
|
||||||
|
acWriter.putUint16(WireFormat.PUBLIC_MESSAGE.value)
|
||||||
|
// FramedContent
|
||||||
|
acWriter.putOpaqueVarInt(pubMsg.groupId)
|
||||||
|
acWriter.putUint64(pubMsg.epoch)
|
||||||
|
encodeSender(acWriter, pubMsg.sender)
|
||||||
|
acWriter.putOpaqueVarInt(pubMsg.authenticatedData)
|
||||||
|
acWriter.putUint8(ContentType.PROPOSAL.value)
|
||||||
|
acWriter.putBytes(pubMsg.content)
|
||||||
|
// FramedContentAuthData (PROPOSAL: signature only)
|
||||||
|
acWriter.putOpaqueVarInt(pubMsg.signature)
|
||||||
|
val authenticatedContentBytes = acWriter.toByteArray()
|
||||||
|
|
||||||
|
pendingProposals.add(
|
||||||
|
PendingProposal(
|
||||||
|
proposal = proposal,
|
||||||
|
senderLeafIndex = senderLeafIndex,
|
||||||
|
authenticatedContentBytes = authenticatedContentBytes,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
data class PendingProposal(
|
data class PendingProposal(
|
||||||
val proposal: Proposal,
|
val proposal: Proposal,
|
||||||
val senderLeafIndex: Int,
|
val senderLeafIndex: Int,
|
||||||
|
/**
|
||||||
|
* Encoded `AuthenticatedContent` bytes for the message that delivered
|
||||||
|
* this proposal — the input to `MakeProposalRef` per RFC 9420 §5.2.
|
||||||
|
* Null when the proposal was created locally and never went through
|
||||||
|
* the MLS framing layer; in that case the lookup falls back to the
|
||||||
|
* bare `proposal.toTlsBytes()` since local commits inline rather than
|
||||||
|
* reference our own pending proposals.
|
||||||
|
*/
|
||||||
|
val authenticatedContentBytes: ByteArray? = null,
|
||||||
)
|
)
|
||||||
|
|
||||||
data class DecryptedMessage(
|
data class DecryptedMessage(
|
||||||
|
|||||||
Reference in New Issue
Block a user