From 42ee18b963777d907bbef3e59665cf80968d57e6 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 25 Feb 2024 16:26:56 +0100 Subject: [PATCH] ssh: return ServerAuthError after too many auth failures if a client is disconnected due to too many authentication attempts we should return a ServerAuthError instead of a generic error. Some users check the error returned by NewServerConn to determine whether or not a client attempted to authenticate. Fixes golang/go#69191 Change-Id: If68fcecdefd6c810fe9df8256b1216e320d8a916 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/566398 Reviewed-by: Filippo Valsorda Reviewed-by: Tim King Auto-Submit: Nicola Murino LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee --- ssh/client_auth_test.go | 25 ++++++++++++++++++------- ssh/server.go | 4 ++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index bf0aa1fe23..e981cc49a6 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -641,17 +641,28 @@ func TestClientAuthMaxAuthTries(t *testing.T) { defer c1.Close() defer c2.Close() - go newServer(c1, serverConfig) - _, _, _, err = NewClientConn(c2, "", clientConfig) - if tries > 2 { - if err == nil { + errCh := make(chan error, 1) + + go func() { + _, err := newServer(c1, serverConfig) + errCh <- err + }() + _, _, _, cliErr := NewClientConn(c2, "", clientConfig) + srvErr := <-errCh + + if tries > serverConfig.MaxAuthTries { + if cliErr == nil { t.Fatalf("client: got no error, want %s", expectedErr) - } else if err.Error() != expectedErr.Error() { + } else if cliErr.Error() != expectedErr.Error() { t.Fatalf("client: got %s, want %s", err, expectedErr) } + var authErr *ServerAuthError + if !errors.As(srvErr, &authErr) { + t.Errorf("expected ServerAuthError, got: %v", srvErr) + } } else { - if err != nil { - t.Fatalf("client: got %s, want no error", err) + if cliErr != nil { + t.Fatalf("client: got %s, want no error", cliErr) } } } diff --git a/ssh/server.go b/ssh/server.go index 3ca9e89e22..c0d1c29e6f 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -510,8 +510,8 @@ userAuthLoop: if err := s.transport.writePacket(Marshal(discMsg)); err != nil { return nil, err } - - return nil, discMsg + authErrs = append(authErrs, discMsg) + return nil, &ServerAuthError{Errors: authErrs} } var userAuthReq userAuthRequestMsg