From 8b8ca37f5c06772b6ec58e2f13aca8b16eda0600 Mon Sep 17 00:00:00 2001 From: Evan Elias Date: Tue, 16 Jul 2024 16:14:12 -0400 Subject: [PATCH] host matching: handle wildcards with non-standard port (#10) 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 https://github.com/golang/go/issues/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. --- README.md | 22 ++++++-- example_test.go | 39 ++++++++++++++ knownhosts.go | 123 +++++++++++++++++++++++++++++++++++---------- knownhosts_test.go | 14 +++++- 4 files changed, 167 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index d569123..4b6f2c1 100644 --- a/README.md +++ b/README.md @@ -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 @@ -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: diff --git a/example_test.go b/example_test.go index 7a502d9..9770819 100644 --- a/example_test.go +++ b/example_test.go @@ -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" diff --git a/knownhosts.go b/knownhosts.go index f1ff837..2b7536e 100644 --- a/knownhosts.go +++ b/knownhosts.go @@ -22,13 +22,21 @@ 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...) @@ -36,8 +44,9 @@ func NewDB(files ...string) (*HostKeyDB, error) { 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 @@ -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 { @@ -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 @@ -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 { @@ -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 @@ -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 @@ -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)} } diff --git a/knownhosts_test.go b/knownhosts_test.go index 473fc1a..520c7e0 100644 --- a/knownhosts_test.go +++ b/knownhosts_test.go @@ -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}, @@ -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)