Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use skeema/knownhosts, not x/crypto/ssh/knownhosts #2212

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -300,6 +300,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 @@ -16,14 +16,20 @@ import (
"strings"
"time"

// 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/containers/common/pkg/config"
"github.com/containers/storage/pkg/fileutils"
"github.com/containers/storage/pkg/homedir"
"github.com/pkg/sftp"
"github.com/sirupsen/logrus"
"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) {
Copy link
Member

Choose a reason for hiding this comment

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

this seems pointless, why check if the dir already exists. We could also just directly call mkdir and ignore ErrExist there

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I just left as it was as it's not doing any harm (except that's hard to read with all those nested conditions...).

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