Skip to content

Commit c35b808

Browse files
evaneliasmcuadros
evanelias
authored and
mcuadros
committed
plumbing: transport/ssh, auto-populate ClientConfig.HostKeyAlgorithms. Fixes #411
This commit adjusts the transport/ssh logic in command.connect(), so that it now auto-populates ssh.ClientConfig.HostKeyAlgorithms. The algorithms are chosen based on the known host keys for the target host, as obtained from the known_hosts file. In order to look-up the algorithms from the known_hosts file, external module github.com/skeema/knownhosts is used. This package is just a thin wrapper around golang.org/x/crypto/ssh/knownhosts, adding an extra mechanism to query the known_hosts keys, implemented in a way which avoids duplication of any golang.org/x/crypto/ssh/knownhosts logic. Because HostKeyAlgorithms vary by target host, some related logic for setting HostKeyCallback has been moved out of the various AuthMethod implementations. This was necessary because the old HostKeyCallbackHelper is not host-specific. Since known_hosts handling isn't really tied to AuthMethod anyway, it seems reasonable to separate these. Previously-exported types/methods remain in place for backwards compat, but some of them are now unused. For testing approach, see pull request. Issue #411 can only be reproduced via end-to-end / integration testing, since it requires actually launching an SSH connection, in order to see the key mismatch error triggered from golang/go#29286 as the root cause.
1 parent af1efaa commit c35b808

File tree

4 files changed

+55
-25
lines changed

4 files changed

+55
-25
lines changed

‎go.mod‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ require (
1717
github.com/jessevdk/go-flags v1.5.0
1818
github.com/kevinburke/ssh_config v0.0.0-20201106050909-4977a11b4351
1919
github.com/sergi/go-diff v1.1.0
20+
github.com/skeema/knownhosts v1.1.0
2021
github.com/xanzy/ssh-agent v0.3.1
21-
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97
22-
golang.org/x/net v0.0.0-20210326060303-6b1517762897
22+
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e
23+
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2
2324
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c
24-
golang.org/x/text v0.3.3
25+
golang.org/x/text v0.3.6
2526
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
2627
gopkg.in/warnings.v0 v0.1.2 // indirect
2728
)

‎go.sum‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
5151
github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
5252
github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
5353
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
54+
github.com/skeema/knownhosts v1.1.0 h1:Wvr9V0MxhjRbl3f9nMnKnFfiWTJmtECJ9Njkea3ysW0=
55+
github.com/skeema/knownhosts v1.1.0/go.mod h1:sKFq3RD6/TKZkSWn8boUbDC7Qkgcv+8XXijpFO6roag=
5456
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
5557
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
5658
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
@@ -59,24 +61,26 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
5961
github.com/xanzy/ssh-agent v0.3.1 h1:AmzO1SSWxw73zxFZPRwaMN1MohDw8UyHnmuxyceTEGo=
6062
github.com/xanzy/ssh-agent v0.3.1/go.mod h1:QIE4lCeL7nkC25x+yA3LBIYfwCc1TFziCtG7cBAac6w=
6163
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
62-
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 h1:/UOmuWzQfxxo9UtlXMwuQU8CMgg1eZXqTRwkSQJWKOI=
6364
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
65+
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e h1:T8NU3HyQ8ClP4SEE+KbFlg6n0NhuTsN4MyznaarGsZM=
66+
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
6467
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
65-
golang.org/x/net v0.0.0-20210326060303-6b1517762897 h1:KrsHThm5nFk34YtATK1LsThyGhGbGe1olrte/HInHvs=
66-
golang.org/x/net v0.0.0-20210326060303-6b1517762897/go.mod h1:uSPa2vr4CLtc/ILN5odXGNXS6mhrKVzTaCXzk9m6W3k=
68+
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2 h1:CIJ76btIcR3eFI5EgSo6k1qKw9KJexJuRLI9G7Hp5wE=
69+
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
6770
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6871
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
6972
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
7073
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
7174
golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
72-
golang.org/x/sys v0.0.0-20210324051608-47abb6519492/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
75+
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
7376
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
7477
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c h1:F1jZWGFhYfh0Ci55sIpILtKKK8p3i2/krTr0H1rg74I=
7578
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
7679
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E=
7780
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
78-
golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=
7981
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
82+
golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
83+
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
8084
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
8185
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
8286
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

