[apparmor] [patch] Allow aa-complain etc. to change profiles for non-existing binaries

[Steve Beattie]

On Thu, Jun 04, 2015 at 01:45:33PM +0200, Christian Boltz wrote:
> > > [ 36-allow-aa-complain-for-non-existing-binary.diff ]
> > 
> > Acked-by: Steve Beattie <steve at nxnw.org>
> 
> Thanks.
> 
> I forgot to ask - should I also commit this patch to the 2.9 branch?

That's fine.

> > Once we had that, a possible solution would be to apply the change
> > above to a regex applied profile if that profile is the only one to
> > apply to that binary; 
> 
> I agree with the intention, but you ignored a side effect.
>
> Let's say I have a profile /usr/lib/dovecot/* and run aa-complain 
> /usr/lib/dovecot/anvil. With your proposal, the /usr/lib/dovecot/* 
> profile would be changed, which means the change affects all the 
> binaries in /usr/lib/dovecot.

Sigh, sorry, I didn't complete what I'd been thinking, which was that it
would need to satisfy to conditions:

  1) only one profile applies

  2) the regex match pattern only applies to one existing binary. I
     grant this latter one is harder to check, but should be possible
     to do (we could start by supporting a subset of aa-regex pattern
     types).

> That will at least surprise the user, and sounds like unexpected 
> behaviour to me ("hey, I only wanted to change the profile for anvil!").

An alternative solution would be to create a separate profile copy
for just the passed path.

> Somehow I'm afraid we should just error out with an useful error message 
> like:
> 
>     /usr/lib/dovecot/anvil is using a profile that is shared with 
>     multiple binaries. If you really want to change it, please use
>         aa-complain '/usr/lib/dovecot/*'
> 
> > if a non-regex match and a regex match would
> > both apply, update the non-regex one (since I believe that's the
> > one the kernel will prefer); 
> 
> Agreed (and that's also what we currently do).
> 
> > if multiple regex matches apply then
> > perhaps an error is in order, but I've forgotten the semantics of
> > kernel behavior there.
> 
> Yes, and see above.
> 
> > > Oh, and this patch also fixes the last failure in minitools_test.py.
> > > (Should we rename it to test-minitools.py to include it in "make
> > > check"?)
> > I get seven failures with minitools_test.py unless I have pre-run make
> > in the profiles directory.
> 
> Indeed. I always have apparmor.d/local/ populated, so I didn't notice 
> this. So we can choose between a) calling "make -C ../../profiles" from 
> utils/test/Makefile and b) copying over the whole ../../profiles 
> directory and run make inside each test tempdir.

Well, the root cause is really that our minitools tests depend on
the state of external files outside of the tests control.

> > It should be using a temp dir per-test. to populate profiles into,
> > rather than CWD (you added infrastructure to common_test.py to make
> > this relatively painless), in no small part to ensure that the tests
> > independent of each other (ensuring that a failure in one test case to
> > change the state of a profile doesn't cause another test case to
> > fail).
> > 
> > The calls to subprocess.check_output() should not need to use a shell.
> > Although, taking the cmd() function from test-aa-decode.py might be
> > preferred, as the output from the cmd can be reported in the test
> > failure, rather than just going to stderr (would need to catch the
> > subprocess.CalledProcessError exception to do the same with
> > subprocess.check_output()).
> 
> Both added to my TODO list.
> 
> > It has the same issue as test-aa.py, in that the import of aa.py
> > fails if /etc/apparmor is not populated. This prevents make check from
> > being run as part of a bootstrapping build process where portions of
> > apparmor have not already been installed.
> 
> Yes, that's https://bugs.launchpad.net/apparmor/+bug/1393979 and 
> probably affects lots of (all?) tests.

Ugh, we've regressed a lot there, these are the tests according to
runtests-py3.sh that fail due to this issue:

*** The following tests failed:
***    aa_test.py minitools_test.py test-aa.py test-dbus_parse.py
       test-mount_parse.py test-pivot_root_parse.py test-ptrace_parse.py
       test-regex_matches.py test-signal_parse.py test-unix_parse.py

-- 
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/20150604/e38c2cca/attachment.pgp>