devfile / devworkspace-operator

Apache License 2.0
67 stars 55 forks source link

DevWorkspace doesn't respect SSH key passphrase when git pushing changes from workspace #1295

Closed dmytro-ndp closed 2 months ago

dmytro-ndp commented 3 months ago

Description

Related changes:

How To Reproduce

Prerequisites:

a) create SSH key with passphrase, using the command ssh-keygen -b 2048 -t rsa -N "test" b) add it to the GitHub.com

Steps to reproduce:

  1. Install Eclipse Che using User Dashboard https://quay.io/repository/eclipse/che-dashboard:pr-1157 with support of SSH key with passphrase
  2. Open User Dashboard and add SSH key to the User Preferences.
  3. Go to GitHub.com and get SSH link to any private repo using.
  4. Go to "Eclipse Che -> Create workspace" page and create workspace using SSH link to the private repo.
  5. Wait on workspace to be created, started, and project to be cloned.
  6. Make changes in the project, commit them and push.
  7. Go to workspace Terminal and try to push changes from there.

Let Dev Spaces keep ssh passphrase for git.webm

Expected behavior

Changes are pushed without errors or asking for passphrase.

Actual behavior

screencast-bpconcjcammlapcogcnnelfmaeghhagj-2024.08.06-14_39_32.webm

At the step 6

there was an error Git: Warning: Permanently added the ECDSA host key for IP address '140.82.112.3' to the list of known hosts. when tried to push changes to the repo using Source Control view of VS Code Editor. git-error log:

> git pull --tags origin main
Warning: Permanently added the ECDSA host key for IP address '140.82.112.3' to the list of known hosts.
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Screenshot from 2024-08-06 14-42-49

At the step 7

there was a request to enter passphrase for key '/etc/ssh/dwo_ssh_key": Screenshot from 2024-08-06 14-42-00

AObuchow commented 3 months ago

@dmytro-ndp to be clear: the issue with CheCode not being able to push to the repo using the UI might be a CheCode issue, though we can only confirm this once the DWO-side issue is resolved (i.e. when the git push over terminal with an ssh passphrase key works without prompting the user for the passphrase)

vinokurig commented 3 months ago

@AObuchow I believe that the problem is in the DWO side indeed. From the recording we see that the git push command called from terminal invokes the default ssh-askpass tool. Looks like we need to propagate the ssh-askpass.sh script to the workspace container as well.

AObuchow commented 3 months ago

I believe the issue with git pushing from terminal prompting the user for the passphrase is because we do not have the required ssh-add environment variables injected into the workspace containers (only the project clone container has them).

We could either add the DISPLAY & SSH_ASKPASS environment variables to all devworkspace containers, or try to add it only to the tooling container, similar to how we infer the tooling container component when determining the image for the persistent-home-init-container.

@vinokurig @dkwon17 any thoughts?

AObuchow commented 3 months ago

@AObuchow I believe that the problem is in the DWO side indeed. From the recording we see that the git push command called from terminal invokes the default ssh-askpass tool. Looks like we need to propagate the ssh-askpass.sh script to the workspace container as well.

My thoughts exactly :) I was just about to finish writing my comment when you posted your comment.

Edit: I hadn't realized we weren't propagating ssh-askpass.sh as well. It seems we'll have to inject it into the workspace container(s) as well as the required environment variables.

AObuchow commented 3 months ago

@vinokurig, here are some thoughts on where in the codebase we could make the required changes:

AObuchow commented 3 months ago

Lastly, something that slipped my mind during the review of your original PR for supporting ssh passphrases: instead of adding the ssh-add environment variables directly into the project clone image, we could add them into the container's environment variables. An argument against this approach however, is that users who provide their own project clone container image might not want these environment variables set for them.

I don't think we need to do anything about this, but I wanted to mention this here incase the topic ever comes up.

ibuziuk commented 3 months ago

moving to the next release for 3.17 - https://issues.redhat.com/browse/CRW-6614?focusedId=25289708&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#commen[…]289708