Hello, (I should have read all mails before replying to the 4/6 patch ;-)
Am Donnerstag, 22. März 2012 schrieb Steve Beattie: > This patch replaces the apparmor.vim generating script with a python > version that eliminates the need for using the replace tool from the > mysql-server package. I'm fine with rewriting the generation of apparmor.vim with a p* language - it's a long-standing issue on my TODO list, but usually there's ENOTIME ;-) Let me warn you that I know nothing about python. This means you'll probably have to maintain the script yourself - or teach me python ;-) > It makes use of the automatically generated lists of capabilities and > network protocols provided by the build infrastructure. :-) > I did not capture all the notes and TODOs that > Christian had in the shell script; I can do so if desired. I won't object ;-) > Index: b/utils/vim/Makefile > =================================================================== > --- a/utils/vim/Makefile > +++ b/utils/vim/Makefile > @@ -1,5 +1,18 @@ > -apparmor.vim: apparmor.vim.in Makefile create-apparmor.vim.sh > - sh create-apparmor.vim.sh > +COMMONDIR=../../common/ > + > +all: > +include common/Make.rules > + > +COMMONDIR_EXISTS=$(strip $(shell [ -d ${COMMONDIR} ] && echo true)) > +ifeq ($(COMMONDIR_EXISTS), true) > +common/Make.rules: $(COMMONDIR)/Make.rules > + ln -sf $(COMMONDIR) . > +endif What's the reason for this COMMONDIR magic? I'd just use include $(COMMONDIR)/Make.rules but probably I'm overlooking the reason why you did it this way ;-) > Index: b/utils/vim/create-apparmor.vim.py > =================================================================== > --- /dev/null > +++ b/utils/vim/create-apparmor.vim.py > @@ -0,0 +1,108 @@ > +#!/usr/bin/python > +# > +# Copyright (C) 2012 Canonical Ltd. > +# > +# This program is free software; you can redistribute it and/or > +# modify it under the terms of version 2 of the GNU General Public > +# License published by the Free Software Foundation. > +# > +# Written by Steve Beattie <st...@nxnw.org>, based on work by > +# Christian Boltz <appar...@cboltz.de> > + > +import os > +import re > +import subprocess > +import sys > + > +# dangerous capabilities > +danger_caps=["audit_control", > + "audit_write", > + "mac_override", > + "mac_admin", > + "set_fcap", > + "sys_admin", > + "sys_module", > + "sys_rawio"] Hmm, would it make sense to get this list from severity.db? Just handle everything with severity 10 (and 9?) as dangerous. > +aa_network_types=r'\s+tcp|\s+udp|\s+icmp' > > +aa_flags=r'(complain|audit|attach_disconnect|no_attach_disconnected|c > hroot_attach|chroot_no_attach|chroot_relative|namespace_relative)' Writing aa_network_types and aa_flags as array and later join'ing them would be better readable. I didn't do this in the shell script because it would have been too difficult, but now that we finally have it in another language, it's an easy change. > +for cap in capabilities: > + if cap not in danger_caps: > + benign_caps.append(cap) IIRC 2.8 will allow to specify more than one capability per line, for example capability sys_admin syslog, This means I'll probably change apparmor.vim to do inline coloring here so that only "sys_admin" will look dangerous instead of the whole line. On the script side, this means the capability list should contain all capabilities including the dangerous ones, and I still need the dangerous capabilities list. That's nothing you need to change now. I just wanted to point it out so that you know which changes you have to expect ;-) > +def my_repl(matchobj): > + #print matchobj.group(1) > + if matchobj.group(1) in aa_regex_map: > + return aa_regex_map[matchobj.group(1)] > + > + return matchobj.group(0) > + > +regex = "@@(" + "|".join(aa_regex_map) + ")@@" > + > +with file("apparmor.vim.in") as template: > + for line in template: > + line = re.sub(regex, my_repl, line.rstrip()) > + print line This looks too easy to be true ;-) There's another thing I'd like to request. You might have noticed that the file rules still contain lots of duplication in apparmor.vim.in - with the exception of "deny x", the only differences between the file rules are - the highlighting name ("sdEntryXYZ") - the permission flags Therefore I'd like to have a function that writes out the file rules. Pseudocode for calling it: filerule( 'sdEntryW' , '(l|r|w|k)+' ) filerule( 'sdEntryUX' , '(r|m|k|ux|pux)+@@TRANSITION@@' ) It would probably also be possible to use an array as long as it uses the permissions as key: $filerules = array( '(l|r|w|k)+' => 'sdEntryW' '(r|m|k|ux|pux)+@@TRANSITION@@' => 'sdEntryUX' ); In theory there can be multiple lines for one highlighting name, and given the restriction of 9 (...) groups per line in vim, I'll probably have to split up some rules sooner or later. In other words: don't even think about using sdEntryXY as array key ;-) Also note the @@TRANSITION@@ part - either filerule() replaces it or I have to switch to the python variable containing its replacement. Final question: Did you test if your python script generates the same apparmor.vim as my shell script? If you can answer this question with "yes" and promise to implement the suggestions above in the next days, then you get an Acked-By: Christian Boltz <appar...@cboltz.de> (having the first version in bzr probably makes testing and implementing my suggestions easier.) Regards, Christian Boltz -- > es ist doch ausgesprochen ruhig hier und das nach dem Release einer > neuen openSUSE Version. Sollte es etwa keine Probleme geben? Vermutlich sind alle damit beschaeftigt, kmail2 ans Laufen zu bekommen. Dann gibt es auch wieder Mails :-) [> Marco Röben und Thomas Moritz in opensuse-de] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor