Skip to content

Commit

Permalink
Only prevent VTs to be mounted inside privileged systemd containers
Browse files Browse the repository at this point in the history
While mounting virtual terminal 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)

Closes containers#16925.

Fixes: 5a2405a ("Don't mount /dev/tty* inside privileged...")
Signed-off-by: Martin Roukala (né Peres) <martin.roukala@mupuf.org>
  • Loading branch information
mupuf committed Jan 10, 2023
1 parent 5b9e068 commit b856b64
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
4 changes: 3 additions & 1 deletion pkg/util/utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/fs"
"os"
"path/filepath"
"regexp"
"strings"
"syscall"

Expand All @@ -20,6 +21,7 @@ import (

var (
errNotADevice = errors.New("not a device node")
regexVirtualTerminalDevice = regexp.MustCompile(`^/dev/tty[\d]+$`)
)

// GetContainerPidInformationDescriptors returns a string slice of all supported
Expand Down Expand Up @@ -104,7 +106,7 @@ func AddPrivilegedDevices(g *generate.Generator, systemdMode bool) error {
}
} else {
for _, d := range hostDevices {
if systemdMode && strings.HasPrefix(d.Path, "/dev/tty") {
if systemdMode && regexVirtualTerminalDevice.MatchString(d.Path) {
continue
}
g.AddDevice(d)
Expand Down
35 changes: 29 additions & 6 deletions test/system/030-run.bats
Original file line number Diff line number Diff line change
Expand Up @@ -928,29 +928,52 @@ $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

# Actual test for 15895: with --systemd, no ttyN devices are passed through
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
Expand Down

0 comments on commit b856b64

Please sign in to comment.