feat(agent): add second SSH listener on port 22 (cherry-pick #16627) (#16763)

Cherry-picked feat(agent): add second SSH listener on port 22 (#16627)

Fixes: https://github.com/coder/internal/issues/377

Added an additional SSH listener on port 22, so the agent now listens on both, port one and port 22.

---
Change-Id: Ifd986b260f8ac317e37d65111cd4e0bd1dc38af8
Signed-off-by: Thomas Kosiewski <tk@coder.com>

agent/agent.go

@@ -1193,19 +1193,22 @@ func (a *agent) createTailnet(
 		return nil, xerrors.Errorf("update host signer: %w", err)
 	}
 
-	sshListener, err := network.Listen("tcp", ":"+strconv.Itoa(workspacesdk.AgentSSHPort))
+	for _, port := range []int{workspacesdk.AgentSSHPort, workspacesdk.AgentStandardSSHPort} {
-	if err != nil {
+		sshListener, err := network.Listen("tcp", ":"+strconv.Itoa(port))
-		return nil, xerrors.Errorf("listen on the ssh port: %w", err)
-	}
-	defer func() {
 		if err != nil {
-			_ = sshListener.Close()
+			return nil, xerrors.Errorf("listen on the ssh port (%v): %w", port, err)
+		}
+		// nolint:revive // We do want to run the deferred functions when createTailnet returns.
+		defer func() {
+			if err != nil {
+				_ = sshListener.Close()
+			}
+		}()
+		if err = a.trackGoroutine(func() {
+			_ = a.sshServer.Serve(sshListener)
+		}); err != nil {
+			return nil, err
 		}
-	}()
-	if err = a.trackGoroutine(func() {
-		_ = a.sshServer.Serve(sshListener)
-	}); err != nil {
-		return nil, err
 	}
 
 	reconnectingPTYListener, err := network.Listen("tcp", ":"+strconv.Itoa(workspacesdk.AgentReconnectingPTYPort))

agent/agent_test.go

@@ -61,38 +61,48 @@ func TestMain(m *testing.M) {
 	goleak.VerifyTestMain(m, testutil.GoleakOptions...)
 }
 
+var sshPorts = []uint16{workspacesdk.AgentSSHPort, workspacesdk.AgentStandardSSHPort}
+
 // NOTE: These tests only work when your default shell is bash for some reason.
 
 func TestAgent_Stats_SSH(t *testing.T) {
 	t.Parallel()
-	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
-	defer cancel()
 
-	//nolint:dogsled
+	for _, port := range sshPorts {
-	conn, _, stats, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
+		port := port
+		t.Run(fmt.Sprintf("(:%d)", port), func(t *testing.T) {
+			t.Parallel()
 
-	sshClient, err := conn.SSHClient(ctx)
+			ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
-	require.NoError(t, err)
+			defer cancel()
-	defer sshClient.Close()
-	session, err := sshClient.NewSession()
-	require.NoError(t, err)
-	defer session.Close()
-	stdin, err := session.StdinPipe()
-	require.NoError(t, err)
-	err = session.Shell()
-	require.NoError(t, err)
 
-	var s *proto.Stats
+			//nolint:dogsled
-	require.Eventuallyf(t, func() bool {
+			conn, _, stats, _, _ := setupAgent(t, agentsdk.Manifest{}, 0)
-		var ok bool
+
-		s, ok = <-stats
+			sshClient, err := conn.SSHClientOnPort(ctx, port)
-		return ok && s.ConnectionCount > 0 && s.RxBytes > 0 && s.TxBytes > 0 && s.SessionCountSsh == 1
+			require.NoError(t, err)
-	}, testutil.WaitLong, testutil.IntervalFast,
+			defer sshClient.Close()
-		"never saw stats: %+v", s,
+			session, err := sshClient.NewSession()
-	)
+			require.NoError(t, err)
-	_ = stdin.Close()
+			defer session.Close()
-	err = session.Wait()
+			stdin, err := session.StdinPipe()
-	require.NoError(t, err)
+			require.NoError(t, err)
+			err = session.Shell()
+			require.NoError(t, err)
+
+			var s *proto.Stats
+			require.Eventuallyf(t, func() bool {
+				var ok bool
+				s, ok = <-stats
+				return ok && s.ConnectionCount > 0 && s.RxBytes > 0 && s.TxBytes > 0 && s.SessionCountSsh == 1
+			}, testutil.WaitLong, testutil.IntervalFast,
+				"never saw stats: %+v", s,
+			)
+			_ = stdin.Close()
+			err = session.Wait()
+			require.NoError(t, err)
+		})
+	}
 }
 
 func TestAgent_Stats_ReconnectingPTY(t *testing.T) {
@@ -266,15 +276,23 @@ func TestAgent_Stats_Magic(t *testing.T) {
 
 func TestAgent_SessionExec(t *testing.T) {
 	t.Parallel()
-	session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
 
-	command := "echo test"
+	for _, port := range sshPorts {
-	if runtime.GOOS == "windows" {
+		port := port
-		command = "cmd.exe /c echo test"
+		t.Run(fmt.Sprintf("(:%d)", port), func(t *testing.T) {
+			t.Parallel()
+
+			session := setupSSHSessionOnPort(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil, port)
+
+			command := "echo test"
+			if runtime.GOOS == "windows" {
+				command = "cmd.exe /c echo test"
+			}
+			output, err := session.Output(command)
+			require.NoError(t, err)
+			require.Equal(t, "test", strings.TrimSpace(string(output)))
+		})
 	}
-	output, err := session.Output(command)
-	require.NoError(t, err)
-	require.Equal(t, "test", strings.TrimSpace(string(output)))
 }
 
 //nolint:tparallel // Sub tests need to run sequentially.
@@ -384,25 +402,33 @@ func TestAgent_SessionTTYShell(t *testing.T) {
 		// it seems like it could be either.
 		t.Skip("ConPTY appears to be inconsistent on Windows.")
 	}
-	session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil)
+
-	command := "sh"
+	for _, port := range sshPorts {
-	if runtime.GOOS == "windows" {
+		port := port
-		command = "cmd.exe"
+		t.Run(fmt.Sprintf("(%d)", port), func(t *testing.T) {
+			t.Parallel()
+
+			session := setupSSHSessionOnPort(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil, port)
+			command := "sh"
+			if runtime.GOOS == "windows" {
+				command = "cmd.exe"
+			}
+			err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{})
+			require.NoError(t, err)
+			ptty := ptytest.New(t)
+			session.Stdout = ptty.Output()
+			session.Stderr = ptty.Output()
+			session.Stdin = ptty.Input()
+			err = session.Start(command)
+			require.NoError(t, err)
+			_ = ptty.Peek(ctx, 1) // wait for the prompt
+			ptty.WriteLine("echo test")
+			ptty.ExpectMatch("test")
+			ptty.WriteLine("exit")
+			err = session.Wait()
+			require.NoError(t, err)
+		})
 	}
-	err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{})
-	require.NoError(t, err)
-	ptty := ptytest.New(t)
-	session.Stdout = ptty.Output()
-	session.Stderr = ptty.Output()
-	session.Stdin = ptty.Input()
-	err = session.Start(command)
-	require.NoError(t, err)
-	_ = ptty.Peek(ctx, 1) // wait for the prompt
-	ptty.WriteLine("echo test")
-	ptty.ExpectMatch("test")
-	ptty.WriteLine("exit")
-	err = session.Wait()
-	require.NoError(t, err)
 }
 
 func TestAgent_SessionTTYExitCode(t *testing.T) {
@@ -596,37 +622,41 @@ func TestAgent_Session_TTY_MOTD_Update(t *testing.T) {
 	//nolint:dogsled // Allow the blank identifiers.
 	conn, client, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, setSBInterval)
 
-	sshClient, err := conn.SSHClient(ctx)
-	require.NoError(t, err)
-	t.Cleanup(func() {
-		_ = sshClient.Close()
-	})
-
 	//nolint:paralleltest // These tests need to swap the banner func.
-	for i, test := range tests {
+	for _, port := range sshPorts {
-		test := test
+		port := port
-		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
-			// Set new banner func and wait for the agent to call it to update the
-			// banner.
-			ready := make(chan struct{}, 2)
-			client.SetAnnouncementBannersFunc(func() ([]codersdk.BannerConfig, error) {
-				select {
-				case ready <- struct{}{}:
-				default:
-				}
-				return []codersdk.BannerConfig{test.banner}, nil
-			})
-			<-ready
-			<-ready // Wait for two updates to ensure the value has propagated.
 
-			session, err := sshClient.NewSession()
+		sshClient, err := conn.SSHClientOnPort(ctx, port)
-			require.NoError(t, err)
+		require.NoError(t, err)
-			t.Cleanup(func() {
+		t.Cleanup(func() {
-				_ = session.Close()
+			_ = sshClient.Close()
-			})
-
-			testSessionOutput(t, session, test.expected, test.unexpected, nil)
 		})
+
+		for i, test := range tests {
+			test := test
+			t.Run(fmt.Sprintf("(:%d)/%d", port, i), func(t *testing.T) {
+				// Set new banner func and wait for the agent to call it to update the
+				// banner.
+				ready := make(chan struct{}, 2)
+				client.SetAnnouncementBannersFunc(func() ([]codersdk.BannerConfig, error) {
+					select {
+					case ready <- struct{}{}:
+					default:
+					}
+					return []codersdk.BannerConfig{test.banner}, nil
+				})
+				<-ready
+				<-ready // Wait for two updates to ensure the value has propagated.
+
+				session, err := sshClient.NewSession()
+				require.NoError(t, err)
+				t.Cleanup(func() {
+					_ = session.Close()
+				})
+
+				testSessionOutput(t, session, test.expected, test.unexpected, nil)
+			})
+		}
 	}
 }
 
@@ -2313,6 +2343,17 @@ func setupSSHSession(
 	banner codersdk.BannerConfig,
 	prepareFS func(fs afero.Fs),
 	opts ...func(*agenttest.Client, *agent.Options),
+) *ssh.Session {
+	return setupSSHSessionOnPort(t, manifest, banner, prepareFS, workspacesdk.AgentSSHPort, opts...)
+}
+
+func setupSSHSessionOnPort(
+	t *testing.T,
+	manifest agentsdk.Manifest,
+	banner codersdk.BannerConfig,
+	prepareFS func(fs afero.Fs),
+	port uint16,
+	opts ...func(*agenttest.Client, *agent.Options),
 ) *ssh.Session {
 	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 	defer cancel()
@@ -2326,7 +2367,7 @@ func setupSSHSession(
 	if prepareFS != nil {
 		prepareFS(fs)
 	}
-	sshClient, err := conn.SSHClient(ctx)
+	sshClient, err := conn.SSHClientOnPort(ctx, port)
 	require.NoError(t, err)
 	t.Cleanup(func() {
 		_ = sshClient.Close()

agent/usershell/usershell_darwin.go

@@ -17,7 +17,7 @@ func Get(username string) (string, error) {
 		return "", xerrors.Errorf("username is nonlocal path: %s", username)
 	}
 	//nolint: gosec // input checked above
-	out, _ := exec.Command("dscl", ".", "-read", filepath.Join("/Users", username), "UserShell").Output()
+	out, _ := exec.Command("dscl", ".", "-read", filepath.Join("/Users", username), "UserShell").Output() //nolint:gocritic
 	s, ok := strings.CutPrefix(string(out), "UserShell: ")
 	if ok {
 		return strings.TrimSpace(s), nil

codersdk/workspacesdk/agentconn.go

@@ -143,6 +143,12 @@ func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, w
 // SSH pipes the SSH protocol over the returned net.Conn.
 // This connects to the built-in SSH server in the workspace agent.
 func (c *AgentConn) SSH(ctx context.Context) (*gonet.TCPConn, error) {
+	return c.SSHOnPort(ctx, AgentSSHPort)
+}
+
+// SSHOnPort pipes the SSH protocol over the returned net.Conn.
+// This connects to the built-in SSH server in the workspace agent on the specified port.
+func (c *AgentConn) SSHOnPort(ctx context.Context, port uint16) (*gonet.TCPConn, error) {
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.End()
 
@@ -150,17 +156,23 @@ func (c *AgentConn) SSH(ctx context.Context) (*gonet.TCPConn, error) {
 		return nil, xerrors.Errorf("workspace agent not reachable in time: %v", ctx.Err())
 	}
 
-	c.Conn.SendConnectedTelemetry(c.agentAddress(), tailnet.TelemetryApplicationSSH)
+	c.SendConnectedTelemetry(c.agentAddress(), tailnet.TelemetryApplicationSSH)
-	return c.Conn.DialContextTCP(ctx, netip.AddrPortFrom(c.agentAddress(), AgentSSHPort))
+	return c.DialContextTCP(ctx, netip.AddrPortFrom(c.agentAddress(), port))
 }
 
 // SSHClient calls SSH to create a client that uses a weak cipher
 // to improve throughput.
 func (c *AgentConn) SSHClient(ctx context.Context) (*ssh.Client, error) {
+	return c.SSHClientOnPort(ctx, AgentSSHPort)
+}
+
+// SSHClientOnPort calls SSH to create a client on a specific port
+// that uses a weak cipher to improve throughput.
+func (c *AgentConn) SSHClientOnPort(ctx context.Context, port uint16) (*ssh.Client, error) {
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.End()
 
-	netConn, err := c.SSH(ctx)
+	netConn, err := c.SSHOnPort(ctx, port)
 	if err != nil {
 		return nil, xerrors.Errorf("ssh: %w", err)
 	}

codersdk/workspacesdk/workspacesdk.go

@@ -29,6 +29,7 @@ var ErrSkipClose = xerrors.New("skip tailnet close")
 
 const (
 	AgentSSHPort             = tailnet.WorkspaceAgentSSHPort
+	AgentStandardSSHPort     = tailnet.WorkspaceAgentStandardSSHPort
 	AgentReconnectingPTYPort = tailnet.WorkspaceAgentReconnectingPTYPort
 	AgentSpeedtestPort       = tailnet.WorkspaceAgentSpeedtestPort
 	// AgentHTTPAPIServerPort serves a HTTP server with endpoints for e.g.

tailnet/conn.go

@@ -52,6 +52,7 @@ const (
 	WorkspaceAgentSSHPort             = 1
 	WorkspaceAgentReconnectingPTYPort = 2
 	WorkspaceAgentSpeedtestPort       = 3
+	WorkspaceAgentStandardSSHPort     = 22
 )
 
 // EnvMagicsockDebugLogging enables super-verbose logging for the magicsock
@@ -745,7 +746,7 @@ func (c *Conn) forwardTCP(src, dst netip.AddrPort) (handler func(net.Conn), opts
 		return nil, nil, false
 	}
 	// See: https://github.com/tailscale/tailscale/blob/c7cea825aea39a00aca71ea02bab7266afc03e7c/wgengine/netstack/netstack.go#L888
-	if dst.Port() == WorkspaceAgentSSHPort || dst.Port() == 22 {
+	if dst.Port() == WorkspaceAgentSSHPort || dst.Port() == WorkspaceAgentStandardSSHPort {
 		opt := tcpip.KeepaliveIdleOption(72 * time.Hour)
 		opts = append(opts, &opt)
 	}