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

Unable to clone with SSH: unprotected private key file #14398

Closed
5 of 19 tasks
davidwindell opened this issue Aug 30, 2019 · 41 comments
Closed
5 of 19 tasks

Unable to clone with SSH: unprotected private key file #14398

davidwindell opened this issue Aug 30, 2019 · 41 comments
Assignees
Labels
area/editor/theia Issues related to the che-theia IDE of Che kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@davidwindell
Copy link
Contributor

davidwindell commented Aug 30, 2019

Describe the bug

When attempting to clone a git repository via the terminal, I get the error:

@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for '/etc/ssh/default-1569588924848/ssh-privatekey' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
Load key "/etc/ssh/default-1569588924848/ssh-privatekey": bad permissions
Permission denied (publickey).
fatal: Could not read from remote repository.

I can't change the permissions as Che is injecting this as readonly.

Che version

  • latest
  • nightly
  • other: please specify

Steps to reproduce

  1. Add SSH key with the SSH: generate key pair... command
  2. Restart workspace
  3. Launch terminal for main workspace container
  4. Run git clone {some git@ URL}

Expected behavior

SSH key should be mounted with appropriate permissions (i.e. 400 or 600).

Runtime

  • kubernetes (include output of kubectl version)
  • Openshift (include output of oc version)
  • minikube (include output of minikube version and kubectl version)
  • minishift (include output of minishift version and oc version)
  • docker-desktop + K8S (include output of docker version and kubectl version)
  • other: Rancher
kubectl version
Client Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.9", GitCommit:"3e4f6a92de5f259ef313ad876bb008897f6a98f0", GitTreeState:"clean", BuildDate:"2019-08-05T09:22:00Z", GoVersion:"go1.11.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.6", GitCommit:"96fac5cd13a5dc064f7d9f4f23030a6aeface6cc", GitTreeState:"clean", BuildDate:"2019-08-19T11:05:16Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"linux/amd64"}

Installation method

Helm charts via Rancher

Environment

  • my computer
    • Windows
    • Linux
    • macOS
  • Cloud
    • Amazon
    • Azure
    • GCE
    • other (please specify)
  • other: please specify
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Aug 30, 2019
@ibuziuk ibuziuk added team/ide and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Sep 2, 2019
@ibuziuk
Copy link
Member

ibuziuk commented Sep 2, 2019

@vinokurig @azatsarynnyy could you please take a look

@ibuziuk ibuziuk added the severity/P1 Has a major impact to usage or development of the system. label Sep 2, 2019
@vinokurig vinokurig added area/editor/theia Issues related to the che-theia IDE of Che kind/bug Outline of a bug - must adhere to the bug report template. labels Sep 2, 2019
@vinokurig
Copy link
Contributor

vinokurig commented Sep 2, 2019

Can not reproduce it in the latest che-theia version.
@davidwindell what is the version of the che-editor are you using?
It is strange that the key folder name is default-redacted, it must be default-{timestamp} in the latest version.

@vinokurig
Copy link
Contributor

@davidwindell Could you please specify the che version, it is viewed in the bottom left corner of the dashboard. Latest is 7.1.0-SNAPSHOT
screenshot-che-che 192 168 39 108 nip io-2019 09 06-08-47-22

@davidwindell
Copy link
Contributor Author

@vinokurig it's 7.0.0

@davidwindell
Copy link
Contributor Author

Could the fact I have to use:

CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_FS__GROUP: "0"
CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_RUN__AS__USER: "0"

As per #14330, cause this?

@l0rd l0rd added this to the Backlog - IDE 1 milestone Sep 23, 2019
@fe-o2
Copy link

fe-o2 commented Sep 24, 2019

I can replicate this bug using a
Che 7.2.0 snapshot (vanilla/not modified in any way during install)
running on osx 10.14.6 -
docker-desktop Engine: 19.03.2
and Kubernetes: v1.14.6

@vinokurig
Copy link
Contributor

vinokurig commented Oct 7, 2019

@davidwindell @fe-o2 Could you please test my fix for this issue? I've pushed the patched che-server image to my dockerhub: ivinokur/che-server:nightly. To specify the image you need to pass -i=ivinokur/che-server:nightly argument to chectl. BTW I couldn't reproduce the issue neither on my local minikube, nor on che.openshift.io

@davidwindell
Copy link
Contributor Author

davidwindell commented Oct 7, 2019

@vinokurig I can't test as with your server image I get a console error:

