security: bind a fetched KeyPackage to the requested signer (group add path) - #202
Open
xAlisher wants to merge 1 commit into
Open
security: bind a fetched KeyPackage to the requested signer (group add path)#202xAlisher wants to merge 1 commit into
xAlisher wants to merge 1 commit into
Conversation
…oup add path 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>
xAlisher
force-pushed
the
security/keypackage-device-binding
branch
from
August 8, 2026 15:18
d8e3af3 to
00870e2
Compare
Author
|
cc @x0net — thank you again for the careful report that surfaced this. You're credited on the fix and here. The downstream fix shipped in Peers v0.9.7 (both the core add-path binding and the #239 offline-contact-card injection point); this PR offers the upstream binding as a courtesy interim. The inbound-attribution variant you flagged is tracked separately (upstream-gated behind de-mls #143 + a libchat re-pin). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On the group add path, a KeyPackage fetched from the registry for a requested signer id is
validate()d 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 inserts the attacker's leaf under the victim's identity: a confidentiality break + sender-attribution spoof.validate()isn't meant to enforce this binding; the AS/application layer is.Fix
Bind the validated leaf's
signature_key(hex) to the requested signer id in bothGroupV1::key_package_for_signerandGroupV2::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.Notes
GHSA-82q9-xqc8-636c); acknowledged as a known issue and slated for the account redesign — offered here as an interim; take it or fold it in as you prefer.member_signature_key— rather than the credential) is the separate structural change you flagged; not included here.