mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-12 01:07:46 +00:00
fix(marmot): enforce required_capabilities on Add and Welcome (RFC 9420 §7.2)
When a group installs a `required_capabilities` extension (every Marmot group does, listing MarmotGroupData=0xF2EE, SelfRemove=0x000A, Basic credential), every member's leaf MUST advertise those types in its own [Capabilities]. We were validating none of that: - `applyProposalAdd` checked version + ciphersuite but never matched the new KP's capabilities against the group's required_capabilities. A non-conformant member silently joined; the next commit that touched their leaf got rejected by spec-conformant peers, splitting the group. - `processWelcome` similarly never checked the joiner's own KP against the group's required set, nor did it sweep existing members' leaves. A misconfigured GroupInfo signer could invite us into an incoherent group whose first commit we'd silently reject forever. Adds two helpers in `MlsGroup.Companion`: - `findRequiredCapabilities(extensions)`: decodes the §7.2 struct from a GroupContext extension list, or returns null if absent. - `requireCapabilitiesMeetRequirements(caps, req, who)`: throws with a specific (extensions=, proposals=, credentials=) diff naming the missing types — turns silent interop breaks into one debuggable line. Wires the gate in two places: - `applyProposalAdd` rejects the Add proposal if the new leaf falls short of the group's required set. - `processWelcome` rejects the join if either (a) our own KP doesn't meet the group's requirements or (b) any existing member's leaf doesn't — the latter catches a malformed GroupInfo at join time. 5 new tests: round-trip decoding, rejection on missing extension / proposal, acceptance when caps are a superset, and an end-to-end `addMember` rejection of a hand-crafted KP with SelfRemove stripped (re-signed so the rejection is from the capability gate, not the signature check). Quartz test suite green; marmot interop 16/16. Closes audit gaps #4, #5, #10. https://claude.ai/code/session_013VYkpz8P1mPh9Ejxy9anhJ
This commit is contained in:
Claude
@@ -2053,6 +2053,16 @@ class MlsGroup private constructor(
|
||||
"KeyPackage does not support ciphersuite 0x0001"
|
||||
}
|
||||
|
||||
// RFC 9420 §12.4.2: an Add proposal MUST be rejected if the new
|
||||
// member's leaf capabilities don't advertise every type listed in
|
||||
// the group's `required_capabilities` extension. Without this gate
|
||||
// a non-conformant member silently joins, and any subsequent commit
|
||||
// that touches their leaf is rejected by peers that DO enforce the
|
||||
// requirement — splitting the group on the next epoch.
|
||||
findRequiredCapabilities(groupContext.extensions)?.let { req ->
|
||||
requireCapabilitiesMeetRequirements(caps, req, "Add KeyPackage leaf")
|
||||
}
|
||||
|
||||
return tree.addLeaf(leafNode)
|
||||
}
|
||||
|
||||
@@ -2545,6 +2555,67 @@ class MlsGroup private constructor(
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Parsed view of the RFC 9420 §7.2 `required_capabilities` extension.
|
||||
*
|
||||
* The on-wire struct is three `uint16<V>` vectors —
|
||||
* `extensions / proposals / credentials` — that name the types every
|
||||
* member of the group MUST advertise in their leaf [Capabilities].
|
||||
*/
|
||||
internal data class RequiredCapabilities(
|
||||
val extensions: List<Int>,
|
||||
val proposals: List<Int>,
|
||||
val credentials: List<Int>,
|
||||
)
|
||||
|
||||
/**
|
||||
* Decode the `required_capabilities` extension from the GroupContext
|
||||
* extension list, or `null` if the group hasn't installed one (some
|
||||
* peers may omit it; treat missing as "no restriction").
|
||||
*/
|
||||
internal fun findRequiredCapabilities(extensions: List<Extension>): RequiredCapabilities? {
|
||||
val ext = extensions.find { it.extensionType == REQUIRED_CAPABILITIES_EXTENSION_TYPE } ?: return null
|
||||
val r = TlsReader(ext.extensionData)
|
||||
|
||||
fun readUint16List(data: ByteArray): List<Int> {
|
||||
val rr = TlsReader(data)
|
||||
val list = mutableListOf<Int>()
|
||||
while (rr.hasRemaining) list.add(rr.readUint16())
|
||||
return list
|
||||
}
|
||||
val exts = readUint16List(r.readOpaqueVarInt())
|
||||
val props = readUint16List(r.readOpaqueVarInt())
|
||||
val creds = readUint16List(r.readOpaqueVarInt())
|
||||
return RequiredCapabilities(exts, props, creds)
|
||||
}
|
||||
|
||||
/**
|
||||
* RFC 9420 §7.2 + §12.4.2: every member's leaf [Capabilities] MUST
|
||||
* advertise every type listed in the group's `required_capabilities`
|
||||
* extension. Adding a member whose KeyPackage doesn't meet the
|
||||
* requirement leaves the group in a non-conformant state where
|
||||
* peers that DO enforce the requirement will reject any commit that
|
||||
* touches that leaf.
|
||||
*
|
||||
* Throws [IllegalStateException] with a precise diff so debugging
|
||||
* an interop break against another implementation is one log line.
|
||||
*/
|
||||
internal fun requireCapabilitiesMeetRequirements(
|
||||
caps: Capabilities,
|
||||
req: RequiredCapabilities,
|
||||
who: String,
|
||||
) {
|
||||
val missingExts = req.extensions.filter { it !in caps.extensions }
|
||||
val missingProps = req.proposals.filter { it !in caps.proposals }
|
||||
val missingCreds = req.credentials.filter { it !in caps.credentials }
|
||||
if (missingExts.isNotEmpty() || missingProps.isNotEmpty() || missingCreds.isNotEmpty()) {
|
||||
throw IllegalStateException(
|
||||
"$who capabilities don't meet required_capabilities: " +
|
||||
"missing extensions=$missingExts proposals=$missingProps credentials=$missingCreds",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Default MLS leaf Capabilities that advertise support for Marmot's
|
||||
* required extensions and proposals so new members can join a group
|
||||
@@ -2734,6 +2805,26 @@ class MlsGroup private constructor(
|
||||
"GroupInfo tree_hash does not match ratchet_tree extension"
|
||||
}
|
||||
|
||||
// RFC 9420 §7.2 + §12.4.3.1: a joiner MUST refuse to join a
|
||||
// group whose `required_capabilities` lists types the joiner's
|
||||
// own KeyPackage doesn't advertise — peers that DO enforce the
|
||||
// requirement would reject every commit the joiner produces
|
||||
// anyway. Catching this at join time turns a silent "your
|
||||
// commits get dropped forever" into an actionable error.
|
||||
findRequiredCapabilities(groupContext.extensions)?.let { req ->
|
||||
val myLeaf = tree.getLeaf(myLeafIndex)
|
||||
requireNotNull(myLeaf) { "Joiner's leaf is blank after tree reconstruction" }
|
||||
requireCapabilitiesMeetRequirements(myLeaf.capabilities, req, "Joiner KeyPackage")
|
||||
// Mirror the same check across every existing member —
|
||||
// if a peer's leaf doesn't meet the group's stated
|
||||
// requirements, the GroupInfo signer mis-installed the
|
||||
// requirement set and the group is incoherent.
|
||||
for (i in 0 until tree.leafCount) {
|
||||
val leaf = tree.getLeaf(i) ?: continue
|
||||
requireCapabilitiesMeetRequirements(leaf.capabilities, req, "Member leaf $i")
|
||||
}
|
||||
}
|
||||
|
||||
// Derive epoch secrets directly from memberSecret (RFC 9420 Section 8.3)
|
||||
// For Welcome, epoch_secret = ExpandWithLabel(member_secret, "epoch", GroupContext, Nh)
|
||||
val epochSecret =
|
||||
|
||||
+149
@@ -551,6 +551,155 @@ class MarmotMipBehaviorTest {
|
||||
)
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// RFC 9420 §7.2 / §12.4.2 required_capabilities enforcement
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Round-trip: the `required_capabilities` extension Marmot installs on
|
||||
* every fresh group decodes back to its declared (extensions, proposals,
|
||||
* credentials) triple.
|
||||
*/
|
||||
@Test
|
||||
fun findRequiredCapabilities_decodesMarmotExtensionInstalledByCreate() {
|
||||
val alice = MlsGroup.create(aliceId.hexToByteArray())
|
||||
val req =
|
||||
MlsGroup.findRequiredCapabilities(alice.extensions)
|
||||
?: error("required_capabilities must be present after create()")
|
||||
assertEquals(listOf(0xF2EE), req.extensions, "MarmotGroupData (0xF2EE) must be required")
|
||||
assertEquals(listOf(0x000A), req.proposals, "SelfRemove (0x000A) must be required")
|
||||
assertEquals(listOf(0x0001), req.credentials, "Basic credential must be required")
|
||||
}
|
||||
|
||||
/**
|
||||
* `requireCapabilitiesMeetRequirements` must throw when ANY required
|
||||
* type is missing — extension OR proposal OR credential — and the
|
||||
* error must name the missing types so an interop debugger can
|
||||
* diagnose without grepping.
|
||||
*/
|
||||
@Test
|
||||
fun requireCapabilitiesMeetRequirements_rejectsMissingExtension() {
|
||||
val req =
|
||||
MlsGroup.Companion.RequiredCapabilities(
|
||||
extensions = listOf(0xF2EE),
|
||||
proposals = listOf(0x000A),
|
||||
credentials = listOf(0x0001),
|
||||
)
|
||||
// Missing 0xF2EE.
|
||||
val caps =
|
||||
com.vitorpamplona.quartz.marmot.mls.tree.Capabilities(
|
||||
extensions = emptyList(),
|
||||
proposals = listOf(0x000A),
|
||||
credentials = listOf(0x0001),
|
||||
)
|
||||
val ex =
|
||||
assertFailsWith<IllegalStateException> {
|
||||
MlsGroup.requireCapabilitiesMeetRequirements(caps, req, "test")
|
||||
}
|
||||
assertTrue(
|
||||
ex.message!!.contains("extensions=[62190]") || ex.message!!.contains("0xF2EE"),
|
||||
"error must name the missing extension type: ${ex.message}",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun requireCapabilitiesMeetRequirements_rejectsMissingProposal() {
|
||||
val req =
|
||||
MlsGroup.Companion.RequiredCapabilities(
|
||||
extensions = emptyList(),
|
||||
proposals = listOf(0x000A),
|
||||
credentials = emptyList(),
|
||||
)
|
||||
val caps =
|
||||
com.vitorpamplona.quartz.marmot.mls.tree.Capabilities(
|
||||
extensions = emptyList(),
|
||||
proposals = emptyList(),
|
||||
credentials = listOf(0x0001),
|
||||
)
|
||||
assertFailsWith<IllegalStateException> {
|
||||
MlsGroup.requireCapabilitiesMeetRequirements(caps, req, "test")
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun requireCapabilitiesMeetRequirements_passesWhenCapsAreSuperset() {
|
||||
val req =
|
||||
MlsGroup.Companion.RequiredCapabilities(
|
||||
extensions = listOf(0xF2EE),
|
||||
proposals = listOf(0x000A),
|
||||
credentials = listOf(0x0001),
|
||||
)
|
||||
val caps =
|
||||
com.vitorpamplona.quartz.marmot.mls.tree.Capabilities(
|
||||
extensions = listOf(0xF2EE, 0x1234),
|
||||
proposals = listOf(0x000A, 0x000B),
|
||||
credentials = listOf(0x0001, 0x0002),
|
||||
)
|
||||
// Should not throw.
|
||||
MlsGroup.requireCapabilitiesMeetRequirements(caps, req, "test")
|
||||
}
|
||||
|
||||
/**
|
||||
* End-to-end gate: `addMember` MUST reject a KeyPackage whose leaf
|
||||
* doesn't advertise the group's `required_capabilities`. We tamper the
|
||||
* KP's leaf capabilities to drop SelfRemove, then re-encode and re-sign
|
||||
* to keep the KP signature valid (RFC 9420 §10.1) so the rejection is
|
||||
* coming from the capability gate and not the signature check.
|
||||
*/
|
||||
@Test
|
||||
fun addMember_rejectsKeyPackageMissingRequiredProposal() =
|
||||
runBlocking<Unit> {
|
||||
// Setup: standard Marmot group (required_capabilities lists
|
||||
// SelfRemove + MarmotGroupData + Basic).
|
||||
val manager = createGroupManager()
|
||||
manager.createGroup(groupId, aliceId.hexToByteArray())
|
||||
|
||||
// Bob's KP, but with SelfRemove stripped from his leaf
|
||||
// capabilities. He's still announcing himself as a Marmot peer
|
||||
// — just lying about supporting SelfRemove.
|
||||
val tampered = createKeyPackageWithoutSelfRemove(bobId)
|
||||
|
||||
assertFailsWith<IllegalStateException> {
|
||||
manager.addMember(groupId, tampered.toTlsBytes())
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a KeyPackage whose leaf [Capabilities] does NOT list 0x000A
|
||||
* (SelfRemove), then re-sign so the KP's outer signature still
|
||||
* validates. Useful for testing the §7.2 gate in isolation.
|
||||
*/
|
||||
private fun createKeyPackageWithoutSelfRemove(identity: String): com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage {
|
||||
val tempGroup = MlsGroup.create(identity.hexToByteArray())
|
||||
val bundle = tempGroup.createKeyPackage(identity.hexToByteArray(), ByteArray(0))
|
||||
val original = bundle.keyPackage
|
||||
val originalLeaf = original.leafNode
|
||||
|
||||
// Strip SelfRemove (0x000A) from the leaf's advertised proposals.
|
||||
val tamperedCaps =
|
||||
originalLeaf.capabilities.copy(
|
||||
proposals = originalLeaf.capabilities.proposals.filter { it != 0x000A },
|
||||
)
|
||||
// Re-build the leaf node and re-sign its TBS so the leaf signature
|
||||
// still verifies (otherwise we'd hit the LeafNode signature check
|
||||
// before the capability gate fires).
|
||||
val tamperedLeaf =
|
||||
originalLeaf.copy(capabilities = tamperedCaps).let { lf ->
|
||||
val tbs = lf.encodeTbs()
|
||||
val sig =
|
||||
com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider
|
||||
.signWithLabel(bundle.signaturePrivateKey, "LeafNodeTBS", tbs)
|
||||
lf.copy(signature = sig)
|
||||
}
|
||||
// Re-sign the KeyPackage TBS over the new leaf.
|
||||
val unsigned = original.copy(leafNode = tamperedLeaf, signature = ByteArray(0))
|
||||
return unsigned.copy(
|
||||
signature =
|
||||
com.vitorpamplona.quartz.marmot.mls.crypto.MlsCryptoProvider
|
||||
.signWithLabel(bundle.signaturePrivateKey, "KeyPackageTBS", unsigned.encodeTbs()),
|
||||
)
|
||||
}
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// RFC 9420 §5.3 psk_secret derivation
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user