diff --git a/ssh/server.go b/ssh/server.go index 0192a67..9292f0b 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -607,6 +607,15 @@ func (b *BannerError) Error() string { return b.Err.Error() } +// maxAuthServerAttempts caps the total number of SSH_MSG_USERAUTH_REQUEST +// messages the server will process on a single connection, regardless of +// outcome (failure, partial success, public key query, or none). It is a +// backstop against clients that drive the authentication loop indefinitely +// without ever incurring a real failure — for example by repeatedly +// triggering PartialSuccessError or by spamming public key offer queries — +// neither of which increment the MaxAuthTries failure counter. +const maxAuthServerAttempts = 128 + func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, error) { if config.PreAuthConnCallback != nil { config.PreAuthConnCallback(s) @@ -617,6 +626,7 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err var perms *Permissions authFailures := 0 + authAttempts := 0 noneAuthCount := 0 var authErrs []error var calledBannerCallback bool @@ -645,6 +655,19 @@ userAuthLoop: return nil, &ServerAuthError{Errors: authErrs} } + if authAttempts >= maxAuthServerAttempts { + discMsg := &disconnectMsg{ + Reason: 2, + Message: "too many authentication attempts", + } + if err := s.transport.writePacket(Marshal(discMsg)); err != nil { + return nil, err + } + authErrs = append(authErrs, discMsg) + return nil, &ServerAuthError{Errors: authErrs} + } + authAttempts++ + var userAuthReq userAuthRequestMsg if packet, err := s.transport.readPacket(); err != nil { if err == io.EOF { diff --git a/ssh/server_test.go b/ssh/server_test.go index 502a25b..01e262b 100644 --- a/ssh/server_test.go +++ b/ssh/server_test.go @@ -166,6 +166,52 @@ func TestMaxAuthTriesNoneMethod(t *testing.T) { } } +func TestMaxAuthServerAttempts(t *testing.T) { + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + invocations := 0 + clientConf := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + AuthCallback: func(ctx *ClientAuthContext) (AuthMethod, error) { + invocations++ + return PublicKeys(testSigners["rsa"]), nil + }, + } + + go NewServerConn(c1, alwaysPartialPubKeyServer()) + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err == nil { + t.Fatal("expected the server to disconnect after exceeding the attempts cap") + } + // The error must be the server's disconnect, not the client-side + // "too many authentication attempts (N), aborting" bound. The + // disconnect format is set by disconnectMsg.Error(). + if !strings.Contains(err.Error(), "ssh: disconnect") || + !strings.Contains(err.Error(), "too many authentication attempts") { + t.Fatalf("expected server disconnect, got: %v", err) + } + // Server-side request sequence per AuthCallback iteration: + // 1 "none" (initial, before any AuthCallback) + // 2 pubkey query (AuthCallback #k, server returns OK) + // 3 pubkey signed (AuthCallback #k, server returns partial) + // So after k complete callbacks the server has processed 1+2k requests. + // The (k+1)-th callback's query is processed as request number 2(k+1); + // the subsequent signed request is rejected by the cap check + // (authAttempts >= maxAuthServerAttempts), so 2(k+1) = maxAuthServerAttempts + // and invocations = maxAuthServerAttempts / 2. + expected := maxAuthServerAttempts / 2 + if invocations != expected { + t.Errorf("AuthCallback invoked %d times; want %d", invocations, expected) + } +} + func TestMaxAuthTriesFirstNoneAuthErrorIgnored(t *testing.T) { username := "testuser" serverConfig := &ServerConfig{