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

Change seccomp default to 'docker/default' #39845

Closed
timstclair opened this issue Jan 13, 2017 · 31 comments · Fixed by #62662, #62671 or #62756

Comments

@timstclair
Copy link

commented Jan 13, 2017

Seccomp is currently defaulted to unconfined on docker:

defaultSeccompOpt = []dockerOpt{{"seccomp", "unconfined", ""}}

I believe this is for historical reasons, from #21790. However, we now have the ability (albeit alpha) to control seccomp profiles, so we should change the default to the more secure docker/default option. This profile is carefully curated, and should provide enhanced security without breaking the majority of users. Unfortunately "the majority" is not "everybody", so changing this default would be a breaking change, and care needs to be taken when rolling it out.

This may also need to wait for seccomp to be promoted to beta.

@vishh @pmorie @kubernetes/sig-node-misc

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2017

fwiw when we launched it with docker i tested all the publically available dockerfiles on github and docker hub with it...

so it was not done without much testing, we also only had one issue iirc after the release which was a lot better than i expected.

@vishh

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

@jessfs

This comment has been minimized.

Copy link

commented Feb 14, 2017

Just want to add my +1 for making this the default.

@timstclair

This comment has been minimized.

Copy link
Author

commented Feb 14, 2017

We might have to develop some tooling to make it easy to consume this
feature in k8s. The current approach of storing profiles on node's root
partition is complicated.

Agreed, I consider this alpha-behavior. For both seccomp & apparmor I'd like to add the ability to consume a ConfigMap (or maybe a new resource) as a profile. However, I consider this tangential to the issue discussed here.

Adjusting the default is already supported through the PodSecurityPolicy, but we still need to put some thought into how we'd go about rolling this out.

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented May 19, 2017

