seccomp: change empty profile behavior #3342

benclarkwood · 2020-03-02T01:17:22Z

If seccomp is enabled, but the profile is empty, fall back to the default seccomp profile rather than unconfined.

Fix for #3079

If seccomp is enabled, but the profile is empty, fall back to the default seccomp profile rather than unconfined. Signed-off-by: Ben Wood <ben@benclarkwood.com>

openshift-ci-robot · 2020-03-02T01:17:36Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: benclarkwood
To complete the pull request process, please assign saschagrunert
You can assign the PR to them by writing /assign @saschagrunert 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

openshift-ci-robot · 2020-03-02T01:17:36Z

Hi @benclarkwood. Thanks for your PR.

I'm waiting for a cri-o 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.

openshift-ci-robot · 2020-03-02T01:17:36Z

Hi @benclarkwood. Thanks for your PR.

I'm waiting for a cri-o 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.

codecov · 2020-03-02T01:24:28Z

Codecov Report

Merging #3342 (62eaf6c) into master (5996f36) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #3342   +/-   ##
=======================================
  Coverage   44.53%   44.53%           
=======================================
  Files          94       94           
  Lines        7731     7731           
=======================================
  Hits         3443     3443           
  Misses       3977     3977           
  Partials      311      311

haircommander · 2020-03-02T02:03:57Z

/ok-to-test

Thanks for the PR! I'm unsure this works with the CRI definition but let's see 😄

saschagrunert · 2020-03-02T07:55:48Z

/ok-to-test

Thanks for the PR! I'm unsure this works with the CRI definition but let's see

Do you have any resource that the CRI should behave like this?

saschagrunert · 2020-03-02T07:56:18Z

server/container_create.go

 	if s.Config().Seccomp().IsDisabled() {
+		if profile == "" {


The test in ctr_seccomp.bats is now failing as well, I think we have to change it too.

http://github.com/kubernetes/cri-api/blob/master/pkg/apis/runtimemptye/v1alpha2/api.proto#L282

server/container_create.go

rhatdan · 2020-03-07T13:30:59Z

/retest

rhatdan · 2020-03-22T19:11:44Z

@benclarkwood I think this might need a rebase.

JAORMX · 2020-04-09T17:22:51Z

Would be great to get this fixed.

JAORMX · 2020-04-09T17:29:12Z

@benclarkwood will you have time to get back to this? Or do you want help in fixing up the tests?

haircommander · 2020-04-09T18:41:07Z

based on the results of both critest runs:

/go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/security_context_linux.go:608 Expected     <string>: Seccomp:	2      to contain substring     <string>: 0 /go/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/security_context_linux.go:1120

it seems this is in violation of the cri. I'm not sure we can do anything here without having the cri changed

JAORMX · 2020-04-10T13:17:26Z

Would this enhancement help change the CRI default? kubernetes/enhancements#1148

haircommander · 2020-04-29T20:02:19Z

Would this enhancement help change the CRI default? kubernetes/enhancements#1148

it seems to be the first step. the proposal would only declare it GA and make it a first class pod spec value. Further work and proposals would be needed to have the CRI default to a profile when none is specified

openshift-ci-robot · 2020-06-22T19:13:05Z

@benclarkwood: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/critest_fedora	`62eaf6c`	link	`/test critest_fedora`
ci/openshift-jenkins/integration_fedora	`62eaf6c`	link	`/test integration_fedora`
ci/openshift-jenkins/integration_crun	`62eaf6c`	link	`/test integration_crun`
ci/openshift-jenkins/critest_rhel	`62eaf6c`	link	`/test critest_rhel`
ci/openshift-jenkins/integration_crun_cgroupv2	`62eaf6c`	link	`/test integration_cgroupv2`
ci/openshift-jenkins/integration_rhel	`62eaf6c`	link	`/test integration_rhel`
ci/prow/images	`62eaf6c`	link	`/test images`
ci/prow/launch-aws	`62eaf6c`	link	`/test launch-aws`
ci/prow/e2e-aws	`62eaf6c`	link	`/test e2e-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

openshift-merge-robot · 2020-11-18T16:12:47Z

@benclarkwood: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-agnostic	`62eaf6c`	link	`/test e2e-agnostic`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

haircommander · 2020-11-18T16:21:58Z

closing this, as we have #4212 now

seccomp: change empty profile behavior

62eaf6c

If seccomp is enabled, but the profile is empty, fall back to the default seccomp profile rather than unconfined. Signed-off-by: Ben Wood <ben@benclarkwood.com>

benclarkwood requested review from mrunalp and runcom as code owners March 2, 2020 01:17

openshift-ci-robot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Mar 2, 2020

openshift-ci-robot requested review from rhatdan and giuseppe March 2, 2020 01:17

openshift-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 2, 2020

openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 2, 2020

saschagrunert reviewed Mar 2, 2020

View reviewed changes

haircommander mentioned this pull request Apr 29, 2020

Unexpected behavior with empty seccomp profile #3079

Closed

haircommander closed this Nov 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

seccomp: change empty profile behavior #3342

seccomp: change empty profile behavior #3342

benclarkwood commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

codecov bot commented Mar 2, 2020 •

edited

haircommander commented Mar 2, 2020

saschagrunert commented Mar 2, 2020

saschagrunert Mar 2, 2020

mrunalp Mar 3, 2020

rhatdan commented Mar 7, 2020

rhatdan commented Mar 22, 2020

JAORMX commented Apr 9, 2020

JAORMX commented Apr 9, 2020

haircommander commented Apr 9, 2020

JAORMX commented Apr 10, 2020

haircommander commented Apr 29, 2020

openshift-ci-robot commented Jun 22, 2020

openshift-merge-robot commented Nov 18, 2020

haircommander commented Nov 18, 2020

seccomp: change empty profile behavior #3342

seccomp: change empty profile behavior #3342

Conversation

benclarkwood commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

openshift-ci-robot commented Mar 2, 2020

codecov bot commented Mar 2, 2020 • edited

Codecov Report

haircommander commented Mar 2, 2020

saschagrunert commented Mar 2, 2020

saschagrunert Mar 2, 2020

Choose a reason for hiding this comment

mrunalp Mar 3, 2020

Choose a reason for hiding this comment

rhatdan commented Mar 7, 2020

rhatdan commented Mar 22, 2020

JAORMX commented Apr 9, 2020

JAORMX commented Apr 9, 2020

haircommander commented Apr 9, 2020

JAORMX commented Apr 10, 2020

haircommander commented Apr 29, 2020

openshift-ci-robot commented Jun 22, 2020

openshift-merge-robot commented Nov 18, 2020

haircommander commented Nov 18, 2020

codecov bot commented Mar 2, 2020 •

edited