‎plumbing/transport/ssh/auth_method.go‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import (
1010

1111
"github.com/go-git/go-git/v5/plumbing/transport"
1212

13+
"github.com/skeema/knownhosts"
1314
sshagent "github.com/xanzy/ssh-agent"
1415
"golang.org/x/crypto/ssh"
15-
"golang.org/x/crypto/ssh/knownhosts"
1616
)
1717

1818
const DefaultUsername = "git"
@@ -43,7 +43,6 @@ const (
4343
type KeyboardInteractive struct {
4444
User string
4545
Challenge ssh.KeyboardInteractiveChallenge
46-
HostKeyCallbackHelper
4746
}
4847

4948
func (a *KeyboardInteractive) Name() string {
@@ -55,19 +54,18 @@ func (a *KeyboardInteractive) String() string {
5554
}
5655

5756
func (a *KeyboardInteractive) ClientConfig() (*ssh.ClientConfig, error) {
58-
return a.SetHostKeyCallback(&ssh.ClientConfig{
57+
return &ssh.ClientConfig{
5958
User: a.User,
6059
Auth: []ssh.AuthMethod{
6160
a.Challenge,
6261
},
63-
})
62+
}, nil
6463
}
6564

6665
// Password implements AuthMethod by using the given password.
6766
type Password struct {
6867
User string
6968
Password string
70-
HostKeyCallbackHelper
7169
}
7270

7371
func (a *Password) Name() string {
@@ -79,18 +77,17 @@ func (a *Password) String() string {
7977
}
8078

8179
func (a *Password) ClientConfig() (*ssh.ClientConfig, error) {
82-
return a.SetHostKeyCallback(&ssh.ClientConfig{
80+
return &ssh.ClientConfig{
8381
User: a.User,
8482
Auth: []ssh.AuthMethod{ssh.Password(a.Password)},
85-
})
83+
}, nil
8684
}
8785

8886
// PasswordCallback implements AuthMethod by using a callback
8987
// to fetch the password.
9088
type PasswordCallback struct {
9189
User string
9290
Callback func() (pass string, err error)
93-
HostKeyCallbackHelper
9491
}
9592

9693
func (a *PasswordCallback) Name() string {
@@ -102,17 +99,16 @@ func (a *PasswordCallback) String() string {
10299
}
103100

104101
func (a *PasswordCallback) ClientConfig() (*ssh.ClientConfig, error) {
105-
return a.SetHostKeyCallback(&ssh.ClientConfig{
102+
return &ssh.ClientConfig{
106103
User: a.User,
107104
Auth: []ssh.AuthMethod{ssh.PasswordCallback(a.Callback)},
108-
})
105+
}, nil
109106
}
110107

111108
// PublicKeys implements AuthMethod by using the given key pairs.
112109
type PublicKeys struct {
113110
User string
114111
Signer ssh.Signer
115-
HostKeyCallbackHelper
116112
}
117113

118114
// NewPublicKeys returns a PublicKeys from a PEM encoded private key. An
@@ -151,10 +147,10 @@ func (a *PublicKeys) String() string {
151147
}
152148

153149
func (a *PublicKeys) ClientConfig() (*ssh.ClientConfig, error) {
154-
return a.SetHostKeyCallback(&ssh.ClientConfig{
150+
return &ssh.ClientConfig{
155151
User: a.User,
156152
Auth: []ssh.AuthMethod{ssh.PublicKeys(a.Signer)},
157-
})
153+
}, nil
158154
}
159155

160156
func username() (string, error) {
@@ -177,7 +173,6 @@ func username() (string, error) {
177173
type PublicKeysCallback struct {
178174
User string
179175
Callback func() (signers []ssh.Signer, err error)
180-
HostKeyCallbackHelper
181176
}
182177

183178
// NewSSHAgentAuth returns a PublicKeysCallback based on a SSH agent, it opens
@@ -212,10 +207,10 @@ func (a *PublicKeysCallback) String() string {
212207
}
213208

214209
func (a *PublicKeysCallback) ClientConfig() (*ssh.ClientConfig, error) {
215-
return a.SetHostKeyCallback(&ssh.ClientConfig{
210+
return &ssh.ClientConfig{
216211
User: a.User,
217212
Auth: []ssh.AuthMethod{ssh.PublicKeysCallback(a.Callback)},
218-
})
213+
}, nil
219214
}
220215

221216
// NewKnownHostsCallback returns ssh.HostKeyCallback based on a file based on a
@@ -231,6 +226,11 @@ func (a *PublicKeysCallback) ClientConfig() (*ssh.ClientConfig, error) {
231226
// ~/.ssh/known_hosts
232227
// /etc/ssh/ssh_known_hosts
233228
func NewKnownHostsCallback(files ...string) (ssh.HostKeyCallback, error) {
229+
kh, err := newKnownHosts(files...)
230+
return ssh.HostKeyCallback(kh), err
231+
}
232+
233+
func newKnownHosts(files ...string) (knownhosts.HostKeyCallback, error) {
234234
var err error
235235

236236
if len(files) == 0 {
@@ -286,6 +286,9 @@ func filterKnownHostsFiles(files ...string) ([]string, error) {
286286

287287
// HostKeyCallbackHelper is a helper that provides common functionality to
288288
// configure HostKeyCallback into a ssh.ClientConfig.
289+
// Deprecated in favor of SetConfigHostKeyFields (see common.go) which provides
290+
// a mechanism for also setting ClientConfig.HostKeyAlgorithms for a specific
291+
// host.
289292
type HostKeyCallbackHelper struct {
290293
// HostKeyCallback is the function type used for verifying server keys.
291294
// If nil default callback will be create using NewKnownHostsCallback

‎plumbing/transport/ssh/common.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,15 @@ func (c *command) connect() error {
121121
if err != nil {
122122
return err
123123
}
124+
hostWithPort := c.getHostWithPort()
125+
config, err = SetConfigHostKeyFields(config, hostWithPort)
126+
if err != nil {
127+
return err
128+
}
124129

125130
overrideConfig(c.config, config)
126131

127-
c.client, err = dial("tcp", c.getHostWithPort(), config)
132+
c.client, err = dial("tcp", hostWithPort, config)
128133
if err != nil {
129134
return err
130135
}
@@ -162,6 +167,23 @@ func dial(network, addr string, config *ssh.ClientConfig) (*ssh.Client, error) {
162167
return ssh.NewClient(c, chans, reqs), nil
163168
}
164169

170+
// SetConfigHostKeyFields sets cfg.HostKeyCallback and cfg.HostKeyAlgorithms
171+
// based on OpenSSH known_hosts. cfg is modified in-place. hostWithPort must be
172+
// supplied, since the algorithms will be set based on the known host keys for
173+
// that specific host. Otherwise, golang.org/x/crypto/ssh can return an error
174+
// upon connecting to a host whose *first* key is not known, even though other
175+
// keys (of different types) are known and match properly.
176+
// For background see https://github.com/go-git/go-git/issues/411 as well as
177+
// https://github.com/golang/go/issues/29286 for root cause.
178+
func SetConfigHostKeyFields(cfg *ssh.ClientConfig, hostWithPort string) (*ssh.ClientConfig, error) {
179+
kh, err := newKnownHosts()
180+
if err == nil {
181+
cfg.HostKeyCallback = kh.HostKeyCallback()
182+
cfg.HostKeyAlgorithms = kh.HostKeyAlgorithms(hostWithPort)
183+
}
184+
return cfg, err
185+
}
186+
165187
func (c *command) getHostWithPort() string {
166188
if addr, found := c.doGetHostWithPortFromSSHConfig(); found {
167189
return addr

0 commit comments

Comments
 (0)