oot ERROR Failed to load plugins: Error: Request 'getDeployedPluginIds' failed
    at Proxy.<anonymous> (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:1789594)
    at e.<anonymous> (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2465965)
    at https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2461450
    at Object.next (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2461555)
    at https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2460468
    at new Promise (<anonymous>)
    at a (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2460245)
    at e.syncPlugins (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2465680)
    at e.<anonymous> (https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2465256)
    at https://static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1:2461450
e.log @ static.developers.redhat.com/che/theia_artifacts/theia.1baa86f0fe74ed25b9d0.js:1

This doesn't happen with the 7.2.0 image.

Looks like the same error here #14767 (comment)

@davidwindell
Copy link
Contributor Author

It's been suggested on Mattermost that I need to use the nightly plugin registry, will try tomorrow

@davidwindell
Copy link
Contributor Author

Unfortunately, I can't test this either because there is a bug in the nightly plugin registry #14704

@vinokurig
Copy link
Contributor

Should be fixed by #14791. @davidwindell @fe-o2 Please reopen the issue if you will be able to reproduce the bug again.

@davidwindell
Copy link
Contributor Author

davidwindell commented Oct 9, 2019

@vinokurig I managed to get this running but the issue still remains, can you re-open? I am using ivinokur/che-server:nightly

screenshot-che-ches2 ches2 che-test edge-servers com-2019 10 09-17_02_08

@davidwindell
Copy link
Contributor Author

According to Rancher, the "default mode" is 600...

screenshot-rancher edge-servers com-2019 10 09-17_34_48

Any thoughts on why they wouldn't appear that way in the terminal?

@davidwindell
Copy link
Contributor Author

davidwindell commented Oct 9, 2019

@vinokurig I think this might hold some clues:

bash-4.4 /etc/ssh/default-1570466985702 $ ls -la
total 4
drwxrwsrwt 3 root root  100 Oct  9 17:06 .
drwxr-xr-x 4 root root 4096 Oct  9 17:09 ..
drwxr-sr-x 2 root root   60 Oct  9 17:06 ..2019_10_09_17_06_26.258433790
lrwxrwxrwx 1 root root   31 Oct  9 17:06 ..data -> ..2019_10_09_17_06_26.258433790
lrwxrwxrwx 1 root root   21 Oct  9 17:06 ssh-privatekey -> ..data/ssh-privatekey
bash-4.4 /etc/ssh/default-1570466985702 $ cd ..data
bash-4.4 /etc/ssh/default-1570466985702/..data $ ls -la
total 4
drwxr-sr-x 2 root root   60 Oct  9 17:06 .
drwxrwsrwt 3 root root  100 Oct  9 17:06 ..
-rw-r----- 1 root root 1675 Oct  9 17:06 ssh-privatekey

It looks like the symbolic link is not 600 which is causing the problem?

Some hints here?

runatlantis/atlantis#775
https://stackoverflow.com/questions/50685385/kubernetes-config-map-symlinks-data-is-there-a-way-to-avoid-them

I don't know if global.securityContext.fsGroup=0 has some impact on this too.

@vinokurig
Copy link
Contributor

@davidwindell Have you reproduced the bug with 640 permissions?

@davidwindell
Copy link
Contributor Author

@vinokurig yes, it's still warning that Permissions 0644 for '/etc/ssh/default-1570466985702/ssh-privatekey' are too open.

@vinokurig
Copy link
Contributor

Looks like this depends on upstream issue: kubernetes/kubernetes#81089

@l0rd
Copy link
Contributor

l0rd commented Nov 4, 2019

@vinokurig 644 is the default file permission but we can specify different file permissions:

  volumes:
  - name: foo
    secret:
      secretName: mysecret
      defaultMode: 416

The upstream issue is about ownership rather then file permissions mode.

@vparfonov vparfonov added status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Nov 6, 2019
@vparfonov
Copy link
Contributor

blocked with kubernetes/kubernetes#81089

@davidwindell
Copy link
Contributor Author

The fix from K8's could be a long time coming. In the meantime, can you implement the workaround of copying the files but only when the pod is running as root. This would at least fix SSH git clone for some users.

@vparfonov vparfonov modified the milestones: 7.4.0, Backlog - IDE 1 Nov 6, 2019
@vinokurig
Copy link
Contributor

@davidwindell AFAIK Che and Che workspaces pods are running under non-root user.

@davidwindell
Copy link
Contributor Author

@vinokurig this can be changed by setting securityContext.runAsUser to '0' which is already necessary because of the bugs in #14330

@l0rd
Copy link
Contributor

l0rd commented Nov 6, 2019

I have created #15138

