Skip to content

Commit

Permalink
ssh: fail client auth immediately on receiving disconnect message
Browse files Browse the repository at this point in the history
Fixes golang/go#66991

Change-Id: I60dd8a807578f162fda0e49bcd6fbf289d444396
GitHub-Last-Rev: f88329d
GitHub-Pull-Request: #293
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/581075
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Nicola Murino <nicola.murino@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
samiponkanen authored and gopherbot committed Jun 4, 2024
1 parent 332fd65 commit d4e7c9c
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 2 deletions.
4 changes: 4 additions & 0 deletions ssh/client_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error {
for auth := AuthMethod(new(noneAuth)); auth != nil; {
ok, methods, err := auth.auth(sessionID, config.User, c.transport, config.Rand, extensions)
if err != nil {
// On disconnect, return error immediately
if _, ok := err.(*disconnectMsg); ok {
return err
}
// We return the error later if there is no other method left to
// try.
ok = authFailure
Expand Down
69 changes: 69 additions & 0 deletions ssh/test/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,72 @@ func TestClientAuthAlgorithms(t *testing.T) {
})
}
}

func TestClientAuthDisconnect(t *testing.T) {
// Use a static key that is not accepted by server.
// This key has been generated with following ssh-keygen command and
// used exclusively in this unit test:
// $ ssh-keygen -t RSA -b 2048 -f /tmp/static_key \
// -C "Static RSA key for golang.org/x/crypto/ssh unit test"

const privKeyData = `-----BEGIN OPENSSH PRIVATE KEY-----
b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAABFwAAAAdzc2gtcn
NhAAAAAwEAAQAAAQEAwV1Zg3MqX27nIQQNWd8V09P4q4F1fx7H2xNJdL3Yg3y91GFLJ92+
0IiGV8n1VMGL/71PPhzyqBpUYSTpWjiU2JZSfA+iTg1GJBcOaEOA6vrXsTtXTHZ//mOT4d
mlvuP4+9NqfCBLGXN7ZJpT+amkD8AVW9YW9QN3ipY61ZWxPaAocVpDd8rVgJTk54KvaPa7
t4ddOSQDQq61aubIDR1Z3P+XjkB4piWOsbck3HJL+veTALy12C09tAhwUnZUAXS+DjhxOL
xpDVclF/yXYhAvBvsjwyk/OC3+nK9F799hpQZsjxmbP7oN+tGwz06BUcAKi7u7QstENvvk
85SDZy1q1QAAA/A7ylbJO8pWyQAAAAdzc2gtcnNhAAABAQDBXVmDcypfbuchBA1Z3xXT0/
irgXV/HsfbE0l0vdiDfL3UYUsn3b7QiIZXyfVUwYv/vU8+HPKoGlRhJOlaOJTYllJ8D6JO
DUYkFw5oQ4Dq+texO1dMdn/+Y5Ph2aW+4/j702p8IEsZc3tkmlP5qaQPwBVb1hb1A3eKlj
rVlbE9oChxWkN3ytWAlOTngq9o9ru3h105JANCrrVq5sgNHVnc/5eOQHimJY6xtyTcckv6
95MAvLXYLT20CHBSdlQBdL4OOHE4vGkNVyUX/JdiEC8G+yPDKT84Lf6cr0Xv32GlBmyPGZ
s/ug360bDPToFRwAqLu7tCy0Q2++TzlINnLWrVAAAAAwEAAQAAAQAIvPDHMiyIxgCksGPF
uyv9F9U4XjVip8/abE9zkAMJWW5++wuT/bRlBOUPRrWIXZEM9ETbtsqswo3Wxah+7CjRIH
qR7SdFlYTP1jPk4yIKXF4OvggBUPySkMpAGJ9hwOMW8Ymcb4gn77JJ4aMoWIcXssje+XiC
8iO+4UWU3SV2i6K7flK1UDCI5JVCyBr3DVf3QhMOgvwJl9TgD7FzWy1hkjuZq/Pzdv+fA2
OfrUFiSukLNolidNoI9+KWa1yxixE+B2oN4Xan3ZbqGbL6Wc1dB+K9h/bNcu+SKf7fXWRi
/vVG44A61xGDZzen1+eQlqFp7narkKXoaU71+45VXDThAAAAgBPWUdQykEEm0yOS6hPIW+
hS8z1LXWGTEcag9fMwJXKE7cQFO3LEk+dXMbClHdhD/ydswOZYGSNepxwvmo/a5LiO2ulp
W+5tnsNhcK3skdaf71t+boUEXBNZ6u3WNTkU7tDN8h9tebI+xlNceDGSGjOlNoHQVMKZdA
W9TA4ZqXUPAAAAgQDWU0UZVOSCAOODPz4PYsbFKdCfXNP8O4+t9txyc9E3hsLAsVs+CpVX
Gr219MGLrublzAxojipyzuQb6Tp1l9nsu7VkcBrPL8I1tokz0AyTnmNF3A9KszBal7gGNS
a2qYuf6JO4cub1KzonxUJQHZPZq9YhCxOtDwTd+uyHZiPy9QAAAIEA5vayd+nfVJgCKTdf
z5MFsxBSUj/cAYg7JYPS/0bZ5bEkLosL22wl5Tm/ZftJa8apkyBPhguAWt6jEWLoDiK+kn
Fv0SaEq1HUdXgWmISVnWzv2pxdAtq/apmbxTg3iIJyrAwEDo13iImR3k6rNPx1m3i/jX56
HLcvWM4Y6bFzbGEAAAA0U3RhdGljIFJTQSBrZXkgZm9yIGdvbGFuZy5vcmcveC9jcnlwdG
8vc3NoIHVuaXQgdGVzdAECAwQFBgc=
-----END OPENSSH PRIVATE KEY-----`

signer, err := ssh.ParsePrivateKey([]byte(privKeyData))
if err != nil {
t.Fatalf("failed to create signer from key: %v", err)
}

// Start server with MaxAuthTries 1 and publickey and password auth
// enabled
server := newServerForConfig(t, "MaxAuthTries", map[string]string{})

// Connect to server, expect failure, that PublicKeysCallback is called
// and that PasswordCallback is not called.
publicKeysCallbackCalled := false
config := clientConfig()
config.Auth = []ssh.AuthMethod{
ssh.PublicKeysCallback(func() ([]ssh.Signer, error) {
publicKeysCallbackCalled = true
return []ssh.Signer{signer}, nil
}),
ssh.PasswordCallback(func() (string, error) {
t.Errorf("unexpected call to PasswordCallback()")
return "notaverygoodpassword", nil
}),
}
client, err := server.TryDial(config)
if err == nil {
t.Errorf("expected TryDial() to fail")
_ = client.Close()
}
if !publicKeysCallbackCalled {
t.Errorf("expected PublicKeysCallback() to be called")
}
}
9 changes: 7 additions & 2 deletions ssh/test/test_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,17 @@ UsePAM yes
PasswordAuthentication yes
ChallengeResponseAuthentication yes
AuthenticationMethods {{.AuthMethods}}
`
maxAuthTriesSshdConfigTail = `
PasswordAuthentication yes
MaxAuthTries 1
`
)

var configTmpl = map[string]*template.Template{
"default": template.Must(template.New("").Parse(defaultSshdConfig)),
"MultiAuth": template.Must(template.New("").Parse(defaultSshdConfig + multiAuthSshdConfigTail))}
"default": template.Must(template.New("").Parse(defaultSshdConfig)),
"MultiAuth": template.Must(template.New("").Parse(defaultSshdConfig + multiAuthSshdConfigTail)),
"MaxAuthTries": template.Must(template.New("").Parse(defaultSshdConfig + maxAuthTriesSshdConfigTail))}

type server struct {
t *testing.T
Expand Down

0 comments on commit d4e7c9c

Please sign in to comment.