fix(types): bind derivation_path in SignTxMessage.Raw() (#162)#163
Merged
Merged
Conversation
SignTxMessage.Raw() builds the canonical payload signed by the event initiator and re-computed by nodes during verification. It previously omitted derivation_path, even though nodes use that field to select the child key for signing (CKD.Derive). An attacker able to tamper messages on the bus could change derivation_path without invalidating the initiator signature, making a node sign the authenticated tx digest with a different derived key than intended. Include DerivationPath in the signed payload so it is authenticated. ComposeAuthorizerRaw wraps Raw(), so authorizer signatures now cover it too. Uses `omitempty` for backward compatibility: messages with an empty derivation_path serialize byte-for-byte like before, so existing signed traffic (e.g. clients that do not use HD derivation) keeps verifying. Adds unit tests for: backward-compat bytes, path inclusion, and the signature-binding property (tampering the path breaks verification), plus a VerifyInitiatorMessage test through the production verify path. Relates to fystack#162
DNK90
force-pushed
the
fix/issue-162-bind-derivation-path
branch
from
f3b0488 to
e99507a
Compare
|
Contributor
|
LGTM! |
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
2 participants
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.
Summary
Fixes the signature-binding gap reported in #162.
SignTxMessage.Raw()builds the canonical payload that the event initiator signs and that nodes re-compute during verification. It previously omittedderivation_path, even though nodes use that field to select the child key for signing (CKD.Derive). An attacker able to tamper messages on the bus could changederivation_pathwithout invalidating the initiator signature, making a node sign the authenticated tx digest with a different derived key than intended. BecauseComposeAuthorizerRaw()wrapsRaw(), authorizer signatures were affected as well.Change
DerivationPathin the signed payload ofSignTxMessage.Raw().ComposeAuthorizerRaw()now covers it automatically (it wrapsRaw()).Backward compatibility
Uses
json:"derivation_path,omitempty". Messages with an emptyderivation_pathserialize byte-for-byte as before, so existing signed traffic (initiators that don't use HD derivation) keeps verifying — no lockstep upgrade required for the empty-path case.Tests
pkg/types: backward-compat bytes (empty path == old output), path inclusion, path changes Raw(), and the signature-binding property (tampering the path breaksed25519.Verify).pkg/identity:VerifyInitiatorMessageend-to-end — a tamperedderivation_pathis rejected through the production verify path.go test ./pkg/types/... ./pkg/identity/...passes;gofmt/go build ./...clean.Note for downstream initiators
Initiators that build & sign
SignTxMessage(e.g. the coordinator/API) must use the same[]uint32 derivation_pathtype and include it in their ownRaw()once they start sending non-empty paths, to keep the canonical bytes identical on both sides.Relates to #162