Date: Thu, 31 Mar 2022 18:55:36 -0400 From: Mathieu <sigsys@gmail.com> To: freebsd-hackers@freebsd.org Subject: Re: curtain: WIP sandboxing mechanism with pledge()/unveil() support Message-ID: <b5d3fc79-5d1b-6946-c6f3-17246897914e@gmail.com> In-Reply-To: <16ab7cdb-32b4-5ffe-f6a8-a657383b3078@FreeBSD.org> References: <25b5c60f-b9cc-78af-86d7-1cc714232364@gmail.com> <01320c49-fa7e-99d2-5840-3c61bb8c0d57@FreeBSD.org> <2d103b77-84d4-fbd7-d957-21b9aa4d5d79@gmail.com> <16ab7cdb-32b4-5ffe-f6a8-a657383b3078@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3/31/22 06:24, David Chisnall wrote: > On 29/03/2022 18:32, Mathieu wrote: >> On 3/29/22 04:34, David Chisnall wrote: >>> Hi, >>> >>> Does pledge actually require kernel support? I'd have thought that >>> it could be implemented on top of Capsicum as a purely userland >>> abstraction (more easily with libc help, but even with an >>> LD_PRELOADed library along the lines of libpreopen). In Verona, >>> we're able to use Capsicum to run unmodified libraries in a sandbox, >>> for example, including handling raw system calls: >>> >>> https://github.com/microsoft/verona/tree/master/experiments/process_sandbox >>> >>> >>> It would be good to understand why this needs more kernel attack >>> surface. >>> >>> David >> >> If it can work like that then it's pretty cool. It could be a lot >> more secure. But it's just not the way I went with. Re-implementing >> so much kernel functionality in userland seems like a lot of work. >> Because I wanted my module to be able to sandbox (almost) everything >> that the OS can run. Including whole process hierarchies that >> execute other programs and use process management and shared memory, >> etc. That's a lot of little details to get right... So I went with >> the same route that jails, other MAC modules and even Capsicum are >> implemented: with access checks in the kernel itself. And most of >> these checks were already in place with MAC hooks. > > My concern with adding it to the kernel is that anything that does > path-based checks is *incredibly* hard to get right and it will fail > open. To date, there are zero examples of path-based sandboxing > mechanisms deployed in the wild that have not had vulnerabilities > arising from the nature of the problem. The filesystem is, > inherently, concurrent. A process can mutate the shape of the > filesystem graph while you are doing path-based checks, mostly around > the handling of '..' in paths. Jails and Capsicum sidestep this in > different ways: > > Jails effectively punt the problem to the jail orchestration code. > They provide very strong restrictions on the paths, with a single root > and allowing all access within this. There are a few restrictions on > what you can do from outside of a jail to avoid allowing the jailed > process to exploit TOCTOU differences and escaping but fortunately > these align with the use of jails as isolated containers containing > (minimal) base system. > > Capsicum simply disallows '..' in paths. If you want to support it in > user code then you must do path resolution in userspace. You may still > have TOCTOU bugs, but they'll all fail closed: you will try to resolve > the result, discover that you don't have a file descriptor > corresponding to the path, and fail. Yeah it's by far the hardest part of this module for the reasons you describe. Directories can move while you're holding references to them or even while you're traversing them. I mean, this is hell. My implementation has flaws, but I believe that they have a limited impact, but one of them is a big deal and it means you have to be careful with what you do from outside the sandbox. I'll try to explain below for anyone interested. It's similar to the situation with jails. I still hope I can improve it though. > >> pledge()/unveil() are usually used for fairly well-disciplined >> applications that either don't run other programs or run very >> specific programs that are also well-disciplined and don't expect too >> much (unless you just drop the pledges on execve()). > > The execve hole is the reason that I have little interest in pledge as > an enforcement mechanism. If a process can just execve itself to > escape, then that's a trivial hole to exploit unless you're incredibly > careful to make sure that the process does not have the ability to > create or read files with executable privilege on the filesystem. I see this with some other sandboxing frameworks too, they allow to execute other programs with higher permissions. That's something I wanted to avoid completely. Look how difficult it is to write safe setugid binaries. All programs you'd allow to run would have to be safe too. pledge() can inherit its restrictions though. If you specify restrictions on the second argument, the executed process will never be able to drop those. Unveils will be inherited as well (but it will not be possible to use unveil() again, so this is not fully "nestable" sandboxing as I call it). But pledge() isn't always used like that because a lot of programs just won't work with inherited pledge restrictions if they weren't designed for it. That's what I wanted my module to support better. > > In contrast, something using Capsicum can create child processes but > they inherit the same limitations. It can inherit file descriptors > from the parent, so if it is using something like libpreopen then it > can inherit a large number of file descriptors for any of the files / > directories that it should be permitted to open. With my module, generally the sandbox is inherited. When you use it with curtain(1), that's how it is. The ability to drop the restrictions on-exec was mostly added for pledge() compatibility. And since my module is designed for arbitrary nesting, what actually happens when you request to drop restrictions on-exec with pledge() is that you return to the restrictions that your process inherited at first (which is no restrictions only if you started with no restrictions). > > Since rtld was extended to allow direct execution mode, you can launch > dynamically linked binaries in Capsicum mode. With the SIGCAP things > in https://reviews.freebsd.org/D33248, it becomes easy to write a > signal handler that intercepts blocked system calls and handles them > (I'm running with this applied and doing exactly that), so this can be > transparent to any dynamically linked binary. Yeah it's interesting. I see the "process descriptor" thing too that could make process management capability-based. Getting a general compat layer built on top of Capsicum could be the best of both worlds. > >> Pledged applications usually reduce the kernel attack surface a lot, >> but you don't run arbitrary programs with pledge (and that wasn't one >> of its goals AFAIK). But that's what I wanted my module to be able to >> do. I'd say it has become a bit of a weird hybrid between a >> "container" framework and an exploit mitigation framework at this >> point. You can run a `make buildworld` with it, build/install/run >> random programs isolated in your project directories, sandbox >> shell/desktop sessions as a whole, etc. And then within those >> sandboxes, nested applications can do their own sandboxing on top of >> it (with this module (and its pledge/unveil compat) or Capsicum (and >> possibly other compat layers built on top of it)). The "inner" >> programs can use more restrictive sandboxes that don't expose as much >> kernel functionality. But for the "outer" programs the whole thing >> slides more towards being "containers"/"jails" (and the more complex >> it would have been to do purely in userland I believe). > > So how do you avoid TOCTOU bugs in your path logic? I don't disagree > with the goals, I worry that you're doing something that is > intrinsically almost impossible to get right. > > David > > You're right, it's not done right. Not fully so. The security rests on the module never allowing sandboxed processes to alter the directory hierarchy in a way that could cause exploitable confusion. But if the alterations come from outside, then it can be unsafe. This is similar to jails but (as you said) it's probably less risky there because of the way jails tend to be used. And I could be wrong about all of this... this is going to need auditing. But IIUC, the risk mainly comes from moving directories from inside of the sandbox to outside of it. Say you sandbox firefox and it has write access to ~/.firefox and a separate $TMPDIR, moving any sub-directory from inside of those to somewhere outside is extremely unsafe. But moving directories between ~/.firefox and the $TMPDIR, this is fine. Moving directories from outside to inside, also fine (but they better stay in there). You could expect no one to do that, but say firefox has access to ~/Downloads though, moving directories out of it is a lot more tempting... So that's a big flaw. Moving directories from a writable sandboxed directory to a read-only sandboxed directory is also dangerous for the same reasons. But all of this has to be done from outside the sandbox. I'm not super happy with it. I wish there could be protections for that. But I'm not sure how. I'd have to keep thinking about it. But for now you have to be careful with what you do from outside of the sandbox to directories unveiled inside of it. I think it's tolerable. Even if just barely... In practice I can imagine it causing problems. The actual implementation is not all that complicated in principle and somewhat similar to jails in some ways. Instead of the process having one "jail root directory", it has as many as it has unveils. Each curtain unveil entry holds a vnode reference to its directory. And, when you unveil a path, ALL parent directories are also added to the curtain as unveil entries (with permissions to traverse them). When path lookup starts, if needed, it'll ascend the directory hierarchy to find the unveil entry that "covers" the start directory. After this, checks are strictly done against the curtain, there's no more searching for parent directories. The module just reacts to traversals of directories that it knows about. Whenever you land on a directory during path lookup (including after ".." lookups), the curtain is searched for the vnode. The dangerous part of it is when you descend into a directory with no corresponding curtain unveil entries. Then it switches into "uncharted" mode where it assumes that ALL files can inherit the parent unveil's permissions. But this is only until you hit another vnode that the curtain knows about. Then it'll leave "uncharted" mode and inherited permissions will not carry upwards during ".." lookups. Each unveil (and all of their parent directories) act like a sort of firewall for the hierarchy confusion down below (or at least that's the idea). And to cause confusion within this outer directory "skeleton", you need to have permissions to alter it in the first place! But within "uncharted" directories anything can happen (using the permissions of the unveil that covers it). If you can move a directory into the depths of another directory, you can trick curtain into using the permissions of the moved directory on the content of the target directory (curtain won't know where the directory really is when it handles ".." lookups). But here's the thing: to move that directory there, the unveils must give write access to both the source and target directories in the first place! So what was gained? The hierarchy confusion isn't so bad. That's a lie though. You can get permissions to modify file attributes on a directory for which you only had write permissions this way (those can be separate unveil permissions). That's something I'll try to fix. It's just a problem for writable directories that can be moved though, separate file attribute change permissions are still useful for /dev files for example (sandboxed root processes are correctly prevented from changing permissions on /dev/null even if they can write to it). All of this is kind of hard to reason about... So I hope I got this right (and there could be bugs too obviously).
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b5d3fc79-5d1b-6946-c6f3-17246897914e>