[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [patch 5/6] rewrite apparmor.vim generation and integrate into build
From:       Christian Boltz <apparmor () cboltz ! de>
Date:       2012-03-22 23:35:32
Message-ID: 1569237.nRfqCbVddM () tux ! boltz ! de ! vu
[Download RAW message or body]

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 <steve@nxnw.org>, based on work by
> +#    Christian Boltz <apparmor@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 <apparmor@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
[prev in list] [next in list] [prev in thread] [next in thread]