ssh: support MaxAuthTries on ServerConfig

This change breaks backwards compatibility.

MaxAuthTries specifies the maximum number of authentication attempts
permitted per connection. If set to a negative number, the server will
allow unlimited authentication attempts. MaxAuthTries defaults to 6 if
not specified, which is a backwards incompatible change. On exceeding
maximum authentication attempts, the server will send a disconnect
message to the client.

This configuration property mirrors a similar property in sshd_config
and prevents bad actors from continuously trying authentication.

Change-Id: Ic77d2c29ee2fd2ae5c764becf7df91d29d03131b
Reviewed-on: https://go-review.googlesource.com/35230
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

2 files changed, 130 insertions, 2 deletions

diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go
index b526f66..dd83a3c 100644
--- a/ssh/client_auth_test.go
+++ b/ssh/client_auth_test.go
@@ -76,8 +76,6 @@ func tryAuth(t *testing.T, config *ClientConfig) error {
 			}
 			return nil, errors.New("keyboard-interactive failed")
 		},
-		AuthLogCallback: func(conn ConnMetadata, method string, err error) {
-		},
 	}
 	serverConfig.AddHostKey(testSigners["rsa"])
 
@@ -482,3 +480,100 @@ func TestClientAuthNone(t *testing.T) {
 		t.Fatalf("server: got %q, want %q", serverConn.User(), user)
 	}
 }
+
+// Test if authentication attempts are limited on server when MaxAuthTries is set
+func TestClientAuthMaxAuthTries(t *testing.T) {
+	user := "testuser"
+
+	serverConfig := &ServerConfig{
+		MaxAuthTries: 2,
+		PasswordCallback: func(conn ConnMetadata, pass []byte) (*Permissions, error) {
+			if conn.User() == "testuser" && string(pass) == "right" {
+				return nil, nil
+			}
+			return nil, errors.New("password auth failed")
+		},
+	}
+	serverConfig.AddHostKey(testSigners["rsa"])
+
+	expectedErr := fmt.Errorf("ssh: handshake failed: %v", &disconnectMsg{
+		Reason:  2,
+		Message: "too many authentication failures",
+	})
+
+	for tries := 2; tries < 4; tries++ {
+		n := tries
+		clientConfig := &ClientConfig{
+			User: user,
+			Auth: []AuthMethod{
+				RetryableAuthMethod(PasswordCallback(func() (string, error) {
+					n--
+					if n == 0 {
+						return "right", nil
+					} else {
+						return "wrong", nil
+					}
+				}), tries),
+			},
+			HostKeyCallback: InsecureIgnoreHostKey(),
+		}
+
+		c1, c2, err := netPipe()
+		if err != nil {
+			t.Fatalf("netPipe: %v", err)
+		}
+		defer c1.Close()
+		defer c2.Close()
+
+		go newServer(c1, serverConfig)
+		_, _, _, err = NewClientConn(c2, "", clientConfig)
+		if tries > 2 {
+			if err == nil {
+				t.Fatalf("client: got no error, want %s", expectedErr)
+			} else if err.Error() != expectedErr.Error() {
+				t.Fatalf("client: got %s, want %s", err, expectedErr)
+			}
+		} else {
+			if err != nil {
+				t.Fatalf("client: got %s, want no error", err)
+			}
+		}
+	}
+}
+
+// Test if authentication attempts are correctly limited on server
+// when more public keys are provided then MaxAuthTries
+func TestClientAuthMaxAuthTriesPublicKey(t *testing.T) {
+	signers := []Signer{}
+	for i := 0; i < 6; i++ {
+		signers = append(signers, testSigners["dsa"])
+	}
+
+	validConfig := &ClientConfig{
+		User: "testuser",
+		Auth: []AuthMethod{
+			PublicKeys(append([]Signer{testSigners["rsa"]}, signers...)...),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+	if err := tryAuth(t, validConfig); err != nil {
+		t.Fatalf("unable to dial remote side: %s", err)
+	}
+
+	expectedErr := fmt.Errorf("ssh: handshake failed: %v", &disconnectMsg{
+		Reason:  2,
+		Message: "too many authentication failures",
+	})
+	invalidConfig := &ClientConfig{
+		User: "testuser",
+		Auth: []AuthMethod{
+			PublicKeys(append(signers, testSigners["rsa"])...),
+		},
+		HostKeyCallback: InsecureIgnoreHostKey(),
+	}
+	if err := tryAuth(t, invalidConfig); err == nil {
+		t.Fatalf("client: got no error, want %s", expectedErr)
+	} else if err.Error() != expectedErr.Error() {
+		t.Fatalf("client: got %s, want %s", err, expectedErr)
+	}
+}
diff --git a/ssh/server.go b/ssh/server.go
index 77c84d1..8e95acc 100644
--- a/ssh/server.go
+++ b/ssh/server.go
@@ -45,6 +45,12 @@ type ServerConfig struct {
 	// authenticating.
 	NoClientAuth bool
 
+	// MaxAuthTries specifies the maximum number of authentication attempts
+	// permitted per connection. If set to a negative number, the number of
+	// attempts are unlimited. If set to zero, the number of attempts are limited
+	// to 6.
+	MaxAuthTries int
+
 	// PasswordCallback, if non-nil, is called when a user
 	// attempts to authenticate using a password.
 	PasswordCallback func(conn ConnMetadata, password []byte) (*Permissions, error)
@@ -141,6 +147,10 @@ type ServerConn struct {
 // Request and NewChannel channels must be serviced, or the connection
 // will hang.
 func NewServerConn(c net.Conn, config *ServerConfig) (*ServerConn, <-chan NewChannel, <-chan *Request, error) {
+	if config.MaxAuthTries == 0 {
+		config.MaxAuthTries = 6
+	}
+
 	fullConf := *config
 	fullConf.SetDefaults()
 	s := &connection{
@@ -267,8 +277,23 @@ func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, err
 	var cache pubKeyCache
 	var perms *Permissions
 
+	authFailures := 0
+
 userAuthLoop:
 	for {
+		if authFailures >= config.MaxAuthTries && config.MaxAuthTries > 0 {
+			discMsg := &disconnectMsg{
+				Reason:  2,
+				Message: "too many authentication failures",
+			}
+
+			if err := s.transport.writePacket(Marshal(discMsg)); err != nil {
+				return nil, err
+			}
+
+			return nil, discMsg
+		}
+
 		var userAuthReq userAuthRequestMsg
 		if packet, err := s.transport.readPacket(); err != nil {
 			return nil, err
@@ -289,6 +314,11 @@ userAuthLoop:
 			if config.NoClientAuth {
 				authErr = nil
 			}
+
+			// allow initial attempt of 'none' without penalty
+			if authFailures == 0 {
+				authFailures--
+			}
 		case "password":
 			if config.PasswordCallback == nil {
 				authErr = errors.New("ssh: password auth not configured")
@@ -360,6 +390,7 @@ userAuthLoop:
 			if isQuery {
 				// The client can query if the given public key
 				// would be okay.
+
 				if len(payload) > 0 {
 					return nil, parseError(msgUserAuthRequest)
 				}
@@ -409,6 +440,8 @@ userAuthLoop:
 			break userAuthLoop
 		}
 
+		authFailures++
+
 		var failureMsg userAuthFailureMsg
 		if config.PasswordCallback != nil {
 			failureMsg.Methods = append(failureMsg.Methods, "password")