[apparmor] [patch] rewrite set_profile_flags() to use write_header()

[Steve Beattie]

On Thu, Apr 02, 2015 at 11:35:58AM -0700, Steve Beattie wrote:
> On Thu, Apr 02, 2015 at 11:15:41AM -0700, Steve Beattie wrote:
> > On Sun, Mar 15, 2015 at 10:20:16PM +0100, Christian Boltz wrote:
> > > Changes in set_profile_flags():
> > > - rewrite set_profile_flags to use parse_profile_start_line() and
> > >   write_header().
> > > - replace the silent failure for non-existing files with a proper
> > >   exception (using lazy programming - the check is done by removing the
> > >   "if os.path.isfile()" check, open_file_read then raises the
> > >   exception ;-)
> > > - comment out regex_hat_flag and the code that was supposed to handle
> > >   hat flags, which were totally broken. We'll need another patch to fix
> > >   it, and we also need to decide if we want to do that because it
> > >   introduces a behaviour change (currently, aa-complain etc. don't
> > >   change hat flags).
> > > 
> > > The tests for set_profile_flags() are also updated:
> > > - prepend a space to comments because write_header always adds a space
> > >   between '{' and the comment
> > > - remove a test with superfluous quotes that are no longer kept (that's
> > >   just a profile cleanup, so dropping that test is the easiest way)
> > > - update test_set_flags_10 and test_set_flags_12 to use the correct
> > >   profile name
> > > - enable the tests for invalid (empty) flags
> > > - update the test for a non-existing file
> > > 
> > > Note: test_set_flags_10, test_set_flags_12 and test_set_flags_nochange_09
> > > will fail with this patch applied. The next patch will fix that.
> > 
> > More than that fail under python2.7 with this patch:
> > 
> > $ PYTHONPATH=.. python test-aa.py  2>&1 | grep ^FAIL
> > FAIL: test_set_flags_10 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_12 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_02 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_05 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_06 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_07 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_09 (__main__.AaTest_set_profile_flags)
> > FAILED (failures=7)
> > 
> > as opposed to
> > 
> > $ PYTHONPATH=.. python3 test-aa.py  2>&1 | grep ^FAIL
> > FAIL: test_set_flags_10 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_12 (__main__.AaTest_set_profile_flags)
> > FAIL: test_set_flags_nochange_09 (__main__.AaTest_set_profile_flags)
> > FAILED (failures=3)
> > 
> > This is as far as I've gotten with debugging (thanks to nose
> > being able to drop me into pdb on a test failiure), this is how
> > AaTest_set_profile_flags.test_set_flags_nochange_02 is failing:
> > 
> > (Pdb) print new_prof
> >    /foo flags=(  complain  ) {
> > 
> >   #include <abstractions/base>
> >   capability chown,
> >   /bar r,
> > }
> > 
> > (Pdb) print real_new_prof
> >   /foo flags=(  complain  ) {
> > 
> >   #include <abstractions/base>
> >   capability chown,
> >   /bar r,
> > }
> > 
> > Note the extra space at the beginning of the profile name line in
> > new_prof? I haven't figured out why that gets added, and why it only
> > gets added with python 2.7.
> 
> The difference is this:
> 
> $ python2.7 -c "print(len('   ')/2)"
> 1
> $ python3 -c "print(len('   ')/2)"
> 1.5

So importing division from __future__ gives floating point division
by default in python 2.7, to match the behavior of division in python3.
Adding it to apparmor/aa.py lets the tests that were failing only under
python 2.7 pass, but I'm a little leery of adding it, as grepping for
division in aa.py gives at least:

                data += write_methods[segs](prof_data, int(depth / 2))
                        data += write_header(write_prof_data[name], int(depth / 2), name, False, include_flags)
                    depth = int(len(line) - len(line.lstrip()) / 2) + 1
                    depth = int((len(line) - len(line.lstrip())) / 2)

so I'm concerned we'd be introducing other changes by doing
so... though getting consistent behavior across python2 and python3
would be a benefit, I'd say.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20150402/3bc54d5d/attachment.pgp>