Match VT device paths to be blocked from mounting exactly #17265
Conversation
As @mheon pointed out in PR containers#17055[^1], isVirtualConsoleDevice() does not only matches VT device paths but also devices named like /dev/tty0abcd. This causes that non VT device paths named /dev/tty[0-9]+[A-Za-z]+ are not mounted into privileged container and systemd containers accidentally. This is an unlikely issue because the Linux kernel does not use device paths like that. To make it failproof and prevent issues in unlikely scenarios, change isVirtualConsoleDevice() to exactly match ^/dev/tty[0-9]+$ paths. Because it is not possible to match this path exactly with Glob syntax, the path is now checked with strings.TrimPrefix() and strconv.ParseUint(). ParseUint uses a bitsize of 16, this is sufficient because the max number of TTY devices is 512 in Linux 6.1.5. (Checked via 'git grep -e '#define' --and -e 'TTY_MINORS'). The commit also adds a unit-test for isVirtualConsoleDevice(). Fixes: f4c81b0 ("Only prevent VTs to be mounted inside...") [^1]: containers#17055 (comment) Signed-off-by: Fabian Holler <mail@fholler.de> Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
openshift-ci
bot
added
do-not-merge/release-note-label-needed
|
Just rebasing #17104 to attempt to get it to pass CI so we can get it into 4.4 |
openshift-ci
bot
added
release-note-none
and removed
do-not-merge/release-note-label-needed
|
@containers/podman-maintainers PTAL |
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick v4.4 |
|
@vrothberg: new pull request created: #17272 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
github-actions
bot
added
the
locked - please file new issue/PR
As @mheon pointed out in PR #170551, isVirtualConsoleDevice() does not only matches VT device paths but also devices named like /dev/tty0abcd.
This causes that non VT device paths named /dev/tty[0-9]+[A-Za-z]+ are not mounted into privileged container and systemd containers accidentally.
This is an unlikely issue because the Linux kernel does not use device paths like that.
To make it failproof and prevent issues in unlikely scenarios, change isVirtualConsoleDevice() to exactly match ^/dev/tty[0-9]+$ paths.
Because it is not possible to match this path exactly with Glob syntax, the path is now checked with strings.TrimPrefix() and strconv.ParseUint().
ParseUint uses a bitsize of 16, this is sufficient because the max number of TTY devices is 512 in Linux 6.1.5.
(Checked via 'git grep -e '#define' --and -e 'TTY_MINORS').
The commit also adds a unit-test for isVirtualConsoleDevice().
Fixes: f4c81b0 ("Only prevent VTs to be mounted inside...")
Signed-off-by: Fabian Holler mail@fholler.de
Signed-off-by: Daniel J Walsh dwalsh@redhat.com
Does this PR introduce a user-facing change?
Footnotes
https://github.com/containers/podman/pull/17055#issuecomment-1378904068 ↩