Fix persistent EOF errors: detect and evict dead SSH connections

danny commented

2026-07-05 17:47:10 +01:00

(Migrated from gitlab.keyop.co.uk)

Closes #21.

Root cause

sshConnections.connection() dialed the SSH connection to the jump host once and cached it forever, with no liveness check. If the underlying connection died silently (e.g. a NAT/firewall idle timeout dropping it without either end sending a FIN/RST), every future packet on that queue kept reusing the same dead client. The per-connection destination dial (sshClient.Dial inside startTunnel) then failed immediately with EOF - forever, since nothing ever evicted the dead connection, until the whole process was restarted. This matches the reported log pattern exactly.

Worse: once a tunnel's DNAT rule existed for a destination, NFQUEUE was never consulted again for it (by design, for normal operation), so a destination whose dial failed even once stayed silently DNAT'd into a dead end forever, with no way to self-heal.

Fix

Three pieces, working together:

sshtunnel.Conn.startKeepalive: a goroutine (started by Dial) that periodically sends an SSH keepalive@openssh.com request and force-closes the connection if no reply arrives within one interval. golang.org/x/crypto/ssh has no built-in equivalent of OpenSSH's ServerAliveInterval, so without this a dead connection is only ever discovered reactively. The interval is configurable via ssh_keepalive_interval_seconds in a --config YAML file (Configuration.SSHKeepaliveIntervalSeconds), defaulting to 30s (sshtunnel.DefaultKeepaliveInterval) when unset.
sshtunnel.Conn.Wait: blocks until the underlying transport closes for any reason. sshConnections.connection() spawns a goroutine (evictOnDeath) that calls this right after a successful dial and removes the conn from the pool once it returns, so the next connection() call redials fresh instead of reusing a client whose transport has already failed.
sshtunnel.Conn.SetTunnelFailedHook: lets startTunnel tear down (and notify on) a specific tunnel whose destination dial fails, rather than leaving it open and DNAT'd forever. sshConnections.connection() wires this to remove the tunnel's DNAT rule (via a new deleteDNATRuleForTunnelFunc indirection, matching the existing addDNATRulesFunc/sshDialFunc testability pattern), so the next connection attempt to that destination gets a fresh chance through NFQUEUE.

Testing

internal/sshtunnel: new real (in-process, ephemeral-key, auth-free) SSH client/server test harness (testserver_test.go) exercises Wait, startKeepalive's timeout/reply/close-during-flight paths, and failTunnel's hook firing on a real rejected destination dial - a mock can't reproduce real SSH transport/global-request/channel-open behavior, so this is a real client/server pair, not a fake.
internal/nfq_forwarder: a similar real (file-backed key/known_hosts) SSH server harness (sshserver_test.go) exercises sshConnections' eviction-and-redial and DNAT-cleanup-hook wiring end to end - a bare sentinel *sshtunnel.Conn{} (used by most other tests in this file) deliberately can't exercise either, since Wait() blocks forever on one by design.
internal/config: new tests for the keepalive-interval field (pass-through, left-zero-for-the-consumer-to-default, negative-value rejection).
go build, make lint (golangci-lint + yamllint), make test (go test -race -cover ./...) all clean.

Not included

Real-host verification (a genuinely dropped jump host connection over a real network, a real firewall-blocked destination) isn't possible in this sandbox - flagged in CLAUDE.md's Known Issues as still open, same as this project's other real-host-only verification items.

Closes #21. ## Root cause `sshConnections.connection()` dialed the SSH connection to the jump host once and cached it forever, with no liveness check. If the underlying connection died silently (e.g. a NAT/firewall idle timeout dropping it without either end sending a FIN/RST), every future packet on that queue kept reusing the same dead client. The per-connection destination dial (`sshClient.Dial` inside `startTunnel`) then failed immediately with `EOF` - forever, since nothing ever evicted the dead connection, until the whole process was restarted. This matches the reported log pattern exactly. Worse: once a tunnel's DNAT rule existed for a destination, NFQUEUE was never consulted again for it (by design, for normal operation), so a destination whose dial failed even once stayed silently DNAT'd into a dead end forever, with no way to self-heal. ## Fix Three pieces, working together: - **`sshtunnel.Conn.startKeepalive`**: a goroutine (started by `Dial`) that periodically sends an SSH `keepalive@openssh.com` request and force-closes the connection if no reply arrives within one interval. `golang.org/x/crypto/ssh` has no built-in equivalent of OpenSSH's `ServerAliveInterval`, so without this a dead connection is only ever discovered reactively. The interval is configurable via `ssh_keepalive_interval_seconds` in a `--config` YAML file (`Configuration.SSHKeepaliveIntervalSeconds`), defaulting to 30s (`sshtunnel.DefaultKeepaliveInterval`) when unset. - **`sshtunnel.Conn.Wait`**: blocks until the underlying transport closes for any reason. `sshConnections.connection()` spawns a goroutine (`evictOnDeath`) that calls this right after a successful dial and removes the conn from the pool once it returns, so the *next* `connection()` call redials fresh instead of reusing a client whose transport has already failed. - **`sshtunnel.Conn.SetTunnelFailedHook`**: lets `startTunnel` tear down (and notify on) a specific tunnel whose destination dial fails, rather than leaving it open and DNAT'd forever. `sshConnections.connection()` wires this to remove the tunnel's DNAT rule (via a new `deleteDNATRuleForTunnelFunc` indirection, matching the existing `addDNATRulesFunc`/`sshDialFunc` testability pattern), so the next connection attempt to that destination gets a fresh chance through NFQUEUE. ## Testing - `internal/sshtunnel`: new real (in-process, ephemeral-key, auth-free) SSH client/server test harness (`testserver_test.go`) exercises `Wait`, `startKeepalive`'s timeout/reply/close-during-flight paths, and `failTunnel`'s hook firing on a real rejected destination dial - a mock can't reproduce real SSH transport/global-request/channel-open behavior, so this is a real client/server pair, not a fake. - `internal/nfq_forwarder`: a similar real (file-backed key/known_hosts) SSH server harness (`sshserver_test.go`) exercises `sshConnections`' eviction-and-redial and DNAT-cleanup-hook wiring end to end - a bare sentinel `*sshtunnel.Conn{}` (used by most other tests in this file) deliberately can't exercise either, since `Wait()` blocks forever on one by design. - `internal/config`: new tests for the keepalive-interval field (pass-through, left-zero-for-the-consumer-to-default, negative-value rejection). - `go build`, `make lint` (golangci-lint + yamllint), `make test` (`go test -race -cover ./...`) all clean. ## Not included - Real-host verification (a genuinely dropped jump host connection over a real network, a real firewall-blocked destination) isn't possible in this sandbox - flagged in `CLAUDE.md`'s Known Issues as still open, same as this project's other real-host-only verification items.

danny commented

2026-07-05 17:47:37 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in issue #21

danny commented

2026-07-05 17:53:24 +01:00

(Migrated from gitlab.keyop.co.uk)

assigned to @danny

danny commented

2026-07-05 17:53:25 +01:00

(Migrated from gitlab.keyop.co.uk)

requested review from @danny

danny commented

2026-07-05 17:53:44 +01:00

(Migrated from gitlab.keyop.co.uk)

Looks good to me. Need to do some real-world testing before merge though.

danny commented

2026-07-05 17:53:44 +01:00

(Migrated from gitlab.keyop.co.uk)

approved this merge request

danny commented

2026-07-05 18:00:09 +01:00

(Migrated from gitlab.keyop.co.uk)

All seems to work as expected. Long term testing will be required to see if this fully resolves the issue, but it certainly doesn't seem to make anything worse.

Will merge.

All seems to work as expected. Long term testing will be required to see if this fully resolves the issue, but it certainly doesn't seem to make anything worse. Will merge.

danny commented

2026-07-05 18:00:34 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit 6e320bbb1b

mentioned in commit 6e320bbb1b8c58705450031daecf8fd759799a75

danny (Migrated from gitlab.keyop.co.uk) merged commit 6e320bbb1b into main

2026-07-05 18:00:34 +01:00

danny commented

2026-07-06 07:42:18 +01:00

(Migrated from gitlab.keyop.co.uk)

mentioned in commit be208f5787

mentioned in commit be208f5787eb27240a4f42d123f34118b232b8cc

Rows
Columns

Fix persistent EOF errors: detect and evict dead SSH connections #63

Root cause

Fix

Testing

Not included