Skip to content

Commit

Permalink
host matching: handle wildcards with non-standard port (#10)
Browse files Browse the repository at this point in the history 
In OpenSSH, wildcard host pattern entries in a known_hosts file can match
hosts regardless of their port number. However, x/crypto/ssh/knownhosts does
not follow this behavior, instead requiring strict port equality; see bug
golang/go#52056 for background.

This commit implements a workaround in skeema/knownhosts, which is enabled
when using the NewDB constructor. Conceptually, the workaround works like
this:

* At constructor time, when re-reading the known_hosts file (originally to
  look for @cert-authority lines), also look for lines that have wildcards
  in the host pattern and no port number specified. Track these lines in a
  new field of the HostKeyDB struct for later use.

* When a host key callback returns no matches (KeyError with empty Want slice)
  and the host had a nonstandard (non-22) port number, try the callback again,
  this time manipulating the host arg to be on port 22.

* If this second call returned nil error, that means the host key now matched
  a known_hosts entry on port 22, so consider the host as known.

* If this second call returned a KeyError with non-empty Want slice, filter
  down the resulting keys to only correspond to lines with known wildcards,
  using the preprocessed information from the first step. This ensures we
  aren't incorrectly returning non-wildcard entries among the Want slice.

The implementation for the latter 3 bullets gets embedded directly in the
host key callback returned by HostKeyDB.HostKeyCallback, by way of some
nested callback wrapping. This only happens if the first bullet actually
found at least one wildcard in the file.
  • Loading branch information
@evanelias
evanelias authored Jul 16, 2024
1 parent 7c797a4 commit 8b8ca37
Show file tree
Hide file tree
Showing 4 changed files with 167 additions and 31 deletions.
22 changes: 18 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@

> This repo is brought to you by [Skeema](https://github.com/skeema/skeema), a
> declarative pure-SQL schema management system for MySQL and MariaDB. Our
> premium products include extensive [SSH tunnel](https://www.skeema.io/docs/options/#ssh)
> premium products include extensive [SSH tunnel](https://www.skeema.io/docs/features/ssh/)
> functionality, which internally makes use of this package.
Go provides excellent functionality for OpenSSH known_hosts files in its
external package [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts).
However, that package is somewhat low-level, making it difficult to implement full known_hosts management similar to command-line `ssh`'s behavior for `StrictHostKeyChecking=no` configuration.
However, that package is somewhat low-level, making it difficult to implement full known_hosts management similar to command-line `ssh`'s behavior for `StrictHostKeyChecking=no` configuration. Additionally, it has several known issues which have been open for multiple years.

This repo ([github.com/skeema/knownhosts](https://github.com/skeema/knownhosts)) is a thin wrapper package around [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts), adding the following functionality:
Package [github.com/skeema/knownhosts](https://github.com/skeema/knownhosts) provides a thin wrapper around [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts), adding the following functionality:

* Look up known_hosts public keys for any given host
* Auto-populate ssh.ClientConfig.HostKeyAlgorithms easily based on known_hosts, providing a solution for [golang/go#29286](https://github.com/golang/go/issues/29286)
* Auto-populate ssh.ClientConfig.HostKeyAlgorithms easily based on known_hosts, providing a solution for [golang/go#29286](https://github.com/golang/go/issues/29286). This also properly handles cert algorithms for hosts using CA keys when [using the NewDB constructor](#enhancements-requiring-extra-parsing) added in v1.3.0.
* Properly match wildcard hostname known_hosts entries regardless of port number, providing a solution for [golang/go#52056](https://github.com/golang/go/issues/52056). (Added in v1.3.0; requires [using the NewDB constructor](#enhancements-requiring-extra-parsing))
* Write new known_hosts entries to an io.Writer
* Properly format/normalize new known_hosts entries containing ipv6 addresses, providing a solution for [golang/go#53463](https://github.com/golang/go/issues/53463)
* Determine if an ssh.HostKeyCallback's error corresponds to a host whose key has changed (indicating potential MitM attack) vs a host that just isn't known yet
Expand Down Expand Up @@ -57,6 +58,19 @@ func sshConfigForHost(hostWithPort string) (*ssh.ClientConfig, error) {
}
```

## Enhancements requiring extra parsing

Originally, this package did not re-read/re-parse the known_hosts files at all, relying entirely on [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) for all known_hosts file reading and processing. This package only offered a constructor called `New`, returning a host key callback, identical to the call pattern of [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) but with extra methods available on the callback type.

However, a couple bugs in [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) cannot possibly be solved without re-reading the known_hosts file. Therefore, as of v1.3.0 of this package, we now offer an alternative constructor `NewDB`, which does an additional read of the known_hosts file (after the one from [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts)), in order to detect:

* @cert-authority lines, so that we can correctly reeturn cert key algorithms instead of normal host key algorithms when appropriate
* host pattern wildcards, so that we can match OpenSSH's behavior for non-standard port numbers, unlike how [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) normally treats them

Aside from *detecting* these special cases, this package otherwise still directly uses [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) for host lookups and all other known_hosts file processing. We do not fork or re-implement core behaviors of [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts).

The performance impact of this extra read should be minimal, as the file should typically be in the filesystem cache already from the original read by [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts). That said, users who wish to avoid the extra read can stay with the `New` constructor, which intentionally retains its pre-v1.3.0 behavior as-is. However, the extra fixes for @cert-authority and host pattern wildcards will not be enabled in that case.

## Writing new known_hosts entries

If you wish to mimic the behavior of OpenSSH's `StrictHostKeyChecking=no` or `StrictHostKeyChecking=ask`, this package provides a few functions to simplify this task. For example:
Expand Down
39 changes: 39 additions & 0 deletions example_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,45 @@ func ExampleNewDB() {
defer client.Close()
}

func ExampleHostKeyCallback_ToDB() {
khFile := "/home/myuser/.ssh/known_hosts"
var kh *knownhosts.HostKeyDB
var err error

// Example of using conditional logic to determine whether or not to perform
// extra parsing pass on the known_hosts file in order to enable enhanced
// behaviors
if os.Getenv("SKIP_KNOWNHOSTS_ENHANCEMENTS") != "" {
// Create a HostKeyDB using New + ToDB: this will skip the extra known_hosts
// processing
var cb knownhosts.HostKeyCallback
if cb, err = knownhosts.New(khFile); err == nil {
kh = cb.ToDB()
}
} else {
// Create a HostKeyDB using NewDB: this will perform extra known_hosts
// processing, allowing proper support for CAs, as well as OpenSSH-like
// wildcard matching on non-standard ports
kh, err = knownhosts.NewDB(khFile)
}
if err != nil {
log.Fatal("Failed to read known_hosts: ", err)
}

sshHost := "yourserver.com:22"
config := &ssh.ClientConfig{
User: "myuser",
Auth: []ssh.AuthMethod{ /* ... */ },
HostKeyCallback: kh.HostKeyCallback(),
HostKeyAlgorithms: kh.HostKeyAlgorithms(sshHost),
}
client, err := ssh.Dial("tcp", sshHost, config)
if err != nil {
log.Fatal("Failed to dial: ", err)
}
defer client.Close()
}

func ExampleWriteKnownHost() {
sshHost := "yourserver.com:22"
khPath := "/home/myuser/.ssh/known_hosts"
Expand Down
123 changes: 97 additions & 26 deletions knownhosts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,31 @@ import (
// behaviors, such as the ability to perform host key/algorithm lookups from
// known_hosts entries.
type HostKeyDB struct {
callback ssh.HostKeyCallback
isCert map[string]bool // keyed by "filename:line"
callback ssh.HostKeyCallback
isCert map[string]bool // keyed by "filename:line"
isWildcard map[string]bool // keyed by "filename:line"
}

// NewDB creates a HostKeyDB from the given OpenSSH known_hosts file(s). It
// reads and parses the provided files one additional time (beyond logic in
// golang.org/x/crypto/ssh/knownhosts) in order to handle CA lines properly.
// golang.org/x/crypto/ssh/knownhosts) in order to:
//
// - Handle CA lines properly and return ssh.CertAlgo* values when calling the
// HostKeyAlgorithms method, for use in ssh.ClientConfig.HostKeyAlgorithms
// - Allow * wildcards in hostnames to match on non-standard ports, providing
// a workaround for https://github.com/golang/go/issues/52056 in order to
// align with OpenSSH's wildcard behavior
//
// When supplying multiple files, their order does not matter.
func NewDB(files ...string) (*HostKeyDB, error) {
cb, err := xknownhosts.New(files...)
if err != nil {
return nil, err
}
hkdb := &HostKeyDB{
callback: cb,
isCert: make(map[string]bool),
callback: cb,
isCert: make(map[string]bool),
isWildcard: make(map[string]bool),
}

// Re-read each file a single time, looking for @cert-authority lines. The
Expand All @@ -59,6 +68,16 @@ func NewDB(files ...string) (*HostKeyDB, error) {
if len(line) > 15 && bytes.HasPrefix(line, []byte("@cert-authority")) && (line[15] == ' ' || line[15] == '\t') {
mapKey := fmt.Sprintf("%s:%d", filename, lineNum)
hkdb.isCert[mapKey] = true
line = bytes.TrimSpace(line[16:])
}
// truncate line to just the host pattern field
if i := bytes.IndexAny(line, "\t "); i >= 0 {
line = line[:i]
}
// Does the host pattern contain a * wildcard and no specific port?
if i := bytes.IndexRune(line, '*'); i >= 0 && !bytes.Contains(line[i:], []byte("]:")) {
mapKey := fmt.Sprintf("%s:%d", filename, lineNum)
hkdb.isWildcard[mapKey] = true
}
}
if err := scanner.Err(); err != nil {
Expand All @@ -73,11 +92,56 @@ func NewDB(files ...string) (*HostKeyDB, error) {
// Alternatively, you can wrap it with an outer callback to potentially handle
// appending a new entry to the known_hosts file; see example in WriteKnownHost.
func (hkdb *HostKeyDB) HostKeyCallback() ssh.HostKeyCallback {
return hkdb.callback
// Either NewDB found no wildcard host patterns, or hkdb was created from
// HostKeyCallback.ToDB in which case we didn't scan known_hosts for them:
// return the callback (which came from x/crypto/ssh/knownhosts) as-is
if len(hkdb.isWildcard) == 0 {
return hkdb.callback
}

// If we scanned for wildcards and found at least one, return a wrapped
// callback with extra behavior: if the host lookup found no matches, and the
// host arg had a non-standard port, re-do the lookup on standard port 22. If
// that second call returns a *xknownhosts.KeyError, filter down any resulting
// Want keys to known wildcard entries.
f := func(hostname string, remote net.Addr, key ssh.PublicKey) error {
callbackErr := hkdb.callback(hostname, remote, key)
if callbackErr == nil || IsHostKeyChanged(callbackErr) { // hostname has known_host entries as-is
return callbackErr
}
justHost, port, splitErr := net.SplitHostPort(hostname)
if splitErr != nil || port == "" || port == "22" { // hostname already using standard port
return callbackErr
}
// If we reach here, the port was non-standard and no known_host entries
// were found for the non-standard port. Try again with standard port.
if tcpAddr, ok := remote.(*net.TCPAddr); ok && tcpAddr.Port != 22 {
remote = &net.TCPAddr{
IP: tcpAddr.IP,
Port: 22,
Zone: tcpAddr.Zone,
}
}
callbackErr = hkdb.callback(justHost+":22", remote, key)
var keyErr *xknownhosts.KeyError
if errors.As(callbackErr, &keyErr) && len(keyErr.Want) > 0 {
wildcardKeys := make([]xknownhosts.KnownKey, 0, len(keyErr.Want))
for _, wantKey := range keyErr.Want {
if hkdb.isWildcard[fmt.Sprintf("%s:%d", wantKey.Filename, wantKey.Line)] {
wildcardKeys = append(wildcardKeys, wantKey)
}
}
callbackErr = &xknownhosts.KeyError{
Want: wildcardKeys,
}
}
return callbackErr
}
return ssh.HostKeyCallback(f)
}

// PublicKey wraps ssh.PublicKey with an additional field, to identify
// whether they key corresponds to a certificate authority.
// whether the key corresponds to a certificate authority.
type PublicKey struct {
ssh.PublicKey
Cert bool
Expand All @@ -96,7 +160,8 @@ func (hkdb *HostKeyDB) HostKeys(hostWithPort string) (keys []PublicKey) {
placeholderAddr := &net.TCPAddr{IP: []byte{0, 0, 0, 0}}
placeholderPubKey := &fakePublicKey{}
var kkeys []xknownhosts.KnownKey
if hkcbErr := hkdb.callback(hostWithPort, placeholderAddr, placeholderPubKey); errors.As(hkcbErr, &keyErr) {
callback := hkdb.HostKeyCallback()
if hkcbErr := callback(hostWithPort, placeholderAddr, placeholderPubKey); errors.As(hkcbErr, &keyErr) {
kkeys = append(kkeys, keyErr.Want...)
knownKeyLess := func(i, j int) bool {
if kkeys[i].Filename < kkeys[j].Filename {
Expand Down Expand Up @@ -190,14 +255,16 @@ func keyTypeToCertAlgo(keyType string) string {
// otherwise identical to ssh.HostKeyCallback, and does not introduce any file-
// parsing behavior beyond what is in golang.org/x/crypto/ssh/knownhosts.
//
// In most situations, use HostKeyDB and its constructor NewDB instead of using
// the HostKeyCallback type. The HostKeyCallback type is only provided for
// backwards compatibility with older versions of this package, as well as for
// very strict situations where any extra known_hosts file-parsing is
// undesirable.
//
// Methods of HostKeyCallback do not provide any special treatment for
// @cert-authority lines, which will (incorrectly) look like normal non-CA host
// keys. HostKeyCallback should generally only be used in situations in which
// @cert-authority lines won't appear, and/or in very strict situations where
// any extra known_hosts file-parsing is undesirable.
//
// In most situations, use HostKeyDB and its constructor NewDB instead of using
// the HostKeyCallback type.
// keys. Additionally, HostKeyCallback lacks the fix for applying * wildcard
// known_host entries to all ports, like OpenSSH's behavior.
type HostKeyCallback ssh.HostKeyCallback

// New creates a HostKeyCallback from the given OpenSSH known_hosts file(s). The
Expand All @@ -207,9 +274,9 @@ type HostKeyCallback ssh.HostKeyCallback
// When supplying multiple files, their order does not matter.
//
// In most situations, you should avoid this function, as the returned value
// does not handle @cert-authority lines correctly. See doc comment for
// HostKeyCallback for more information. Instead, use NewDB to create a
// HostKeyDB with proper CA support.
// lacks several enhanced behaviors. See doc comment for HostKeyCallback for
// more information. Instead, most callers should use NewDB to create a
// HostKeyDB, which includes these enhancements.
func New(files ...string) (HostKeyCallback, error) {
cb, err := xknownhosts.New(files...)
return HostKeyCallback(cb), err
Expand All @@ -222,16 +289,20 @@ func (hkcb HostKeyCallback) HostKeyCallback() ssh.HostKeyCallback {
}

// ToDB converts the receiver into a HostKeyDB. However, the returned HostKeyDB
// lacks proper CA support. It is usually preferable to create a CA-supporting
// HostKeyDB instead, by using NewDB.
// This method is provided for situations in which the calling code needs to
// make CA support optional / user-configurable. This way, calling code can
// conditionally create a non-CA-supporting HostKeyDB by calling New(...).ToDB()
// or a CA-supporting HostKeyDB by calling NewDB(...).
// lacks the enhanced behaviors described in the doc comment for NewDB: proper
// CA support, and wildcard matching on nonstandard ports.
//
// It is generally preferable to create a HostKeyDB by using NewDB. The ToDB
// method is only provided for situations in which the calling code needs to
// make the extra NewDB behaviors optional / user-configurable, perhaps for
// reasons of performance or code trust (since NewDB reads the known_host file
// an extra time, which may be undesirable in some strict situations). This way,
// callers can conditionally create a non-enhanced HostKeyDB by using New and
// ToDB. See code example.
func (hkcb HostKeyCallback) ToDB() *HostKeyDB {
// This intentionally leaves the isCert map field as nil, as there is no way
// to retroactively populate it from just a HostKeyCallback. Methods of
// HostKeyDB will skip any CA-related behaviors accordingly.
// This intentionally leaves the isCert and isWildcard map fields as nil, as
// there is no way to retroactively populate them from just a HostKeyCallback.
// Methods of HostKeyDB will skip any related enhanced behaviors accordingly.
return &HostKeyDB{callback: ssh.HostKeyCallback(hkcb)}
}

Expand Down
14 changes: 13 additions & 1 deletion knownhosts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,13 @@ func TestWithCertLines(t *testing.T) {
expectedAlgos: []string{ssh.KeyAlgoRSASHA512, ssh.KeyAlgoRSASHA256, ssh.KeyAlgoRSA, ssh.CertAlgoECDSA256v01},
},
{
host: "whatever.lol.test:22", // only matches the * entry
host: "whatever.test:22", // only matches the * entry
expectedKeyTypes: []string{ssh.KeyAlgoECDSA256},
expectedIsCert: []bool{true},
expectedAlgos: []string{ssh.CertAlgoECDSA256v01},
},
{
host: "whatever.test:22022", // only matches the * entry
expectedKeyTypes: []string{ssh.KeyAlgoECDSA256},
expectedIsCert: []bool{true},
expectedAlgos: []string{ssh.CertAlgoECDSA256v01},
Expand All @@ -202,6 +208,12 @@ func TestWithCertLines(t *testing.T) {
expectedIsCert: []bool{true, true, true},
expectedAlgos: []string{ssh.CertAlgoRSASHA512v01, ssh.CertAlgoRSASHA256v01, ssh.CertAlgoRSAv01, ssh.CertAlgoECDSA256v01, ssh.CertAlgoED25519v01},
},
{
host: "oddport.certy.test:2345",
expectedKeyTypes: []string{ssh.KeyAlgoRSA, ssh.KeyAlgoECDSA256, ssh.KeyAlgoED25519},
expectedIsCert: []bool{true, true, true},
expectedAlgos: []string{ssh.CertAlgoRSASHA512v01, ssh.CertAlgoRSASHA256v01, ssh.CertAlgoRSAv01, ssh.CertAlgoECDSA256v01, ssh.CertAlgoED25519v01},
},
}
for _, tc := range testCases {
annotatedKeys := kh.HostKeys(tc.host)
Expand Down

0 comments on commit 8b8ca37

Please sign in to comment.