Skip to content

fix(ssh): store host key buffer in addHostKey so verifyHostKey matches#1361

Open
naaa760 wants to merge 1 commit intogeneralaction:mainfrom
naaa760:fix/ssh-key-ver-brk
Open

fix(ssh): store host key buffer in addHostKey so verifyHostKey matches#1361
naaa760 wants to merge 1 commit intogeneralaction:mainfrom
naaa760:fix/ssh-key-ver-brk

Conversation

@naaa760
Copy link
Contributor

@naaa760 naaa760 commented Mar 9, 2026

fix: #1360

description

  • We were saving the host key fingerprint in the key field. Reconnect decoded it as base64 and re-hashed it, so it never matched. Accepted hosts showed as "changed" and TOFU was broken.

Changes

  • addHostKey() now takes a key Buffer and stores its base64; no fingerprint.
  • Tests pass a key buffer and assert file has base64 key data.

Testing

  • pnpm exec vitest run src/main/services/ssh/tests/SshHostKeyService.test.ts — 29 tests pass.

@vercel
Copy link

vercel bot commented Mar 9, 2026

@naaa760 is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 9, 2026

Greptile Summary

This PR fixes a critical TOFU (Trust On First Use) regression in SshHostKeyService. Previously, addHostKey stored a fingerprint string (e.g. SHA256:abc123) directly in the keyBase64 field of the in-memory store. When verifyHostKey later tried to recover the key via Buffer.from(stored.keyBase64, 'base64'), it decoded the ASCII bytes of the fingerprint string — not a real key — causing the re-computed fingerprint to never match. Every reconnect would return 'changed' instead of 'known', completely breaking host trust tracking.

The fix correctly changes addHostKey to accept a raw Buffer and converts it with .toString('base64') before storage, restoring the lossless round-trip that verifyHostKey relies on. Tests are updated accordingly and verify the fix works as intended.

Key observations:

  • The fix is minimal, correct, and well-targeted — the verifyHostKeypersistKnownHosts → re-parse round-trip now works correctly.
  • The code is clean and well-tested across both standard ports and non-standard port formats.
  • The in-memory store, file persistence, and verification all use consistent base64 encoding/decoding.

Confidence Score: 5/5

  • The fix is correct and safe to merge. The TOFU bug is properly resolved with consistent base64 round-tripping.
  • The core bug fix is straightforward and correct — addHostKey now accepts a raw Buffer, converts it to base64 for storage, and verifyHostKey correctly decodes it back to compute matching fingerprints. Tests are comprehensive and pass. No breaking changes to the API surface; the parameter type change in addHostKey is fixed simultaneously in all callers within the test suite. The two changed files are clean and well-tested.
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant SSH as SSH Connection
    participant SVC as SshHostKeyService
    participant FS as known_hosts file

    Note over SSH,FS: First connection (TOFU - Trust On First Use)
    SSH->>SVC: addHostKey(host, port, keyType, rawKeyBuffer)
    SVC->>SVC: keyBase64 = rawKeyBuffer.toString('base64')
    SVC->>SVC: knownHosts.set(hostPort, { algorithm, keyBase64 })
    SVC->>FS: writeFile → "host ssh-ed25519 <base64key>"

    Note over SSH,FS: Subsequent connection (verification)
    SSH->>SVC: verifyHostKey(host, port, keyType, fingerprint)
    SVC->>FS: (already initialized, reads from memory)
    SVC->>SVC: stored.keyBase64 → Buffer.from(keyBase64, 'base64')
    SVC->>SVC: knownFingerprint = SHA256(rawKeyBuffer)
    SVC->>SVC: knownFingerprint === fingerprint?
    SVC-->>SSH: 'known' ✓ (previously: always 'changed' ✗)

    Note over SSH,FS: OLD BUGGY FLOW (before fix)
    SSH->>SVC: addHostKey(host, port, keyType, "SHA256:abc123")
    SVC->>SVC: keyBase64 = "SHA256:abc123" (fingerprint string, not key!)
    SVC->>FS: writeFile → "host ssh-ed25519 SHA256:abc123"
    SSH->>SVC: verifyHostKey(host, port, keyType, "SHA256:abc123")
    SVC->>SVC: Buffer.from("SHA256:abc123", 'base64') → garbled bytes
    SVC->>SVC: SHA256(garbledBytes) ≠ "SHA256:abc123"
    SVC-->>SSH: 'changed' ✗ (TOFU broken)
Loading

Last reviewed commit: e8a82ef

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.

[bug]: SSH Host Key Verification is Broken

1 participant