Auto-add unknown jump host keys, on by default

danny commented

2026-07-05 19:25:27 +01:00

(Migrated from gitlab.keyop.co.uk)

Closes #17 (https://gitlab.keyop.co.uk/keyop/go/nfq_forwarder/-/issues/17).

Background

Connecting to a jump host with no known_hosts entry at all currently fails outright:

FATAL Failed to connect to jump host: sshtunnel.Dial(...) failed: ssh: handshake failed:
knownhosts: key is unknown

requiring the host's key to be added manually (e.g. via ssh-keyscan, or ssh-ing to it once)
before nfq_forwarder can be used against it at all. This is reasonable, safe-by-default
behavior, but inconvenient - this MR makes it optional (and off by default, i.e. auto-add on).

Fix

golang.org/x/crypto/ssh/knownhosts already distinguishes the two cases this needs treated
differently, via *knownhosts.KeyError.Want:

empty -> the host has no existing entry at all - genuinely unknown.
non-empty -> the host has an entry, but with a different key - a mismatch, and exactly
the signal of a possible MITM attack.

sshtunnel.Dial's HostKeyCallback now wraps knownhosts.New's own callback
(buildHostKeyCallback): on an empty-Want KeyError, it appends a new line to known_hosts
(knownhosts.Line, the same helper the file format is built from) and accepts the connection,
logging that it did so; on anything else - including a real mismatch - the error is returned
unchanged and the connection is refused, exactly as today. A changed key is never
auto-corrected, regardless of this setting.

New config option ssh_strict_host_key_checking (Configuration.SSHStrictHostKeyChecking),
a global setting alongside ssh_key_path/known_hosts_path. Named after OpenSSH's own
well-known option for the same behavior. Defaults to false (auto-add on) -
deliberately phrased as the "strict" opt-in so Go's normal zero-value already matches the
wanted default, with no special defaulting logic needed in config.LoadFromFile. Applies
uniformly to both the --config YAML and legacy CLI-flags paths.
known_hosts_path is now created automatically (empty) when it doesn't already exist and
auto-add is enabled (ensureFileExists) - a brand new setup that's never manually ssh'd
anywhere works out of the box, no manual touch known_hosts needed first. With
ssh_strict_host_key_checking: true, a missing file remains a hard error, matching that
setting's explicit intent that every key must already be recorded.

Testing

Pure knownhosts-package logic plus file I/O, so no live SSH handshake is needed -
internal/sshtunnel/hostkey_test.go builds real ssh.PublicKey values (fresh ed25519 keys) and
temp known_hosts files/paths directly:

a genuinely unknown host's key is added, and a second, fresh callback built from the
now-updated file also accepts it (round-trip proof, not just "a line got written");
a host already known with a different key is still rejected, and the file is left
byte-for-byte unmodified (the critical security case);
a host already known with the matching key is accepted, unaffected by this change;
with ssh_strict_host_key_checking: true, an unknown host is still rejected exactly as
today, and the file is left unmodified;
a missing known_hosts file is created automatically when auto-add is enabled (and the newly
created file works correctly for a subsequent check), but remains a hard error in strict mode
(and the file is not created in that case);
ensureFileExists itself is unit tested directly: creates when missing, leaves an existing
file's content untouched, and a missing parent directory is still a real error;
appendKnownHostKey's open-failure path (e.g. path is a directory) returns an error rather
than panicking.
go build ./..., go vet ./...
make lint (golangci-lint + yamllint)
make test (go test -race -cover ./...)
Manual sanity check: docs/config.example.yaml loads ssh_strict_host_key_checking: false correctly via config.LoadFromFile

Docs

Updated README.md, docs/README.md, docs/config.example.yaml, and CLAUDE.md (Design
notes + Testing sections).

