Skip to content

Commit

Permalink
Use skeema/knownhosts, not x/crypto/ssh/knownhosts
Browse files Browse the repository at this point in the history
Package [golang.org/x/crypto/ssh/knownhosts](https://pkg.go.dev/golang.org/x/crypto/ssh/knownhosts)
has an issue when an SSH server has many public keys (i.e. supports multiple crypto algorithms).

For instance, if the local `known_hosts` file entries don't match the first SSH server key but match
other keys of the SSH server, the handshake fails with a key mismatch error.

See golang/go#29286 and containers/podman#23575.

Package [github.com/skeema/knownhosts](https://github.com/skeema/knownhosts) is a wrapper of
`x/crypto/ssh/knownhosts` that addresses this issue.

This commit replaces the usage of `x/crypto/ssh/knownhosts` in containers/common with
`github.com/skeema/knownhosts`.

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
  • Loading branch information
l0rd committed Oct 22, 2024
1 parent 88d52c6 commit ca63ba0
Show file tree
Hide file tree
Showing 9 changed files with 875 additions and 34 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ require (
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2
github.com/seccomp/libseccomp-golang v0.10.0
github.com/sirupsen/logrus v1.9.3
github.com/skeema/knownhosts v1.3.0
github.com/spf13/cobra v1.8.1
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ github.com/sigstore/sigstore v1.8.4 h1:g4ICNpiENFnWxjmBzBDWUn62rNFeny/P77HUC8da3
github.com/sigstore/sigstore v1.8.4/go.mod h1:1jIKtkTFEeISen7en+ZPWdDHazqhxco/+v9CNjc7oNg=
github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ=
github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ=
github.com/skeema/knownhosts v1.3.0 h1:AM+y0rI04VksttfwjkSTNQorvGqmwATnvnAHpSgc0LY=
github.com/skeema/knownhosts v1.3.0/go.mod h1:sPINvnADmT/qYH1kfv+ePMmOBTH6Tbl7b5LvTDjFK7M=
github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM=
github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
Expand Down
73 changes: 39 additions & 34 deletions pkg/ssh/connection_golang.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,15 @@ import (
"github.com/containers/storage/pkg/homedir"
"github.com/pkg/sftp"
"github.com/sirupsen/logrus"
// We are using skeema/knownhosts rather than
// golang.org/x/crypto/ssh/knownhosts because the
// latter has an issue when the first key returned
// by the server doesn't match the one in known_hosts:
// https://github.com/golang/go/issues/29286
// https://github.com/containers/podman/issues/23575
"github.com/skeema/knownhosts"
"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/agent"
"golang.org/x/crypto/ssh/knownhosts"
"golang.org/x/exp/maps"
)

Expand Down Expand Up @@ -301,46 +307,44 @@ func ValidateAndConfigure(uri *url.URL, iden string, insecureIsMachineConnection
return nil, err
}

keyFilePath := filepath.Join(homedir.Get(), ".ssh", "known_hosts")
known, err := knownhosts.NewDB(keyFilePath)
if err != nil {
if !errors.Is(err, os.ErrNotExist) {
return nil, err
}
keyDir := path.Dir(keyFilePath)
if err := fileutils.Exists(keyDir); errors.Is(err, os.ErrNotExist) {
if err := os.Mkdir(keyDir, 0o700); err != nil {
return nil, err
}
}
k, err := os.OpenFile(keyFilePath, os.O_RDWR|os.O_CREATE, 0o600)
if err != nil {
return nil, err
}
k.Close()
known, err = knownhosts.NewDB(keyFilePath)
if err != nil {
return nil, err
}
}

var callback ssh.HostKeyCallback
if insecureIsMachineConnection {
callback = ssh.InsecureIgnoreHostKey()
} else {
callback = ssh.HostKeyCallback(func(host string, remote net.Addr, pubKey ssh.PublicKey) error {
keyFilePath := filepath.Join(homedir.Get(), ".ssh", "known_hosts")
known, err := knownhosts.New(keyFilePath)
if err != nil {
if !errors.Is(err, os.ErrNotExist) {
return err
}
keyDir := path.Dir(keyFilePath)
if err := fileutils.Exists(keyDir); errors.Is(err, os.ErrNotExist) {
if err := os.Mkdir(keyDir, 0o700); err != nil {
return err
}
}
k, err := os.OpenFile(keyFilePath, os.O_RDWR|os.O_CREATE, 0o600)
if err != nil {
return err
}
k.Close()
known, err = knownhosts.New(keyFilePath)
if err != nil {
return err
}
}
// we need to check if there is an error from reading known hosts for this public key and if there is an error, what is it, and why is it happening?
// if it is a key mismatch we want to error since we know the host using another key
// however, if it is a general error not because of a known key, we want to add our key to the known_hosts file
hErr := known(host, remote, pubKey)
var keyErr *knownhosts.KeyError
// if keyErr.Want is not empty, we are receiving a different key meaning the host is known but we are using the wrong key
as := errors.As(hErr, &keyErr)
hErr := known.HostKeyCallback()(host, remote, pubKey)
switch {
case as && len(keyErr.Want) > 0:
case knownhosts.IsHostKeyChanged(hErr):
logrus.Warnf("ssh host key mismatch for host %s, got key %s of type %s", host, ssh.FingerprintSHA256(pubKey), pubKey.Type())
return keyErr
return hErr
// if keyErr.Want is empty that just means we do not know this host yet, add it.
case as && len(keyErr.Want) == 0:
case knownhosts.IsHostUnknown(hErr):
// write to known_hosts
err := addKnownHostsEntry(host, pubKey)
if err != nil {
Expand All @@ -358,10 +362,11 @@ func ValidateAndConfigure(uri *url.URL, iden string, insecureIsMachineConnection
}

cfg := &ssh.ClientConfig{
User: uri.User.Username(),
Auth: authMethods,
HostKeyCallback: callback,
Timeout: tick,
User: uri.User.Username(),
Auth: authMethods,
HostKeyCallback: callback,
Timeout: tick,
HostKeyAlgorithms: known.HostKeyAlgorithms(uri.Host),
}
return cfg, nil
}
Expand Down
36 changes: 36 additions & 0 deletions vendor/github.com/skeema/knownhosts/CONTRIBUTING.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

201 changes: 201 additions & 0 deletions vendor/github.com/skeema/knownhosts/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions vendor/github.com/skeema/knownhosts/NOTICE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit ca63ba0

Please sign in to comment.