containerd: Enable enable_unprivileged_ports and enable_unprivileged_icmp by default

[olljanat]

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. Those settings was added on containerd 1.6.x version.

Verification

Deploy following test service:

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-tmp
          mountPath: /var/cache/nginx/
        securityContext:
          capabilities:
            drop:
            - ALL
      securityContext:
        runAsNonRoot: true
        runAsUser: 1001
      volumes:
      - name: nginx-tmp
        emptyDir: {}

Check that NGINX is running and listening port 80

On theory also ping with command like:

kubectl exec -it test-... -- ping 1.1.1.1

should works but at least on my env it does not.

However at least sysctl setting net.ipv4.ping_group_range got value value 0 2147483647 as expected. It might be configuration issue on my env or to be related the fact that kind run inside of docker.

Change can be tested without building kind with configuration like this:

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
name: test
containerdConfigPatches:
- |-
  [plugins."io.containerd.grpc.v1.cri"]
    enable_unprivileged_ports = true
    enable_unprivileged_icmp = true

[linux-foundation-easycla]

• ❌ - login: @olljanat / name: Olli Janatuinen . The commit (f177395) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[k8s-ci-robot]

Welcome @olljanat!

It looks like this is your first PR to kubernetes-sigs/kind 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kind has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @olljanat. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: olljanat
To complete the pull request process, please assign bentheelder after the PR has been reviewed.
You can assign the PR to them by writing /assign @bentheelder in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[BenTheElder]

Hmm, I don't know that we should be enabling non-default containerd settings that aren't strictly necessary or documented as part of https://kubernetes.io/docs/setup/production-environment/container-runtimes/

I appreciate the desire to have these settings enabled, but doing so only in kind makes kind an inaccurate way to test applications. I would like to see this enabled, but not so much as a special case in KIND only.

AFAICT kubernetes/kubernetes#102612 did not reach consensus to enable it. containerd/containerd#6170 certainly did not.

. Change can be tested without building kind with configuration like this:

As you noted these settings can be enabled in your own clusters without being enabled by default by using a containerd config patch (which we need to document under https://kind.sigs.k8s.io/docs/user/configuration/, an example is in https://kind.sigs.k8s.io/docs/user/local-registry/).

kubectl exec -it test-... -- ping 1.1.1.1

ICMP is generally not enabled by Kubernetes's networking stack like TCP and UDP are.

[olljanat]

AFAICT kubernetes/kubernetes#102612 did not reach consensus to enable it. containerd/containerd#6170 certainly did not.

Some containerd developers was worry about some possible corner cases / unknown unknowns which might came with this one. I assumed that kind would be good place to try find those so they can be fixed.

FYI. I have same proposal open also on k3s side k3s-io/k3s#5538

[BenTheElder]

Some containerd developers was worry about some possible corner cases / unknown unknowns which might came with this one. I assumed that kind would be good place to try find those so they can be fixed.

Our first and foremost goal still is testing Kubernetes itself https://kind.sigs.k8s.io/docs/contributing/project-scope/

I think edge cases could be discovered by using containerdConfigPAtches to turn it on and explore for those cases, but I don't think we should find them by turning it on and waiting for users to complain. Instead we're more likely to have users complain that they tested their application in kind but then it didn't work on their "real" cluster since there's currently no plan to roll this out in containerd by default or in any of the major vendors, and we try to avoid that.

See e.g. #1331

FYI. I have same proposal open also on k3s side k3s-io/k3s#5538

ACK, but K3s is very clearly a forked subset of Kubernetes, as a general rule we're not inclined to follow that project.

[olljanat]

Ok. I noticed that containerd has added 2.0 milestone so opened this one now containerd/containerd#6924

However it comes to details enable_unprivileged_ports would be enought to handle kubernetes/kubernetes#102612 and I think that it was enable_unprivileged_icmp which containerd developers was worry about and I just didn't really realized option to separate those two when worked with PR.

What you mean by saying:

ICMP is generally not enabled by Kubernetes's networking stack like TCP and UDP are.

That ping test case works fine before dropping all capabilities so I would assume that ICMP need to be enabled at least on some level.

EDIT: that looks to be busybox issue. Top of this change there need to be proper ping package installed on container like mentioned on https://serverfault.com/a/1001312

[aojea]

does securityContext work in all kind environments?
does it work in rootless?

[olljanat]

@aojea containerd run PR validations also against of rootless. On rootless enable_unprivileged_icmp is actually ignored by https://github.com/containerd/containerd/blob/v1.6.4/pkg/cri/server/sandbox_run_linux.go#L147 as other why it didn't passed those tests.

What comes to securityContext like on my example test case above I'm not actually sure if those works on rootless. Maybe someone who have access to that kind of environment should test? It is bit outside of this proposal but valuable information anyway.

[aojea]

I prefer to not enable non-defaults options too, we already have a lot of exposure surface and we don't know the fallout ... people can always patch it in kind for their use cases...

[BenTheElder]

as of yet there is no response to containerd/containerd#6924 containerd/containerd#6170 (comment), I'd be curious to see those responses, but in the current state I think we should stick to:

kind: Cluster
apiVersion: kind.x-k8s.io/v1alpha4
name: test
containerdConfigPatches:
- |-
  [plugins."io.containerd.grpc.v1.cri"]
    enable_unprivileged_ports = true
    enable_unprivileged_icmp = true

[olljanat]

@BenTheElder btw I wonder that has there been yet any discussion about Kubernetes 2.0 and collecting requirements for it? There is so many of those "secure by default" changes which would be needed that perhaps it would make sense to agree that new major version is needed to be able to include those breaking changes and that those need to live side by side quite long time?

I'm thinking changes like that pods should run as non-root, without capabilities and using read only root filesystem by default unless user specify other why.

[BenTheElder]

I wonder that has there been yet any discussion about Kubernetes 2.0 and collecting requirements for it?

I don't think there's currently any particularly serious discussion.

There is so many of those "secure by default" changes which would be needed that perhaps it would make sense to agree that new major version is needed to be able to include those breaking changes and that those need to live side by side quite long time?

I think it would be difficult to reach agreement on to be honest, there's usually a lot of pushback against any breaking changes, they do happen though. Which is why I don't think there's any major 2.0 discussion happening, most users want less breakage and more portability, AFAICT.