[ssh-gateway] fix missing output when running simple command #18366

iQQBot · 2023-07-27T08:26:33Z

Description

This PR fixes missing output in SSH gateway when running a simple command, and also upgrade golang crypto library

Summary generated by Copilot

`🤖 Generated by Copilot at e9c1f91`

Fixed security and compatibility issues in ws-proxy component. Refactored SSH proxy forwarding code to prevent crashes and handle errors better.

Related Issue(s)

Fixes EXP-246

How to test

start a workspace in preview environment
get ssh connection string via access token
run watch -n 0.1 ssh -T 'workspaceID#[email protected]' 'date +%s%N' test for 1minute
also run this command in gitpod.io workspace, and compare output.

Documentation

Preview status

Gitpod was successfully deployed to your preview environment.

🏷️ Name - pd-ssh-output
🔗 URL - pd-ssh-output.preview.gitpod-dev.com/workspaces.
📚 Documentation - See our internal documentation for information on how to interact with your preview environment.
📦 Version - pd-ssh-output-gha.14207

Build Options

Build

/werft with-werft
Run the build with werft instead of GHA
leeway-no-cache
/werft no-test
Run Leeway with --dont-test

Publish

/werft publish-to-npm
/werft publish-to-jb-marketplace

Installer

analytics=segment
with-dedicated-emulation
workspace-feature-flags
Add desired feature flags to the end of the line above, space separated

Preview Environment

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-gce-vm
If enabled this will create the environment on GCE infra
with-integration-tests=ssh
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh
with-monitoring

/hold

akosyakov · 2023-07-28T07:47:49Z

@iQQBot there is the actual fix? in the library?

iQQBot · 2023-07-28T07:50:31Z

@iQQBot there is the actual fix? in the library?

Move close to another place, it's because golang routine competition, we need send all date before we call close.

iQQBot · 2023-07-28T07:53:05Z

/hold

maybe we should wait all of stdout, stderr, request then call close

akosyakov · 2023-07-28T07:57:28Z

ok, I tried with VS Code Desktop and Local SSH over direct SSH, it does not help there, it now fails differently though, it seems it can tunnel, but then interrupts, before could not forward at all.

[09:55:56.820] Remote server is listening on 36127
[09:55:56.820] Parsed server configuration: {"serverConfiguration":{"remoteListeningOn":{"port":36127},"osReleaseId":"ubuntu","arch":"x86_64","webUiAccessToken":"","sshAuthSock":"","display":"","tmpDir":"/tmp","platform":"linux","connectionToken":"1a1aa111-a1aa-111a-1111-aa1a1a1a111a"},"extInstallTime":239,"installUnpackCode":""}
[09:55:56.821] Persisting server connection details to /Users/[user]/Library/Application Support/Code/User/globalStorage/ms-vscode-remote.remote-ssh/vscode-ssh-host-0fdc06be-660393deaaa6d1996740ff4880f1bad43768c814-0.102.0/data.json
[09:55:56.826] Starting forwarding server. localPort 64392 -> socksPort 64384 -> remotePort 36127
[09:55:56.827] Forwarding server listening on 64392
[09:55:56.827] Waiting for ssh tunnel to be ready
[09:55:56.828] [Forwarding server 64392] Got connection 0
[09:55:56.829] Tunneled 36127 to local port 64392
[09:55:56.829] Resolved "ssh-remote+7b22686f73744e616d65223a22616b6f7379616b6f762d70617263656c64656d6f2d373476766964317a3762702e7673732e70642d7373682d6f75747075742e707265766965772e676974706f642d6465762e636f6d222c2275736572223a22616b6f7379616b6f762d70617263656c64656d6f2d373476766964317a376270227d" to "127.0.0.1:64392"
[09:55:56.837] ------




[09:55:56.866] [Forwarding server 64392] Got connection 1
[09:55:56.961] > local-server-2> ssh child died, shutting down
[09:55:56.961] Failed to set up socket for dynamic port forward to remote port 36127: Socket closed. Is the remote port correct?
[09:55:56.962] Failed to set up socket for dynamic port forward to remote port 36127: Socket closed. Is the remote port correct?
[09:55:56.966] Local server exit: 0

iQQBot · 2023-07-28T08:00:42Z

ok, I tried with VS Code Desktop and Local SSH over direct SSH, it does not help there, it now fails differently though, it seems it can tunnel, but then interrupts, before could not forward at all.

Yeah, It's another problem, not related this PR

akosyakov · 2023-07-28T08:02:27Z

not related this PR

What interesting though that before it won't ever reach [Forwarding server 64392] Got connection 1 so this PR did fix initiation of the forwarding somehow.

akosyakov · 2023-07-31T07:10:51Z

@iQQBot Good to retest?

akosyakov · 2023-07-31T07:13:06Z

components/ws-proxy/pkg/sshproxy/forward.go

+		defer wg.Done()
+		_, _ = io.Copy(targetChan, originChan)
+		targetChannelWg.Done()
+		targetChannelWg.Wait()


I wonder should not we wait for stderr/stdout in both directions? won't close on one end trigger close on another

In fact, they are closed together. It's just that due to the nature of go routines, it is possible that the last few bytes may not be guaranteed to be sent completely, so we need to wait for all of them to be sent before sending a close signal.

yeah, I meant there we can simplify with one wg, which waits for both directions, but like that is fine as well 👍

akosyakov · 2023-07-31T07:17:55Z

@iQQBot preview envs are down?

iQQBot · 2023-07-31T07:21:07Z

@iQQBot preview envs are down?

Rerun

mustard-mh · 2023-07-31T07:23:24Z

ok, I tried with VS Code Desktop and Local SSH over direct SSH, it does not help there, it now fails differently though, it seems it can tunnel, but then interrupts, before could not forward at all.

[09:55:56.820] Remote server is listening on 36127
[09:55:56.820] Parsed server configuration: {"serverConfiguration":{"remoteListeningOn":{"port":36127},"osReleaseId":"ubuntu","arch":"x86_64","webUiAccessToken":"","sshAuthSock":"","display":"","tmpDir":"/tmp","platform":"linux","connectionToken":"1a1aa111-a1aa-111a-1111-aa1a1a1a111a"},"extInstallTime":239,"installUnpackCode":""}
[09:55:56.821] Persisting server connection details to /Users/[user]/Library/Application Support/Code/User/globalStorage/ms-vscode-remote.remote-ssh/vscode-ssh-host-0fdc06be-660393deaaa6d1996740ff4880f1bad43768c814-0.102.0/data.json
[09:55:56.826] Starting forwarding server. localPort 64392 -> socksPort 64384 -> remotePort 36127
[09:55:56.827] Forwarding server listening on 64392
[09:55:56.827] Waiting for ssh tunnel to be ready
[09:55:56.828] [Forwarding server 64392] Got connection 0
[09:55:56.829] Tunneled 36127 to local port 64392
[09:55:56.829] Resolved "ssh-remote+7b22686f73744e616d65223a22616b6f7379616b6f762d70617263656c64656d6f2d373476766964317a3762702e7673732e70642d7373682d6f75747075742e707265766965772e676974706f642d6465762e636f6d222c2275736572223a22616b6f7379616b6f762d70617263656c64656d6f2d373476766964317a376270227d" to "127.0.0.1:64392"
[09:55:56.837] ------




[09:55:56.866] [Forwarding server 64392] Got connection 1
[09:55:56.961] > local-server-2> ssh child died, shutting down
[09:55:56.961] Failed to set up socket for dynamic port forward to remote port 36127: Socket closed. Is the remote port correct?
[09:55:56.962] Failed to set up socket for dynamic port forward to remote port 36127: Socket closed. Is the remote port correct?
[09:55:56.966] Local server exit: 0

@akosyakov Found bug in https://github.com/microsoft/dev-tunnels-ssh that it omit SSH_MSG_CHANNEL_EXTENDED_DATA and throw error, will create a fix PR later today.

Which should make local-ssh x gateway works

akosyakov · 2023-07-31T07:24:52Z

