Hello,
Am Donnerstag, 4. Juni 2015 schrieb Steve Beattie:
> On Thu, Jun 04, 2015 at 01:45:33PM +0200, Christian Boltz wrote:
> > > 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
So for the /{usr/,}bin/ping profile, we have:
# ls -l /bin/ping /usr/bin/ping
lrwxrwxrwx 1 root root 13 27. Apr 22:23 /bin/ping -> /usr/bin/ping
-rwxr-xr-x 1 root root 39384 27. Apr 22:23 /usr/bin/ping
Well, things aren't that easy... ;-)
(and no, I don't want to readlink() etc. to check if all names point to
the same binary - that would mean to cover one of 100 corner cases, and
leave the other 99 uncovered)
> 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).
It's probably even less likely that a * or ** applies to only one binary ;-)
> > 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.
I hope you are joking ;-) - that would end up with a big mess.
> > 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/*'
I still think this is the only reliable solution.
> > > 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.
I know, but even if it's annoying, it has an advantage: we automatically
make sure it works with real-world profiles, not only outdated testcases.
> > > 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:
That's not a regression. We have this problem since we have aa.py -but
nobody noticed it (until I opened the bugreport) ;-)
> *** 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
I'm not surprised.
Minimal testcase:
python3 -c 'from apparmor.aa import fetch_profiles_by_name' # that function
just does "return None, None"
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/cb/apparmor/HEAD-patches-applied/utils/apparmor/aa.py", line
4354, in <module>
if cfg['settings'].get('default_owner_prompt', False):
File "/usr/lib64/python3.4/configparser.py", line 937, in __getitem__
raise KeyError(key)
KeyError: 'settings'
The problem is that aa.py reads logparser.conf in the global area
(for details what it does exactly, scroll down to the end of the file),
so you'll trigger reading /etc/apparmor/logprof.conf by just importing
a function from apparmor.aa
An interesting detail is that it errors out quite late - so configparser
doesn't error out on non-existing files:
# python3
Python 3.4.1 (default, May 23 2014, 17:48:28) [GCC] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import configparser
>>> config = configparser.ConfigParser()
>>> config.read('/foo/bar')
[]
>>>
Oh, and the documentation says that's intended:
https://docs.python.org/3.4/library/configparser.html#configparser.ConfigParser.read
Hmm, should we add a "file exists?" check in apparmor.config so that we
get a clear error message instead of an empty result? Or should we just
make sure to have good defaults for everything so that the tools (and
the tests) work even without /etc/apparmor/logprof.conf?
(That would still leave the issue that an existing logprof.conf can
influence test results.)
Regards,
Christian Boltz
--
It's never too late for a new feature
(C) Novell
:-)
[Marcel Hilzinger in opensuse-factory]
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
