[4.11] add support for seccomp flags #57
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
This is to include Linux seccomp flags. Identical to upstream commit c152e83. Signed-off-by: Kir Kolyshkin <[email protected]>
List of seccomp flags defined in runtime-spec: * SECCOMP_FILTER_FLAG_TSYNC * SECCOMP_FILTER_FLAG_LOG * SECCOMP_FILTER_FLAG_SPEC_ALLOW Note that runc does not apply SECCOMP_FILTER_FLAG_TSYNC. It does not make sense to apply the seccomp filter on only one thread; other threads will be terminated after exec anyway. See similar commit in crun: containers/crun@fefabff Note that SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV (introduced by https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?id=c2aa2dfef243 in Linux 5.19-rc1) is not added yet because Linux 5.19 is not released yet. Signed-off-by: Alban Crequy <[email protected]> (cherry picked from commit 58ea21d) Signed-off-by: Kir Kolyshkin <[email protected]>
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]> (cherry picked from commit c7dc8b1) Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
4.11-seccomp-ssb
branch
from
b44b590 to
80e31ac
Compare
|
LGTM, once the upstream PR is approved |
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]> (cherry picked from commit 26dc55e) Signed-off-by: Kir Kolyshkin <[email protected]>
|
Refreshed, added backport of opencontainers/runc#3581 |
kolyshkin
force-pushed
the
4.11-seccomp-ssb
branch
from
32196c8 to
0a43c9d
Compare
1. This valid warning is reported by shellcheck v0.8.0:
In tests/integration/helpers.bash line 38:
KERNEL_MINOR="${KERNEL_VERSION#$KERNEL_MAJOR.}"
^-----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
Did you mean:
KERNEL_MINOR="${KERNEL_VERSION#"$KERNEL_MAJOR".}"
Fix this.
2. These (invalid) warnings are also reported by the new version:
In tests/integration/events.bats line 13:
@test "events --stats" {
^-- SC2030 (info): Modification of status is local (to subshell caused by @BATS test).
In tests/integration/events.bats line 41:
[ "$status" -eq 0 ]
^-----^ SC2031 (info): status was modified in a subshell. That change might be lost.
Basically, this is happening because shellcheck do not really track
the call tree and/or local variables. This is a known (and reported)
deficiency, and the alternative to disabling these warnings is moving
the code around, which is worse due to more changes in git history.
So we have to silence/disable these.
3. Update shellcheck to 0.8.0.
Signed-off-by: Kir Kolyshkin <[email protected]>
(cherry picked from commit be00ae0)
Signed-off-by: Kir Kolyshkin <[email protected]>
(cherry picked from commit 631343689d08dd7d4d4ba79027af9a1b8e93184f)
Signed-off-by: Kir Kolyshkin <[email protected]>
Fix a few copy-paste errors. Fixes: 520702d Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit e45f75ff654ec51dad8c71c7cd2b0dd2220c31bd) Signed-off-by: Kir Kolyshkin <[email protected]>
Amend runc features to print seccomp flags. Two set of flags are added: * known flags are those that this version of runc is aware of; * supported flags are those that can be set; normally, this is the same set as known flags, but due to older version of kernel and/or libseccomp, some known flags might be unsupported. This commit also consolidates three different switch statements dealing with flags into one, in func setFlag. A note is added to this function telling what else to look for when adding new flags. Unfortunately, it also adds a list of known flags, that should be kept in sync with the switch statement. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit cb15546f50c04f375d30bde87be77a8fd3b73e72) 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 as the default (if it's supported). Otherwise, use the flags as they are set (that includes no flags for empty seccomp.Flags array). This mimics the crun behavior, and makes runc seccomp performance on par with crun. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit c162ecc3a1dc314ae78797c83b3adac7bb6f0374) Signed-off-by: Kir Kolyshkin <[email protected]>
This test (initially added by commit 58ea21d and later amended in commit 26dc55e) currently has two major deficiencies: 1. All possible flag combinations, and their respective numeric values, have to be explicitly listed. Currently we support 3 flags, so there is only 2^3 - 1 = 7 combinations, but adding more flags will become increasingly difficult (for example, 5 flags will result in 31 combinations). 2. The test requires kernel 4.17 (for SECCOMP_FILTER_FLAG_SPEC_ALLOW), and not doing any tests when running on an older kernel. This, too, will make it more difficult to add extra flags in the future. Both issues can be solved by using runc features which now prints all known and supported runc flags. We still have to hardcode the numeric values of all flags, but most of the other work is coded now. In particular: * The test only uses supported flags, meaning it can be used with older kernels, removing the limitation (2) above. * The test calculates the powerset (all possible combinations) of flags and their numeric values. This makes it easier to add more flags, removing the limitation (1) above. * The test will fail (in flags_value) if any new flags will be added to runc but the test itself is not amended. Signed-off-by: Kir Kolyshkin <[email protected]> (cherry picked from commit c7f672428d810c0428b53d76903d0fdc4f6f6c9c) Signed-off-by: Kir Kolyshkin <[email protected]>
kolyshkin
force-pushed
the
4.11-seccomp-ssb
branch
from
0a43c9d to
1c20848
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of upstream PRs:
seccomp: set SPEC_ALLOW by default opencontainers/runc#3581to rhaos-4.11 branch (which is a copy of release-1.1 branch from upstream, up to and including v1.1.4)