@akosyakov Found bug in https://github.com/microsoft/dev-tunnels-ssh that it omit SSH_MSG_CHANNEL_EXTENDED_DATA and throw error, will create a fix PR later today.

@mustard-mh I checked it is the same with the tunnel but it works. I wonder whether maybe it works with all changes together now. Although if we resolved localhost hanging then exploring direct SSH is not a priority.

mustard-mh · 2023-07-31T07:35:51Z

@mustard-mh I checked it is the same with the tunnel but it works. I wonder whether maybe it works with all changes together now. Although if we resolved localhost hanging then exploring direct SSH is not a priority.

@akosyakov It's because golang lib doesn't ignore SSH_MSG_UNIMPLEMENTED as rfc said https://github.com/gitpod-io/golang-crypto/blob/master/ssh/messages.go#L844

When we use local-ssh x gateway, dev-tunnel will send SSH_MSG_UNIMPLEMENTED since it doesn't resolve SSH_MSG_CHANNEL_EXTENDED_DATA.

So we can fix it in golang-crypto or dev-tunnel

iQQBot · 2023-07-31T19:20:51Z

/unhold

roboquat added do-not-merge/hold do-not-merge/work-in-progress size/XS size/S and removed size/XS labels Jul 27, 2023

iQQBot force-pushed the pd/ssh-output branch from 850b509 to 5747b58 Compare July 27, 2023 17:40

[ssh-gateway] fix missing output when running simple command

1bc508b

iQQBot force-pushed the pd/ssh-output branch from 5747b58 to e9c1f91 Compare July 27, 2023 17:46

iQQBot changed the title ~~[DO NOT MERGE] SSH output debug~~ [ssh-gateway] fix missing output when running simple command Jul 27, 2023

upgrade golang-crypto library

0e64d05

iQQBot force-pushed the pd/ssh-output branch from e9c1f91 to 0e64d05 Compare July 27, 2023 17:55

roboquat added size/M and removed size/S labels Jul 27, 2023

iQQBot marked this pull request as ready for review July 28, 2023 07:41

iQQBot requested a review from a team as a code owner July 28, 2023 07:41

roboquat removed the do-not-merge/work-in-progress label Jul 28, 2023

github-actions bot added the team: team-engine label Jul 28, 2023

aledbf approved these changes Jul 28, 2023

View reviewed changes

only close when all of stdout stderr request stream finish

2e2cd8d

akosyakov reviewed Jul 31, 2023

View reviewed changes

upgrade golang-crypto

f1be8da

roboquat removed the do-not-merge/hold label Jul 31, 2023

roboquat merged commit 2e2833a into main Jul 31, 2023

roboquat deleted the pd/ssh-output branch July 31, 2023 19:21

[ssh-gateway] fix missing output when running simple command #18366

[ssh-gateway] fix missing output when running simple command #18366

Uh oh!

Conversation

iQQBot commented Jul 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

🤖 Generated by Copilot at e9c1f91

Related Issue(s)

How to test

Documentation

Preview status

Build Options

Uh oh!

akosyakov commented Jul 28, 2023

Uh oh!

iQQBot commented Jul 28, 2023

Uh oh!

iQQBot commented Jul 28, 2023

Uh oh!

akosyakov commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iQQBot commented Jul 28, 2023

Uh oh!

akosyakov commented Jul 28, 2023

Uh oh!

akosyakov commented Jul 31, 2023

Uh oh!

akosyakov Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

iQQBot Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

akosyakov Jul 31, 2023

Choose a reason for hiding this comment

Uh oh!

akosyakov commented Jul 31, 2023

Uh oh!

iQQBot commented Jul 31, 2023

Uh oh!

mustard-mh commented Jul 31, 2023

Uh oh!

akosyakov commented Jul 31, 2023

Uh oh!

mustard-mh commented Jul 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iQQBot commented Jul 31, 2023

Uh oh!

Uh oh!

iQQBot commented Jul 27, 2023 •

edited

Loading

`🤖 Generated by Copilot at e9c1f91`

akosyakov commented Jul 28, 2023 •

edited

Loading

mustard-mh commented Jul 31, 2023 •

edited

Loading