[apparmor] [patch] Make sure aa-cleanprof de-duplicates capability rules

[Kshitij Gupta]

Hello,

On Wed, Apr 15, 2015 at 12:34 AM, Christian Boltz <apparmor at cboltz.de>
wrote:

> Hello,
>
> Am Montag, 13. April 2015 schrieb Steve Beattie:
> > On Tue, Apr 14, 2015 at 12:50:26AM +0200, Christian Boltz wrote:
> > > Am Montag, 13. April 2015 schrieb Steve Beattie:
> > > > On Sun, Apr 12, 2015 at 03:32:25AM +0200, Christian Boltz wrote:
> > > > > CleanProf.remove_duplicate_rules() didn't call
> > > > >
> > > > >   $profile['capability'].delete_duplicates()
> > > > >
> > > > > because aa-cleanprof sets same_file=True.
> > > > >
> > > > > Fix this by calling delete_duplicates(None) so that it
> > > > > only checks the profile against itsself.
> > > > >
> > > > > [ 43-cleanprof-do-in-profile-run.diff ]
> > > > >
> > > > > === modified file 'utils/apparmor/cleanprofile.py'
> > > > > --- utils/apparmor/cleanprofile.py      2014-12-16 22:13:25
> > > > > +0000
> > > > > +++ utils/apparmor/cleanprofile.py      2015-04-11 22:35:00
> > > > > +0000
> > > > > @@ -67,6 +67,8 @@
> > > > >
> > > > >              #Clean the duplicates of caps in other profile
> > > > >
> > > > >              if not self.same_file:
> > > > >                  deleted +=
> > > > >
> > > > >self.other.aa[program][hat]['capability'].delete_duplicates(self.
> > > > >pro
> > > > >file.aa[program][hat]['capability'])
> > > > >
> > > > > +            else:
> > > > > +                deleted +=
> > > > > self.other.aa[program][hat]['capability'].delete_duplicates(None
> > > > > )>
> > > >
> > > > This patch does not seem to do what you claim it does:
> > > Did you also apply 42-in-profile-deduplication.diff before testing?
> > > Without that, there's no in-profile deduplication (removing lines
> > > covered by includes should work without patch 42).
> >
> > I didn't initially (nothing in this patch description called out
> > that it depended on that one. However, when I tried path 42 without
> > patch 43 applied, the testing that I did showed that it deleted the
> > in-profile duplicated capability, so I'm still not clear on why this
> > patch is necessary.
>
> The strange thing is that it's clearly necessary for me - I just tested
> without it, and it didn't remove in-profile duplicates.
>
> Both your test-profile (the one mentioned below) and the one Steve used
had their duplicate rules removed by the tool in my bzr branch which had I
pulled afresh from upstream (at revision 3018).

Note: I'm testing on Kubuntu 14.10 with Python 3.4 if that matters.

Thanks.

Regards,

Kshitij Gupta



> Note that I'm testing with all my pending patches applied [1], however I
> think only patch 42 is related to cleanprof.
>
> My test profile:
>
> # cat usr.bin.echo
> /usr/bin/echo {
>    audit capability chown, # drop (1)
>    capability dac_override, # drop
>    deny capability dac_override,
>    capability dac_override, # drop
>    audit capability chown, # drop (2)
>    deny capability chown, # drop
>    audit deny capability chown,
>    capability, # drop
>    audit capability,
> }
>
> Without patch 43, aa-cleanprof doesn't remove any of those rules.
> With patch 43, aa-cleanprof shrinks the profile to
>
>  /usr/bin/echo {
>   audit deny capability chown,
>   deny capability dac_override,
>
>   audit capability,
> }
>
>
> Regards,
>
> Christian Boltz
>
> [1] all pending patches means:
>     30-logparser-change-mask-only-for-path-events.diff
>     31-enable-testloops-for-nosetests.diff
>     33-fix-add-to-variable-and-add-tests.diff
>     35-fix-serialize_profile_from_old_profiles-variable-add.diff
>     36-fix-crash-in-serialize_profile_from_old_profiles.diff
>     39-aatest-maxdiff.diff
>     41-add-baserule-tests.diff
>     42-in-profile-deduplication.diff
>     43-cleanprof-do-in-profile-run.diff
>
> --
> > > dank meiner Versionitis verwende ich längst die 10.1 ;-)
> > Das Spielchen habe ich auch mitgemacht - von 6.0 bis 9.3. Nu reichts,
> > man soll schließlich arbeiten mit dem Ding.
> Zum Arbeiten braucht es kein unsupportetes Supplementary!
> [>>Christian Boltz, > Christian Lepper & Marcus Meissner in suse-laptop]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150426/a7bdf82f/attachment.html>