[apparmor] [PATCH 4/4] dconf patch

[Christian Boltz]

Hello,

Am Montag, 14. Dezember 2015 schrieb William Hua:
> Just made one minor change to make dconf rules more consistent with
> other rules (parsing permissions after paths).

Some notes about 0004-Add-support-for-dconf-confinement.patch:

> --- a/parser/parser_lex.l
> +++ b/parser/parser_lex.l

> +<DCONF_MODE>{
> +       r(ead)?                                 { RETURN_TOKEN(TOK_READ); }
> +       w(rite)?                                { RETURN_TOKEN(TOK_WRITE); }
> +       (rw|wr)                                 { RETURN_TOKEN(TOK_READWRITE); }
> +       ({PATHNAME}|{QPATHNAME})        {

That's much better than the loooong list of possible keywords in the 
last round. I still wonder if we really need "read" and "write" or if 
"r" and "w" would be enough ;-)

(Yes, I know we allow "read" and "write" at other places, but we don't
need to repeat that error ;-)


> --- a/parser/parser_yacc.y
> +++ b/parser/parser_yacc.y

> +dconf_perm: TOK_READ { $$ = AA_DCONF_READ; }
> +       | TOK_WRITE { $$ = AA_DCONF_READWRITE; /* writable implies readable */ }
> +       | TOK_READWRITE { $$ = AA_DCONF_READWRITE; }

I still don't like the idea to implicitely grant read permissions if 
something has write permissions.

This needs *at least* a very clear note in the documentation (BTW: did
I overlook the apparmor.d.pod patch?). The more strict and IMHO better
way would be to error out if only write is allowed in a profile.


Also, can you please add a parser/tst/simple_tests/dconf/ directory 
with some example profiles (some with valid, some with invalid syntax)?


Regards,

Christian Boltz
-- 
Bugzilla beißt nicht und ist viel, viel netter als ich. ;)
[Lars Müller in opensuse-de]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160110/c63b7255/attachment.pgp>