On Mon, Sep 16, 2013 at 05:06:51PM -0700, Seth Arnold wrote:
> On Mon, Sep 16, 2013 at 04:49:49PM -0700, Steve Beattie wrote:
> > Index: b/parser/tst/simple_tests/capability/ok_dup_allow5.sd
> > ===================================================================
> > --- /dev/null
> > +++ b/parser/tst/simple_tests/capability/ok_dup_allow5.sd
> > @@ -0,0 +1,17 @@
> > +#
> > +#=DESCRIPTION validate duplicate multiple capabilities w/differing perm
> > mods.
> > +#=EXRESULT PASS
> > +# vim:syntax=subdomain
> > +# Last Modified: Sun Apr 17 19:44:44 2005
> > +#
> > +
> > +/does/not/exit102 {
> > + allow capability chown dac_override dac_read_search fowner fsetid kill
> > setgid setuid setpcap linux_immutable net_bind_service net_broadcast
> > net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot
> > sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time
> > sys_tty_config mknod lease audit_write audit_control,
> > +
> > + audit allow capability chown dac_override dac_read_search fowner fsetid
> > kill setgid setuid setpcap linux_immutable net_bind_service net_broadcast
> > net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot
> > sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time
> > sys_tty_config mknod lease audit_write audit_control,
> > +
> > + audit deny capability chown dac_override dac_read_search fowner fsetid
> > kill setgid setuid setpcap linux_immutable net_bind_service net_broadcast
> > net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot
> > sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time
> > sys_tty_config mknod lease audit_write audit_control,
> > +
> > + deny capability chown dac_override dac_read_search fowner fsetid kill
> > setgid setuid setpcap linux_immutable net_bind_service net_broadcast
> > net_admin net_raw ipc_lock ipc_owner sys_module sys_rawio sys_chroot
> > sys_ptrace sys_pacct sys_admin sys_boot sys_nice sys_resource sys_time
> > sys_tty_config mknod lease audit_write audit_control,
> > +}
> > +
>
> I think I'd expect the above to FAIL instead of PASS -- 'allow capability
> chown' followed by 'deny capability chown' doesn't make much sense. I know
> we've decided that 'deny' rules should subtract from the profile, but
> having two conflicting lines in one profile hurts my head.
>
> This feels complicated.I started typing up an email prepared to state that this patch was extending behavior in the same direction as before, that 'deny' rules always override 'allow' rules, and that this patch just adds the ability to make explicit the implicit 'allow' in our regular rules. But thinking about the patch as it stands now, I don't believe that's the case. I'll try to write some behavioral regression tests around this tomorrow, rather than just the 'does it parse okay?' tests that are included in this patch, to help us determine what correct behavior should be and whether this patch achieves that (I have my opinions, but would like to hear others', including John's as the original author of the patch). -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
