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

.ssh/* files do not have expected/required permissions when copied into custom home dir #3509

Closed
itewk opened this issue Nov 9, 2020 · 18 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@itewk
Copy link

itewk commented Nov 9, 2020

Expected Behavior

When the .ssh/* files are copied into the home dir of the runAsUser of the container they should have the expected standard .ssh file permissions

Actual Behavior

sh-4.4$ ls -al  ~/.ssh
total 20
drwxrws---. 2 tssc 1001 4096 Nov  9 14:44 .
drwxrwsr-x. 7 root 1001 4096 Nov  9 14:44 ..
-rw-rw-r--. 1 tssc 1001  348 Nov  9 14:45 config
-rw-rw----. 1 tssc 1001 2610 Nov  9 14:45 id_git-ssh-keys-ploigos-workflow-reference-quarkus-mvn-foobar-frui
-rw-rw-r--. 1 tssc 1001  869 Nov  9 14:45 known_hosts

And because the permissions on .ssh/config are not 644 I get an error from git about about permissions on that file.

Steps to Reproduce the Problem

  1. have a TaskRun using a custom UID and custom home dir
  2. add ssh key secret to the ServiceAccount running the TaskRun container
  3. check the permisions on the .ssh files

Additional Info

  • Kubernetes version:

    Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.8", GitCommit:"9f2892aab98fe339f3bd70e3c470144299398ace", GitTreeState:"clean", BuildDate:"2020-08-13T16:12:48Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"19", GitVersion:"v1.19.0+d59ce34", GitCommit:"d59ce3486ae3ca3a0c36e5498e56f51594076596", GitTreeState:"clean", BuildDate:"2020-10-08T15:58:07Z", GoVersion:"go1.15.0", Compiler:"gc", Platform:"linux/amd64"}
  • Tekton Pipeline version:

    Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

v0.16.3%

Workaround

I added the following to my ClusterTask

      echo "****************************"
      echo "* Fix .ssh dir permissions *"
      echo "****************************"
      passwd_home=$(getent passwd $(whoami) | cut -d: -f6)
      chmod 700 ${passwd_home}/.ssh
      chmod 644 ${passwd_home}/.ssh/known_hosts
      chmod 644 ${passwd_home}/.ssh/config
      chmod 600 ${passwd_home}/.ssh/id_*
@itewk itewk added the kind/bug Categorizes issue or PR as related to a bug. label Nov 9, 2020
@ghost
Copy link

ghost commented Nov 11, 2020

Hm, interesting. In the code we create directories with 0700 perms and files with 0600 perms 😕

@itewk
Copy link
Author

itewk commented Nov 11, 2020

@sbwsg maybe its somethiung to do with my home directory being in a workspace mounted to a PV/PVC?

@ghost
Copy link

ghost commented Nov 11, 2020

@sbwsg maybe its somethiung to do with my home directory being in a workspace mounted to a PV/PVC?

Interesting, yeah I wonder if that's related. I noticed in another of your comments that the home directory has the setgid bit set on it (/home/tssc has perms drwxrwsr-x). Is that similarly true here?

@StrongMonkey
Copy link

@sbwsg Just hitting this. Is this something to be scheduled into next milestone?

@ghost
Copy link

ghost commented Jan 7, 2021

@StrongMonkey I cannot reproduce and as mentioned in an earlier comment we create the files with 0700 and 0600 perms. Can you provide any info to help try and reproduce this? What platform are you on? k8s version? tekton version?

@StrongMonkey
Copy link

@sbwsg I am using the customized build of tekton which is based of v0.16.3. But as @itewk mentioned, I reproduced it by specifying the pod's runAsUser to 1000(non-root). I am cloning a private repo with ssh auth.

Platform: rancher 2.5.4
K8s version: v1.19.3
Tekton: v0.16.3

There are multiple places which create files with 600 permission, this includes basic auth and ssh afaik.

@ghost
Copy link

ghost commented Jan 8, 2021

So, I just want to clarify what behaviour you're seeing. Here's an example TaskRun I run in my minikube:

kind: TaskRun
apiVersion: tekton.dev/v1beta1
metadata:
  name: test
spec:
  serviceAccountName: ssh-key-service-account
  taskSpec:
    steps:
    - name: foo
      image: alpine:3.12
      securityContext:
        runAsUser: 1000
      script: |
        ls -lah $HOME/.ssh

When I fetch the logs from this I see the following:

+ ls -lah /tekton/home/.ssh
total 16K
drwx------    2 1000     root        4.0K Jan  7 23:55 .
drwxrwxrwx    3 root     root        4.0K Jan  7 23:55 ..
-rw-------    1 1000     root         105 Jan  7 23:55 config
-rw-------    1 1000     root        2.5K Jan  7 23:55 id_ssh-key-for-git

Can you paste the logs from running the same thing in your cluster?

Edit to add: you'll need to create the ssh-key-service-account with a tekton ssh key too.

@matthewdevenny
Copy link

matthewdevenny commented Jan 8, 2021

The issue is due to defaultMode of a secret volume the default is 0644. If you mount a secret on the Tekton container for the ssh keys then you need to set defaultMode: 0600

kubectl explain pod.spec.volumes.secret
KIND:     Pod
VERSION:  v1

RESOURCE: secret <Object>

DESCRIPTION:
     Secret represents a secret that should populate this volume. More info:
     https://kubernetes.io/docs/concepts/storage/volumes#secret

     Adapts a Secret into a volume. The contents of the target Secret's Data
     field will be presented in a volume as files using the keys in the Data
     field as the file names. Secret volumes support ownership management and
     SELinux relabeling.

FIELDS:
   defaultMode	<integer>
     Optional: mode bits to use on created files by default. Must be a value
     between 0 and 0777. Defaults to 0644. Directories within the path are not
     affected by this setting. This might be in conflict with other options that
     affect the file mode, like fsGroup, and the result can be other mode bits
     set.

   items	<[]Object>
     If unspecified, each key-value pair in the Data field of the referenced
     Secret will be projected into the volume as a file whose name is the key
     and content is the value. If specified, the listed keys will be projected
     into the specified paths, and unlisted keys will not be present. If a key
     is specified which is not present in the Secret, the volume setup will
     error unless it is marked optional. Paths must be relative and may not
     contain the '..' path or start with '..'.

   optional	<boolean>
     Specify whether the Secret or its keys must be defined

   secretName	<string>
     Name of the secret in the pod's namespace to use. More info:
     https://kubernetes.io/docs/concepts/storage/volumes#secret

@ghost
Copy link

ghost commented Jan 8, 2021

Hi @boxboatmatt thanks for jumping in. I don't think that's the issue here but appreciate your input. The files are being created in the original issue description with permissions 664 (config and known_hosts) and 660 (id_git...). As you mentioned, Secret's default permission bits are 644, which would actually be an improvement over what we've got here.

I haven't yet seen any clear pointer to what it is Tekton Pipelines should be doing differently. What I need is a short reproducible example that demonstrates the incorrect behaviour so I can debug it.

@matthewdevenny
Copy link

@sbwsg not sure about the original issue the author was experiencing.

I should have clarified that I believe the issue @StrongMonkey is experiencing is due to the rancher/gitjob use case for tekton, which mounts a git client secret on the Tekton container.

For reference:
rancher/gitjob#26

@StrongMonkey
Copy link

I think I figured out the problem. This is happening when HOME env is set in the container.

I think there is a bug in the workaround to make ssh respect HOME env setting, the code handles it by doing symlink from $HOME/.ssh to /home/$user/.ssh. However if HOME is set the homeDir will always return HOME env first. Then it will never try to setup $HOME/user/.ssh symlink. This is happening to my setup since I am setting HOME env to something else.

Does that look correct? @sbwsg

@ghost
Copy link

ghost commented Jan 8, 2021

I'm still not sure what error you're facing 😆 Can you please describe the error message or incorrect behaviour that you are seeing?

I think I figured out the problem. This is happening when HOME env is set in the container.

I think there is a bug in the workaround to make ssh respect HOME env setting, the code handles it by doing symlink from $HOME/.ssh to /home/$user/.ssh. However if HOME is set the homeDir will always return HOME env first. Then it will never try to setup $HOME/user/.ssh symlink. This is happening to my setup since I am setting HOME env to something else.

Does that look correct? @sbwsg

We have some documentation on working with a HOME environment variable as a non-root user here: https://github.com/tektoncd/pipeline/blob/master/docs/auth.md#using-secrets-as-a-non-root-user But I can't tell if what you're describing is buggy or working-as-intended without knowing what the actual problem you're experiencing is 😄

@StrongMonkey
Copy link

@sbwsg oh I missed the doc reference, thanks for that! I was just reading through the symlink code and figure out if this could be a problem. I am just tracking down into this method which does the symlink function. So if HOME env is set then ensureHomeEnvSSHLinkedFromPath will not setup any symlink because homepath and homeenv are the same value.

@tekton-robot
Copy link
Collaborator

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 8, 2021
@tekton-robot
Copy link
Collaborator

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@tekton-robot tekton-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels May 8, 2021
@tekton-robot
Copy link
Collaborator

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

@tekton-robot
Copy link
Collaborator

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@itewk
Copy link
Author

itewk commented Jun 28, 2021

i guess this died on the vine?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

4 participants