From 1dae66f74871e63325ef0293ef3d808bc888610e Mon Sep 17 00:00:00 2001 From: Rodrigo Campos Date: Fri, 26 Jan 2024 17:13:39 +0100 Subject: [PATCH] libct/dmz: Require RUNC_DMZ=true to opt-in If it is compiled, the user needs to opt-in with this env variable to use it. While we are there, remove the RUNC_DMZ=legacy as that is now the default. Signed-off-by: Rodrigo Campos --- README.md | 2 +- libcontainer/dmz/dmz_linux.go | 16 +++++++++++++--- tests/integration/exec.bats | 6 +++--- tests/integration/run.bats | 20 ++++++++++---------- tests/integration/selinux.bats | 15 +++++++-------- 5 files changed, 34 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 7e40776bb..df5061a37 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ make BUILDTAGS="" | Build Tag | Feature | Enabled by Default | Dependencies | |---------------|---------------------------------------|--------------------|---------------------| | `seccomp` | Syscall filtering using `libseccomp`. | yes | `libseccomp` | -| `!runc_nodmz` | Reduce memory usage for CVE-2019-5736 protection by using a small C binary, [see `memfd-bind` for more details][contrib-memfd-bind]. `runc_nodmz` disables this feature and causes runc to use a different protection mechanism which will further increases memory usage temporarily during container startup. This feature can also be disabled at runtime by setting the `RUNC_DMZ=legacy` environment variable. | yes || +| `!runc_nodmz` | Reduce memory usage for CVE-2019-5736 protection by using a small C binary, [see `memfd-bind` for more details][contrib-memfd-bind]. `runc_nodmz` disables this **experimental feature** and causes runc to use a different protection mechanism which will further increases memory usage temporarily during container startup. To enable this feature you also need to set the `RUNC_DMZ=true` environment variable. | yes || | `runc_dmz_selinux_nocompat` | Disables a SELinux DMZ workaround (new distros should set this). See [dmz README] for details. | no || The following build tags were used earlier, but are now obsoleted: diff --git a/libcontainer/dmz/dmz_linux.go b/libcontainer/dmz/dmz_linux.go index 0985677ea..eddbbc701 100644 --- a/libcontainer/dmz/dmz_linux.go +++ b/libcontainer/dmz/dmz_linux.go @@ -7,7 +7,9 @@ import ( "bytes" "debug/elf" "embed" + "fmt" "os" + "strconv" "sync" "github.com/sirupsen/logrus" @@ -43,11 +45,19 @@ var ( // If the runc-dmz binary is not embedded into the runc binary, Binary will // return ErrNoDmzBinary as the error. func Binary(tmpDir string) (*os.File, error) { - // Setting RUNC_DMZ=legacy disables this dmz method. - if os.Getenv("RUNC_DMZ") == "legacy" { - logrus.Debugf("RUNC_DMZ=legacy set -- switching back to classic /proc/self/exe cloning") + // Only RUNC_DMZ=true enables runc_dmz. + runcDmz := os.Getenv("RUNC_DMZ") + if runcDmz == "" { + logrus.Debugf("RUNC_DMZ is not set -- switching back to classic /proc/self/exe cloning") return nil, ErrNoDmzBinary } + if dmzEnabled, err := strconv.ParseBool(runcDmz); err == nil && !dmzEnabled { + logrus.Debugf("RUNC_DMZ is false -- switching back to classic /proc/self/exe cloning") + return nil, ErrNoDmzBinary + } else if err != nil { + return nil, fmt.Errorf("parsing RUNC_DMZ: %w", err) + } + runcDmzBinaryOnce.Do(func() { runcDmzBinary, _ = runcDmzFs.ReadFile("binary/runc-dmz") // Verify that our embedded binary has a standard ELF header. diff --git a/tests/integration/exec.bats b/tests/integration/exec.bats index 280907048..a92a23780 100644 --- a/tests/integration/exec.bats +++ b/tests/integration/exec.bats @@ -323,14 +323,14 @@ function check_exec_debug() { [ "$status" -eq 0 ] } -@test "RUNC_DMZ=legacy runc exec [execve error]" { +@test "runc exec [execve error]" { cat <rootfs/run.sh #!/mmnnttbb foo bar sh EOF chmod +x rootfs/run.sh - RUNC_DMZ=legacy runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox - RUNC_DMZ=legacy runc exec -t test_busybox /run.sh + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + runc exec -t test_busybox /run.sh [ "$status" -ne 0 ] # After the sync socket closed, we should not send error to parent diff --git a/tests/integration/run.bats b/tests/integration/run.bats index 82c580277..b49f3ac4c 100644 --- a/tests/integration/run.bats +++ b/tests/integration/run.bats @@ -127,20 +127,20 @@ function teardown() { [ "${lines[0]}" = "410" ] } -@test "runc run [runc-dmz]" { - runc --debug run test_hello +@test "RUNC_DMZ=true runc run [runc-dmz]" { + RUNC_DMZ=true runc --debug run test_hello [ "$status" -eq 0 ] [[ "$output" = *"Hello World"* ]] # We use runc-dmz if we can. [[ "$output" = *"runc-dmz: using runc-dmz"* ]] } -@test "runc run [cap_sys_ptrace -> /proc/self/exe clone]" { +@test "RUNC_DMZ=true runc run [cap_sys_ptrace -> /proc/self/exe clone]" { # Add CAP_SYS_PTRACE to the bounding set, the minimum needed to indicate a # container process _could_ get CAP_SYS_PTRACE. update_config '.process.capabilities.bounding += ["CAP_SYS_PTRACE"]' - runc --debug run test_hello + RUNC_DMZ=true runc --debug run test_hello [ "$status" -eq 0 ] [[ "$output" = *"Hello World"* ]] if [ "$EUID" -ne 0 ] && is_kernel_gte 4.10; then @@ -154,8 +154,8 @@ function teardown() { fi } -@test "RUNC_DMZ=legacy runc run [/proc/self/exe clone]" { - RUNC_DMZ=legacy runc --debug run test_hello +@test "runc run [/proc/self/exe clone]" { + runc --debug run test_hello [ "$status" -eq 0 ] [[ "$output" = *"Hello World"* ]] [[ "$output" = *"runc-dmz: using /proc/self/exe clone"* ]] @@ -231,14 +231,14 @@ function teardown() { grep -E '^boottime\s+1337\s+3141519$' <<<"$output" } -@test "runc run [exec error]" { +@test "RUNC_DMZ=true runc run [exec error]" { cat <rootfs/run.sh #!/mmnnttbb foo bar sh EOF chmod +x rootfs/run.sh update_config '.process.args = [ "/run.sh" ]' - runc run test_hello + RUNC_DMZ=true runc run test_hello # Ensure that the output contains the right error message. For runc-dmz, both # nolibc and libc have the same formatting string (but libc will print the @@ -248,14 +248,14 @@ EOF [[ "$output" = *"exec /run.sh: "* ]] } -@test "RUNC_DMZ=legacy runc run [execve error]" { +@test "runc run [execve error]" { cat <rootfs/run.sh #!/mmnnttbb foo bar sh EOF chmod +x rootfs/run.sh update_config '.process.args = [ "/run.sh" ]' - RUNC_DMZ=legacy runc run test_hello + runc run test_hello [ "$status" -ne 0 ] # After the sync socket closed, we should not send error to parent diff --git a/tests/integration/selinux.bats b/tests/integration/selinux.bats index 6265bd461..19bc9d207 100644 --- a/tests/integration/selinux.bats +++ b/tests/integration/selinux.bats @@ -39,17 +39,16 @@ function teardown() { } # https://github.com/opencontainers/runc/issues/4057 +@test "runc run (custom selinux label, RUNC_DMZ=true)" { + update_config ' .process.selinuxLabel |= "system_u:system_r:container_t:s0:c4,c5" + | .process.args = ["/bin/true"]' + RUNC_DMZ=true runc run tst + [ "$status" -eq 0 ] +} + @test "runc run (custom selinux label)" { update_config ' .process.selinuxLabel |= "system_u:system_r:container_t:s0:c4,c5" | .process.args = ["/bin/true"]' runc run tst [ "$status" -eq 0 ] } - -@test "runc run (custom selinux label, RUNC_DMZ=legacy)" { - export RUNC_DMZ=legacy - update_config ' .process.selinuxLabel |= "system_u:system_r:container_t:s0:c4,c5" - | .process.args = ["/bin/true"]' - runc run tst - [ "$status" -eq 0 ] -}