[apparmor] [patch] update aa.py is_skippable_file() according to libapparmor
Kshitij Gupta
kgupta8592 at gmail.com
Tue Feb 3 20:52:37 UTC 2015
Hello,
On Wed, Feb 4, 2015 at 1:28 AM, Christian Boltz <apparmor at cboltz.de> wrote:
> Hello,
>
> [resent without linebreaks in the patch]
>
> Am Dienstag, 3. Februar 2015 schrieb Christian Boltz:
> > Am Donnerstag, 25. Dezember 2014 schrieb Kshitij Gupta:
> > > On Thu, Dec 25, 2014 at 6:02 AM, Christian Boltz wrote:
> > > > Am Mittwoch, 24. Dezember 2014 schrieb Kshitij Gupta:
> > > > > On Sun, Dec 7, 2014 at 2:49 AM, Christian Boltz wrote:
> > <big snip, scroll down for updated patch>
> >
> > > > I'd also be interested in some numbers - how long do the following
> > > > variants take?
> > > >
> > > > a) the "old" code we currently have in bzr (hint: it does 4
> > > > re.search)
> > >
> > > 0.185 s for 1000 (x13 in each for all cases) calls to functions
> > >
> > > > b) with my patch applied (only one re.search, so I'd expect it to
> > > > be
> > > >
> > > > faster)
> > >
> > > 0.160 s for 1000 (x13 in each for all cases) calls to functions
> >
> > That difference isn't too big.
> >
> > >> c) with my patch applied + using re.compile for skippable_files
> > >>
> > > > c1) with re.compile inside the function (= one re.compile per
> > > > file)
> > >
> > > 0.144 s for 1000 (x13 in each for all cases) calls to functions
> >
> > So re.compile already improves things for a regex used only once.
> >
> > > > c2) with re.compile outside the function (= only one re.compile
> > > > in total)
> > >
> > > 0.065 s for 1000 (x13 in each for all cases) calls to functions
> >
> > That's a clear sign that we should use re.compile() everywhere,
> > ideally in the global area (outside of any functions) for often-used
> > regexes so that each regex is compiled only once.
> >
> > > d) with the code using os.path.* you proposed
> > >
> > > Using os.path.* and in operator:
> > > 0.082 s for 1000 (x13 in each for all cases) calls to functions
> > >
> > > Using os.path.basename followed by your suggested endswith for
> > > basename: 0.052 s for 1000 (x13 in each for all cases) calls to
> > > functions
> >
> > This variant is fast, easily readable, avoids the escaping we would
> > need for a regex and avoids having something (the compiled regex)
> > "polluting" the module namespace, so it's the way to go :-)
> >
> > > I've attach the test scripts, separate files for all 6 cases.
> >
> > Thanks, and thanks for all the testing!
> >
> > Here's the updated patch, based on your "ext6.py". I only changed the
> > aa.py part, the testcases didn't change since v1.
> >
> >
> >
> > Update is_skippable_file() to match all extensions that are listed in
> > libapparmor _aa_is_blacklisted() - some extensions were missing in the
> > python code.
> >
> > Also make the code more readable and add some testcases.
> >
> > Notes:
> > - the original code additionally ignored *.swp. I didn't include that
> > - *.swp looks like vim swap files which are also dot files
> > - the python code ignores README files, but the C code doesn't
> > (do we need to add README in the C code?)
> >
> >
> [ is_skippable_file.diff ]
>
> === modified file 'utils/apparmor/aa.py'
> --- utils/apparmor/aa.py 2015-01-30 20:08:17 +0000
> +++ utils/apparmor/aa.py 2015-02-03 18:12:37 +0000
> @@ -2539,14 +2539,22 @@
> else:
> return False
>
> -# rpm backup files, dotfiles, emacs backup files should not be processed
> -# The skippable files type needs be synced with apparmor initscript
> +
> def is_skippable_file(path):
> - """Returns True if filename matches something to be skipped"""
> - if (re.search('(^|/)\.[^/]*$', path) or re.search('\.rpm(save|new)$',
> path)
> - or re.search('\.dpkg-(old|new)$', path) or
> re.search('\.swp$', path)
> - or path[-1] == '~' or path == 'README'):
> - return True
> + """Returns True if filename matches something to be skipped (rpm or
> dpkg backup files, hidden files etc.)
> + The list of skippable files needs to be synced with apparmor
> initscript and libapparmor _aa_is_blacklisted()
> + path: filename (with or without directory)"""
> +
> + basename = os.path.basename(path)
> +
> + if not basename or basename[0] == '.' or basename == 'README':
> + return True
> +
> + skippable_suffix = ('.dpkg-new', '.dpkg-old', '.dpkg-dist',
> '.dpkg-bak', '.rpmnew', '.rpmsave', '.orig', '.rej', '~')
> + if basename.endswith(skippable_suffix):
> + return True
> +
> + return False
>
> def is_skippable_dir(path):
> if re.search('(disable|cache|force-complain|lxc)', path):
>
> === modified file 'utils/test/test-aa.py'
> --- utils/test/test-aa.py 2014-11-27 22:20:26 +0000
> +++ utils/test/test-aa.py 2015-02-03 17:57:25 +0000
> @@ -15,7 +15,7 @@
> import tempfile
> from common_test import write_file
>
> -from apparmor.aa import check_for_apparmor
> +from apparmor.aa import check_for_apparmor, is_skippable_file
>
> class AaTest_check_for_apparmor(unittest.TestCase):
> FILESYSTEMS_WITH_SECURITYFS =
> 'nodev\tdevtmpfs\nnodev\tsecurityfs\nnodev\tsockfs\n\text3\n\text2\n\text4'
> @@ -70,6 +70,48 @@
> mounts = write_file(self.tmpdir, 'mounts',
> self.MOUNTS_WITH_SECURITYFS % self.tmpdir)
> self.assertEqual('%s/security/apparmor' % self.tmpdir,
> check_for_apparmor(filesystems, mounts))
>
> +class AaTest_is_skippable_file(unittest.TestCase):
> + def test_not_skippable_01(self):
> + self.assertFalse(is_skippable_file('bin.ping'))
> + def test_not_skippable_02(self):
> + self.assertFalse(is_skippable_file('usr.lib.dovecot.anvil'))
> + def test_not_skippable_03(self):
> + self.assertFalse(is_skippable_file('bin.~ping'))
> + def test_not_skippable_04(self):
> + self.assertFalse(is_skippable_file('bin.rpmsave.ping'))
> + def test_not_skippable_05(self):
> + # normally is_skippable_file should be called without directory,
> but it shouldn't hurt too much
> + self.assertFalse(is_skippable_file('/etc/apparmor.d/bin.ping'))
> + def test_not_skippable_06(self):
> + self.assertFalse(is_skippable_file('bin.pingrej'))
> +
> + def test_skippable_01(self):
> + self.assertTrue(is_skippable_file('bin.ping.dpkg-new'))
> + def test_skippable_02(self):
> + self.assertTrue(is_skippable_file('bin.ping.dpkg-old'))
> + def test_skippable_03(self):
> + self.assertTrue(is_skippable_file('bin.ping..dpkg-dist'))
> + def test_skippable_04(self):
> + self.assertTrue(is_skippable_file('bin.ping..dpkg-bak'))
> + def test_skippable_05(self):
> + self.assertTrue(is_skippable_file('bin.ping.rpmnew'))
> + def test_skippable_06(self):
> + self.assertTrue(is_skippable_file('bin.ping.rpmsave'))
> + def test_skippable_07(self):
> + self.assertTrue(is_skippable_file('bin.ping.orig'))
> + def test_skippable_08(self):
> + self.assertTrue(is_skippable_file('bin.ping.rej'))
> + def test_skippable_09(self):
> + self.assertTrue(is_skippable_file('bin.ping~'))
> + def test_skippable_10(self):
> + self.assertTrue(is_skippable_file('.bin.ping'))
> + def test_skippable_11(self):
> + self.assertTrue(is_skippable_file('')) # empty filename
> + def test_skippable_12(self):
> + self.assertTrue(is_skippable_file('/etc/apparmor.d/')) #
> directory without filename
> + def test_skippable_13(self):
> + self.assertTrue(is_skippable_file('README'))
> +
>
> if __name__ == '__main__':
> unittest.main(verbosity=2)
>
> Thanks for the patch and tests.
Acked for 2.9 and mainline.
Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>.
Regards,
Kshitij Gupta
>
>
> Regards,
>
> Christian Boltz
> --
> "Bei mir" läuft KDE gar nicht.
> Völlig korrekt. Logisch. Aber sinnfrei.
> [David Haller in opensuse-de]
>
>
> --
> 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/20150204/2c677ed2/attachment-0001.html>
More information about the AppArmor
mailing list