Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/build/cmd/gomote: add "gomote rdp" #26090

Closed
bradfitz opened this issue Jun 27, 2018 · 12 comments

Comments

@bradfitz
Copy link
Member

@bradfitz bradfitz commented Jun 27, 2018

For Windows builders, occasionally we want GUI/desktop access.

Add a "gomote rdp" to proxy a gomote buildlet's TCP port 3389 to gomote client's localhost:3389, restricted to same users who have "gomote ssh" access.

/cc @aclements

@gopherbot gopherbot added this to the Unreleased milestone Jun 27, 2018
@gopherbot gopherbot added the Builders label Jun 27, 2018
@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Aug 3, 2018

/cc @dmitshur

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Aug 13, 2018

Perhaps the best way to implement this is to just fix our "gomote ssh" support to pass-through port forwarding requests. (perhaps filtered)

And because we support ssh to Windows buildlets, a user wanting gomote rdp could just "gomote ssh -L3389:localhost:3389 ..."

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Aug 13, 2018

That also might mean you could fix the ssh scp/sftp channel support at the same time, which is currently a TODO in the code.

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Aug 16, 2018

I investigated the current state of port forwarding, since this was also my first idea for how to implement this under the hood.

To test things, I started a simple HTTP server responding on port 8080 on a buildlet user-dmitshur-linux-amd64-0, then ran on my local machine (where I run gomote):

$ ssh -p 2222 -N -L 8080:localhost:8080 user-dmitshur-linux-amd64-0@farmer.golang.org

And tried to:

$ curl localhost:8080/hi
curl: (56) Recv failure: Connection reset by peer

The ssh client printed the following error:

channel 2: open failed: administratively prohibited: port forwarding is disabled

So, port forwarding isn't currently working (at least not for linux-amd64 buildlets), and I want to figure out which layer along the ssh proxying pipeline the "port forwarding is disabled" error is coming from (i.e., where it needs to be enabled).

(perhaps filtered)

Did you have something specific in mind @bradfitz? What kind of filtering would we want (and why, given how buildlets are meant to run arbitrary things as long as the user has access)?

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Aug 16, 2018

If you wanted to do it at the ssh level, we might just get it for free if you did #21140 (comment) (the io.Copy pairs proxying ssh channel requests). Or pretty close.

But rather than mess with that, the other option is to do it with magic on gomote client and coordinator.

Flow with the non-ssh option would be:

  • user runs gomote INSTANCE rdp
  • user's gomote client machine does a net.Listen("tcp", "localhost:3389")
  • gomote says "Hey, connect your RDP client to 'localhost:3389'.")
  • gomote Accepts a connection, and then writes an authenticated https request to https://farmer.golang.org/remote/rdp?instance=......
  • cmd/coordinator checks auth/etc, then net.Dial("tcp", "internal-ip-of-instance:3389") and replies 101 Switching Protocols and then glues together the internal 3389 with the client's 3389.
  • gomote then proxies between its localhost 3389 and the http request. (we have similar code already in cmd/buildlet/remote.go IIRC)

Later we could generalize this to other port numbers if needed.

Perhaps we could even use this for port 22 (or internal port 2200) so things like scp/sftp for #21140 work, avoiding the cmd/coordinator ssh server in most cases (except plan9).

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Aug 17, 2018

If you wanted to do it at the ssh level, we might just get it for free if you did #21140 (comment) (the io.Copy pairs proxying ssh channel requests). Or pretty close.

I'm looking at that issue simultaneously. I experimented with doing scp to farmer.golang.org and the error I get in that case is from this branch:

ptyReq, winCh, isPty := s.Pty()
if !isPty {
	fmt.Fprintf(s, "scp etc not yet supported; https://golang.org/issue/21140\n")
	return
}

So in the case of scp, that is definitely the blocker. For SSH port forwarding, it's either hitting the "administratively prohibited: port forwarding is disabled" problem before the if !isPty branch has a chance to run, or it might be getting past that branch without triggering it.

It might make sense to work on the scp case before this.

But rather than mess with that, the other option is to do it with magic on gomote client and coordinator. Flow with the non-ssh option would be: ...

If I understand you correctly, you're suggesting implementing rdp with the same general approach as ssh is implemented (except more directly, without an extra SSH server in between), right? If so, that makes sense, and it was the other very clear resolution path I have considered.

However, you go on to say:

Perhaps we could even use this for port 22 (or internal port 2200) so things like scp/sftp for #21140 work, avoiding the cmd/coordinator ssh server in most cases (except plan9).

This is very intriguing. You're suggesting that it might be possible and desirable to simplify the current ssh implementation by getting rid of the middle-layer SSH server, and just proxying a raw TCP connection from the user's local machine (running gomote) to the buildlet (where a real OpenSSL SSH server is running). For all OSes other than Plan 9, at least. Am I understanding your suggestion correctly?

If so, I'd be very eager to try to go in that direction, because it'd be very nice to reduce the complexity of the current solution and be able to solve scp/ssh port forwarding and any other issues just by being a dumb TCP proxy (which is easy) rather than a SSH proxy (can be harder to support all features and be confident about correctness).

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Aug 17, 2018

Yup, that's what I was implying. The only downside is that it removes some control from us. For instance, it lets any gomote ssh user setup TCP tunnels from anywhere to anywhere on their own once they're in. But--- we're already giving them full code execution by design, so it doesn't really matter. We already have to fully trust these users.

Note we'll still need to keep the middle-layer ssh server for plan9.

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Nov 12, 2019

A year later and @aclements needs this again. Fortunately I wrote a design above that I still like, so I'll look into this today.

@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Nov 12, 2019

Fortunately, our Windows machines have RDP enabled:

gopher@SERVER-2016-V7- C:\Users\gopher>netstat /p tcp /a | findstr 3389                                                         
  TCP    0.0.0.0:3389           server-2016-v7-buildlet:0  LISTENING                                                            
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 14, 2019

Change https://golang.org/cl/207357 mentions this issue: cmd/coordinator: add buildlet TCP proxy handler, for gomote rdp

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 15, 2019

Change https://golang.org/cl/207378 mentions this issue: cmd/gomote: add gomote rdp subcommand to open an RDP proxy to a Windows buildlet

gopherbot pushed a commit to golang/build that referenced this issue Nov 15, 2019
Updates golang/go#26090

Change-Id: I095f70baceb23cf28fcd70a78fd72df29603370e
Reviewed-on: https://go-review.googlesource.com/c/build/+/207357
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@bradfitz

This comment has been minimized.

Copy link
Member Author

@bradfitz bradfitz commented Nov 15, 2019

Filed #35629 to make the Windows environment better once we're RDP'ed in.

/cc @aclements @cherrymui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.