[apparmor] [PATCH 4/4] dconf patch

[Christian Boltz]

Hello,

Am Dienstag, 6. Oktober 2015 schrieb John Johansen:
> On 10/06/2015 11:05 AM, Christian Boltz wrote:
> > Am Dienstag, 6. Oktober 2015 schrieb John Johansen:
> >> diff --git a/parser/Makefile b/parser/Makefile
> >> index 1f0db8d..ec54f96 100644
> >> --- a/parser/Makefile
> >> +++ b/parser/Makefile
...
> > I know that list is chaotic already (probably for historical
> > reasons?), but what about sorting the HDRS files by alphabet? (same
> > question for SRCS and maybe some other file lists in the Makefile)
> 
> yeah we can get to doing something like that, once my make file
> patches land. 

Most of them are acked, so feel free to commit those ;-)
I'd also accept a *.h wildcard to make maintaining the Makefile easier.

> This is based on work William did months ago and I am
> only now getting a reply out to.

no problem ;-)

> >> --- a/parser/tst/equality.sh
> >> +++ b/parser/tst/equality.sh
> >> 
> >> +verify_binary_equality "dconf read" \
> >> +	"/t { dconf / r, }" \
> >> +	"/t { dconf / read, }"
> >> +
> >> +verify_binary_equality "dconf write" \
> >> +	"/t { dconf / w, }" \
> >> +	"/t { dconf / write, }"
> >> +
> >> +verify_binary_equality "dconf read-write" \
> >> +	"/t { dconf / rw, }" \
> >> +	"/t { dconf / wr, }" \
> >> +	"/t { dconf / readwrite, }" \
> >> +	"/t { dconf / writeread, }" \
> >> +	"/t { dconf / read-write, }" \
> >> +	"/t { dconf / write-read, }" \
> >> +	"/t { dconf / read_write, }" \
> >> +	"/t { dconf / write_read, }"

BTW: I'd add another test here:
    "/t { dconf / r, dconf / w, }"

> > Seriously?
> > 
> > I have to admit that I don't really know dconf, but having 8
> > different ways to allow read and write (one letter vs. word, no
> > separator vs - vs. _) is too much. We don't win anything with it,
> > but it makes
> > implementation of the parser and the tools more difficult than
> > needed.
> > 
> > IMHO the single-letter syntax we already use in file rules ("rw" or
> > "wr") is enough and will save us some headache.
> 
> gah, no that was supposed to be cut out, notice in my intro reply that
> I moved it back to an apparmor style syntax. I must have either
> missed this block or missed git adding the change back into the patch

Note that it's not only in the tests. The parsing code (parser_lex.l) 
also allows "r(ead)?" and "w(rite)?", and maybe I missed another place 

I also just noticed another interesting bit in parser_yacc.y [1]

 +       | TOK_WRITE { $$ = AA_DCONF_READWRITE; /* writable implies 
                                                   readable */ }

This sounds like surprising behaviour to me - does this really make 
sense?,If yes, this needs to be documented in bold letters or - IMHO 
better - rules with only w permissions should be rejected as invalid to 
enforce that the profile always contains rw permissions, not only w.


Regards,

Christian Boltz

[1] I should have read the patch a bit slower before writing the 
    previous mail ;-)
-- 
Graphisch??? Wie meinen? Hast du zuviel Fleisch von zu "gluecklichen"
Rindern gefuttert? *scnr*  Wozu zum Henker sollte man sowas brauchen?
Logo ginge auch per ASCII :)  (Logo?  welches Logo? Wozu ueberhaupt?)
[David Haller in suse-linux]