[apparmor] [patch] Write unix rules when saving a profile

Tyler Hicks tyhicks at canonical.com
Thu Dec 17 16:48:51 UTC 2015


On 2015-12-05 13:09:25, Christian Boltz wrote:
> Hello,
> 
> Am Freitag, 4. Dezember 2015 schrieb Christian Boltz:
> > r2637 added support for parsing unix rules, but forgot to add write
> > support. The result was that a profile lost its unix rules when it was
> > saved.
> > 
> > This patch adds the write_unix_rules() and write_unix() functions
> > (based on the write_pivot_root() and write_pivot_root_rules()
> > functions) and makes sure they get called at the right place.
> > 
> > The cleanprof testcase gets an unix rule added to ensure it's not
> > deleted when writing the profile. (Note that minitools_test.py is not
> > part of the default "make check", however I always run it.)
> > 
> > 
> > I propose this patch for trunk, 2.10 and 2.9, which all share this
> > bug.
> > 
> > 
> > References: https://bugs.launchpad.net/apparmor/+bug/1522938
> >             https://bugzilla.opensuse.org/show_bug.cgi?id=954104
> 
> Some manual testing with aa-logprof showed that I missed to add 'unix' 
> in one place in serialize_profile_from_old_profile(), causing a crash 
> when using "(V)iew changes". Here's v2 to fix this:

This patch looks pretty good. Can you take a look at whether or not you
need to update profile_storage() for 'unix' rules? I don't know the code
well enough to say if it is needed or not.

After making that determination, feel free to add

  Acked-by: Tyler Hicks <tyhicks at canonical.com>

Thanks!

Tyler

> 
> 
> [ 27-write-unix-rules.diff ]
> 
> === modified file ./utils/apparmor/aa.py
> --- utils/apparmor/aa.py        2015-12-05 13:02:38.958092363 +0100
> +++ utils/apparmor/aa.py        2015-12-05 13:00:37.667275750 +0100
> @@ -3369,6 +3369,24 @@
>      data += write_pivot_root_rules(prof_data, depth, 'allow')
>      return data
>  
> +def write_unix_rules(prof_data, depth, allow):
> +    pre = '  ' * depth
> +    data = []
> +
> +    # no unix rules, so return
> +    if not prof_data[allow].get('unix', False):
> +        return data
> +
> +    for unix_rule in prof_data[allow]['unix']:
> +        data.append('%s%s' % (pre, unix_rule.serialize()))
> +    data.append('')
> +    return data
> +
> +def write_unix(prof_data, depth):
> +    data = write_unix_rules(prof_data, depth, 'deny')
> +    data += write_unix_rules(prof_data, depth, 'allow')
> +    return data
> +
>  def write_link_rules(prof_data, depth, allow):
>      pre = '  ' * depth
>      data = []
> @@ -3480,6 +3498,7 @@
>      data += write_signal(prof_data, depth)
>      data += write_ptrace(prof_data, depth)
>      data += write_pivot_root(prof_data, depth)
> +    data += write_unix(prof_data, depth)
>      data += write_links(prof_data, depth)
>      data += write_paths(prof_data, depth)
>      data += write_change_profile(prof_data, depth)
> @@ -3636,6 +3655,7 @@
>                           'signal': write_signal,
>                           'ptrace': write_ptrace,
>                           'pivot_root': write_pivot_root,
> +                         'unix': write_unix,
>                           'link': write_links,
>                           'path': write_paths,
>                           'change_profile': write_change_profile,
> @@ -3651,6 +3671,7 @@
>                                  'signal',
>                                  'ptrace',
>                                  'pivot_root',
> +                                'unix',
>                                  'link',
>                                  'path',
>                                  'change_profile',
> @@ -3667,6 +3688,7 @@
>                      'signal': True, # not handled otherwise yet
>                      'ptrace': True, # not handled otherwise yet
>                      'pivot_root': True, # not handled otherwise yet
> +                    'unix': True, # not handled otherwise yet
>                      'link': False,
>                      'path': False,
>                      'change_profile': False,
> === modified file ./utils/test/cleanprof_test.in
> --- utils/test/cleanprof_test.in        2015-12-05 13:02:38.958092363 +0100
> +++ utils/test/cleanprof_test.in        2015-12-04 22:16:24.161247856 +0100
> @@ -8,6 +8,8 @@
>         allow /usr/share/X11/locale/**  r,
>         allow /home/*/** r,
>  
> +    unix (receive) type=dgram,
> +
>      ^foo {
>              /etc/fstab r,
>          capability dac_override,
> === modified file ./utils/test/cleanprof_test.out
> --- utils/test/cleanprof_test.out       2015-12-05 13:02:38.958092363 +0100
> +++ utils/test/cleanprof_test.out       2015-12-04 22:16:36.529169204 +0100
> @@ -6,6 +6,8 @@
>  /usr/bin/a/simple/cleanprof/test/profile {
>    #include <abstractions/base>
>  
> +  unix (receive) type=dgram,
> +
>    /home/*/** r,
>    /home/foo/** w,
>  
> 
> 
> 
> Regards,
> 
> Christian Boltz
> 
> PS: I know the sig is too long - but it's just one sentence ;-)
> -- 
> Wenn Du dermaleinst Deinen Enkeln von Deinen Erlebnissen im Usenet
> erzählst, daß Du also einmal ganz unschuldig eine einfache Frage
> gestellt hast, und schilderst, wie Du daraufhin wegen Deiner Tastatur,
> Einleitungszeile und allem möglichen anderen Kram angemacht wurdest,
> aber niemand auch nur im Traum daran dachte, die Frage zu beantworten,
> dann kannst Du fortfahren mit dem Satz "Also frage ich noch einmal ganz
> brav, ob jemand ein oben genanntes Tool kenne", wobei Du auch noch das
> perfektische Präsens verwendest, durch das die Erzählung lebendiger
> wirkt als in einer grammatikalischen Vergangenheitsform - ein weiteres
> jener unzähligen Details, an denen ein Grammatikprüfprogramm zuverlässig
> scheitern wird, abgesehen von all den anderen Problemen, die es mit
> einem Satz wie diesem voraussichtlich ohnehin haben wird, an dem man
> übrigens auch sehen kann, warum im Deutschen im Gegensatz zu manch
> anderen Sprachen, in denen die Sätze kürzer zu sein pflegen, die Groß-
> und Kleinschreibung wichtig ist.   [Ralf Bader in desd]
> 
> 
> -- 
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20151217/1bbdcb05/attachment-0001.pgp>


More information about the AppArmor mailing list