SSH: Upload Private Key does not work as expected #1020

vitaliy-guliy · 2021-03-05T16:14:33Z

Signed-off-by: Vitaliy Gulyy vgulyy@redhat.com

What does this PR do?

Allows to upload and use encrypted private SSH key.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

eclipse-che/che#19109

How to test this PR?

Devfile to test

---
apiVersion: 1.0.0
metadata:
  generateName: clone-ssh-
projects:
  -
    name: che-theia
    source:
      type: git
      location: "git@github.com:eclipse/che-theia.git"
  -
    name: atlascode
    source:
      type: git
      location: "git@bitbucket.org:atlassianlabs/atlascode.git"
components:
  - type: cheEditor
    alias: che-theia
    reference: https://raw.githubusercontent.com/chepullreq4/pr-check-files/master/che-theia/pr-1020/che_theia_meta.yaml
    memoryLimit: 2G

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

codecov · 2021-03-05T16:14:39Z

Codecov Report

Merging #1020 (ff37397) into master (d690740) will increase coverage by 1.59%.
The diff coverage is 2.60%.

@@            Coverage Diff             @@
##           master    #1020      +/-   ##
==========================================
+ Coverage   21.46%   23.05%   +1.59%     
==========================================
  Files         315      328      +13     
  Lines       11495    12318     +823     
  Branches     1710     1880     +170     
==========================================
+ Hits         2467     2840     +373     
- Misses       8882     9325     +443     
- Partials      146      153       +7

Impacted Files	Coverage Δ
plugins/ssh-plugin/src/agent/ssh-agent.ts	`0.00% <0.00%> (ø)`
...lugins/ssh-plugin/src/command/add-key-to-github.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/command/command.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/command/create-key.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/command/delete-key.ts	`0.00% <0.00%> (ø)`
...ns/ssh-plugin/src/command/generate-key-for-host.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/command/generate-key.ts	`0.00% <0.00%> (ø)`
...ugins/ssh-plugin/src/command/upload-private-key.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/command/view-public-key.ts	`0.00% <0.00%> (ø)`
plugins/ssh-plugin/src/git/git-listener.ts	`0.00% <0.00%> (ø)`
... and 39 more

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 d690740...ff37397. Read the comment docs.

che-bot · 2021-03-05T16:49:14Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

che-bot · 2021-03-09T17:25:15Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

che-bot · 2021-03-17T12:01:02Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

che-bot · 2021-03-18T17:45:38Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

vinokurig · 2021-03-19T14:02:14Z

When we have a previously uploaded encrypted ssh key, after workspace start, there is an input message asking to enter a passphrase for the encrypted ssh key:

vitaliy-guliy · 2021-03-19T14:51:18Z

When we have a previously uploaded encrypted ssh key, after workspace start, there is an input message asking to enter a passphrase for the encrypted ssh key:

It looks like this prompt for a key, that you have been just uploaded. It's just because it's in /home/theia/.ssh/ directory.
After workspace startup, all private keys placed to /etc/ssh/private/ directory.

In order to add them to the ssh-agent we need to provide a passphrase.
We can skip this step and ask the user for the passphrase on demand.

vinokurig · 2021-03-19T15:42:51Z

@vitaliy-guliy

It looks like this prompt for a key, that you have been just uploaded. It's just because it's in /home/theia/.ssh/ directory.
After workspace startup, all private keys placed to /etc/ssh/private/ directory.

No, It happens after workspace restart, I think it is caused by this bug: eclipse-che/che#19345

vitaliy-guliy · 2021-03-22T10:33:23Z

@vitaliy-guliy

It looks like this prompt for a key, that you have been just uploaded. It's just because it's in /home/theia/.ssh/ directory.
After workspace startup, all private keys placed to /etc/ssh/private/ directory.

No, It happens after workspace restart, I think it is caused by this bug: eclipse/che#19345

This PR must fix that bug.

vinokurig · 2021-03-22T15:18:45Z

This PR must fix that bug.

I've checked, it doesn't.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy · 2021-03-22T16:27:29Z

This PR must fix that bug.

I've checked, it doesn't.

If your workspace was created from the devfile, taken from this PR description, after restarting the workspace you will have following

The private key after restarting appears in /ect/ssh/private directory, not in /home/theia/.ssh/.

che-bot · 2021-03-22T16:54:42Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

vinokurig · 2021-03-23T06:42:25Z

The private key after restarting appears in /ect/ssh/private directory, not in /home/theia/.ssh/.

And if the key is encrypted an input message to enter a paraphrase appears, even if the paraphrase has been entered before.

vinokurig · 2021-03-23T10:14:32Z

@vitaliy-guliy It is not easy to see the fix in 29 changed files. It would be great to have a separate commit for refactoring and a separate commit for the fix.

vitaliy-guliy · 2021-03-23T10:44:51Z

@vitaliy-guliy It is not easy to see the fix in 29 changed files. It would be great to have a separate commit for refactoring and a separate commit for the fix.

