-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[ssh-gateway] Reject ssh connection when workspace not found #11433
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
Conversation
started the job as gitpod-build-pd-reject-if-ws-nf.1 because the annotations in the pull request description changed |
f059dcd
to
cb6e6c5
Compare
This is because we want to be compatible with |
cb6e6c5
to
47ccb93
Compare
What does it mean for user if workspace is running and private key is present? They will be able to connect successfully? What would be your expectation? |
I expectation we only support currently we support |
started the job as gitpod-build-pd-reject-if-ws-nf.6 because the annotations in the pull request description changed |
|
||
func NewSSHError(shortName string, description string) SSHError { | ||
return SSHError{shortName: shortName, description: description} | ||
} | ||
|
||
func NewSSHErrorWithReject(shortName string, description string) SSHError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that no more attempts are allowed and the connection is simply disconnected
ok, let's try, @jeanp413 @felladrin could you check that it is aligned with expectations in JB GW plugin and VS Code Desktop extension 🙏 |
I deleted the |
@iQQBot it works but I see in the logs that it still tries to authenticate with |
This does not affect the fact that we do not have GSSAPIAuthentication enabled, but the client can still send it. Due to the limitations of the standard library, we cannot disconnect immediately, but will wait for a timeout of about 1-2s, during which time the client can still try to send authentication, but it will fail due to lack of response |
@felladrin Assigned you to test how JB GW behaves with this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides testing the SSH connection, as instructed in the How to Test section, I checked JetBrains IntelliJ (Stable and EAP) and JetBrains Gateway (Stable and EAP).
- Tested the full connection flow (start, connect, stop) (start, connect, disconnect, stop). All good.
- Tested trying to connect simultaneously as I stopped the workspaces, and Gateway will simply indicate the workspace is stopping/stopped and won't try to connect.
- Tested trying to connect to non-running workspaces, and I couldn't find any way to do it in JetBrains Gateway flow. It will always take the user to the workspace page where they need to re-start it.
@aledbf Could you have a look? It would be good if this PR can include gen55 🙏 |
Description
[ssh-gateway] Reject ssh connection when workspace not found
For reviewer: In general, the SSH authentication process is as follows
NoClientAuthCallback
->PublicKeyCallback(if private key provider)
->PasswordCallback
there are 2 special error type
ssh.ErrNoAuth
This Error only exists in theNoClientAuthCallback
, which means that this callback is skipped and not counted as attemptsssh.ErrDenied
This Error means that retries are no longer allowed and the validation process is complete.All error created by
NewSSHErrorWithReject
is base onssh.ErrDenied
The new authentication process is simple than before
NoClientAuthCallback
responsible for checking if workspace exists, if not, close the connection, if it does, determine if the username is in the form ofworkspaceId#ownerToken
, if so, verify ownerToken, if the authentication fails, close the connectionPublicKeyCallback
responsible for verifying the private key, the username check is removed here because theNoClientAuthCallback
has already done it for us, so there is only one possibility that the username is a exists workspaceID.PasswordCallback
is similar withPublicKeyCallback
but verify password (ownerToken).Related Issue(s)
Fixes #11428
How to test
Connection closed
instead of ask your provider passwordRelease Notes
Documentation
Werft options: