crun follows symlinks when creating mount points about crun HOT 7 CLOSED

The obvious fix is to do the equivalent of https://github.com/cyphar/filepath-securejoin.

However, that library is not race-safe at all (see CVE-2018-18554 to see what I mean). I have been working on a race-safe version (which also makes use of new kernel features I'm working on), and that would be a better thing to eventually switch to (https://github.com/openSUSE/libpathrs). Unfortunately it's currently not packaged by anyone (though we should also fix that).

Interestingly, one thing I just figured out is that mount(2) is actually completely symlink-unsafe (there is no way to stop it from following trailing symlinks). mount the command resolves the symlinks in the path manually, but that's not necessary since mount(2) will do it for you. However it is also completely magic-link unsafe (magic-link targets resolve to the path that was open)! Which means that in libpathrs we can actually implement a safe mount(2) API. Interestingly this means that symlinks are weirdly always safe to not be mount-points (which means that files guaranteed to be symlinks in procfs are "always safe" -- though it's possible the new mount API changes this behaviour).

Since we're doing everything in a mount namespace the symlink-unsafety of mount(2) is not that bad though you could use it to trick the process into mounting over parts of /proc.

from crun.

Yes, I've spent some time playing with that. It's possible to replace /sys and /proc in the rootfs with symlinks pointing somewhere else and the runtime will mount sysfs and bind sysfs and procfs there, but I haven't been able to abuse it usefully since everything else will follow the symlinks, too.

from crun.

What I meant is that you could create a symlink to /proc and then add a volume mount for that symlink -- the host /proc (inside the container's mount namespace -- though you have to be sure you mark everything as MS_SLAVE beforehand) gets mounted over because mount(2) follows the symlink. This results in the runtime program possibly being tricked by a fake procfs.

EDIT: Oh shit, I just checked and runc doesn't do an MS_SLAVE of /... This isn't good.

EDIT2: Ah I forgot we do SecureJoin in runc. But this will be a problem for crun. Something like the following:

% cat >Dockerfile <<EOF
FROM busybox
RUN ln -s /proc/irq /foo
VOLUME /foo
EOF
% docker build -t foo .
% docker run --runtime=crun foo bash

from crun.

EDIT2: Ah I forgot we do SecureJoin in runc. But this will be a problem for crun. Something like the following.

There is something similar to SecureJoin in crun based on https://git.busybox.net/uClibc/tree/utils/chroot_realpath.c.

I've done some minor changes to adapt it to crun, such as return partial resolved paths, and now to resolve also the last component.

I'll look into using https://github.com/openSUSE/libpathrs.

Do you have any example of using it from C? Can it be used as a copylib?

from crun.

I'll look into using https://github.com/openSUSE/libpathrs.

Note that at the moment it is still in the "heavy development" phase (so there's a lot of emphasis ojn the "eventually" in "eventually switch to") -- though I would appreciate feedback regarding the APIs. It definitely works as-is (I have some not-yet-pushed tests that use it with a Python FFI wrapper to double-check that CVE-2018-16554 attacks are defended against).

Do you have any example of using it from C?

There is a trivial example in the README -- it looks like this:

#include <pathrs.h>

int get_my_fd(void)
{
	int fd = -1;
	pathrs_root_t *root = NULL;
	pathrs_handle_t *handle = NULL;
	pathrs_error_t error = {};

	root = pathrs_open("/path/to/root");
	if (!root)
		goto err;

	handle = pathrs_inroot_resolve(root, "/etc/passwd");
	if (!handle)
		goto err;

	fd = pathrs_reopen(handle, O_RDONLY);
	if (fd < 0)
		goto err;

	goto out;

err:
	if (pathrs_error(&error) <= 0)
		abort();
	fprintf(stderr, "Uh-oh: %s (errno=%d)\n", error.description, error.errno);

out:
	pathrs_hfree(handle);
	pathrs_rfree(root);
	return fd;
}

The key interfaces all look like the above:

• You open a pathrs_root_t (which is an O_PATH descriptor).
• You either resolve a pathrs_handle_t relative to the pathrs_root_t (which is also an O_PATH descriptor) or you do an operation relative to pathrs_root_t (which may or may not return a pathrs_handle_t).
• If you have a pathrs_handle_t and you want an actual file descriptor, then you pathrs_reopen it.
• Cleanups are done with pathrs_{h,r}free.

If you mean an actual program which uses it, I'm currently working on porting umoci to using it -- but I need to iron out a few more details specific to umoci (mainly related to how umoci unpack --rootless should be implemented).

Can it be used as a copylib?

I'm not sure what that means?

from crun.

Can it be used as a copylib?

I'm not sure what that means?

From what I understood you'd like to get libpathrs packaged. I was just wondering if I could just add it as a git submodule for now and start using it without waiting for packages. Would you have anything against it?

Can I get a resolved path from a handle so that I can use it with mount(2)?

from crun.

From what I understood you'd like to get libpathrs packaged. I was just wondering if I could just add it as a git submodule for now and start using it without waiting for packages. Would you have anything against it?

I have no problem with that at all -- as long as you can build it and it works for you that's totally fine with me (obviously please avoid out-of-tree patches but that's a given).

Can I get a resolved path from a handle so that I can use it with mount(2)?

I am about to write a wrapper for mount(2) -- but what I just discovered above is that you don't need to get a resolved path for mount(2) (in fact the whole idea behind libpathrs is that paths are the wrong model because you can get symlink attacks). You can just mount over /proc/self/fd/$n and it will all work out (this is what I meant by magic-link unsafe).

If you really try, you can get resolved pathrs through libpathrs but I don't expose that as a feature because (IMHO) using string paths is a misfeature. You should be doing all operations on handles (read: file descriptors) where you can actually protect against TOCTOU attacks completely.

from crun.