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

[SSH Gateway] remove private key requirement when ownerToken is provide #10704

Merged
merged 1 commit into from
Jun 17, 2022

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Jun 16, 2022

Description

[SSH Gateway] remove private key requirement when ownerToken is provide

At past, we introduce a simple copy/paste command to let users can simply copy it into the terminal to get an ssh connection to the workspace, but there are limitations to the golang.org/x/crypto/ssh library, it require user provider a private key in order to skip password prompt, this can be confusing for users and cause an incident for VSCode Desktop can not connect to workspace.

So we move to fork golang.org/x/crypto/ssh repo link, and do some change, to make it suits us better.

The first change is cherry-pick some commit from tailscale 🙏 , the most important thing is support NoClientAuthCallback which let us can verify username in this check, and don't need the user to provide private key. PR link

see internal discuss

NOTE: this PR is base on #10573 for test, it need rebase to main before merge.

Related Issue(s)

Fixes #

How to test

  1. start a workspace in preview environment
  2. start another workspace or use your local computer (move your private key to another folder)
  3. using copy/paste ssh command to connect workspace, it should success
    4. upload your public key in setting -> SSH Key
    5. use your private key to connect with the workspace

4 and 5 is check this feature doesn't break #10573

Release Notes

[SSH Gateway] remove priavte key requirement when ownerToken is provide

Documentation

@iQQBot iQQBot marked this pull request as ready for review June 16, 2022 18:08
@iQQBot iQQBot requested review from a team June 16, 2022 18:08
@github-actions github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Jun 16, 2022
@iQQBot iQQBot self-assigned this Jun 16, 2022
@jenting jenting changed the title [SSH Gateway] remove priavte key requirement when ownerToken is provide [SSH Gateway] remove private key requirement when ownerToken is provide Jun 17, 2022
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-try-no-auth-ssh.6 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-try-no-auth-ssh.7 because the annotations in the pull request description changed
(with .werft/ from main)

@iQQBot iQQBot requested a review from a team June 17, 2022 04:37
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-try-no-auth-ssh.8 because the annotations in the pull request description changed
(with .werft/ from main)

@akosyakov
Copy link
Member

The PR should be opened again @mustard-mh branch not main?

@iQQBot
Copy link
Contributor Author

iQQBot commented Jun 17, 2022

NOTE: this PR is base on #10573 for test, it need rebase to main before merge.

@akosyakov
Copy link
Member

akosyakov commented Jun 17, 2022

@iQQBot the trouble is that now we need review from webapp and self hosted teams? Maybe we test SSH keys separately? and first solve one problem?

@iQQBot
Copy link
Contributor Author

iQQBot commented Jun 17, 2022

@iQQBot the trouble is that now we need review from webapp and self hosted teams? Maybe we test SSH keys separately? and first solve one problem?

Self-hosted team review is required, after you test I will rebase to main, just want to make sure it doesn't break user-uploaded ssh key

@akosyakov
Copy link
Member

akosyakov commented Jun 17, 2022

It works in copy/paste via Dashboard 🎉 We should also remove a note about private keys?
Screenshot 2022-06-17 at 06 59 00

It does not seem to help with auth in VS Code Desktop:
Screenshot 2022-06-17 at 07 04 58

@akosyakov
Copy link
Member

I see we will need to update VS Code desktop extension. It only uses ownerToken if private keys are present: https://github.com/gitpod-io/gitpod-vscode-desktop/blob/1b62c01a8d8c45739a637439161a4b8ed2fa51f6/src/remoteConnector.ts#L522

@akosyakov
Copy link
Member

akosyakov commented Jun 17, 2022

@iQQBot it looks good to me. what next? wait I did not test upload

@iQQBot iQQBot force-pushed the pd/try-no-auth-ssh branch from d5a6977 to c883698 Compare June 17, 2022 05:14
@roboquat roboquat added size/M and removed size/XXL labels Jun 17, 2022
@iQQBot iQQBot removed the request for review from a team June 17, 2022 05:15
@akosyakov
Copy link
Member

tested uploaded, it worked as well, great work

@iQQBot
Copy link
Contributor Author

iQQBot commented Jun 17, 2022

It works in copy/paste via Dashboard 🎉 We should also remove a note about private keys?

Yes, but will do it in another PR, because we don't know when is workspace team deploy this PR, after workspace team is deployed, remove note will safe.

It looks good to me. what next

I already rebase and remove review request from webapp team, but it still need self-hosted team approve, because it change go.sum in installer, if this PR get merge and deploy, we can update vscode desktop extension and dashboard note and document

@iQQBot iQQBot removed team: webapp Issue belongs to the WebApp team team: IDE labels Jun 17, 2022
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-pd-try-no-auth-ssh.10 because the annotations in the pull request description changed
(with .werft/ from main)

@akosyakov
Copy link
Member

akosyakov commented Jun 17, 2022

@iQQBot don't forget to add notes in our team sync that it does not slip 🙏 I'm super excited with all improvements 🚀

@roboquat roboquat merged commit 9dc436a into main Jun 17, 2022
@roboquat roboquat deleted the pd/try-no-auth-ssh branch June 17, 2022 06:43
@roboquat roboquat added the deployed: workspace Workspace team change is running in production label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production release-note size/M team: delivery Issue belongs to the self-hosted team team: workspace Issue belongs to the Workspace team
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants