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

seccomp: set SPEC_ALLOW by default #3581

Closed
wants to merge 3 commits into from

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Aug 30, 2022

If no seccomps flags are set in OCI runtime spec (not even the empty
set), set SPEC_ALLOW by default. Otherwise, use the flags set.

This mimics the crun behavior, and makes runc seccomp performance on par
with crun.

The test case added in #3580 checks all the cases:

  • no flags (this is when we set SPEC_ALLOW)
  • empty set of flags (this is when we don't set SPEC_ALLOW)
  • specific flags set

This currently includes #3580, thus a draft pending its merge.

Commit 58ea21d added support for seccomp flags such as
SPEC_ALLOW, but it does not work as expected, because since commit
7a8d716 we do not use libseccomp-golang's Load(), but
handle flags separately in patchbfp.

This fixes setting SPEC_ALLOW flag.

Add a comment to not forget to amend filterFlags when adding new flags.

Signed-off-by: Kir Kolyshkin <[email protected]>
Add a debug print of seccomp flags value, so the test can check
those (without using something like strace, that is).

Amend the flags setting test with the numeric values expected, and the
logic to check those.

Signed-off-by: Kir Kolyshkin <[email protected]>
If no seccomps flags are set in OCI runtime spec (not even the empty
set), set SPEC_ALLOW by default. Otherwise, use the flags set.

This mimics the crun behavior, and makes runc seccomp performance on par
with crun.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Sep 1, 2022

CentOS 7 failed with:

time="2022-08-31T02:39:05Z" level=error msg="runc run failed: unable to start container process: unable to init seccomp: error adding SSB flag to seccomp filter: SetSSB requires libseccomp >= 2.5.0 and API level >= 4 (current version: 2.3.1, API level: 0)"

OK, we should check if the flag is supported by the running kernel/libseccomp before adding it.

@kolyshkin
Copy link
Contributor Author

Closing in favor of #3588 (which does the same thing and more).

@kolyshkin
Copy link
Contributor Author

Did I say close?

@kolyshkin kolyshkin closed this Sep 2, 2022
@kolyshkin kolyshkin deleted the seccomp-ssb-default branch September 2, 2022 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant