diff --git a/README.md b/README.md index d569123..87c187c 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,9 @@ This repo ([github.com/skeema/knownhosts](https://github.com/skeema/knownhosts)) * 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) + * As of v1.3.0, this also properly handles CAs when [using the NewDB constructor](#enhancements-requiring-extra-parsing) +* 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 +60,19 @@ func sshConfigForHost(hostWithPort string) (*ssh.ClientConfig, error) { } ``` +## Enhancements requiring extra parsing + +Originally, this package did not re-read/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). This package just added extra methods on top of its returned 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 returns a struct type `HostKeyDB`. When using `NewDB`, this package 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, which require returning different host key algorithm values than regular host keys +* host pattern wildcards, which [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts) doesn't properly match against all port numbers, like OpenSSH does + +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 these 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 retains its pre-v1.3.0 behavior as-is. However, the extra fixes for @cert-authority and host pattern wildcards will not be present 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)