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

feat: avoid setting privileged flag if seLinuxOptions is not null #599

Merged
merged 4 commits into from
Jun 3, 2021

Conversation

backjo
Copy link
Contributor

@backjo backjo commented Jun 3, 2021

This PR sets the privileged flag to false if SELinuxOptions are present/defined. This is needed because containerd treats SELinux and Privileged controls as mutually exclusive - see https://github.com/containerd/cri/blob/aa2d5a97c/pkg/server/container_create.go#L164 .

This allows users who use SELinux for managing privileged processes to use GH Actions - otherwise, based on the SELinux policy, the Docker in Docker container might not be privileged enough.

Signed-off-by: Jonah Back jonah@jonahback.com

Signed-off-by: Jonah Back <jonah@jonahback.com>
@mumoshu
Copy link
Collaborator

mumoshu commented Jun 3, 2021

@backjo Hey! Thanks for your contribution. A bit hesitant to ask, but would you mind adding a short description and example to explain how you would use this feature in practice?

I have literally no experience using seLinux thing with containers.

Probably one would start with a "allow all" selinux policy(is there the term "policy" really?) that contains a full list of allowed operations, and gradually trim it down so that you can only allow specific operations. But I don't even know what a "allow all" selinux policies would look like and therefore no idea how I could test or maintain this feature.

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 3, 2021

a short description and example to explain how you would use this feature in practice?

Whatever place or whatever style is okay. As long as it is written somewhere in the README, I can move or edit it appropriately later. The hardest part for me is about how I could start testing this.

@backjo
Copy link
Contributor Author

backjo commented Jun 3, 2021

Yeah - I can try, but SELinux is pretty complicated, even for me :/ . We leverage AWS BottleRocket OS, which comes with a built-in set of policies. I imagine most folks who are using SELinuxOptions will be somewhat familiar already - maybe just a link to something like https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux_atomic_host/7/html/container_security_guide/docker_selinux_security_policy ?

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 3, 2021

@backjo Thanks for clarifying! I see. Yes, the redhat doc seems great so a snippet of Runner/RunnerDeployment manifest and a short description and a link to the redhat doc seems to do the job.

backjo and others added 3 commits June 2, 2021 19:00
…inux for docker inside of containers

Signed-off-by: Jonah Back <jonah@jonahback.com>
Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your contribution @backjo ☺️

@mumoshu mumoshu merged commit 8c42f99 into actions:master Jun 3, 2021
@peimanja
Copy link

peimanja commented Sep 27, 2021

@backjo I appreciate if you can help me with this issue that I'm facing since we are also running github action runners on AWS BottleRocket OS. So I've been seeing this error on runners startup:

mv: setting attribute 'security.selinux' for 'security.selinux': Permission denied
mv: setting attribute 'security.selinux' for 'security.selinux': Permission denied
mv: setting attribute 'security.selinux' for 'security.selinux': Permission denied
mv: setting attribute 'security.selinux' for 'security.selinux': Permission denied
mv: setting attribute 'security.selinux' for 'security.selinux': Permission denied

but runners working fine and get started without any issue.
the we've started seeing errors like this on some actions:

Status: Downloaded newer image for mysql:5.7
docker: Error response from daemon: failed to copy xattrs: failed to set xattr "security.selinux" on /var/lib/docker/volumes/ac04276306bcdac9522a2e785260017fe7fceed01382998b528f684aac5b0150/_data: permission denied.

I've looked around and noticed we can set the SELinuxOptions on runners. So I set it like this:

      securityContext:
        seLinuxOptions:
          level: "s0"
          role: "system_r"
          type: "super_t"
          user: "system_u"

When I set these the startup errors go away and runners come up without any issue. However now looks like docker having issue

time="2021-09-27T07:50:08.623439127Z" level=error msg="failed to mount overlay: operation not permitted" storage-driver=overlay2
time="2021-09-27T07:50:08.623481986Z" level=error msg="exec: \"fuse-overlayfs\": executable file not found in $PATH" storage-driver=fuse-overlayfs
time="2021-09-27T07:50:08.625989494Z" level=error msg="AUFS was not found in /proc/filesystems" storage-driver=aufs
time="2021-09-27T07:50:08.626346406Z" level=error msg="failed to mount overlay: operation not permitted" storage-driver=overlay
time="2021-09-27T07:50:08.626360291Z" level=error msg="Failed to built-in GetDriver graph devicemapper /var/lib/docker"
time="2021-09-27T07:50:08.630144575Z" level=warning msg="Running iptables --wait -t nat -L -n failed with message: `iptables v1.8.4 (legacy): can't initialize iptables table `nat': Permission denied (you must be root)\nPerhaps iptables or your kernel needs to be upgraded.`, error: exit status 3"

I'm running latest version of the controller + using runner docker in docker latest image and have Bottlerocket OS 1.2.0 and also can test on Bottlerocket OS 1.0.5

I also noticed that RunnerDeployment crd does not support all the configuration https://github.com/actions-runner-controller/actions-runner-controller/blob/3f331e9a3965f38fc98f452571735e0469de8c6d/charts/actions-runner-controller/crds/actions.summerwind.dev_runnerdeployments.yaml#L3052 such as allowPrivilegeEscalation or capabilities since I wanted to play around with those as I saw this issue bottlerocket-os/bottlerocket#1569 (comment).

So not sure what's the issue and how you got around it?

I'm also watching this PR to see if that can help bottlerocket-os/bottlerocket#1733

@backjo
Copy link
Contributor Author

backjo commented Sep 27, 2021

Hey @peimanja - here's the resource we define, hope it helps

apiVersion: actions.summerwind.dev/v1alpha1
kind: RunnerDeployment
metadata:
  name: devops-runners
  namespace: actions-runner-system
spec:
  replicas: 1
  template:
    spec:
      volumes:
      - name: cgroupfs
        hostPath:
          path: /sys/fs/cgroup
      securityContext:
        seLinuxOptions:
          level: s0
          role: system_r
          type: super_t
          user: system_u
      labels:
        - runner-devops
        - ubuntu-latest
      organization: devops
      nodeSelector:
        kubernetes.io/os: linux
        kubernetes.io/arch: amd64
      #image: ubuntu:focal
      #imagePullPolicy: Always
      #dockerdWithinRunnerContainer: false
      dockerVolumeMounts:
      - mountPath: /sys/fs/cgroup
        name: cgroupfs
      resources:
        limits:
          cpu: "2"
          memory: "2Gi"
        requests:
          cpu: "2"
          memory: "2Gi"
      # Timeout after a node crashed or became unreachable to evict your pods somewhere else (default 5mins)
      tolerations:
        - key: "node.kubernetes.io/unreachable"
          operator: "Exists"
          effect: "NoExecute"
          tolerationSeconds: 30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants