Only prevent VTs to be mounted inside privileged systemd containers

While mounting virtual console devices in a systemd container is a
recipe for disaster (I experienced it first hand), mounting serial
console devices, modems, and others should still be done by default
for privileged systemd-based containers.

v2, addressing the review from @fho:
 - use backticks in the regular expression to remove backslashes
 - pre-compile the regex at the package level
 - drop IsVirtualTerminalDevice (not needed for a one-liner)

v3, addressing the review from @fho and @rhatdan:
 - re-introduce a private function for matching the device names
 - use path.Match rather than a regex not to slow down startup time

Closes #16925.

Fixes: 5a2405ae1b3a ("Don't mount /dev/tty* inside privileged...")
Signed-off-by: Martin Roukala (né Peres) <martin.roukala@mupuf.org>
This commit is contained in:
Martin Roukala (né Peres)
2023-01-10 12:10:13 +02:00
parent 382c55eeaa
commit f4c81b0aa5
2 changed files with 47 additions and 7 deletions

View File

@ -5,6 +5,7 @@ import (
"fmt"
"io/fs"
"os"
"path"
"path/filepath"
"strings"
"syscall"
@ -70,6 +71,22 @@ func FindDeviceNodes() (map[string]string, error) {
return nodes, nil
}
func isVirtualConsoleDevice(device string) bool {
/*
Virtual consoles are of the form `/dev/tty\d+`, any other device such as
/dev/tty, ttyUSB0, or ttyACM0 should not be matched.
See `man 4 console` for more information.
NOTE: Matching is done using path.Match even though a regular expression
would have been more accurate. This is because a regular
expression would have required pre-compilation, which would have
increase the startup time needlessly or made the code more complex
than needed.
*/
matched, _ := path.Match("/dev/tty[0-9]*", device)
return matched
}
func AddPrivilegedDevices(g *generate.Generator, systemdMode bool) error {
hostDevices, err := getDevices("/dev")
if err != nil {
@ -104,7 +121,7 @@ func AddPrivilegedDevices(g *generate.Generator, systemdMode bool) error {
}
} else {
for _, d := range hostDevices {
if systemdMode && strings.HasPrefix(d.Path, "/dev/tty") {
if systemdMode && isVirtualConsoleDevice(d.Path) {
continue
}
g.AddDevice(d)

View File

@ -928,15 +928,16 @@ $IMAGE--c_ok" \
# ('skip' would be nicer in some sense... but could hide a regression.
# Fedora, RHEL, Debian, Ubuntu, Gentoo, all have /dev/ttyN, so if
# this ever triggers, it means a real problem we should know about.)
assert "$(ls /dev/tty* | grep -vx /dev/tty)" != "" \
vt_tty_devices_count=$(find /dev -regex '/dev/tty[0-9].*' | wc -w)
assert "$vt_tty_devices_count" != "0" \
"Expected at least one /dev/ttyN device on host"
# Ok now confirm that without --systemd, podman exposes ttyNN devices
run_podman run --rm -d --privileged $IMAGE ./pause
cid="$output"
run_podman exec $cid sh -c 'ls /dev/tty*'
assert "$output" != "/dev/tty" \
run_podman exec $cid sh -c "find /dev -regex '/dev/tty[0-9].*' | wc -w"
assert "$output" = "$vt_tty_devices_count" \
"ls /dev/tty* without systemd; should have lots of ttyN devices"
run_podman stop -t 0 $cid
@ -944,13 +945,35 @@ $IMAGE--c_ok" \
run_podman run --rm -d --privileged --systemd=always $IMAGE ./pause
cid="$output"
run_podman exec $cid sh -c 'ls /dev/tty*'
assert "$output" = "/dev/tty" \
"ls /dev/tty* with --systemd=always: should have no ttyN devices"
run_podman exec $cid sh -c "find /dev -regex '/dev/tty[0-9].*' | wc -w"
assert "$output" = "0" \
"ls /dev/tty[0-9] with --systemd=always: should have no ttyN devices"
run_podman stop -t 0 $cid
}
# 16925: --privileged + --systemd = share non-virtual-terminal TTYs
@test "podman run --privileged as root with systemd mounts non-vt /dev/tty devices" {
skip_if_rootless "this test only makes sense as root"
# First, confirm that we _have_ non-virtual terminal /dev/tty* devices on
# the host.
non_vt_tty_devices_count=$(find /dev -regex '/dev/tty[^0-9].*' | wc -w)
if [ "$non_vt_tty_devices_count" -eq 0 ]; then
skip "The server does not have non-vt TTY devices"
fi
# Verify that all the non-vt TTY devices got mounted in the container
run_podman run --rm -d --privileged --systemd=always $IMAGE ./pause
cid="$output"
run_podman '?' exec $cid find /dev -regex '/dev/tty[^0-9].*'
assert "$status" = 0 \
"No non-virtual-terminal TTY devices got mounted in the container"
assert "$(echo "$output" | wc -w)" = "$non_vt_tty_devices_count" \
"Some non-virtual-terminal TTY devices are missing in the container"
run_podman stop -t 0 $cid
}
@test "podman run read-only from containers.conf" {
containersconf=$PODMAN_TMPDIR/containers.conf
cat >$containersconf <<EOF