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

[Christian Boltz]

Hello,

Am Dienstag, 22. Dezember 2015 schrieb Christian Boltz:
> 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.
...
> [ 43-prevent-crash-by-serialize_profile_from_old_profile.diff ]

To answer Steve's comment at the "right" place:

    | (Yes, I know I missed 43. It requires a little more thought/effort
    | than I have time for at the moment.)

Well, the TL;DR version is that we can choose between
a) catch errors in serialize_profile_from_old_profile() and handle them in
   a as-sane-as-possible way (that's what this patch does)
b) crash aa-logprof if serialize_profile_from_old_profile() fails

The long version:

serialize_profile_from_old_profile() has some known breakage [1] and needs 
a full rewrite, but that needs quite some work and thought (and will 
_really_ be a patch that requires "a little more thought/effort" ;-)
(I have an idea how to implement it, but I'm not yet sure if and how 
good it will work.)

That said - I'll probably do more cleanup before touching 
serialize_profile_from_old_profile() because the cleanup will hopefully 
make it easier to rewrite this beast ;-)

So: Yes, I know this patch isn't perfect, but IMHO it is an improvement, 
but for now, I'd prefer to display an error message with a hint to use 
"view (C)lean changes" over crashing aa-logprof ;-)
The good thing is that this will only happen in corner cases (one file 
containing multiple profiles), so users won't see that message too often.

On the long term, serialize_profile_from_old_profile() needs to be 
replaced by something that is easier to maintain, has less bugs and 
doesn't crash ;-)


Regards,

Christian Boltz

[1] without looking at the bugtracker or the code, I know that
    - it fails to handle hats and child profiles (the whole hats will
      appear as removed lines in the diff, which makes the diff view 
      useless for them)
    - it crashes if a file contains more than one (main) profile 
      (prevented by this patch)
    - it sometimes adds new rules at not-so-perfect places
    or, to sum it up, it feels like the (random!) sig ;-)

-- 
wodim is based on a cdrecord from September 2004 with additional bugs
added by Debian and with the DVD support code ripped off and replaced
by something that works on weekends wit full-moon.
[Jörg Schilling in opensuse-factory]
-------------- 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/20160125/875ce703/attachment.pgp>