Skip to content

secure: use HKDF for session key derivation, bind transcript to both peers#35

Open
SonOfTroll wants to merge 7 commits into
ethical-buddy:mainfrom
SonOfTroll:main
Open

secure: use HKDF for session key derivation, bind transcript to both peers#35
SonOfTroll wants to merge 7 commits into
ethical-buddy:mainfrom
SonOfTroll:main

Conversation

@SonOfTroll

Copy link
Copy Markdown
Contributor

hey, i noticed the handshake was deriving the session key by just SHA256-ing the raw shared secret with the kind byte appended. it works, but SHA256 isn't really a proper KDF, and the resulting key isn't bound to the actual session participants, which leaves a small misbinding risk if someone is relaying the handshake in the middle
i switched it over to HKDF-SHA256 and included a transcript hash that binds both ephemeral keys and both node IDs
same overall behavior, but the key derivation is now tied to the session context
changes:

  • deriveKey() now uses HKDF with the transcript hash as the salt
  • added buildTranscript() to canonicalize the ordering (client ephemeral + ID first, server second), so both sides derive the same key regardless of who initiates
  • added x/crypto as a direct dependency (it was already present indirectly)

@ethical-buddy

Copy link
Copy Markdown
Owner

Thanks for taking this on. The HKDF direction is the right direction, but I am going to request changes before we merge this.

Main blockers:

  1. This changes the secure-session wire behavior. Older nodes derive the session key as sha256(shared || kind), while this PR derives it through HKDF over shared secret plus transcript context. That means mixed-version peers can complete the visible handshake path and then fail encrypted traffic. We need an explicit protocol/version migration path or compatibility gate before this lands.

  2. The transcript currently binds shortened VX6 node IDs, not the full static Ed25519 public keys. Since VX6 node IDs are truncated hashes, the transcript should bind the full static public keys from both handshake messages if the goal is peer-context binding.

  3. The tests currently prove the new path can round-trip, but they do not prove the security properties introduced by this change. Please add focused tests for transcript canonicalization, key separation across kind/identity/ephemeral changes, and mixed-version behavior.

I am going to make a more detailed plan in the issues section and assign it to you so we can move this forward in smaller pieces. The change is valuable, but I want us to test and roll out the HKDF shift slowly and securely instead of merging it as one implicit protocol break.

@SonOfTroll

Copy link
Copy Markdown
Contributor Author

@ethical-buddy
sorry for the insanely late follow up life happened and this kinda sat on the backburner for way longer than i intended
addressed the review comments
added version gating to the hello handshake so peers not on v2 fail immediately with a clear error instead of breaking later on the first encrypted frame
replaced node ids in the transcript with the full 32-byte ed25519 pubkeys since node ids are just truncated hashes and don't really provide useful binding also bumped the transcript and info labels to v2
added 7 focused tests covering transcript consistency key separation full pubkey binding and version mismatch handling
all 9 tests are passing and the build is clean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants