Skip to content

[jb] fix #8296: validate host key fingerprint #8317

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

Merged
merged 1 commit into from
Feb 22, 2022
Merged

Conversation

akosyakov
Copy link
Member

@akosyakov akosyakov commented Feb 18, 2022

Description

Gateway plugin intercepts SSH fingerprint check and validates it against public ssh key of SSH gateway instead of asking a user. If SSH keys are not present then an error is presented to a user that Gitpod installation is not supported, if keys are not matched then an error presented as well.

Related Issue(s)

fixes #8296

How to test

Release Notes

NONE

Documentation

return fetchWS(resolveJoinLinkUrl, connectParams, ownerToken)
}

private fun resolveCredentials(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iQQBot relevant functions are resolveCredentials, resolveHostKeys and most interesting is acceptHostKey

@codecov
Copy link

codecov bot commented Feb 18, 2022

Codecov Report

Merging #8317 (c2df2f4) into main (e111296) will decrease coverage by 1.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8317      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e111296...c2df2f4. Read the comment docs.

@akosyakov
Copy link
Member Author

akosyakov commented Feb 21, 2022

/werft run

👍 started the job as gitpod-build-ak-jb-host-key.1

Base automatically changed from pd/ssh-hostkey to main February 21, 2022 11:13
): AskAboutHostKey {
val hostKeysByType = hostKeys.groupBy({ it.type.lowercase() }) { it.hostKey }
val acceptHostKey: AskAboutHostKey = { hostName, keyType, fingerprint, _ ->
if (hostName != ideUrl.host) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iQQBot Could you have a look please whether it makes sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the latest plugin URL changed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, now yes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can print an URL in werft job somehow. We know a version and can query JB marketplace for URL, but not sure how to display it on results page.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try and checked, it works, The process was very smooth and there were no more pop-ups

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's wait for workspace team to deploy and then we can merge

@akosyakov akosyakov marked this pull request as ready for review February 22, 2022 03:38
@akosyakov akosyakov requested a review from a team February 22, 2022 03:38
@akosyakov
Copy link
Member Author

@iQQBot changes to ws-proxy in production, it should be good to merge and deploy

@filiptronicek
Copy link
Member

Did I miss something or is the Restart IDE button gone by mistake, @akosyakov? It seems I still need to restart it in order for it to work.

@akosyakov
Copy link
Member Author

Did I miss something or is the Restart IDE button gone by mistake, @akosyakov? It seems I still need to restart it in order for it to work.

It is on purpose, next version of GW does not require restart which is going to be released on 24th.

@mustard-mh
Copy link
Contributor

/werft run

@akosyakov
Copy link
Member Author

akosyakov commented Feb 22, 2022

@mustard-mh @filiptronicek you can test against gitpod.io to be sure, you don't need prev envs

I updated How to test section.

@filiptronicek
Copy link
Member

I guess this is unrelated, but is our API broken that it returns null for a workspace that I don't even see in my dashboard and here it looks like it is starting/stopping?

image

@akosyakov
Copy link
Member Author

akosyakov commented Feb 22, 2022

I guess this is unrelated, but is our API broken that it returns null for a workspace that I don't even see in my dashboard and here it looks like it is starting/stopping?

Right I don't think it is related to this PR. Let's file another issue. It seems you get a workspace which does not have a normalised context URL. Please capture in the issue: How did you start a workspace? Using a link from How to start?

Copy link
Member

@filiptronicek filiptronicek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the issue I mentioned (unrelated to changes from this PR), it works for me a-ok. I wasn't aware that we could get rid of the dialog and I'm so happy we did!

image

I'll hold for the review from @mustard-mh and/or any potential further code review. /hold

@mustard-mh
Copy link
Contributor

Do steps like How to Test, and it works well

@akosyakov
Copy link
Member Author

/unhold

@roboquat roboquat merged commit 70aea6c into main Feb 22, 2022
@roboquat roboquat deleted the ak/jb_host_key branch February 22, 2022 08:12
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production release-note-none size/XL team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove SSH fingerprint confirm with JetBrains IDEs
5 participants