security: bind a fetched KeyPackage to the requested signer on the group add path (#202)

On the group add path, a KeyPackage fetched from the registry for a requested
signer id is validated but never checked to belong to that signer: the member id
is read from the package's own credential. A registry that returns an attacker's
(validly self-signed) package for a victim's id therefore inserts the attacker's
leaf under the victim's identity — a confidentiality break and sender-attribution
spoof. validate() is not meant to enforce this; the application (AS) layer is.

Bind the validated leaf's signature_key (hex) to the requested signer id in both
GroupV1::key_package_for_signer and GroupV2::add_member, rejecting a mismatch.
Bind to the key, not the credential bytes: an impostor can copy an id into a
credential but cannot sign a leaf with the victim's key.

Courtesy patch from the downstream Peers fork; coordinated via the libchat
security advisory. Reported by @x0net.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

core/conversations/src/conversation/group_v1.rs

@@ -157,6 +157,19 @@ impl GroupV1Convo {
 
         let key_package_in = KeyPackageIn::tls_deserialize(&mut keypkg_bytes.as_slice())?;
         let keypkg = key_package_in.validate(provider.crypto(), ProtocolVersion::Mls10)?; //TODO: P3 - Hardcoded Protocol Version
+        // SECURITY: validate() only proves the package is well-formed and self-signed
+        // — NOT that it belongs to the signer we asked the registry for. Bind the
+        // fetched leaf's signature_key to the requested id (a signer id is
+        // hex(Ed25519 verifying key)); reject a mismatch so a malicious/compromised
+        // registry cannot insert an attacker's leaf under a victim's identity
+        // (confidentiality break + sender-attribution spoof). Bind to the key, not the
+        // spoofable credential bytes.
+        let leaf_key = hex::encode(keypkg.leaf_node().signature_key().as_slice());
+        if leaf_key != signer.as_str() {
+            return Err(ChatError::Protocol(format!(
+                "keypackage for signer {signer} is bound to a different signing key ({leaf_key})"
+            )));
+        }
         Ok(keypkg)
     }
 

core/conversations/src/conversation/group_v2.rs

@@ -152,8 +152,24 @@ fn fetch_key_packages<S: ExternalServices>(
                 .retrieve(member.as_str())
                 .map_err(ChatError::generic)?
                 .ok_or_else(|| ChatError::generic("No key package"))?;
-            let member_id = KeyPackageIn::tls_deserialize(&mut key_package.as_slice())?
-                .validate(service_ctx.mls_provider.crypto(), ProtocolVersion::Mls10)?
+            let validated = KeyPackageIn::tls_deserialize(&mut key_package.as_slice())?
+                .validate(service_ctx.mls_provider.crypto(), ProtocolVersion::Mls10)?;
+            // SECURITY: a validated KeyPackage only proves it is well-formed and
+            // self-signed — NOT that it belongs to the signer we asked the registry
+            // for. `member_id` below is read from the package's OWN credential and was
+            // never checked equal to `member`, so a malicious/compromised registry (or
+            // a cache poisoned by an untrusted transport) can return an attacker's
+            // package for a victim's id, inserting the attacker's leaf under the
+            // victim's identity: confidentiality break + sender-attribution spoof.
+            // A signer id is hex(Ed25519 verifying key), so bind the leaf's
+            // signature_key (not the spoofable credential bytes) to the requested id.
+            let leaf_key = hex::encode(validated.leaf_node().signature_key().as_slice());
+            if leaf_key != member.as_str() {
+                return Err(ChatError::generic(format!(
+                    "key package for {member} is bound to a different signing key ({leaf_key})"
+                )));
+            }
+            let member_id = validated
                 .leaf_node()
                 .credential()
                 .serialized_content()