@vinokurig
Copy link
Contributor

I have some ideas with init container, playing with it now.

@vinokurig vinokurig added status/in-progress This issue has been taken by an engineer and is under active development. and removed status/blocked Issue that can’t be moved forward. Must include a comment on the reason for the blockage. labels Nov 7, 2019
@vinokurig
Copy link
Contributor

@davidwindell I've prepared an image ivinokur/che-server:nightly. It does the workaround that described in https://stackoverflow.com/a/50426726 from init container. Can you test the image?

@vinokurig
Copy link
Contributor

Dont forget to set

CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_FS__GROUP: 0
CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_RUN__AS__USER: 0

@vinokurig vinokurig removed the status/in-progress This issue has been taken by an engineer and is under active development. label Nov 8, 2019
@davidwindell
Copy link
Contributor Author

Awesome, yes I'll test today! 👍

@davidwindell
Copy link
Contributor Author

@vinokurig it worked!! Great job, I am now able to clone via SSH.

@l0rd
Copy link
Contributor

l0rd commented Nov 17, 2019

@vinokurig I have been testing this and managed to get file permission 0600 for the key files. I have found out that when you specify fsGroup the secret file permissions get overridden. So the trick consist simply in not specifying fsGroup at all.

I suppose that in Che we are forced to specify the fsGroup if we are running the pod as non root user (not sure about that though). But we do not need to specify the fsGroup when running the pod as root.

Anyway here are the steps I have followed (on minikube):

  1. Create a secret with the keys
$ kubectl create secret generic ssh-key-secret --from-file=id_rsa.pub=/Users/mloriedo/.ssh/id_rsa.pub --from-file=id_rsa=/Users/mloriedo/.ssh/id_rsa
secret/my-ssh-key created
  1. Start the pod
cat <<EOF | kubectl apply -f -
kind: Pod
apiVersion: v1
metadata:
  name: secret-test-pod
  labels:
    name: secret-test
spec:
  securityContext:
    runAsUser: 0
    runAsGroup: 0
  containers:
  - name: ssh-test-container
    image: busybox
    volumeMounts:
    - name: secret-volume
      readOnly: true
      mountPath: "/etc/secret-volume"
    command: ["tail"]
    args: ["-f", "/dev/null"]
  volumes:
  - name: secret-volume
    secret:
      secretName: ssh-key-secret
      defaultMode: 384
EOF
  1. Verify
$ kubectl exec -ti secret-test-pod -- ls -laR /etc/secret-volume/
/etc/secret-volume/:
total 4
drwxrwxrwt    3 root     root           120 Nov 16 10:47 .
drwxr-xr-x    1 root     root          4096 Nov 16 10:47 ..
drwxr-xr-x    2 root     root            80 Nov 16 10:47 ..2019_11_16_10_47_47.045033508
lrwxrwxrwx    1 root     root            31 Nov 16 10:47 ..data -> ..2019_11_16_10_47_47.045033508
lrwxrwxrwx    1 root     root            13 Nov 16 10:47 id_rsa -> ..data/id_rsa
lrwxrwxrwx    1 root     root            17 Nov 16 10:47 id_rsa.pub -> ..data/id_rsa.pub

/etc/secret-volume/..2019_11_16_10_47_47.045033508:
total 8
drwxr-xr-x    2 root     root            80 Nov 16 10:47 .
drwxrwxrwt    3 root     root           120 Nov 16 10:47 ..
-rw-------    1 root     root          1675 Nov 16 10:47 id_rsa
-rw-------    1 root     root           417 Nov 16 10:47 id_rsa.pub

@vinokurig
Copy link
Contributor

@lord Do you mean that we need to have an ability not to specify fsGroup ?

@l0rd
Copy link
Contributor

l0rd commented Nov 19, 2019

@vinokurig I think that in general we should not apply fsGroup and runAsUser if the user do not ask it specifically. And we should probably support runAsGroup as well. What do you think?

@vinokurig
Copy link
Contributor

@l0rd
Thank you for your researches, I've found that we can unset SecurityContext parameters by setting NULL to them in the CHE configmap.
@davidwindell
As a workaround you can set

CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_FS__GROUP: NULL
CHE_INFRA_KUBERNETES_POD_SECURITY__CONTEXT_RUN__AS__USER: 0

and the key files will have real 600 permissions. Could you please test it?

@davidwindell
Copy link
Contributor Author

@vinokurig thank you very much, I can confirm that setting FS__GROUP to NULL allows the SSH key to work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/editor/theia Issues related to the che-theia IDE of Che kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

8 participants