Date: Fri, 8 Dec 2017 13:17:19 +0100 From: Solar Designer <solar@...nwall.com> To: Salvatore Mesoraca <s.mesoraca16@...il.com> Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>, Matthew Wilcox <willy@...radead.org> Subject: Re: [PATCH v3 0/2] Restrict dangerous open in sticky directories On Thu, Dec 07, 2017 at 11:23:34PM +0100, Solar Designer wrote: > On Tue, Dec 05, 2017 at 11:13:50AM +0100, Salvatore Mesoraca wrote: > > 2017-11-30 20:05 GMT+01:00 Solar Designer <solar@...nwall.com>: > > > So the current name is "protected_sticky_child_create" (I couldn't even > > > recall it, had to look it up for this reply). This unnecessarily > > > bundles this potentially more general policy stuff with the existing > > > "protections" against specific attacks, unnecessarily limits scope to > > > "sticky" and "create", and talks about some "child". How about we use > > > something totally different, focusing on "policy"? It could be simply > > > "policy" (we're already in "fs"), or if that won't fly then how about > > > "security_policy" or "dac_policy"? We will be imposing extra > > > restrictions on top of usual Unix discretionary access control > > > permission bits while not going all the way to mandatory access control > > > (not tying objects to subjects). So in a sense we'll have an extension > > > of Unix DAC. > > > > Yea, I like "dac_policy" very much. > > OK. Thinking of this further, what we might want to have is a generic > policy against file accesses that would negate a privilege boundary > between Unix (pseudo-)users. And one of those would be execve(2) of a > file writable by someone else, or with any of its parent directories > writable by someone else (unless that's root, indeed). I think the name > "dac_policy" still fits this well, but anything with "create" or even > "open" in the name would not fit it. > > Another way of looking at this is that we'd be creating a reverse DAC - > optionally blocking unsafe accesses made possible by too permissive DAC. > But I don't have a good sysctl name suggestion along these lines. Yet another thought is that while we won't go for MAC ("not tying objects to subjects") in our additional policy against unsafe file accesses, it could reasonably be extended to cover not only accesses made possible by too permissive DAC, but also those made possible by too permissive ACLs. I doubt we'd want to actually add ACL support in there (too obscure, unnecessary when auditing software), but if someone eventually does that would fit in with our general approach (and wouldn't even require separate flags, as it's conceptually similar to group-writable). So an ideal name would not conflict with that. What would be a good short name for "file access safety policy"? Maybe we can omit "file" considering we're in "fs". "access_safety_policy"? The individual flags might correspond to detecting and/or blocking: O_CREAT w/o O_EXCL in +t world-writable directory except if file exists and is owned by root or by current fsuid and the directory owner is root or current fsuid O_CREAT w/o O_EXCL in +t group-writable directory except if file exists and is owned by root or by current fsuid and the directory owner is root or current fsuid, or O_CREAT w/o O_EXCL in +t directory owned by another non-root user (regardless of whether the file exists and its ownership) (Note: the "+t directory owned by another non-root user" case is for consistency and for software testing only, not for enforcing a policy like this in production, because the directory owner can remove the +t.) O_CREAT w/o O_EXCL in -t world-writable directory (no exceptions) O_CREAT w/o O_EXCL in -t group-writable directory (no exceptions), or O_CREAT w/o O_EXCL in any directory owned by another non-root user (regardless of the sticky bit, and also without exceptions) Same as above for O_WRONLY or O_RDWR, so 4 more flags. Same as above for O_RDONLY, so 4 more flags. Other unsafe file accesses via pathname, including execve(2) (would consider permissions on the file itself, as well as the directory), chmod(2), chown(2). That's maybe 6 more flags if we continue to separate world vs. group/user, even if we don't separate by +t (but we could nevertheless trust a pre-existing file in +t under the same conditions listed for "O_CREAT w/o O_EXCL in +t"). Then we could want to eventually add checking of all parent directories, and this in turn would make more syscalls relevant (in the sense of being usable for an immediate attack on the invoking user, making them do something unintended) in the special case when an upper level directory is unsafe: symlink(2), link(2), unlink(2), rename(2), mkdir(2), rmdir(2), perhaps more. Just for these 6 I quickly identified, and if we continue to separate world vs. group/user, this could be 12 more flags. So this would be 4+4+6+12 = 26 flags already. This feels like way too many, as well as not leaving enough bits e.g. in a 32-bit mask in case we end up identifying many more relevant syscalls. So perhaps the approach above is too fine-grained and should be revised. If so, maybe our start with two bits for "O_CREAT w/o O_EXCL in +t" is already too fine-grained and should be revised as well. What do you think? How to group these categories in fewer flags best? Perhaps less separation of the open(2) flags and/or of the syscalls relevant only with unsafe upper-level directories (one flag for all of them?) Drop the +t / -t separation for open(2)? (But nevertheless trust a pre-existing file in +t under the same conditions as I listed for "O_CREAT w/o O_EXCL in +t".) Drop the world vs. group/user separation? Introduce more sysctl's - separate for open(2) vs. other syscalls? And/or maybe separate for world vs. group/user-writable? This would be convenient in that the same bit positions would refer to the same open(2) modes in the same directory categories, or the same syscalls. Extending a previously tested policy for world-writable to cover also group/user-writable would be as simple as setting the other sysctl to the same value. Any other ideas? > > > > If I implement something like what Matthew proposed it will be easy to > > > > extend scope and functionalities of this feature without complicating too much > > > > the interface. > > > > > > >  https://lkml.org/lkml/2017/11/22/319 > > > > > > Right. I tried thinking of a way to specify all reasonable combinations > > > without the likely unreasonable ones, but couldn't come up with anything > > > elegant. So I'm fine with Matthew's proposal as-is. > > > > Great. > > Thinking of this further, maybe it'd be friendlier to further expansion > if we separate the policy and notify vs. block into two bitmasks, in > separate sysctl's. If a sysadmin wants only notifications or only > blocking, that would then be easier to achieve, by setting the notify > vs. block sysctl to 0 or to all 1's (to -1 maybe, although this would > give the wrong impression of being "below 0"). Then the sysadmin would > be able to focus on the actual policy bits separately, without the > distraction of the notify vs. block bits inbetween. > > The names could then be "dac_policy" and "dac_policy_enforce" maybe? > Any other suggestions? Unfortunately, my suggestion above wouldn't address Matthew's criticism about the lack of flexibility. It wouldn't be possible to independently control notifications and enforcement. So perhaps the two sysctl name suffixes can be "notify" and "enforce", and they'd independently define two policies - one producing notifications only and the other actually blocking syscalls. Usually they'd be the same or one of them would be 0, but arbitrary two policies could also be specified. Now how to combine this with extensibility of the policies themselves, potentially eventually going to the extent I outlined above, which might require multiple sysctl's per policy? Perhaps just double their number? If so, that may be a reason for keeping each of the two policies in fewer sysctl's, which is in turn a reason for lower granularity. Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.