ssh: cap total userauth attempts per server connection
serverAuthenticate only bounded real failures via MaxAuthTries.
PartialSuccessError responses and the publickey isQuery short-circuit
both kept the loop running without incrementing authFailures, so a
client could keep the server processing SSH_MSG_USERAUTH_REQUEST
messages indefinitely on a single connection.
Add an unconditional cap, maxAuthServerAttempts = 128, on the total
number of userauth requests handled per connection. The counter is
incremented at the top of the loop before method dispatch, so every
method and every isQuery / partial-success path counts. When the cap
is exceeded the server sends SSH_MSG_DISCONNECT with reason 2 ("too
many authentication attempts"), mirroring the MaxAuthTries handling.
The bound is well below OpenSSH's hard cap of 1024 but above any
realistic multi-step auth flow.
Change-Id: I56779fc55cd00ddfd32ec938f8de3a49be0145dc
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781903
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
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 @@
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 @@
var perms *Permissions
authFailures := 0
+ authAttempts := 0
noneAuthCount := 0
var authErrs []error
var calledBannerCallback bool
@@ -645,6 +655,19 @@
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 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{