aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAyman Bagabas <ayman.bagabas@gmail.com>2023-01-09 17:34:24 +0300
committerAyman Bagabas <ayman.bagabas@gmail.com>2023-03-05 18:28:47 -0500
commit3ba636d6c9e247882798714e3233930441e0a64e (patch)
tree8ce1df952a6c1d56aa55e130f0c6d82a9ef57380
parent5dabd83e3712e2554745c736b55df405a0ba4f33 (diff)
downloadgo-git-3ba636d6c9e247882798714e3233930441e0a64e.tar.gz
fix(ssh): unable to pass a custom HostKeyCallback func
Don't overwrite HostKeyCallback if one is provided. Fixes: c35b8082c863 ("plumbing: transport/ssh, auto-populate ClientConfig.HostKeyAlgorithms. Fixes #411") Fixes: https://github.com/go-git/go-git/issues/654 Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
-rw-r--r--plumbing/transport/ssh/auth_method.go28
-rw-r--r--plumbing/transport/ssh/common.go33
-rw-r--r--plumbing/transport/ssh/common_test.go79
-rw-r--r--plumbing/transport/ssh/upload_pack_test.go4
4 files changed, 101 insertions, 43 deletions
diff --git a/plumbing/transport/ssh/auth_method.go b/plumbing/transport/ssh/auth_method.go
index 9d3bcd3..e89ce4b 100644
--- a/plumbing/transport/ssh/auth_method.go
+++ b/plumbing/transport/ssh/auth_method.go
@@ -43,6 +43,7 @@ const (
type KeyboardInteractive struct {
User string
Challenge ssh.KeyboardInteractiveChallenge
+ HostKeyCallbackHelper
}
func (a *KeyboardInteractive) Name() string {
@@ -54,18 +55,19 @@ func (a *KeyboardInteractive) String() string {
}
func (a *KeyboardInteractive) ClientConfig() (*ssh.ClientConfig, error) {
- return &ssh.ClientConfig{
+ return a.SetHostKeyCallback(&ssh.ClientConfig{
User: a.User,
Auth: []ssh.AuthMethod{
a.Challenge,
},
- }, nil
+ })
}
// Password implements AuthMethod by using the given password.
type Password struct {
User string
Password string
+ HostKeyCallbackHelper
}
func (a *Password) Name() string {
@@ -77,10 +79,10 @@ func (a *Password) String() string {
}
func (a *Password) ClientConfig() (*ssh.ClientConfig, error) {
- return &ssh.ClientConfig{
+ return a.SetHostKeyCallback(&ssh.ClientConfig{
User: a.User,
Auth: []ssh.AuthMethod{ssh.Password(a.Password)},
- }, nil
+ })
}
// PasswordCallback implements AuthMethod by using a callback
@@ -88,6 +90,7 @@ func (a *Password) ClientConfig() (*ssh.ClientConfig, error) {
type PasswordCallback struct {
User string
Callback func() (pass string, err error)
+ HostKeyCallbackHelper
}
func (a *PasswordCallback) Name() string {
@@ -99,16 +102,17 @@ func (a *PasswordCallback) String() string {
}
func (a *PasswordCallback) ClientConfig() (*ssh.ClientConfig, error) {
- return &ssh.ClientConfig{
+ return a.SetHostKeyCallback(&ssh.ClientConfig{
User: a.User,
Auth: []ssh.AuthMethod{ssh.PasswordCallback(a.Callback)},
- }, nil
+ })
}
// PublicKeys implements AuthMethod by using the given key pairs.
type PublicKeys struct {
User string
Signer ssh.Signer
+ HostKeyCallbackHelper
}
// NewPublicKeys returns a PublicKeys from a PEM encoded private key. An
@@ -147,10 +151,10 @@ func (a *PublicKeys) String() string {
}
func (a *PublicKeys) ClientConfig() (*ssh.ClientConfig, error) {
- return &ssh.ClientConfig{
+ return a.SetHostKeyCallback(&ssh.ClientConfig{
User: a.User,
Auth: []ssh.AuthMethod{ssh.PublicKeys(a.Signer)},
- }, nil
+ })
}
func username() (string, error) {
@@ -173,6 +177,7 @@ func username() (string, error) {
type PublicKeysCallback struct {
User string
Callback func() (signers []ssh.Signer, err error)
+ HostKeyCallbackHelper
}
// NewSSHAgentAuth returns a PublicKeysCallback based on a SSH agent, it opens
@@ -207,10 +212,10 @@ func (a *PublicKeysCallback) String() string {
}
func (a *PublicKeysCallback) ClientConfig() (*ssh.ClientConfig, error) {
- return &ssh.ClientConfig{
+ return a.SetHostKeyCallback(&ssh.ClientConfig{
User: a.User,
Auth: []ssh.AuthMethod{ssh.PublicKeysCallback(a.Callback)},
- }, nil
+ })
}
// NewKnownHostsCallback returns ssh.HostKeyCallback based on a file based on a
@@ -286,9 +291,6 @@ func filterKnownHostsFiles(files ...string) ([]string, error) {
// HostKeyCallbackHelper is a helper that provides common functionality to
// configure HostKeyCallback into a ssh.ClientConfig.
-// Deprecated in favor of SetConfigHostKeyFields (see common.go) which provides
-// a mechanism for also setting ClientConfig.HostKeyAlgorithms for a specific
-// host.
type HostKeyCallbackHelper struct {
// HostKeyCallback is the function type used for verifying server keys.
// If nil default callback will be create using NewKnownHostsCallback
diff --git a/plumbing/transport/ssh/common.go b/plumbing/transport/ssh/common.go
index 4b9ac07..e06958a 100644
--- a/plumbing/transport/ssh/common.go
+++ b/plumbing/transport/ssh/common.go
@@ -10,6 +10,7 @@ import (
"github.com/go-git/go-git/v5/plumbing/transport"
"github.com/go-git/go-git/v5/plumbing/transport/internal/common"
+ "github.com/skeema/knownhosts"
"github.com/kevinburke/ssh_config"
"golang.org/x/crypto/ssh"
@@ -122,9 +123,18 @@ func (c *command) connect() error {
return err
}
hostWithPort := c.getHostWithPort()
- config, err = SetConfigHostKeyFields(config, hostWithPort)
- if err != nil {
- return err
+ if config.HostKeyCallback == nil {
+ kh, err := newKnownHosts()
+ if err != nil {
+ return err
+ }
+ config.HostKeyCallback = kh.HostKeyCallback()
+ config.HostKeyAlgorithms = kh.HostKeyAlgorithms(hostWithPort)
+ } else if len(config.HostKeyAlgorithms) == 0 {
+ // Set the HostKeyAlgorithms based on HostKeyCallback.
+ // For background see https://github.com/go-git/go-git/issues/411 as well as
+ // https://github.com/golang/go/issues/29286 for root cause.
+ config.HostKeyAlgorithms = knownhosts.HostKeyAlgorithms(config.HostKeyCallback, hostWithPort)
}
overrideConfig(c.config, config)
@@ -167,23 +177,6 @@ func dial(network, addr string, config *ssh.ClientConfig) (*ssh.Client, error) {
return ssh.NewClient(c, chans, reqs), nil
}
-// SetConfigHostKeyFields sets cfg.HostKeyCallback and cfg.HostKeyAlgorithms
-// based on OpenSSH known_hosts. cfg is modified in-place. hostWithPort must be
-// supplied, since the algorithms will be set based on the known host keys for
-// that specific host. Otherwise, golang.org/x/crypto/ssh can return an error
-// upon connecting to a host whose *first* key is not known, even though other
-// keys (of different types) are known and match properly.
-// For background see https://github.com/go-git/go-git/issues/411 as well as
-// https://github.com/golang/go/issues/29286 for root cause.
-func SetConfigHostKeyFields(cfg *ssh.ClientConfig, hostWithPort string) (*ssh.ClientConfig, error) {
- kh, err := newKnownHosts()
- if err == nil {
- cfg.HostKeyCallback = kh.HostKeyCallback()
- cfg.HostKeyAlgorithms = kh.HostKeyAlgorithms(hostWithPort)
- }
- return cfg, err
-}
-
func (c *command) getHostWithPort() string {
if addr, found := c.doGetHostWithPortFromSSHConfig(); found {
return addr
diff --git a/plumbing/transport/ssh/common_test.go b/plumbing/transport/ssh/common_test.go
index 6d634d5..496e82d 100644
--- a/plumbing/transport/ssh/common_test.go
+++ b/plumbing/transport/ssh/common_test.go
@@ -5,23 +5,25 @@ import (
"github.com/go-git/go-git/v5/plumbing/transport"
+ "github.com/gliderlabs/ssh"
"github.com/kevinburke/ssh_config"
- "golang.org/x/crypto/ssh"
+ stdssh "golang.org/x/crypto/ssh"
+ "golang.org/x/crypto/ssh/testdata"
. "gopkg.in/check.v1"
)
func Test(t *testing.T) { TestingT(t) }
func (s *SuiteCommon) TestOverrideConfig(c *C) {
- config := &ssh.ClientConfig{
+ config := &stdssh.ClientConfig{
User: "foo",
- Auth: []ssh.AuthMethod{
- ssh.Password("yourpassword"),
+ Auth: []stdssh.AuthMethod{
+ stdssh.Password("yourpassword"),
},
- HostKeyCallback: ssh.FixedHostKey(nil),
+ HostKeyCallback: stdssh.FixedHostKey(nil),
}
- target := &ssh.ClientConfig{}
+ target := &stdssh.ClientConfig{}
overrideConfig(config, target)
c.Assert(target.User, Equals, "foo")
@@ -30,11 +32,11 @@ func (s *SuiteCommon) TestOverrideConfig(c *C) {
}
func (s *SuiteCommon) TestOverrideConfigKeep(c *C) {
- config := &ssh.ClientConfig{
+ config := &stdssh.ClientConfig{
User: "foo",
}
- target := &ssh.ClientConfig{
+ target := &stdssh.ClientConfig{
User: "bar",
}
@@ -93,12 +95,69 @@ func (s *SuiteCommon) TestDefaultSSHConfigWildcard(c *C) {
c.Assert(cmd.getHostWithPort(), Equals, "github.com:22")
}
+func (s *SuiteCommon) TestIgnoreHostKeyCallback(c *C) {
+ uploadPack := &UploadPackSuite{
+ opts: []ssh.Option{
+ ssh.HostKeyPEM(testdata.PEMBytes["ed25519"]),
+ },
+ }
+ uploadPack.SetUpSuite(c)
+ // Use the default client, which does not have a host key callback
+ uploadPack.Client = DefaultClient
+ auth, err := NewPublicKeys("foo", testdata.PEMBytes["rsa"], "")
+ c.Assert(err, IsNil)
+ c.Assert(auth, NotNil)
+ auth.HostKeyCallback = stdssh.InsecureIgnoreHostKey()
+ ep := uploadPack.newEndpoint(c, "bar.git")
+ ps, err := uploadPack.Client.NewUploadPackSession(ep, auth)
+ c.Assert(err, IsNil)
+ c.Assert(ps, NotNil)
+}
+
+func (s *SuiteCommon) TestFixedHostKeyCallback(c *C) {
+ hostKey, err := stdssh.ParsePrivateKey(testdata.PEMBytes["ed25519"])
+ c.Assert(err, IsNil)
+ uploadPack := &UploadPackSuite{
+ opts: []ssh.Option{
+ ssh.HostKeyPEM(testdata.PEMBytes["ed25519"]),
+ },
+ }
+ uploadPack.SetUpSuite(c)
+ // Use the default client, which does not have a host key callback
+ uploadPack.Client = DefaultClient
+ auth, err := NewPublicKeys("foo", testdata.PEMBytes["rsa"], "")
+ c.Assert(err, IsNil)
+ c.Assert(auth, NotNil)
+ auth.HostKeyCallback = stdssh.FixedHostKey(hostKey.PublicKey())
+ ep := uploadPack.newEndpoint(c, "bar.git")
+ ps, err := uploadPack.Client.NewUploadPackSession(ep, auth)
+ c.Assert(err, IsNil)
+ c.Assert(ps, NotNil)
+}
+
+func (s *SuiteCommon) TestFailHostKeyCallback(c *C) {
+ uploadPack := &UploadPackSuite{
+ opts: []ssh.Option{
+ ssh.HostKeyPEM(testdata.PEMBytes["ed25519"]),
+ },
+ }
+ uploadPack.SetUpSuite(c)
+ // Use the default client, which does not have a host key callback
+ uploadPack.Client = DefaultClient
+ auth, err := NewPublicKeys("foo", testdata.PEMBytes["rsa"], "")
+ c.Assert(err, IsNil)
+ c.Assert(auth, NotNil)
+ ep := uploadPack.newEndpoint(c, "bar.git")
+ _, err = uploadPack.Client.NewUploadPackSession(ep, auth)
+ c.Assert(err, NotNil)
+}
+
func (s *SuiteCommon) TestIssue70(c *C) {
uploadPack := &UploadPackSuite{}
uploadPack.SetUpSuite(c)
- config := &ssh.ClientConfig{
- HostKeyCallback: ssh.InsecureIgnoreHostKey(),
+ config := &stdssh.ClientConfig{
+ HostKeyCallback: stdssh.InsecureIgnoreHostKey(),
}
r := &runner{
config: config,
diff --git a/plumbing/transport/ssh/upload_pack_test.go b/plumbing/transport/ssh/upload_pack_test.go
index e65e04a..f172fee 100644
--- a/plumbing/transport/ssh/upload_pack_test.go
+++ b/plumbing/transport/ssh/upload_pack_test.go
@@ -25,6 +25,7 @@ import (
type UploadPackSuite struct {
test.UploadPackSuite
fixtures.Suite
+ opts []ssh.Option
port int
base string
@@ -57,6 +58,9 @@ func (s *UploadPackSuite) SetUpSuite(c *C) {
s.UploadPackSuite.NonExistentEndpoint = s.newEndpoint(c, "non-existent.git")
server := &ssh.Server{Handler: handlerSSH}
+ for _, opt := range s.opts {
+ opt(server)
+ }
go func() {
log.Fatal(server.Serve(l))
}()