From db3efd125e4ec39d3f63f676f3302fd9fc96ea0d Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Tue, 17 Mar 2026 10:38:12 +0200 Subject: [PATCH] Fix SSH agent forwarding handled in wrong request handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move auth-agent-req@openssh.com handling from handleGlobalRequests (SSH_MSG_GLOBAL_REQUEST) to handleSession (SSH_MSG_CHANNEL_REQUEST), matching the SSH spec and what real clients like Go's agent.RequestAgentForwarding() actually send. The old handler was dead code — real clients send this as a session channel request, which hit the default reject case in handleSession. Tests passed only because they used client.SendRequest() (global) instead of session-scoped requests. Tests now use agent.RequestAgentForwarding(session) to exercise the correct code path, including a same-session end-to-end test that verifies SSH_AUTH_SOCK is set. Co-Authored-By: Claude Opus 4.6 (1M context) --- guest/sshd/server.go | 30 +++++------------------------ guest/sshd/server_test.go | 40 +++++++++++++++++++++++++++------------ guest/sshd/session.go | 14 ++++++++++++++ 3 files changed, 47 insertions(+), 37 deletions(-) diff --git a/guest/sshd/server.go b/guest/sshd/server.go index f64c437..cfbe1b3 100644 --- a/guest/sshd/server.go +++ b/guest/sshd/server.go @@ -274,32 +274,12 @@ func (s *Server) handleConnection(netConn net.Conn) { } // handleGlobalRequests processes connection-level SSH requests. -// It handles agent forwarding requests when enabled and discards -// all other global requests. -func (s *Server) handleGlobalRequests(reqs <-chan *ssh.Request, conn *ssh.ServerConn) { +// It rejects all global requests; session-specific requests like +// agent forwarding are handled in handleSession. +func (s *Server) handleGlobalRequests(reqs <-chan *ssh.Request, _ *ssh.ServerConn) { for req := range reqs { - switch req.Type { - case "auth-agent-req@openssh.com": - if s.cfg.AgentForwarding { - s.setAgentForwarding(conn, true) - s.logger.Info("agent forwarding enabled", - "remote", conn.RemoteAddr(), - ) - if req.WantReply { - _ = req.Reply(true, nil) - } - } else { - s.logger.Debug("agent forwarding rejected (disabled)", - "remote", conn.RemoteAddr(), - ) - if req.WantReply { - _ = req.Reply(false, nil) - } - } - default: - if req.WantReply { - _ = req.Reply(false, nil) - } + if req.WantReply { + _ = req.Reply(false, nil) } } } diff --git a/guest/sshd/server_test.go b/guest/sshd/server_test.go index 5c5db01..bbc00c9 100644 --- a/guest/sshd/server_test.go +++ b/guest/sshd/server_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/agent" ) // generateTestKeyPair creates an ECDSA P-256 key pair for testing and @@ -327,10 +328,13 @@ func TestAgentForwardingDisabled(t *testing.T) { client := dialSSH(t, addr, signer) - // Request agent forwarding — should be rejected. - ok, _, err := client.SendRequest("auth-agent-req@openssh.com", true, nil) + session, err := client.NewSession() require.NoError(t, err) - assert.False(t, ok, "agent forwarding should be rejected when disabled") + defer func() { _ = session.Close() }() + + // Request agent forwarding via the real API — should be rejected. + err = agent.RequestAgentForwarding(session) + assert.Error(t, err, "agent forwarding should be rejected when disabled") } func TestAgentForwardingEnabled(t *testing.T) { @@ -352,10 +356,23 @@ func TestAgentForwardingEnabled(t *testing.T) { client := dialSSH(t, addr, signer) - // Request agent forwarding — should be accepted. - ok, _, err := client.SendRequest("auth-agent-req@openssh.com", true, nil) + session, err := client.NewSession() + require.NoError(t, err) + defer func() { _ = session.Close() }() + + // Request agent forwarding via the real API — should be accepted. + err = agent.RequestAgentForwarding(session) + require.NoError(t, err, "agent forwarding should be accepted when enabled") + + // Verify the flag was set by running a command on a second session. + session2, err := client.NewSession() require.NoError(t, err) - assert.True(t, ok, "agent forwarding should be accepted when enabled") + defer func() { _ = session2.Close() }() + + output, err := session2.CombinedOutput("echo ${SSH_AUTH_SOCK:-unset}") + require.NoError(t, err) + result := strings.TrimSpace(string(output)) + assert.Contains(t, result, "/tmp/ssh-", "agent socket should be set on connection after forwarding request") } func TestAgentSocketCreated(t *testing.T) { @@ -377,16 +394,15 @@ func TestAgentSocketCreated(t *testing.T) { client := dialSSH(t, addr, signer) - // Request agent forwarding. - ok, _, err := client.SendRequest("auth-agent-req@openssh.com", true, nil) - require.NoError(t, err) - require.True(t, ok) - - // Run a command that checks if SSH_AUTH_SOCK is set. + // Request agent forwarding and run a command on the same session, + // which is the real client flow: auth-agent-req arrives before exec. session, err := client.NewSession() require.NoError(t, err) defer func() { _ = session.Close() }() + err = agent.RequestAgentForwarding(session) + require.NoError(t, err) + output, err := session.CombinedOutput("echo $SSH_AUTH_SOCK") require.NoError(t, err) diff --git a/guest/sshd/session.go b/guest/sshd/session.go index 8e4fbce..ec99665 100644 --- a/guest/sshd/session.go +++ b/guest/sshd/session.go @@ -119,6 +119,20 @@ func (s *Server) handleSession(ch ssh.Channel, requests <-chan *ssh.Request, con replyRequest(req, false) } + case "auth-agent-req@openssh.com": + if s.cfg.AgentForwarding { + s.setAgentForwarding(conn, true) + s.logger.Info("agent forwarding enabled", + "remote", conn.RemoteAddr(), + ) + replyRequest(req, true) + } else { + s.logger.Debug("agent forwarding rejected (disabled)", + "remote", conn.RemoteAddr(), + ) + replyRequest(req, false) + } + case "exec": var payload struct { Command string