Skip to content

feat(ssh): wire CheckHostIP from sshconfig for DNS-spoofing detection#378

Merged
kke merged 2 commits into
mainfrom
cfg-check-host-ip
Jun 7, 2026
Merged

feat(ssh): wire CheckHostIP from sshconfig for DNS-spoofing detection#378
kke merged 2 commits into
mainfrom
cfg-check-host-ip

Conversation

@kke

@kke kke commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

When CheckHostIP is enabled, the host key callback additionally resolves the connecting hostname to IP addresses and checks whether any of those IPs appear in known_hosts with a different key. A mismatch returns ErrHostKeyMismatch; unknown IPs and DNS failures are non-fatal.

The IP check is read-only — no new entries are written for resolved IPs (writing both hostname and IP on first connect is deferred as a follow-on).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR wires the CheckHostIP ssh config option into the SSH host-key verification flow by wrapping the existing known_hosts host key callback with an additional “resolved IP mismatch” check to detect potential DNS spoofing.

Changes:

  • Added hostkey.WithCheckHostIP(...) wrapper that resolves hostnames to IPs and checks those IPs in known_hosts for conflicting keys (read-only, DNS failures/nonexistent IP entries are non-fatal).
  • Updated protocol/ssh/connection.go to conditionally apply the wrapper when sshconfig.CheckHostIP is enabled, and adjusted global known_hosts selection to return the chosen path for wrapping.
  • Added focused unit tests for the new behavior and a test-only hook to stub DNS resolution.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
protocol/ssh/hostkey/export_test.go Exposes a test-only hook to override hostname resolution.
protocol/ssh/hostkey/callbacks.go Implements WithCheckHostIP and the IP-verification wrapper logic.
protocol/ssh/hostkey/callbacks_test.go Adds unit tests covering mismatch detection, unknown IP non-fatal behavior, DNS failure behavior, permissive behavior, and IP-host skip behavior.
protocol/ssh/connection.go Wraps host-key callbacks with WithCheckHostIP when CheckHostIP is enabled; returns selected global known_hosts path for wrapping.
protocol/ssh/connection_test.go Adds an integration-style test ensuring hostkeyCallback enables CheckHostIP wrapping without breaking known_hosts verification.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread protocol/ssh/hostkey/callbacks.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread protocol/ssh/hostkey/callbacks.go
Comment thread protocol/ssh/hostkey/callbacks.go
Comment thread protocol/ssh/connection.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go Outdated
Comment thread protocol/ssh/hostkey/callbacks.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go Outdated
Comment thread sshconfig/config.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go Outdated
Comment thread sshconfig/config.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go Outdated
Comment thread protocol/ssh/hostkey/callbacks_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go
Comment thread protocol/ssh/hostkey/callbacks.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread protocol/ssh/hostkey/callbacks.go
Comment thread protocol/ssh/hostkey/callbacks.go
When CheckHostIP is enabled, the host key callback additionally resolves
the connecting hostname to IP addresses and checks whether any of those
IPs appear in known_hosts with a different key. A mismatch returns
ErrHostKeyMismatch; unknown IPs and DNS failures are non-fatal.

The IP check is read-only — no new entries are written for resolved IPs
(writing both hostname and IP on first connect is deferred as a follow-on).

Signed-off-by: Kimmo Lehto <klehto@mirantis.com>
@kke kke force-pushed the cfg-check-host-ip branch from 9714e05 to 45e10f8 Compare June 7, 2026 07:37
@kke kke requested a review from Copilot June 7, 2026 07:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

protocol/ssh/hostkey/callbacks_test.go:16

  • The tests override a package-level variable (hostkey.LookupHostFunc). Without any synchronization, this becomes race-prone if these tests (or future tests in the same package) are run with t.Parallel(), since multiple tests could swap the global resolver concurrently. Adding a local mutex around the override makes the helper safe and documents that the override is global.
import (
	"crypto/ed25519"
	"crypto/rand"
	"fmt"
	"net"
	"os"
	"path/filepath"
	"testing"

	"github.com/k0sproject/rig/v2/protocol/ssh/hostkey"
	"github.com/stretchr/testify/require"
	"golang.org/x/crypto/ssh"
	"golang.org/x/crypto/ssh/knownhosts"
)

Comment thread protocol/ssh/hostkey/callbacks_test.go
Signed-off-by: Kimmo Lehto <klehto@mirantis.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@kke kke marked this pull request as ready for review June 7, 2026 08:03
@kke kke merged commit eee9c8f into main Jun 7, 2026
13 checks passed
@kke kke deleted the cfg-check-host-ip branch June 7, 2026 08:04
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