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 containers#16925.

Fixes: 5a2405a ("Don't mount /dev/tty* inside privileged...")
Signed-off-by: Martin Roukala (né Peres) <[email protected]>

pkg/util/utils_linux.go

@@ -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)

test/system/030-run.bats

@@ -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