The fix was added when the code was partially refactored. In fact, it's almost the same code as it was before. One huge class was splitted on different classes and linked with inversify. In any case, the refactored code always looks like a new code.

But if we talk about the fixup I can explain here how it works.

We launch the ssh-agent when we starting the SSH plugin and take values of SSH_AGENT_PID and SSH_AUTH_SOCK environment variables from ssh-agent output. SSH plugin model has an API to provide that values.
When workspace plugin clones the git repository, it asks for that environment variables and sets them to the process.
SSH_ASKPASS environment variable was introduced (like GIT_ASKPASS) to handle requests on the ssh passphrase.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

che-bot · 2021-03-23T16:43:19Z

✅ E2E Happy path tests succeed 🎉

See Details

name	link
che-theia	quay.io/crw_pr/che-theia:1020
che-theia-endpoint-runtime-binary	quay.io/crw_pr/che-theia-endpoint-runtime-binary:1020

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

Use comment "[crw-ci-test]" to rerun happy path E2E test.
Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

vinokurig

I confirm that the PR fixes the described problem and the SSH plugin works as expected after the refactoring.

vitaliy-guliy · 2021-03-25T13:56:50Z

I confirm that the PR fixes the described problem and the SSH plugin works as expected after the refactoring.

Thanks for the review

Use inversify to binding modules

9281312

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

Code refactoring

17373d7

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy added 2 commits March 17, 2021 13:18

Upload private SSH key

bf7fa9d

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

Merge with master

9b24486

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy added 3 commits March 18, 2021 18:52

Use askpass for GIT_ASKPASS and SSH_ASKPASS

9e53713

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

Update comment

6805d8d

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

Remove redundant file

80c636d

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vitaliy-guliy marked this pull request as ready for review March 19, 2021 10:24

vitaliy-guliy requested review from benoitf, ericwill, sunix, svor, tsmaeder and vinokurig as code owners March 19, 2021 10:24

Merge branch 'master' into upload-private-key

ccd21f6

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

Do not ask for ssh passphrase on startup

ff37397

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>

vinokurig approved these changes Mar 25, 2021

View reviewed changes

vitaliy-guliy merged commit c93df2a into master Mar 25, 2021

vitaliy-guliy deleted the upload-private-key branch March 25, 2021 14:00

che-bot added this to the 7.29 milestone Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSH: Upload Private Key does not work as expected #1020

SSH: Upload Private Key does not work as expected #1020

vitaliy-guliy commented Mar 5, 2021 •

edited

Loading

codecov bot commented Mar 5, 2021 •

edited

Loading

che-bot commented Mar 5, 2021

che-bot commented Mar 9, 2021

che-bot commented Mar 17, 2021

che-bot commented Mar 18, 2021

vinokurig commented Mar 19, 2021

vitaliy-guliy commented Mar 19, 2021

vinokurig commented Mar 19, 2021

vitaliy-guliy commented Mar 22, 2021 •

edited

Loading

vinokurig commented Mar 22, 2021

vitaliy-guliy commented Mar 22, 2021 •

edited

Loading

che-bot commented Mar 22, 2021

vinokurig commented Mar 23, 2021

vinokurig commented Mar 23, 2021

vitaliy-guliy commented Mar 23, 2021 •

edited

Loading

che-bot commented Mar 23, 2021

vinokurig left a comment

vitaliy-guliy commented Mar 25, 2021

SSH: Upload Private Key does not work as expected #1020

SSH: Upload Private Key does not work as expected #1020

Conversation

vitaliy-guliy commented Mar 5, 2021 • edited Loading

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Reviewers

Happy Path Channel

codecov bot commented Mar 5, 2021 • edited Loading

Codecov Report

che-bot commented Mar 5, 2021

che-bot commented Mar 9, 2021

che-bot commented Mar 17, 2021

che-bot commented Mar 18, 2021

vinokurig commented Mar 19, 2021

vitaliy-guliy commented Mar 19, 2021

vinokurig commented Mar 19, 2021

vitaliy-guliy commented Mar 22, 2021 • edited Loading

vinokurig commented Mar 22, 2021

vitaliy-guliy commented Mar 22, 2021 • edited Loading

che-bot commented Mar 22, 2021

vinokurig commented Mar 23, 2021

vinokurig commented Mar 23, 2021

vitaliy-guliy commented Mar 23, 2021 • edited Loading

che-bot commented Mar 23, 2021

vinokurig left a comment

Choose a reason for hiding this comment

vitaliy-guliy commented Mar 25, 2021

vitaliy-guliy commented Mar 5, 2021 •

edited

Loading

codecov bot commented Mar 5, 2021 •

edited

Loading

vitaliy-guliy commented Mar 22, 2021 •

edited

Loading

vitaliy-guliy commented Mar 22, 2021 •

edited

Loading

vitaliy-guliy commented Mar 23, 2021 •

edited

Loading