refactor: check a group's signature against the room id, not against a key
Phase 1 of docs/member-archive.md. No wire change, no behaviour change, and one function where there was one. `GroupKeyStateEvent.isSignedByGroup` did three things: walk a threshold key to the room it derives, compare that to the event's author, and check the id and the signature. Only the first of those needs a key. The other two need the id the walk produces -- and a room's id *is* that value, held from both ends by `GroupKeyState.verifies` and `FrostSigningManager.signingPath`. So the walk splits off and `isSignedByRoom(event, chatRoomId)` is what remains: the same three checks, with the one input a caller might not have already resolved. `isSignedByGroup` becomes the one-line caller that walks first, and every existing call site and test is untouched. **Why this is worth a commit of its own.** A member added after the ceremony holds no `DkgSession`, no share, and -- until a state is re-announced, which nothing does -- no `GroupKeyState` row either. Under the old signature they could not check a group signature at all, and an archive of the group's work would have had to be believed because a member said so. Under the new one they check it against the id in their own Welcome, and the sender of an archive stops needing to be trusted. That is the property phases 2-9 are built on, so it lands first and lands alone. **The catch moved and had to be kept.** `marmotGroupId` is now called outside `isSignedByRoom`, so `isSignedByGroup` keeps a `runCatching` of its own. Without it a threshold key that is not a point stops being a refused state and becomes an exception in the middle of the inbound path -- every input here is off the wire, and the whole contract of these functions is that malformed means no. There is a test that fails if the catch is dropped. **Tests**, added to `GroupKeyStateTest` where the FROST key material, the second group and the real-quorum `groupSignature` helper already live: - a room's id is the only key its signature verifies against -- the same group's sibling room fails, and so does another group entirely; - the verifier does not care what kind it is looking at, over four kinds including `GroupKeyStateEvent` itself. That last one is not incidental: a key state signed by the room passes exactly as a dialect does, which is why the archive needs an allowlist of kinds on top of this and cannot read "the group signed it" as permission to apply it; - a rumor nobody signed is not a group signature -- empty sig, member author, which is what every nip30303 event on the wire looks like today; - claiming the room as author proves nothing without the signature. The room id is in the h tag of every kind:445 the group has sent, so writing it into `pubKey` is free; the author check and the id check both pass and the signature is the whole feature; - an event edited after signing fails on the id, not on the signature -- and the original still passes, which is why the id check is not redundant; - malformed input is a no rather than a throw, on both forms. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -125,33 +125,37 @@ object GroupKeyStateEvent {
|
||||
.takeIf { it.length == 66 && it.all { char -> char.isDigit() || char in 'a'..'f' || char in 'A'..'F' } }
|
||||
|
||||
/**
|
||||
* Whether the room derived from [thresholdPublicKey] at [path] actually
|
||||
* signed [event].
|
||||
* Whether the room with id [chatRoomId] signed [event].
|
||||
*
|
||||
* Three things, and all three are needed. The author has to be the key that
|
||||
* derivation reaches, or a real signature by some other group -- or by the
|
||||
* same group in another of its rooms -- would pass. The id has to be the
|
||||
* hash of the fields it is sitting next to, or the signature covers a
|
||||
* message that is not this event and the fields could then say anything. And
|
||||
* the signature has to verify.
|
||||
* Three things, and all three are needed. The author has to be the room, or
|
||||
* a real signature by some other group -- or by the same group in another of
|
||||
* its rooms -- would pass. The id has to be the hash of the fields it is
|
||||
* sitting next to, or the signature covers a message that is not this event
|
||||
* and the fields could then say anything. And the signature has to verify.
|
||||
*
|
||||
* Note what is being checked against what: the key in the *content*, walked
|
||||
* to the path in the *tags*, is what has to have signed. Since that walk is
|
||||
* also the room's id, the author of a state that passes is the room it
|
||||
* belongs to -- so a state signed by one group about another group's key
|
||||
* fails here, which is the point. Only a room may say what it signs with.
|
||||
* ### Why a room id is enough
|
||||
*
|
||||
* A room's id *is* the group's threshold key derived at the room's path --
|
||||
* see `shared-key-derivation.md`, and `GroupKeyState.verifies` and
|
||||
* `FrostSigningManager.signingPath`, both of which hold that invariant from
|
||||
* their own ends. So the key a signature has to verify against is not looked
|
||||
* up; it is the id of the room the event was found in.
|
||||
*
|
||||
* That is what makes a group's signature checkable by a member holding
|
||||
* nothing else. No `GroupKeyState` row, no threshold key, no derivation path,
|
||||
* no ceremony -- which is exactly the position a member added after the
|
||||
* ceremony is in, and the reason `docs/member-archive.md` can hand them
|
||||
* history without asking them to trust whoever sent it.
|
||||
*
|
||||
* Everything is caught, because every input is off the wire: a pubkey that
|
||||
* is not a point, a signature that is not 64 bytes, hex that is not hex.
|
||||
* All of them mean the same thing here, which is no.
|
||||
*/
|
||||
fun isSignedByGroup(
|
||||
fun isSignedByRoom(
|
||||
event: Event,
|
||||
thresholdPublicKey: HexKey,
|
||||
path: List<Long>
|
||||
chatRoomId: HexKey
|
||||
): Boolean = runCatching {
|
||||
val author = SharedKeyDerivation.marmotGroupId(thresholdPublicKey, path)
|
||||
if (!event.pubKey.equals(author, ignoreCase = true)) return false
|
||||
if (!event.pubKey.equals(chatRoomId, ignoreCase = true)) return false
|
||||
|
||||
val hashes = EventHasher.hashIdCheck(
|
||||
id = event.id,
|
||||
@@ -169,4 +173,27 @@ object GroupKeyStateEvent {
|
||||
pubKey = event.pubKey.hexToByteArray()
|
||||
)
|
||||
}.getOrDefault(false)
|
||||
|
||||
/**
|
||||
* Whether the room derived from [thresholdPublicKey] at [path] actually
|
||||
* signed [event].
|
||||
*
|
||||
* Note what is being checked against what: the key in the *content*, walked
|
||||
* to the path in the *tags*, is what has to have signed. Since that walk is
|
||||
* also the room's id, the author of a state that passes is the room it
|
||||
* belongs to -- so a state signed by one group about another group's key
|
||||
* fails here, which is the point. Only a room may say what it signs with.
|
||||
*
|
||||
* The walk is the only thing this adds to [isSignedByRoom], and it is inside
|
||||
* the catch for the same reason everything else is: [thresholdPublicKey]
|
||||
* comes off the wire, and a key that is not a point is a no rather than a
|
||||
* throw.
|
||||
*/
|
||||
fun isSignedByGroup(
|
||||
event: Event,
|
||||
thresholdPublicKey: HexKey,
|
||||
path: List<Long>
|
||||
): Boolean = runCatching {
|
||||
isSignedByRoom(event, SharedKeyDerivation.marmotGroupId(thresholdPublicKey, path))
|
||||
}.getOrDefault(false)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user