On Tue, Sep 17, 2013 at 02:01:44PM +0200, Christian Boltz wrote: > Hello, > > Am Montag, 16. September 2013 schrieb Steve Beattie: > > - fix a bug in apparmor.vim to let it recognize multiple > > capability entries in a single line. > > I didn't know this is possible ;-)
See, for example, the last few lines in parser/tst/simple_tests/capability/ok1.sd. (It makes for a good test case for whether apparmor.vim is handling multiple entries properly.) > @Kshitij: you'll also need to allow this in the py tools. > > Index: b/parser/tst/simple_tests/capability/ok_allow1.sd > > =================================================================== > > --- /dev/null > > +++ b/parser/tst/simple_tests/capability/ok_allow1.sd > > @@ -0,0 +1,41 @@ > > +# > > +#=DESCRIPTION validate uses of allow/capabilities. > > +#=EXRESULT PASS > > +# vim:syntax=subdomain > > new files should use > # vim:syntax=apparmor > (and when you are bored, you should also change all existing profiles ;-) > In other words: please s/syntax=subdomain/syntax=apparmor/ in the patch ;-) Okay, I can do that. > > +++ b/parser/tst/simple_tests/file/allow/ok_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_3.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_append_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_carat_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_carat_2.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_comma_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_comma_2.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_embedded_spaces_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_embedded_spaces_2.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_embedded_spaces_3.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_inv_char_class.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_lock_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_mmap_1.sd > > > +++ b/parser/tst/simple_tests/file/allow/ok_mmap_2.sd > > All those files don't have a vim: header. Maybe you can add it? Sure. Though, while syntax highlighting with apparmor.vim is a pretty important tool for me when examining profiles, I don't enable modelines in vim because it's a bit dangerous to give random files you open access to vim settings. But it's still a useful hint that syntax highlighting for apparmor profiles is available, but should give you an idea why I'm not careful about getting them correct. > > Index: b/utils/vim/create-apparmor.vim.py > > =================================================================== > > --- a/utils/vim/create-apparmor.vim.py > > +++ b/utils/vim/create-apparmor.vim.py > > > - 'FILE': r'\v^\s*(audit\s+)?(deny\s+)?(owner\s+)?' + > > filename + r'\s+', # Start of a file rule > > + 'FILE': r'\v^\s*(audit\s+)?((deny|allow)\s+)?(owner\s+)?' > > + filename + r'\s+', # Start of a file rule > > vi has a restriction on how many (...) you use per line (IIRC 9 - I hit > the limit in the past), so we shouldn't add (...) if not really needed. > > Please use (deny\s+|allow\s+) instead of ((deny|allow)\s+) > (like you did for 'auditdenyowner' and 'auditdeny') Yes, I hit that parentheses limit when I tried to use '((deny|allow)\s+)?' in 'auditdenyowner' and 'auditdeny'. I didn't go back and adjust FILE because when I looked, I couldn't see anything in create-apparmor.vim.py or apparmor.vim.in that used FILE. Everything seems to use FILENAME instead. > > Index: b/utils/vim/apparmor.vim.in > > =================================================================== > > --- a/utils/vim/apparmor.vim.in > > +++ b/utils/vim/apparmor.vim.in > > @@ -132,7 +132,7 @@ syn keyword sdCapKey @@sdKapKe > > > -syn match sdCap /\v^\s*@@auditdeny@@capability\s+(@@sdKapKeyRegex@@) > > @@EOL@@ ... > > +syn match sdCap > > /\v^\s*@@auditdeny@@capability\s+((@@sdKapKeyRegex@@)\s+)*(@@sdKapKeyRegex@@)@@EOL@@ > > ... > > (lines shortened for readability) > > Looks good, even if it adds two more (...) ;-) Yeah, it doesn't seem to go over vim's limit. The other way to do it, which would add only a single of parens, would be to create a second capability list regex that in included the '\s+' trailing spaces requirement with each capability name, to be used to match all the capability names other than the last one that occurs in the line. This way seemed cleaner. As an aside, I probably should have submitted this patch separately as it's an independent fix from adding support for the 'access' keyword, and it's easily cherry-pickable for the 2.8 branch. > The more interesting question is: how do we color the line if it contains > "dangerous" and "normal" capabilities? The way things work currently is that dangerous capabilities get highlighted differently (foreground color red) when they occur, whether they are the only capability in a rule or mixed with less dangerous ones. They stand out pretty well against the dark blue foreground color of the rest of the capability rule. Again, parser/tst/simple_tests/capability/ok1.sd makes for a useful example. Thanks for the feedback. -- 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
