feat(cli): implement ssh remote forward #8515

mtojek · 2023-07-14T11:47:36Z

Related: #6818

This PR enables remote forward (ssh -R) for the coder CLI to allow reverse TCP tunnels.

Changes:

parse the --remote-forward flag (remote_port:local_address:local_port)
reuse sshRemoteForward for proxying traffic
adjust tryPollWorkspaceAutostop to prevent blocking when 2 ssh -R processes are trying to open the same remote port
move sshRemoteForward to cli/remoteforward.go

mtojek · 2023-07-18T14:37:15Z

cli/remoteforward.go

+// local address via SSH in a goroutine.
+//
+// Accepts a `cookieAddr` as the local address.
+func sshRemoteForward(ctx context.Context, stderr io.Writer, sshClient *gossh.Client, localAddr, remoteAddr net.Addr) (io.Closer, error) {


for reviewers: moved from ssh.go

bpmct

Can you also add support for this with coder port-forward?

johnstcn · 2023-07-19T08:35:41Z

cli/remoteforward.go

+	cookie []byte
+}
+
+var remoteForwardRegex = regexp.MustCompile(`^(\d+):(.+):(\d+)$`)


nit: add a comment to clarify what this should match

This could also be done with a simple strings.Split()

nit: add a comment to clarify what this should match

👍

This could also be done with a simple strings.Split()

Right, but with regexp we can check if ports are numbers :)

cli/remoteforward.go

cli/ssh.go

johnstcn

LGTM, some nits in comments.

mtojek · 2023-07-19T15:07:29Z

Can you also add support for this with coder port-forward

Thank you for raising this question, @bpmct.

The main reason why it is paired with ssh is because remote forward requires a daemon on the remote site to serve forwarded TCP endpoints. SSH server supports this very nicely and in a stable way.

Also, if we want to forward multiple ports, then it is advised to run multiple ssh -R ... (or coder ssh -R ...) commands.

Enabling remote forward as part of the "port-forward" will be tricky, as it means that "port-forward" must establish SSH connection with the workspace agent, contrary to local forward which just needs Tailscale/Wireguard.

I see the following options:

Leave remote forward as-is, as feature parity with traditional ssh -R command.
Establish SSH connections with port-forward, but it may require extra flags (GPG agent, keys, etc.) added to the "port-forward".
Implement a homebrew forward proxy daemon as part Coder agent, which will do exactly the same things as the SSH server - unfortunately, this means reinventing the wheel.

I'm fine with implementing 2. and 3. in separate PRs. Option 3. might be a relatively tricky feature to implement.

@bpmct Please let me know your thoughts!

bpmct · 2023-07-19T18:06:05Z

I see. My main concern is around feature discoverability since many will probably look in coder port-forward --help to see if they can do reverse port forwarding. Would it be possible to add help text to coder port-forward --help pointing users to coder ssh -R to do reverse port forwarding?

Otherwise, I think option 2 would be fine to embed a ssh server only if -R is specified, and also limit to one port per-command. I can't really think of a use case to add support for those extra flags for port-forward. Am I missing something? What I don't understand, and maybe should require an engineering discussion, is the level of difficulty or debt from option 2.

mtojek · 2023-07-20T07:23:34Z

Would it be possible to add help text to coder port-forward --help pointing users to coder ssh -R to do reverse port forwarding?

Absolutely, this is something I can easily do. It is a no-brainer.

I can't really think of a use case to add support for those extra flags for port-forward. Am I missing something?

I'm looking at these SSH flags:

  -A, --forward-agent bool, $CODER_SSH_FORWARD_AGENT
          Specifies whether to forward the SSH agent specified in $SSH_AUTH_SOCK.

  -G, --forward-gpg bool, $CODER_SSH_FORWARD_GPG
          Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be
          running locally and will not be started for you. If a GPG agent is already running in the workspace, it will be attempted to be killed.

      --identity-agent string, $CODER_SSH_IDENTITY_AGENT
          Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled.

If we're going to use an SSH connection, these flags may be required for port-forward to work in some circumstances. @johnstcn Please correct me if I'm wrong.

What I don't understand, and maybe should require an engineering discussion, is the level of difficulty or debt from option 2.

Alright, I'm going to finish work on coder ssh -R in this PR, and if we want to proceed with port-forward -R, I will do a follow-up 👍

cli/portforward.go

make gen

cb0f964

mtojek self-assigned this Jul 14, 2023

mtojek added 7 commits July 14, 2023 14:09

TODOs

a532f7b

basic implementation

6df2a9b

Forwarding works

7506df9

Merge branch 'main' into 6818-reverse-port-fwd

7d3d47f

Cancel condition

ece557e

Remote forward

9f5e500

httptest

0b53a5c

mtojek commented Jul 18, 2023

View reviewed changes

mtojek requested review from johnstcn and bpmct July 18, 2023 14:38

mtojek marked this pull request as ready for review July 18, 2023 14:38

bpmct reviewed Jul 18, 2023

View reviewed changes

johnstcn reviewed Jul 19, 2023

View reviewed changes

cli/remoteforward.go Show resolved Hide resolved

johnstcn reviewed Jul 19, 2023

View reviewed changes

cli/ssh.go Outdated Show resolved Hide resolved

johnstcn reviewed Jul 19, 2023

View reviewed changes

cli/ssh.go Show resolved Hide resolved

johnstcn approved these changes Jul 19, 2023

View reviewed changes

mtojek marked this pull request as draft July 19, 2023 10:15

mtojek added 4 commits July 19, 2023 12:25

Merge branch 'main' into 6818-reverse-port-fwd

2471dd5

format

5e87bc6

format

f4d5c68

WIP

9ae8f1d

mtojek marked this pull request as ready for review July 19, 2023 15:13

Add notice about ssh -R

3e77af3

johnstcn reviewed Jul 20, 2023

View reviewed changes

cli/portforward.go Outdated Show resolved Hide resolved

Rephrase

d8859c3

mtojek merged commit 9689bca into main Jul 20, 2023

mtojek deleted the 6818-reverse-port-fwd branch July 20, 2023 10:05

github-actions bot locked and limited conversation to collaborators Jul 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): implement ssh remote forward #8515

feat(cli): implement ssh remote forward #8515

mtojek commented Jul 14, 2023 •

edited

Loading

mtojek Jul 18, 2023

bpmct left a comment

johnstcn Jul 19, 2023 •

edited

Loading

mtojek Jul 19, 2023

johnstcn left a comment

mtojek commented Jul 19, 2023

bpmct commented Jul 19, 2023 •

edited

Loading

mtojek commented Jul 20, 2023

feat(cli): implement ssh remote forward #8515

feat(cli): implement ssh remote forward #8515

Conversation

mtojek commented Jul 14, 2023 • edited Loading

mtojek Jul 18, 2023

Choose a reason for hiding this comment

bpmct left a comment

Choose a reason for hiding this comment

johnstcn Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

mtojek Jul 19, 2023

Choose a reason for hiding this comment

johnstcn left a comment

Choose a reason for hiding this comment

mtojek commented Jul 19, 2023

bpmct commented Jul 19, 2023 • edited Loading

mtojek commented Jul 20, 2023

mtojek commented Jul 14, 2023 •

edited

Loading

johnstcn Jul 19, 2023 •

edited

Loading

bpmct commented Jul 19, 2023 •

edited

Loading