Skip to content

Commit

Permalink
Merge pull request #17055 from mupuf/mount-non-vt-tty
Browse files Browse the repository at this point in the history
Only prevent VTs to be mounted inside privileged systemd containers
  • Loading branch information
openshift-merge-robot authored Jan 12, 2023
2 parents 39ffcb8 + f4c81b0 commit a7ba63d
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 7 deletions.
19 changes: 18 additions & 1 deletion pkg/util/utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io/fs"
"os"
"path"
"path/filepath"
"strings"
"syscall"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
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 a7ba63d

Please sign in to comment.