fix: treat negative keepalive replies as liveness

server.go

@@ -516,7 +516,7 @@ func (srv *Server) HandleConn(newConn net.Conn) {
 // of sshConn. It mirrors OpenSSH's client_alive_check(): if at least one
 // channel is open, the keepalive is sent as a SSH2_MSG_CHANNEL_REQUEST on
 // that channel; otherwise it falls back to a SSH2_MSG_GLOBAL_REQUEST. After
-// ClientAliveCountMax consecutive intervals with no successful reply,
+// ClientAliveCountMax consecutive intervals with no reply received,
 // sshConn is closed so HandleConn unblocks. Stops when `done` closes
 // (HandleConn returning).
 func (srv *Server) connectionKeepAlive(
@@ -563,35 +563,36 @@ func (srv *Server) connectionKeepAlive(
 				keepAlive.ServerRequestedKeepAliveCallback()
 				// Mirror OpenSSH client_alive_check(): prefer a channel
 				// request on an open channel; fall back to a global
-				// request if no channel is open or the channel send
-				// fails (channel was closed mid-flight).
+				// request only if no channel is open or the channel send
+				// fails at the transport level (for example, because the
+				// channel closed mid-flight). A negative SSH reply still
+				// proves that the peer received and processed the request.
 				//
 				// No outer timeout is needed here: the inFlight semaphore
 				// already prevents overlapping probes, TimeIsUp() at the
 				// next tick enforces the deadline, and if SendRequest
 				// hangs forever it will be unblocked when the TimeIsUp
 				// branch closes sshConn.
-				var (
-					ok  bool
-					err error
-				)
+				var err error
 				ch := openChans.any()
 				if ch != nil {
-					ok, err = ch.SendRequest(keepAliveRequestType, true, nil)
+					_, err = ch.SendRequest(keepAliveRequestType, true, nil)
 					if err != nil {
 						openChans.remove(ch)
-					}
-					if err != nil || !ok {
 						ch = nil
 					}
 				}
 				if ch == nil {
-					ok, _, err = sshConn.SendRequest(keepAliveRequestType, true, nil)
+					_, _, err = sshConn.SendRequest(keepAliveRequestType, true, nil)
 				}
-				if err == nil && ok {
+				// keepalive@openssh.com is intentionally unsupported by
+				// many peers. Both success and failure replies prove that
+				// the peer is alive; only a transport error means no reply
+				// was received.
+				if err == nil {
 					keepAlive.Reset()
 				} else {
-					log.Printf("ssh: keepalive request failed: ok=%t err=%v", ok, err)
+					log.Printf("ssh: keepalive request failed: err=%v", err)
 				}
 			}()
 		}

server_test.go

@@ -373,12 +373,14 @@ func TestConnectionKeepAliveUsesChannelRequestWhenSessionOpen(t *testing.T) {
 	}
 }
 
-// TestConnectionKeepAliveNegativeGlobalReplyDoesNotReset verifies that a
-// protocol-level negative response is not counted as a successful keepalive.
-func TestConnectionKeepAliveNegativeGlobalReplyDoesNotReset(t *testing.T) {
+// TestConnectionKeepAliveNegativeGlobalReplyKeepsAlive verifies OpenSSH-style
+// keepalive semantics: a protocol-level failure reply still proves that the
+// peer received and processed the request, so it resets the liveness deadline.
+func TestConnectionKeepAliveNegativeGlobalReplyKeepsAlive(t *testing.T) {
 	t.Parallel()
 
 	closingFired := make(chan struct{})
+	var globalCount atomic.Int64
 	srv := &Server{
 		Handler:             func(_ Session) {},
 		ClientAliveInterval: 100 * time.Millisecond,
@@ -414,26 +416,34 @@ func TestConnectionKeepAliveNegativeGlobalReplyDoesNotReset(t *testing.T) {
 	go func() {
 		for req := range reqs {
 			if req.Type == keepAliveRequestType {
+				globalCount.Add(1)
 				_ = req.Reply(false, nil)
 			} else if req.WantReply {
 				_ = req.Reply(true, nil)
 			}
 		}
 	}()
 
+	// The configured timeout is 200ms. Negative replies must keep the
+	// transport alive well beyond that window.
 	select {
 	case <-closingFired:
-	case <-time.After(5 * time.Second):
-		t.Fatal("negative global keepalive replies incorrectly reset the deadline")
+		t.Fatal("negative global keepalive reply did not preserve liveness")
+	case <-time.After(700 * time.Millisecond):
+	}
+	if got := globalCount.Load(); got < 2 {
+		t.Fatalf("expected multiple negative global keepalive replies, got %d", got)
 	}
 }
 
-// TestConnectionKeepAliveNegativeChannelReplyDoesNotReset is the channel
-// request counterpart to TestConnectionKeepAliveNegativeGlobalReplyDoesNotReset.
-func TestConnectionKeepAliveNegativeChannelReplyDoesNotReset(t *testing.T) {
+// TestConnectionKeepAliveNegativeChannelReplyKeepsAlive is the channel
+// counterpart to TestConnectionKeepAliveNegativeGlobalReplyKeepsAlive. A
+// negative channel reply proves liveness and must not trigger global fallback.
+func TestConnectionKeepAliveNegativeChannelReplyKeepsAlive(t *testing.T) {
 	t.Parallel()
 
 	closingFired := make(chan struct{})
+	var globalCount, channelCount atomic.Int64
 	srv := &Server{
 		Handler:             func(s Session) { <-s.Context().Done() },
 		ClientAliveInterval: 100 * time.Millisecond,
@@ -466,7 +476,6 @@ func TestConnectionKeepAliveNegativeChannelReplyDoesNotReset(t *testing.T) {
 		for range chans { //nolint:revive // intentional drain
 		}
 	}()
-	var globalCount atomic.Int64
 	go func() {
 		for req := range reqs {
 			if req.Type == keepAliveRequestType {
@@ -486,20 +495,26 @@ func TestConnectionKeepAliveNegativeChannelReplyDoesNotReset(t *testing.T) {
 	go func() {
 		for req := range chReqs {
 			if req.Type == keepAliveRequestType {
+				channelCount.Add(1)
 				_ = req.Reply(false, nil)
 			} else if req.WantReply {
 				_ = req.Reply(true, nil)
 			}
 		}
 	}()
 
+	// The configured timeout is 200ms. Repeated negative channel replies
+	// must keep the connection alive and must not cause a global fallback.
 	select {
 	case <-closingFired:
-	case <-time.After(5 * time.Second):
-		t.Fatal("negative channel keepalive replies incorrectly reset the deadline")
+		t.Fatal("negative channel keepalive reply did not preserve liveness")
+	case <-time.After(700 * time.Millisecond):
 	}
-	if globalCount.Load() == 0 {
-		t.Fatal("negative channel keepalive reply did not fall back to a global request")
+	if got := channelCount.Load(); got < 2 {
+		t.Fatalf("expected multiple negative channel keepalive replies, got %d", got)
+	}
+	if got := globalCount.Load(); got != 0 {
+		t.Fatalf("negative channel replies unexpectedly triggered %d global keepalives", got)
 	}
 }