On Thu, 2017-07-13 at 11:48 -0400, Stephen Smalley wrote: > On Thu, 2017-07-13 at 09:25 -0400, Paul Moore wrote: > > On Thu, Jul 13, 2017 at 8:44 AM, Stephen Smalley <sds@xxxxxxxxxxxxx > > > > > wrote: > > > On Wed, 2017-07-12 at 20:27 -0400, Chris PeBenito wrote: > > > > On 07/12/2017 05:38 PM, Paul Moore wrote: > > > > > On Wed, Jul 12, 2017 at 9:26 AM, Stephen Smalley <sds@tycho.n > > > > > sa > > > > > .gov > > > > > > wrote: > > > > > > On Tue, 2017-07-11 at 17:00 -0400, Paul Moore wrote: > > > > > > > On Mon, Jul 10, 2017 at 4:25 PM, Stephen Smalley <sds@tyc > > > > > > > ho > > > > > > > .nsa > > > > > > > .gov> > > > > > > > wrote: > > > > > > > > > > While I think splitting the NNP/nosuid transition > > > > > restrictions > > > > > might > > > > > be a good idea under the new policy capability, I'm not sure > > > > > it > > > > > is > > > > > worth the cost of a "process2" object class. > > > > > > > > > > With that in mind, let's do two things with this patch: > > > > > > > > > > * Mention the nosuid restrictions in the patch > > > > > description. It > > > > > doesn't need much text, but something would be good so we > > > > > have > > > > > documentation in the git log. > > > > > > > > > > * Let's pick a new permission name that is not specific to > > > > > NNP > > > > > or > > > > > nosuid. IMHO, nnpnosuid_transition is ... less than good. > > > > > Unfortunately, I'm not sure I'm much better at picking names; > > > > > how > > > > > about "protected_transition"? "restricted_transition"? > > > > > "enable_transition"? "override_transition"? > > > > > > > > I vote for nnp_transition anyway. "No New Privileges" > > > > encompasses > > > > nosuid in my mind. If the two perms had been separated I would > > > > have > > > > been inclined to allow both every time one of them was needed, > > > > to > > > > make > > > > sure no one was surprised by the behavior difference. > > > > > > I agree; I'll keep it as nnp_transition and just document it in > > > the > > > patch description. > > > > Sorry to be a stubborn about this, but nnp_transition makes little > > sense for the nosuid restriction. Like it or not, NNP has a > > concrete > > meaning which is distinct from nosuid mounts. We don't have to > > pick > > any of the permission names I tossed out, none of those were very > > good, but I would like to see something that takes both NNP and > > nosuid > > into account, or preferably something that doesn't use either name > > explicitly but still conveys the meaning. > > NNP is essentially a superset of nosuid; it applies to all execve() > calls by the process and its descendants rather than only to execve() > calls on specially marked filesystems. So I viewed it as the more > general term. > > If that's not viable, I can't think of anything clearer or better > than > nnp_nosuid_transition. That clearly links it to what it means (allow > a > SELinux domain transition under NNP or nosuid). It is somewhat ugly > and verbose but it is clear in what it means, which I think is more > important. All of your suggestions IMHO were less clear since they > had > no clear linkage to either NNP or nosuid, and I couldn't tell from > reading the permission name what it meant. The SELinux domain > transition isn't protected, it isn't restricted, it isn't clear what > enable_transition means versus the regular transition or > dyntransition > permissions, and we aren't overriding a transition but rather > allowing > one under NNP/nosuid. > > The only other alternative I see is to introduce a process2 class and > use separate nnp_transition and nosuid_transition permissions (but in > practice I expect them to be both allowed or denied together). Note > that this will require two avtab and AVC entries for every domain > pair > (if we allow whichever one ends up going in the process2 class), so > that has an impact on policy and AVC size. Don't really see that as > worthwhile. > > Other approach would be to make the nosuid transition checks file- > based > instead so that it would go in the file class (while the nnp one > would > remain in the process class), but I don't think that's really what we > want either. Difference between "Can domain D1 transition under > nosuid > to D2?" and "Can domain D1 transition under nosuid when executing > file > with type T1?". Just to be clear, we would also be separately checking regular transition permission between the old and new contexts on these transitions, so you would need both: allow D1 D2:process transition; allow D1 T1:file nosuid_transition; if we took the latter approach. So we wouldn't lose entirely on a domain-to-domain check, but it would no longer be domain-to-domain for the nosuid part. Whereas with original approach, we end up requiring: allow D1 D2:process transition; allow D1 D2:process nnp_nosuid_transition; # or whatever permission name is used > > On a separate note, I plan to cc luto on the next version of the > patch > as I suspect he will have concerns about relaxing this constraint on > NNP and this likely requires updating > Documentation/prctl/no_new_privs* > and the man pages that describe NNP behavior. > > The other model would be to figure out a way to make the typebounds > logic work cleanly in a manner that preserves the desired NNP/nosuid > invariant _and_ doesn't require leaking unnecessary accesses into the > ancestor domains that make them less secure, plus CIL support for > automatically propagating permissions in the desired way. But I > haven't yet come up with a way to do that. We can do it in some > cases > by creating typebounds between the object types, e.g.: > typebounds parent_t child_t; > allow child_t self:process execmem; > allow child_t child_exec_t:file entrypoint; > allow child_t child_tmp_t:file create; > can be allowed via: > allow parent_t child_t:process execmem; # an otherwise nonsensical > rule > typebounds parent_exec_t child_exec_t; > typebounds parent_tmp_t child_tmp_t; > but this breaks down when there isn't an equivalent type and > permission > set already allowed to the parent for every type allowed to the > child.