So there are a few ways I can imagine going about this, happy to open a proposal if you if agree with any:

  1. defaulting to docker/default
  2. forking the docker/default and maintaining our own default so we can edit easily in the future (i'm happy to do this since I wrote a lot of the docker default)
  3. (2) might require making our own "seccomp profile spec" and adding as an api somewhere (idk which group) if we don't want to rely on another runtime's.
  4. an option to set a different default maybe as a kubelet config option (also might require 3 unless we just default to another runtimes)

I am kinda in favor of 2, 3, and 4 so we have the most control.

@timstclair

This comment has been minimized.

Copy link
Author

commented May 22, 2017

2,3 sound good to me. I think we can do (2) before fully implementing (3), since it's technically still an "alpha" feature. I think we should use the OCI spec, but I don't know the details on it. (4) is essentially already implemented on the PodSecurityPolicy, via an annotation (should graduate to a field before seccomp support goes to beta): "seccomp.security.alpha.kubernetes.io/defaultProfileName" (see https://github.com/kubernetes/kubernetes/blob/master/pkg/security/podsecuritypolicy/seccomp/strategy.go)

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented May 22, 2017

(4) is essentially already implemented on the PodSecurityPolicy, via an annotation

makes sense

I think we should use the OCI spec, but I don't know the details on it

Should we use theirs as a base and make our own or vendor theirs (ie what happens if they change theirs)

@timstclair

This comment has been minimized.

Copy link
Author

commented May 22, 2017

Should we use theirs as a base and make our own or vendor theirs (ie what happens if they change theirs)

That's a question for @kubernetes/sig-api-machinery-misc, but my hunch is that we create our own copy (rather than vendoring), since I don't think we have any other examples of embedding a third party API in ours (unless it's opaque)

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented May 22, 2017

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented May 24, 2017

@DjangoPeng

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@jessfraz So can we use the default profile to set seccomp as docker/default now?

@feiskyer

This comment has been minimized.

Copy link
Member

commented May 25, 2017

I think we should spec the seccomp format first, refer #39128. or else, other container runtimes couldn't know how to process docker/default.

@jessfraz

This comment has been minimized.

Copy link
Contributor

commented May 25, 2017

@tallclair tallclair referenced this issue Sep 23, 2017
6 of 22 tasks complete
@fejta-bot

This comment has been minimized.

Copy link

commented Dec 25, 2017

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@justincormack

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2018

/remove-lifecycle stale

@dims

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

discussion on seccomp format is here : #52827

@liggitt

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

not sure a docker-specific seccomp name is good as a default

@tallclair

This comment has been minimized.

Copy link
Member

commented Apr 6, 2018

/assign @wangzhen127

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2018

@tallclair: GitHub didn't allow me to assign the following users: wangzhen127.

Note that only kubernetes members and repo collaborators can be assigned.

In response to this:

/assign @wangzhen127

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-github-robot pushed a commit that referenced this issue Apr 26, 2018
Kubernetes Submit Queue
Merge pull request #62662 from wangzhen127/runtime-default
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Change seccomp annotation from "docker/default" to "runtime/default"

**What this PR does / why we need it**:
This PR changes seccomp annotation from "docker/default" to "runtime/default", so that it is can be applied to all kinds of container runtimes. This PR is a followup of [#1963](kubernetes/community#1963).

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #39845

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@wangzhen127

This comment has been minimized.

Copy link
Member

commented Apr 26, 2018

/reopen

There is followup work.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2018

@wangzhen127: you can't re-open an issue/PR unless you authored it or you are assigned to it.

In response to this:

/reopen

There is followup work.

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-github-robot pushed a commit that referenced this issue May 17, 2018
Kubernetes Submit Queue
Merge pull request #62671 from wangzhen127/seccomp-in-psp
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use 'docker/default' as default seccomp profile for unprivileged PodSecurityPolicy

**What this PR does / why we need it**:
This PR sets the default seccomp profile for unprivileged PodSecurityPolicy to 'docker/default'. This PR is a followup of [#62662](#62662). We are using 'docker/default' instead of 'runtime/default' in addons in order to handle node version skew. When default seccomp profile is applied later, we can remove those annotations.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #39845

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
grayluck pushed a commit to grayluck/kubernetes that referenced this issue May 24, 2018
Kubernetes Submit Queue
Merge pull request kubernetes#62756 from wangzhen127/seccomp-in-addon
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use default seccomp profile for unprivileged addons

**What this PR does / why we need it**:
This PR sets the default seccomp profile of unprivileged addons to 'docker/default'. This PR is a followup of [kubernetes#62662](kubernetes#62662) and [kubernetes#62671](kubernetes#62671). We are using 'docker/default' instead of 'runtime/default' in addons in order to handle node version skew. When seccomp profile is applied automatically by default later, we can remove those annotations.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes#39845

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
k8s-github-robot pushed a commit that referenced this issue May 25, 2018
Kubernetes Submit Queue
Merge pull request #64279 from wangzhen127/dns-seccomp
Automatic merge from submit-queue (batch tested with PRs 61963, 64279, 64130, 64125, 64049). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use default seccomp profile for DNS addons.

**What this PR does / why we need it**:
This PR sets the default seccomp profile of DNS addons to 'docker/default'. This PR is a followup of #62662. We are using 'docker/default' instead of 'runtime/default' in addons in order to handle node version skew. When seccomp profile is applied automatically by default later, we can remove those annotations.

This is PR is part of #39845.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
k8s-github-robot pushed a commit that referenced this issue May 30, 2018
Kubernetes Submit Queue
Merge pull request #64281 from wangzhen127/es-seccomp
Automatic merge from submit-queue (batch tested with PRs 64281, 62991). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use default seccomp profile for flutend-elasticsearch addons

**What this PR does / why we need it**:
This PR sets the default seccomp profile to 'docker/default' for:
- fluentd-es daemon set.
- kibana-logging deployment.

The elasticsearch-logging stateful set is still unconfined because it uses gce:podsecuritypolicy:privileged.

This PR is a followup of #62662. We are using 'docker/default' instead of 'runtime/default' in addons in order to handle node version skew. When seccomp profile is applied automatically by default later, we can remove those annotations.

This is PR is part of #39845.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
dims pushed a commit to dims/kubernetes that referenced this issue Jun 5, 2018
Kubernetes Submit Queue
Merge pull request kubernetes#64276 from wangzhen127/manifests-seccomp
Automatic merge from submit-queue (batch tested with PRs 64276, 64094, 64719, 64766, 64750). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Use default seccomp profile for GCE manifests

**What this PR does / why we need it**:
This PR sets the default seccomp profile of unprivileged addons to 'docker/default' for GCE manifests. This PR is a followup of kubernetes#62662. We are using 'docker/default' instead of 'runtime/default' in addons in order to handle node version skew. When seccomp profile is applied automatically by default later, we can remove those annotations.

This is PR is part of kubernetes#39845.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

@tallclair tallclair reopened this Jun 7, 2018

@fejta-bot

This comment has been minimized.

Copy link

commented Sep 5, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
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.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@RRAlex

This comment has been minimized.

Copy link

commented Sep 18, 2018

/remove-lifecycle stale

@jesseendahl

This comment has been minimized.

Copy link

commented Sep 18, 2018

What's blocking this from happening at this point? cc @mayakacz @destijl

@dims

This comment has been minimized.

Copy link
Member

commented Sep 19, 2018

long-term-issue (note to self)

@tallclair

This comment has been minimized.

Copy link
Member

commented Sep 19, 2018

What's blocking this from happening at this point?

Generally, the fact that this a breaking change, and it's very hard to get breaking changes into Kubernetes these days, especially something that is hard to get visibility into.

Other issues are that this is still an "alpha" feature, but predates feature gates, so it's an alpha feature that's enabled in every cluster. Before changing the default, we probably need to promote the feature to GA, which includes:

  1. Migrating the annotation to a pod field
  2. Improving the profile management, so that profiles can be written as configmaps or k8s resources, and loaded by the node from there.
  3. Create Kubernetes standard profiles, which can eventually be made the default (can be a copy of the docker default)

And possibly:

  1. Create a tool for running seccomp in an "audit only" mode to capture any violations before potentially breaking workloads.
@fejta-bot

This comment has been minimized.

Copy link

commented Dec 18, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
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.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@fejta-bot

This comment has been minimized.

Copy link

commented Jan 17, 2019

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@fejta-bot

This comment has been minimized.

Copy link

commented Feb 16, 2019

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2019

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.