jail: fix -j user namespace join, deferred userns, masks and sysfs - #46
Open
joshuacov1 wants to merge 9 commits into
Open
joshuacov1 wants to merge 9 commits into
joshuacov1 wants to merge 9 commits into
Conversation
userns_wait_idmaps() remounts / MS_REC|MS_PRIVATE after the user namespace exists. On the clone() path the mount namespace is owned by that userns and the call succeeds. On the deferred path the mount namespace is still owned by the initial userns, so after unshare(CLONE_NEWUSER) the call returns EPERM and every jail joining a namespace by path fails with "private mount failed". isolate_mountns_and_detach_inherited() already makes the tree private on the deferred path while privileged. Move the remount to the clone() call site in exec_jail(). Signed-off-by: Joshua Covington <joshuacov@gmail.com>
remount_proc_sys_after_unshare() and remount_readonly_now() run after
the deferred unshare(CLONE_NEWNS), in a mount namespace that does not
own the mounts copied into it. Those mounts carry
MNT_LOCK_{NOSUID,NODEV,NOEXEC,ATIME}; a remount passing only MS_RDONLY
asks to clear them and fails with EPERM. /proc/sys stayed writable in
every deferred-userns jail, and an OCI readonlyPath would as well.
do_mount() already ORs in the flags read back from mountinfo for phase
1. Add bind_remount_readonly() doing the same and use it in both
phase-2 helpers.
Signed-off-by: Joshua Covington <joshuacov@gmail.com>
The file masks (/proc/kcore, /proc/sysrq-trigger, OCI maskedPaths) bind the noafile and remount it with a hard-coded MS_RELATIME. Under a userns from clone() the inherited mounts are MNT_LOCK_ATIME, the bind inherits that, and changing the atime mode is EPERM. procd mounts /tmp with MS_NOATIME, so on OpenWrt the remount fails for every -f -p jail: critical masks make mount_all() fail without a message, optional ones are left read-write. Not visible while the masks were applied privileged; exposed by 6aa23a8 ("jail: give the container's namespaces to its own user namespace"). Split the remount half of bind_remount_readonly() into remount_readonly() and use it for both noafile mask sites. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
Several checks key on CLONE_NEWUSER, which -j never sets: the euid switched to before clone() so that files created for the jail belong to the uid its root maps to, the owner of staged device nodes, the overlay upper chown, the console hand-over, the securebits restore and the devpts gid. With -j these ran as host root and the jail's root saw nobody-owned files it could not write. root_map_uid was also never derived for a joined namespace. Have a helper setns() into it and report the mapping of uid 0 from its own uid_map; this also covers a namespace kept alive only by a bind-mounted nsfs file, as OCI runtimes hand them over, where no process exists to read /proc/<pid>/uid_map from. Keep the default with a warning on failure. Add jail_has_userns() and use it in place of the CLONE_NEWUSER tests. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
-j entered the user namespace at the top of exec_jail(), before any mount. Mounting procfs needs CAP_SYS_ADMIN in the userns owning the pid namespace; that pidns comes from clone() in the parent and belongs to the initial userns, which the joined one can never own. Every -j user jail with -p failed with EPERM on its own /proc mount. Treat the join as the deferred case: build the jail fs privileged, then setns() in enter_userns() where the deferred userns is created, and follow the same phase-2 path. Default masks, the read-only /proc/sys and OCI maskedPaths/readonlyPaths thereby leave the locked phase-1 mount list for these jails as well. Factor the credential drop shared by all three paths into userns_become_root(). Signed-off-by: Joshua Covington <joshuacov@gmail.com>
The return value of jail_join_ns() was ignored. A -j naming an exited pid or an unknown namespace type left every requested namespace unset and the jail started without them; a -j user jail whose target was gone ran as host root in the initial userns with nothing logged. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
CLONE_NEWPID and CLONE_NEWIPC were added to every non-OCI jail regardless of -j. For a joined pid namespace the parent has already called setns(CLONE_NEWPID), after which clone(CLONE_NEWPID) is EINVAL, so "-j <pid>:pid" never got past clone(). Only create the two when nothing is joined in their place. A jail joining another container's pid and user namespaces thereby runs in a pidns owned by its userns and can mount its own procfs, which a pidns created by the parent never allows. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
Mounting sysfs needs CAP_SYS_ADMIN in the userns owning the network namespace. A jail with a userns from clone() that keeps the netns it was started in cannot mount one: -s, or an OCI mount of type sysfs, failed with EPERM and the jail did not start. This worked on the tip of openwrt#39, where every userns was entered after the mounts, and broke again with 6aa23a8 ("jail: give the container's namespaces to its own user namespace"). Create the mount in the parent before clone() with fsopen()/fsmount(), carrying the flags the mount queue asked for, and leave the fd on the queue for do_mount_fd() to move_mount() into place. The jail gets the sysfs it asked for, with nothing but its own masks beneath it. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
The mask branch of do_mount() returns error without a message when the tmpfs or noafile mount fails. A critical mask (/proc/kcore, /proc/sysrq-trigger, /sys/firmware) then only shows up as "mount_all() failed", with no path and no errno. Signed-off-by: Joshua Covington <joshuacov@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a follow up to #39: the
-j /procEPERM left open there, plus what testing surfaced beyond it. All nine patches are on c230ad8. In details:-f -pjail failsmount_all()silently, every OCI container with maskedPaths has them rw. Keep the flags in effect (3)-j <pid>:pidfailed with clone3 EINVAL (7)Clone-time userns path (uidMappings, uxc/runc/crun parity) is unchanged except in 3 and 8, which turn EPERM into a working mount; namespace ownership is left untouched, the container mounts its own procfs, nested runtimes are ok. The following behaviour changes worth knowing of: on -j user the cgroupns is now host-owned like on the deferred-create path; a netifd-triggered restart whose -j target has exited now fails instead of starting unconfined. Previously masks that were silently left rw, now become ro.
A side note:
The error I hit is more-or-less /tmp's noatime being remapped as relatime by ujail's hard-coded flag, and this only becomes a kernel-enforced failure once a userns is involved. Own, deferred, or joined, doesn't matter which but what matters is that the process is no longer the owner of the original /tmp mount. That's exactly why in my case /etc/init.d/radiusd's -f -p (own userns) hits it: -f is precisely the condition that turns the pre-existing flag mismatch into an enforced EPERM.
Assisted-by: Claude Fable 5.1
@dangowrt since you wrote 6aa23a8 ("jail: give the container's namespaces to its own user namespace"), can you look at this?