[apparmor] RFC: The future of "(V)iew Changes" in aa-logprof

[Christian Boltz]

Hello,

at the openSUSE Conference, I talked with John about how aa-logprof 
writes profiles for the diff view, just came up with a possible solution 
I didn't consider before, and would now like to hear some feedback ;-)


As you know, aa-logprof has two options to display a profile diff:
- (V)iew Changes
- View Changes b/w (C)lean profiles

The first option uses some quite old code [1] (including a few known 
bugs) to write the updated profile, and then shows the diff to the 
profile in /etc/apparmor.d/. It has the intention to keep the diff 
readable, but this doesn't always work.

The second option writes both the old and new profile in "clean" mode 
[2] and then diffs them - which in practise makes the diff much shorter, 
but also hides the cleanup changes that will happen to the profile when 
writing it.

When actually writing the profile, it always gets written in clean mode. 
This means both diff variants do not really represent what will happen 
to the file in /etc/apparmor.d:
- (V)iew Changes uses write code that differs from how the profile 
  really gets written (diff view uses [1], but writing the profile 
  happens in clean mode [2])
- View Changes b/w (C)lean profiles uses the correct write code [2], but
  does the diff to a "clean" old profile instead of the real old profile
  in /etc/apparmor.d/. This keeps the diff very readable, but also means
  the cleanup that will be done to the profile doesn't get displayed in 
  the diff.


Now the question is - what behaviour do we want in the future?

My prefered option would be to change (V)iew Changes so that it writes 
the new profile in clean mode instead of least-possible-changes mode.
Advantages would be:
- it gives users the "real" diff that will be applied when writing the 
  profile
-  it would be the easiest way - basically I could drop the code to 
  write the profile with minimum changes [1], and would have at least
  400 lines less to maintain [3]
- it would make future changes to aa-logprof (like support for nested 
  child profiles) easier because we'd have only one way to write 
  profiles


An alternative would be to completely rewrite the (V)iew Changes code so 
that it writes a profile with minimum changes - and then introduce an 
option to really write the profile in that mode.
Needless to say that this is quite some work, and I seriously doubt if 
it's worth the effort.



Since this mail is quite long, let me sum it up in pseudocode:

(V)iew Changes (current implementation):
    - write_new_profile_with_minimum_changes to tempfile
    - diff /etc/apparmor.d/$profile $new_profile_with_minimum_changes
    - write new_profile_clean to /etc/apparmor.d

Proposed behaviour for (V)iew Changes:
    - write_new_profile_clean to tempfile
    - diff /etc/apparmor.d/$profile $new_profile_clean
    - write new_profile_clean to /etc/apparmor.d

View Changes b/w (C)lean profiles:
    - write_new_profile_clean to tempfile
    - write_old_profile_clean to tempfile
    - diff $old_profile_clean $new_profile_clean
   - write new_profile_clean to /etc/apparmor.d

Opinions?


Regards,

Christian Boltz

[1] serialize_profile_from_old_profile() in aa.py

[2] serialize_profile() in aa.py

[3] serialize_profile_from_old_profile() has about 400 lines, so this
    number doesn't even include helper functions that might become 
    superfluous

-- 
Well, I guess, Stephan knows very well, what the fuzz is about: it's
about hundreds of patches, which will have to be regenerated, done as
an employment-creation measure for this lazy gang of packagers.
[Hans-Peter Jansen in opensuse-packaging]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20180603/68b11600/attachment.sig>