Skip to content

containerd: Enable enable_unprivileged_ports and enable_unprivileged_… #5538

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 15, 2022
Merged

containerd: Enable enable_unprivileged_ports and enable_unprivileged_… #5538

merged 1 commit into from
Jun 15, 2022

Conversation

olljanat
Copy link
Contributor

@olljanat olljanat commented May 4, 2022 •
edited
Loading

Proposed Changes

Make using hardened containers a bit easier by:

  • Allowing non-root containers to listen ports < 1024
  • Allowing ICMP on containers without any capabilities

Types of Changes

Containerd configuration change.

Verification

Create test deployment based on standard NGINX image which wants to listen port 80 and would fail as non-root before this change:

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: test
  name: test
spec:
  selector:
    matchLabels:
      app: test
  template:
    metadata:
      labels:
        app: test
    spec:
      containers:
      - name: test
        image: nginx
        ports:
        - containerPort: 80
          protocol: TCP
        volumeMounts:
        - name: nginx-cache
          mountPath: /var/cache/nginx/
        - name: nginx-run
          mountPath: /var/run/
        securityContext:
          capabilities:
            drop:
            - ALL
      securityContext:
        runAsNonRoot: true
        runAsUser: 65534
        runAsGroup: 65534
      volumes:
      - name: nginx-cache
        emptyDir: {}
      - name: nginx-run
        emptyDir: {}

Check that NGINX is running and listening port 80

Start debug session inside of same pod and test that ping works.

kubectl debug -it <pod name> --image=ollijanatuinen/debug:v2 --target=test
ping -c 1 k3s.io

Linked Issues

Closes #4545

User-Facing Change

NONE

Further Comments

containerd 1.6.x is needed so waiting for #4761 to be merged first. PR which added those flags and discussion related to it can be found from containerd/containerd#6170

@olljanat olljanat requested a review from a team as a code owner May 4, 2022 21:17
@olljanat olljanat mentioned this pull request May 10, 2022
Closed
@brandond
Copy link
Member

This looks good, but can you rebase on to the recent changes for 1.24 and confirm that it's doing what you want?

@olljanat olljanat force-pushed the enable-unprivileged-options branch from 00166b4 to 9701dcd Compare May 13, 2022 02:22
@olljanat
Copy link
Contributor Author

@brandond done and afaiu CI fails from same reason why it is failing on master branch which have nothing to do with this PR.

I updated testing guide to first message and here is log from testing which proof that pod is running as non-root without any capabilities and nginx is still able to listen port 80 and ping works.

$ kubectl get pods -o wide
NAME                    READY   STATUS    RESTARTS   AGE   IP           NODE        NOMINATED NODE   READINESS GATES
test-5ff75f7d8c-vhfld   1/1     Running   0          9s    10.42.0.31   vmi821450   <none>           <none>

$ kubectl exec -it test-5ff75f7d8c-vhfld -- whoami
nobody

$ kubectl exec -it test-5ff75f7d8c-vhfld -- id
uid=65534(nobody) gid=65534(nogroup) groups=65534(nogroup)

$ curl http://10.42.0.31
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
<style>
html { color-scheme: light dark; }
body { width: 35em; margin: 0 auto;
font-family: Tahoma, Verdana, Arial, sans-serif; }
</style>
</head>
<body>
<h1>Welcome to nginx!</h1>
<p>If you see this page, the nginx web server is successfully installed and
working. Further configuration is required.</p>

<p>For online documentation and support please refer to
<a href="http://nginx.org/">nginx.org</a>.<br/>
Commercial support is available at
<a href="http://nginx.com/">nginx.com</a>.</p>

<p><em>Thank you for using nginx.</em></p>
</body>
</html>


$ kubectl debug -it test-5ff75f7d8c-vhfld --image=ollijanatuinen/debug:v2 --target=test
Targeting container "test". If you don't see processes from this container it may be because the container runtime doesn't support this feature.
Defaulting debug container name to debugger-2h4vn.
If you don't see a command prompt, try pressing enter.

