From d4e7c9cb6cb8bb64ad4a1988cd26328ef6cb9023 Mon Sep 17 00:00:00 2001 From: samiponkanen Date: Fri, 17 May 2024 12:09:53 +0000 Subject: [PATCH] ssh: fail client auth immediately on receiving disconnect message Fixes golang/go#66991 Change-Id: I60dd8a807578f162fda0e49bcd6fbf289d444396 GitHub-Last-Rev: f88329d35712873d0d7e3b39b9b11e7bfbc28e71 GitHub-Pull-Request: golang/crypto#293 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/581075 Reviewed-by: Cherry Mui Reviewed-by: Roland Shoemaker Auto-Submit: Nicola Murino Reviewed-by: Nicola Murino LUCI-TryBot-Result: Go LUCI --- ssh/client_auth.go | 4 +++ ssh/test/session_test.go | 69 ++++++++++++++++++++++++++++++++++++++ ssh/test/test_unix_test.go | 9 +++-- 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/ssh/client_auth.go b/ssh/client_auth.go index 9486c59862..b93961010d 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -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 diff --git a/ssh/test/session_test.go b/ssh/test/session_test.go index e9bfa9ec27..53e6645633 100644 --- a/ssh/test/session_test.go +++ b/ssh/test/session_test.go @@ -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") + } +} diff --git a/ssh/test/test_unix_test.go b/ssh/test/test_unix_test.go index 9695de7319..12698e49ab 100644 --- a/ssh/test/test_unix_test.go +++ b/ssh/test/test_unix_test.go @@ -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