fix: avoid useless "Failed authentication attempt" logs (#39602)

modules/ssh/server.go

@@ -84,23 +84,30 @@ func (srv *sshServer) serve(listener net.Listener) error {
 }
 
 func (srv *sshServer) handleConn(netConn net.Conn) {
-	ctx, cancel := context.WithCancel(graceful.GetManager().HammerContext())
-	defer cancel()
 	defer netConn.Close()
 
-	conn, chans, reqs, err := gossh.NewServerConn(netConn, srv.newServerConfig(ctx))
+	ctx, cancel := context.WithCancel(graceful.GetManager().HammerContext())
+	defer cancel()
+
+	sshConn, sshChannels, sshReqs, err := gossh.NewServerConn(netConn, srv.newServerConfig(ctx))
 	if err != nil {
-		sshConnectionFailed(netConn, err)
+		// OpenSSH's logs are something like:
+		// * disconnect without sending anything: "Connection closed by 1.2.3.4 port 5678"
+		// * send invalid bytes: "banner exchange: Connection from 1.2.3.4 port 5678: invalid format"
+		// * preauth failed: "Connection closed by authenticating user SYSOP 1.2.3.4 port 5678 [preauth]"
+		// * successfully logon and disconnect: "Disconnected from user SYSOP 1.2.3.4 port 5678"
+		log.Warn("Failed connection from %s with error: %v", netConn.RemoteAddr(), err)
 		return
 	}
+	defer sshConn.Close()
 
-	go gossh.DiscardRequests(reqs)
-	for newChan := range chans {
+	go gossh.DiscardRequests(sshReqs)
+	for newChan := range sshChannels {
 		if newChan.ChannelType() != "session" {
 			_ = newChan.Reject(gossh.UnknownChannelType, "unsupported channel type")
 			continue
 		}
-		go handleSessionChannel(ctx, conn, newChan)
+		go handleSessionChannel(ctx, sshConn, newChan)
 	}
 }
 

modules/ssh/ssh.go

@@ -250,15 +250,6 @@ func publicKeyHandler(ctx context.Context, conn gossh.ConnMetadata, key gossh.Pu
 	return keyPermissions(pkey.ID), nil
 }
 
-// sshConnectionFailed logs a failed connection
-// - this mainly exists to give a nice function name in logging
-func sshConnectionFailed(conn net.Conn, err error) {
-	// Log the underlying error with a specific message
-	log.Warn("Failed connection from %s with error: %v", conn.RemoteAddr(), err)
-	// Log with the standard failed authentication from message for simpler fail2ban configuration
-	log.Warn("Failed authentication attempt from %s", conn.RemoteAddr())
-}
-
 // Listen starts an SSH server listening on given port.
 func Listen(host string, port int, ciphers, keyExchanges, macs []string) {
 	hostKeyFiles := make([]string, 0, len(setting.SSH.ServerHostKeys))

routers/api/v1/api.go

@@ -946,7 +946,7 @@ func verifyAuthWithOptionsAPI(options *common.VerifyOptions) func(ctx *context.A
 				ctx.JSON(http.StatusForbidden, map[string]string{"message": "This account is not activated."})
 				return
 			} else if check.LoginIsProhibited {
-				log.Info("Failed authentication attempt for %s from %s", ctx.Doer.Name, ctx.RemoteAddr())
+				log.Info("Failed authentication attempt for %s from %s (prohibited)", ctx.Doer.Name, ctx.RemoteAddr())
 				ctx.JSON(http.StatusForbidden, map[string]string{"message": "This account is prohibited from signing in, please contact your site administrator."})
 				return
 			} else if check.NeedChangePassword {

routers/web/web.go

@@ -185,7 +185,11 @@ func verifyAuthWithOptionsWeb(options *common.VerifyOptions) func(ctx *context.C
 				ctx.HTML(http.StatusOK, "user/auth/activate")
 				return
 			} else if check.LoginIsProhibited {
-				log.Info("Failed authentication attempt for %s from %s", ctx.Doer.Name, ctx.RemoteAddr())
+				// FIXME: there are a lot of "Failed authentication attempt" log messages, and there are many problems:
+				// * Inconsistent log levels: sometimes "info" sometimes "warning"
+				// * Unclear criteria, no context: invalid password, prohibited user, etc.
+				// It was designed for "fail2ban". If it is still really useful, need to improve or clean up.
+				log.Info("Failed authentication attempt for %s from %s (prohibited)", ctx.Doer.Name, ctx.RemoteAddr())
 				ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
 				ctx.HTML(http.StatusOK, "user/auth/prohibit_login")
 				return