$ capsh --print
Current: =
Bounding set =cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap
Ambient set =
Current IAB: !cap_dac_read_search,!cap_linux_immutable,!cap_net_broadcast,!cap_net_admin,!cap_ipc_lock,!cap_ipc_owner,!cap_sys_module,!cap_sys_rawio,!cap_sys_ptrace,!cap_sys_pacct,!cap_sys_admin,!cap_sys_boot,!cap_sys_nice,!cap_sys_resource,!cap_sys_time,!cap_sys_tty_config,!cap_lease,!cap_audit_control,!cap_mac_override,!cap_mac_admin,!cap_syslog,!cap_wake_alarm,!cap_block_suspend,!cap_audit_read
Securebits: 00/0x0/1'b0
 secure-noroot: no (unlocked)
 secure-no-suid-fixup: no (unlocked)
 secure-keep-caps: no (unlocked)
 secure-no-ambient-raise: no (unlocked)
uid=65534(nobody) euid=65534(nobody)
gid=65534(nogroup)
groups=
Guessed mode: UNCERTAIN (0)

nobody@test-5ff75f7d8c-vhfld:/$ ping -c 1 k3s.io
PING k3s.io (104.26.15.42) 56(84) bytes of data.
64 bytes from 104.26.15.42 (104.26.15.42): icmp_seq=1 ttl=58 time=4.71 ms

--- k3s.io ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 4.705/4.705/4.705/0.000 ms

@brandond
Copy link
Member

brandond commented May 13, 2022 •
edited
Loading

I see that there's some concern from upstream about things breaking when these are enabled by default, see containerd/containerd#6170 (comment)

Can you speak to that? It sounds like we may need to leave them off by default, but add a way to enable them - other than just having the user provide a config template, which is already possible.

@olljanat
Copy link
Contributor Author

Can you speak to that?

As far I understand those concerns are coming from Docker side and about some non-Kubernetes related issue which they have seen somewhere in past. Other challenge would be that if those would be enabled on containerd side then people might miss those release notes.

Here that should't be issue if these would be enabled as part 1.24 release it would be easy highlight it on release notes.

Those same settings was released as part of Docker 20.10.0 https://github.com/moby/moby/blob/v20.10.0/daemon/oci_linux.go#L763-L778

It sounds like we may need to leave them off by default, but add a way to enable them - other than just having the user provide a config template, which is already possible.

As compromise we might also consider to activate those by default only on RKE2 side first? As there rootless mode, etc are not supported and there any help to make it easier to get existing container images working with those security hardenings would be useful.

And just FYI about where I got idea for adding those settings to containerd side https://twitter.com/ibuildthecloud/status/1333901962568368128

@olljanat
Copy link
Contributor Author

@manuelbuil perhaps you can be second reviewer for this? Would be nice to get it merged before first 1.24.x release version.

@brandond
Copy link
Member

It is too late for 1.24.1 at this point, we can evaluate it for 1.24.2.

@brandond brandond force-pushed the enable-unprivileged-options branch from 9701dcd to 41aec65 Compare June 14, 2022 17:34
@brandond
Copy link
Member

Rebased to master HEAD for CI fixes

@brandond brandond merged commit 2968a83 into k3s-io:master Jun 15, 2022
@cguertin14 cguertin14 mentioned this pull request Jun 28, 2022
Merged
@helloqiu helloqiu mentioned this pull request Jul 28, 2022
Closed
1 task
@olljanat olljanat mentioned this pull request Aug 4, 2022
Open
@brandond brandond mentioned this pull request Aug 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandond brandond brandond approved these changes

@manuelbuil manuelbuil manuelbuil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable enable_unprivileged_ports and enable_unprivileged_icmp
3 participants
@olljanat @brandond @manuelbuil