[apparmor] [PATCH 3/8] add optional allow prefix to the language v2

[Steve Beattie]

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
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20130916/cf724e18/attachment.pgp>