[apparmor] [patch] Prevent crash caused by by serialize_profile_from_old_profile()

[Christian Boltz]

Hello,

if a profile file contains multiple profiles and one of those profiles
contains a rule managed by a *Ruleset class,
serialize_profile_from_old_profile() crashes with an AttributeError.

This happens because profile_data / write_prof_data contain only one
profile with its hats, which explodes if a file contains multiple
profiles, as reported in lp#1528139

Fixing this would need lots of
    write_prof_data[hat] -> write_prof_data[profile][hat]
changes (and of course also a change in the calling code) or, better
option, a full rewrite of serialize_profile_from_old_profile().

Unfortunately I don't have the time to do the rewrite at the moment (I
have other things on my TODO list), and changing write_prof_data[hat]
-> write_prof_data[profile][hat] is something that might introduce more
breakage, so I'm not too keen to do that.

Therefore this patch wraps the serialize_profile_from_old_profile() call
in try/except. If it fails, the diff will include an error message and
recommend to use 'View Changes b/w (C)lean profiles' instead, which is
known to work.

Note: I know using an error message as 'newprofile' isn't an usual way
to display an error message, but I found it more intuitive than
displaying it as a warning (without $PAGER).


References: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139


I propose this patch for trunk and 2.10
(2.9 "just" displays a wrong diff, but doesn't crash.)


[ 43-prevent-crash-by-serialize_profile_from_old_profile.diff ]

=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py        2015-12-21 00:13:57.215799543 +0100
+++ utils/apparmor/aa.py        2015-12-22 00:04:28.306757169 +0100
@@ -2368,7 +2368,12 @@
                         oldprofile = aa[which][which]['filename']
                     else:
                         oldprofile = get_profile_filename(which)
-                    newprofile = serialize_profile_from_old_profile(aa[which], which, '')
+
+                    try:
+                        newprofile = serialize_profile_from_old_profile(aa[which], which, '')
+                    except AttributeError:  
+                        # see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
+                        newprofile = "###\n###\n### Internal error while generating diff, please use '%s' instead\n###\n###\n" % _('View Changes b/w (C)lean profiles')
 
                     display_changes_with_comments(oldprofile, newprofile)
 
@@ -3638,6 +3643,11 @@
     write_filelist = deepcopy(filelist[prof_filename])
     write_prof_data = deepcopy(profile_data)
 
+    # XXX profile_data / write_prof_data contain only one profile with its hats
+    # XXX this will explode if a file contains multiple profiles, see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
+    # XXX fixing this needs lots of write_prof_data[hat] -> write_prof_data[profile][hat] changes (and of course also a change in the calling code)
+    # XXX (the better option is a full rewrite of serialize_profile_from_old_profile())
+
     if options:  # and type(options) == dict:
         if options.get('METADATA', False):
             include_metadata = True


Regards,

Christian Boltz
-- 
mv ~/Hirn ~/Sieb
[David Haller in suse-linux]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20151222/ebfcbf30/attachment.pgp>