Add Seccomp to Annotations #25324

jessfraz · 2016-05-08T23:20:42Z

Add Seccomp API

This is a WIP of #24602, with some included changes to that proposal that I mentioned on it. For the most part those changes are that instead of passing the path to the profiles, the SeccompProfile struct is passed to the API. This way if using kubectl you can pass a local profile.

Things left to do:

Add Tests
Add ability to pass seccomp profile to kubectl, mostly wondering about the design on this one

This is my first PR to kubernetes so without a doubt some test somewhere will probably fail ;)

derekwaynecarr · 2016-05-09T01:27:11Z

@pmorie

thockin · 2016-05-09T03:23:13Z

pkg/api/types.go

@@ -1362,6 +1362,8 @@ type PodSecurityContext struct {
 	// Use the host's ipc namespace.
 	// Optional: Default to false.
 	HostIPC bool `json:"hostIPC,omitempty"`
+	// The SeccompProfile is the seccomp profile this pod should run under.


I'd appreciate a comment about the semantics wrt the per-container value and the per-pod value. Is it a total override? Additive? Subtractive?

I need to define that in the proposal, I want total override personally.

This should be a name right now to match the proposal, I do not want to specify these profiles inline -- it makes the policy much harder to write.

Will update!

On Monday, May 9, 2016, Paul Morie notifications@github.com wrote:

In pkg/api/types.go
#25324 (comment)
:

@@ -1362,6 +1362,8 @@ type PodSecurityContext struct {
// Use the host's ipc namespace.
// Optional: Default to false.
HostIPC bool json:"hostIPC,omitempty"

// The SeccompProfile is the seccomp profile this pod should run under.

This should be a name right now to match the proposal, I do not want to
specify these profiles inline -- it makes the policy much harder to write.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/kubernetes/kubernetes/pull/25324/files/ec54b7e13bbd79087fb34f0973f4482c0150c9c6#r62559109

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

thockin · 2016-05-09T03:34:39Z

I just scanned for API style, not full code review.

@erictune since this is VERY similar (and totally different impl) than the apparmor stuff we discussed.

eparis · 2016-05-09T13:30:13Z

ok to test

pmorie · 2016-05-23T18:15:25Z

@jfrazelle proposal is merged so you should be able to update with confidence from that now. We are trying to get LGTM on all 1.3 features today.

jessfraz · 2016-05-23T23:12:41Z

Will do as sson as done with work! sorry!

On Mon, May 23, 2016 at 11:40 AM, Paul Morie notifications@github.com
wrote:

@jfrazelle https://github.com/jfrazelle proposal is merged so you
should be able to update with confidence from that now. We are trying to
get LGTM on all 1.3 features today.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25324 (comment)

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

pmorie · 2016-05-24T00:44:30Z

pkg/kubelet/dockertools/manager.go

@@ -949,18 +946,33 @@ func (dm *DockerManager) checkVersionCompatibility() error {
 	return nil
 }

-func (dm *DockerManager) getDefaultSecurityOpt() ([]string, error) {
+func (dm *DockerManager) getSecurityOpt(container *api.Container) ([]string, error) {


Look for the annotation on the pod for this container (needs to be changed to accept the pod)

The precedent should be:

container-specific annotation, if any

pod-level annotation, if any

default (unconfined?)

jessfraz · 2016-05-24T03:46:00Z

updated and added tests

pmorie · 2016-05-24T04:26:54Z

@jfrazelle Looks like you need to run hack/update-all.sh. Can we get an E2E in a follow-up?

jessfraz · 2016-05-24T04:28:02Z

ah my b

pmorie · 2016-05-24T05:16:06Z

LGTM.

Let's do E2E coverage in a follow-up. Appreciate the effort getting this in for 1.3!

jessfraz · 2016-05-24T15:51:24Z

Did I miss another generation is that why the test is failing?

ncdc · 2016-05-24T17:00:28Z

@jfrazelle more generation/verification fun:

Verifying ./hack/../hack/verify-flags-underscore.py
Found flags in golang files not in the list of known flags. Please add these to hack/verify-flags/known-flags.txt
seccomp-profile-root
FAILED   ./hack/../hack/verify-flags-underscore.py  8s

jessfraz · 2016-05-24T17:15:42Z

ah got it

On Tue, May 24, 2016 at 10:05 AM, Andy Goldstein notifications@github.com
wrote:

@jfrazelle https://github.com/jfrazelle more generation/verification
fun:

Verifying ./hack/../hack/verify-flags-underscore.py
Found flags in golang files not in the list of known flags. Please add these to hack/verify-flags/known-flags.txt
seccomp-profile-root
FAILED ./hack/../hack/verify-flags-underscore.py 8s

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25324 (comment)

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

ncdc · 2016-05-24T18:26:14Z

@jfrazelle squash please?

Signed-off-by: Jess Frazelle <me@jessfraz.com>

jessfraz · 2016-05-24T19:25:36Z

done

pmorie · 2016-05-24T20:19:36Z

LGTM, thanks!

jessfraz · 2016-05-25T15:19:43Z

Nooooooooooooooo :(

pweil- · 2016-05-25T15:35:52Z

ERROR: (gcloud.compute.networks.create) Some requests did not succeed:
 - Quota 'NETWORKS' exceeded. Limit: 75.0

Hmm, looks like you hit this: #26270

eparis · 2016-05-25T15:44:56Z

@k8s-bot e2e test this flake: #26270

ncdc · 2016-05-26T13:27:54Z

@k8s-bot e2e test this issue: #IGNORE
(seems like something quite bad happened in the test run https://console.cloud.google.com/storage/browser/kubernetes-jenkins/pr-logs/pull/25324/kubernetes-pull-build-test-e2e-gce/41970/)

will try to investigate if time permits

k8s-bot · 2016-05-26T14:19:38Z

GCE e2e build/test passed for commit aa8c72a.

jessfraz · 2016-05-26T17:53:08Z

Yay! thanks!

On Thu, May 26, 2016 at 10:51 AM, Alex Mohr notifications@github.com
wrote:

Merged #25324 #25324.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#25324 (comment)

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

sttts · 2016-06-08T10:20:45Z

pkg/kubelet/dockertools/manager.go

+		return nil, err
+	}
+
+	return []string{fmt.Sprintf("seccomp=%s", b.Bytes())}, nil


Should this really be the compacted file content or the filename itself? Compare https://docs.docker.com/engine/security/seccomp/

Yes it needs to be the file contents. The docker cli takes a file name,
reads its contents, and transmits the contents to the daemon.

On Wednesday, June 8, 2016, Dr. Stefan Schimanski notifications@github.com
wrote:

In pkg/kubelet/dockertools/manager.go
#25324 (comment)
:

if !strings.HasPrefix(profile, "localhost") {

return nil, fmt.Errorf("unknown seccomp profile option: %s", profile)

}

file, err := ioutil.ReadFile(filepath.Join(dm.seccompProfileRoot, strings.TrimPrefix(profile, "localhost/")))

if err != nil {

return nil, err

}

b := bytes.NewBuffer(nil)

if err := json.Compact(b, file); err != nil {

return nil, err

}

return []string{fmt.Sprintf("seccomp=%s", b.Bytes())}, nil

Should this really be the compacted file content or the filename itself?
Compare https://docs.docker.com/engine/security/seccomp/

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/25324/files/aa8c72adaaab6cba51ff868516ac5b423a95f069#r66230042,
or mute the thread
https://github.com/notifications/unsubscribe/AAABYoMSK4m547F8EFFS3otQ56-RjCKAks5qJpe1gaJpZM4IZwJt
.

googlebot added the cla: yes label May 8, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels May 8, 2016

thockin assigned pmorie May 9, 2016

thockin reviewed May 9, 2016
View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2016

pmorie reviewed May 24, 2016
View reviewed changes

jessfraz force-pushed the add-seccomp branch from ec54b7e to a623537 Compare May 24, 2016 03:45

jessfraz changed the title ~~WIP: Add Seccomp API~~ Add Seccomp to Annotations May 24, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2016

pmorie added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label May 24, 2016

k8s-github-robot removed the release-note-label-needed label May 24, 2016

jessfraz force-pushed the add-seccomp branch from a623537 to a241903 Compare May 24, 2016 05:02

pmorie added this to the v1.3 milestone May 24, 2016

jessfraz force-pushed the add-seccomp branch from 97f3060 to 1629674 Compare May 24, 2016 17:15

k8s-github-robot added the needs-ok-to-merge label May 24, 2016

ncdc added the ok-to-merge label May 24, 2016

k8s-github-robot removed the needs-ok-to-merge label May 24, 2016

seccomp: add annotations and test for docker runtime

aa8c72a

Signed-off-by: Jess Frazelle <me@jessfraz.com>

jessfraz force-pushed the add-seccomp branch from 1629674 to aa8c72a Compare May 24, 2016 19:25

pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016

pmorie mentioned this pull request May 25, 2016

Seccomp umbrella issue #24611

Closed

2 tasks

alex-mohr merged commit 4357b8a into kubernetes:master May 26, 2016

mdshuai mentioned this pull request Jun 1, 2016

Seccomp annotations name format need redefine #26610

Closed

sttts reviewed Jun 8, 2016
View reviewed changes

jessfraz deleted the add-seccomp branch July 6, 2016 22:46

pweil- mentioned this pull request Oct 25, 2016

Seccomp kubernetes/enhancements#135

Closed

22 tasks

feiskyer mentioned this pull request Dec 22, 2016

Proposal: spec the seccomp profile format #39128

Closed

knabben mentioned this pull request May 18, 2020

Mark Kubelet --seccomp-profile-root deprecated #91182

Merged

pjbgf mentioned this pull request Aug 20, 2021

Add note about deprecated seccomp annotation kubernetes/website#29433

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Seccomp to Annotations #25324

Add Seccomp to Annotations #25324

jessfraz commented May 8, 2016

derekwaynecarr commented May 9, 2016

thockin May 9, 2016

pmorie May 9, 2016

pmorie May 9, 2016

jessfraz May 12, 2016

thockin commented May 9, 2016

eparis commented May 9, 2016

pmorie commented May 23, 2016

jessfraz commented May 23, 2016

pmorie May 24, 2016

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 24, 2016

ncdc commented May 24, 2016

jessfraz commented May 24, 2016

ncdc commented May 24, 2016

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 25, 2016

pweil- commented May 25, 2016

eparis commented May 25, 2016

ncdc commented May 26, 2016 •

edited

Loading

k8s-bot commented May 26, 2016

jessfraz commented May 26, 2016

sttts Jun 8, 2016

ncdc Jun 8, 2016

Add Seccomp to Annotations #25324

Add Seccomp to Annotations #25324

Conversation

jessfraz commented May 8, 2016

derekwaynecarr commented May 9, 2016

thockin May 9, 2016

Choose a reason for hiding this comment

pmorie May 9, 2016

Choose a reason for hiding this comment

pmorie May 9, 2016

Choose a reason for hiding this comment

jessfraz May 12, 2016

Choose a reason for hiding this comment

thockin commented May 9, 2016

eparis commented May 9, 2016

pmorie commented May 23, 2016

jessfraz commented May 23, 2016

pmorie May 24, 2016

Choose a reason for hiding this comment

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 24, 2016

ncdc commented May 24, 2016

jessfraz commented May 24, 2016

ncdc commented May 24, 2016

jessfraz commented May 24, 2016

pmorie commented May 24, 2016

jessfraz commented May 25, 2016

pweil- commented May 25, 2016

eparis commented May 25, 2016

ncdc commented May 26, 2016 • edited Loading

k8s-bot commented May 26, 2016

jessfraz commented May 26, 2016

sttts Jun 8, 2016

Choose a reason for hiding this comment

ncdc Jun 8, 2016

Choose a reason for hiding this comment

ncdc commented May 26, 2016 •

edited

Loading