Closes #17 (https://gitlab.keyop.co.uk/keyop/go/nfq_forwarder/-/issues/17). ## Background Connecting to a jump host with no `known_hosts` entry at all currently fails outright: ``` FATAL Failed to connect to jump host: sshtunnel.Dial(...) failed: ssh: handshake failed: knownhosts: key is unknown ``` requiring the host's key to be added manually (e.g. via `ssh-keyscan`, or `ssh`-ing to it once) before `nfq_forwarder` can be used against it at all. This is reasonable, safe-by-default behavior, but inconvenient - this MR makes it optional (and off by default, i.e. auto-add on). ## Fix `golang.org/x/crypto/ssh/knownhosts` already distinguishes the two cases this needs treated differently, via `*knownhosts.KeyError.Want`: - **empty** -> the host has no existing entry at all - genuinely unknown. - **non-empty** -> the host has an entry, but with a *different* key - a mismatch, and exactly the signal of a possible MITM attack. `sshtunnel.Dial`'s `HostKeyCallback` now wraps `knownhosts.New`'s own callback (`buildHostKeyCallback`): on an empty-`Want` `KeyError`, it appends a new line to `known_hosts` (`knownhosts.Line`, the same helper the file format is built from) and accepts the connection, logging that it did so; on anything else - including a real mismatch - the error is returned unchanged and the connection is refused, exactly as today. **A changed key is never auto-corrected, regardless of this setting.** - New config option `ssh_strict_host_key_checking` (`Configuration.SSHStrictHostKeyChecking`), a global setting alongside `ssh_key_path`/`known_hosts_path`. Named after OpenSSH's own well-known option for the same behavior. **Defaults to `false`** (auto-add on) - deliberately phrased as the "strict" opt-*in* so Go's normal zero-value already matches the wanted default, with no special defaulting logic needed in `config.LoadFromFile`. Applies uniformly to both the `--config` YAML and legacy CLI-flags paths. - **`known_hosts_path` is now created automatically** (empty) when it doesn't already exist and auto-add is enabled (`ensureFileExists`) - a brand new setup that's never manually `ssh`'d anywhere works out of the box, no manual `touch known_hosts` needed first. With `ssh_strict_host_key_checking: true`, a missing file remains a hard error, matching that setting's explicit intent that every key must already be recorded. ## Testing Pure `knownhosts`-package logic plus file I/O, so no live SSH handshake is needed - `internal/sshtunnel/hostkey_test.go` builds real `ssh.PublicKey` values (fresh ed25519 keys) and temp `known_hosts` files/paths directly: - a genuinely unknown host's key is added, and a *second*, fresh callback built from the now-updated file also accepts it (round-trip proof, not just "a line got written"); - a host already known with a **different** key is still rejected, and the file is left byte-for-byte unmodified (the critical security case); - a host already known with the matching key is accepted, unaffected by this change; - with `ssh_strict_host_key_checking: true`, an unknown host is still rejected exactly as today, and the file is left unmodified; - a missing `known_hosts` file is created automatically when auto-add is enabled (and the newly created file works correctly for a subsequent check), but remains a hard error in strict mode (and the file is *not* created in that case); - `ensureFileExists` itself is unit tested directly: creates when missing, leaves an existing file's content untouched, and a missing parent directory is still a real error; - `appendKnownHostKey`'s open-failure path (e.g. path is a directory) returns an error rather than panicking. - [x] `go build ./...`, `go vet ./...` - [x] `make lint` (golangci-lint + yamllint) - [x] `make test` (`go test -race -cover ./...`) - [x] Manual sanity check: `docs/config.example.yaml` loads `ssh_strict_host_key_checking: false` correctly via `config.LoadFromFile` ## Docs Updated `README.md`, `docs/README.md`, `docs/config.example.yaml`, and `CLAUDE.md` (Design notes + Testing sections).

danny commented

2026-07-05 19:39:29 +01:00

(Migrated from gitlab.keyop.co.uk)

added 1 commit

1a791604 - Pre-create known_hosts when auto-adding unknown host keys

Compare with previous version

added 1 commit <ul><li>1a791604 - Pre-create known_hosts when auto-adding unknown host keys</li></ul> [Compare with previous version](/keyop/go/nfq_forwarder/-/merge_requests/25/diffs?diff_id=282&start_sha=e7b8178413ec9f92b891af732c8e4193bf1bcb4a)

danny commented

2026-07-05 19:39:51 +01:00

(Migrated from gitlab.keyop.co.uk)

changed the description

danny commented

2026-07-05 19:40:03 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in issue #17

danny commented

2026-07-05 21:22:25 +01:00

(Migrated from gitlab.keyop.co.uk)

added 1 commit

4810b624 - Updated notes with redacted gitlab tokens.

Compare with previous version

added 1 commit <ul><li>4810b624 - Updated notes with redacted gitlab tokens.</li></ul> [Compare with previous version](/keyop/go/nfq_forwarder/-/merge_requests/25/diffs?diff_id=284&start_sha=1a791604b3acc2d057713c7cf66f13a3d5dc9055)

danny commented

2026-07-05 21:23:35 +01:00

(Migrated from gitlab.keyop.co.uk)

approved this merge request

danny commented

2026-07-05 21:27:23 +01:00

(Migrated from gitlab.keyop.co.uk)

requested review from @danny