Thanks to reading the wrong directory in read_inactive_profiles()
(profile_dir instead of extra_profile_dir), aa-genprof never asked about
using a profile from the extra_profile_dir.
Sounds like an easy fix, right? ;-)
After fixing this (last chunk), several other errors popped up, one
after the other:
- get_profile() missed a required parameter in a serialize_profile() call
- when saving the profile, it was written to extra_profile_dir, not to
profile_dir where it (as a now-active profile) should be. This is
fixed by removing the filename from existing_profiles{} so that it can
pick up the default name.
- CMD_FINISHED (when asking if the extra profile should be used or a new
one) behaved exactly like CMD_CREATE_PROFILE, but this is surprising
for the user. Remove it to avoid confusion.
- displaying the extra profile was only implemented in YaST mode
- get_pager() returned None, not an actual pager. Since we have 'less'
hardcoded at several places, also return it in get_pager()
Finally, also remove CMD_FINISHED from the get_profile() test in
test-translations.py.
(test-translations.py is only in trunk, therefore this part of the patch
is obviously trunk-only.)
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk
Acked-by: John Johansen <john.johansen@canonical.com> for trunk + a 50% ACK for 2.10 and 2.9
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9
"Everywhere" means aa-mergeprof and aa-cleanprof. In theory also
aa-logprof, but that needs some code that parses dbus log events ;-)
Also add some dbus rules to the aa-cleanprof test profiles to ensure
superfluous dbus rules get deleted.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Change aa.py to use DbusRule and DbusRuleset in profile_storage,
parse_profile_data() and write_dbus. This also means we can drop the
now unused parse_dbus_rule() and write_dbus_rules() functions.
Raw_DBUS_Rule in rules.py is now also unused and can be dropped.
Also shorten the list of known-failing tests in
test-parser-simple-tests.py. Even if the list of removals doesn't look
too long, the generated_dbus/* removals mean 1989 tests now cause the
expected failures.
OTOH, I had to add 4 tests to the known-failing list:
- 3 tests with a "wrong" order of the conditionals which the parser
accepts (which is slightly surprising, because usually we enforce the
order of rule parts)
- one test fails because the path in the path= conditional doesn't start
with / or a variable. Instead, it starts with an alternation, which
wouldn't be allowed in file rules.
Those 4 failures need more investigation, but shouldn't block this
patchset.
Finally, adjust test-regex_matches.py to import RE_PROFILE_DBUS from
apparmor.regex instead of apparmor.aa.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The tests include the two tests from test-dbus_parse.py, therefore
delete this file.
As usual, we have 100% coverage :-)
Also addd an explicit str() conversion to common_test.py to avoid
TypeError: not all arguments converted during string formatting
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Some dbus rule conditionals come with optional parenthesis. Instead of
making the regex even more complicated, use a small function to strip
those parenthesis.
Also add some tests for strip_parenthesis() to test-regex.py.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
As a preparation for the DbusRule class, add a <details> match group
to RE_PROFILE_DBUS.
Also adjust test-regex_matches.py for the added group.
Note: RE_PROFILE_DBUS is only used in aa.py, and only matches[0..2]
are used. 0 and 1 are audit and allow/deny and 2 is and stays the whole
rule (except audit and allow/deny). Therefore no aa.py changes are
needed.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
While running test-translations.py with the fixed german translations,
I noticed that I still get errors about hotkey conflicts
It turned out that test-translations.py reads the system-wide
apparmor-utils.mo in addition to the in-tree translations.
(I have the 2.11 beta1 translations installed, which contain hotkey
conflicts for the german translations).
This is surprising because test-translations.py explicitely sets the
locale path. Interestingly, this happens only 4 times (checked with a
temp profile with audit for those files) while test-translations.py has
9 tests).
(Any idea if this behaviour is normal or a bug?)
This patch adds LC_ALL=C to the make check and make coverage commandline
so that the system-wide translations don't get used.
I checked with a modified de.po that in-tree hotkey conflicts still get
detected.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This test builds and installs the apparmor-utils translations into a
tempdir, and then checks if there's any hotkey conflict in one of the
languages. This is based on a manually maintained list of "buttons" that
are displayed at the same time.
To make things a bit easier to test, add CMD_CANCEL to ui.py CMDS[].
Also replace hardcoded usage of '(Y)es', '(N)o' and '(C)ancel' with
CMDS['CMD_YES'], CMDS['CMD_NO'] and CMDS['CMD_CANCEL'].
Acked-by: Seth Arnold <seth.arnold@canonical.com>
If the program specified as get_output param isn't executable or doesn't
exist at all, get_output() returns with ret = -1.
Raising an exception looks like a better option, especially because
other possible exec failures already raise an exception ("Unable to
fork").
Note: get_output is only used by get_reqs() which also does the
os.access() check for x permissions (and raises an exception), so in
practise raising an exception in get_output() doesn't change anything.
This change also allows to rewrite and simplify get_output() quite a bit.
Another minor change (and fix) is in the removal of the last line. The
old code removed the last line if output contained at least two items.
This had two not-so-nice effects:
- an empty output resulted in [''] instead of []
- if a command didn't add a \n on the last line, this line was deleted
nevertheless
The patch changes that to always remove the last line if it is empty,
which fixes both issues mentioned above.
Also add a test to ensure the exception is really raised, and adjust the
test that expects an empty stdout.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
To make these tests independent from the underlaying system, add a
fake_ldd script that provides hardcoded ldd output for the "known"
executables and libraries.
To avoid interferences with the real system (especially symlinks), all
paths in fake_ldd have '/AATest' prepended.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
To ensure aa-cleanprof works as expected (and writing the rules works
as expected), add some rules for every rule class to the cleanprof.in
and cleanprof.out test profiles.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
parser/tst/simple_tests/profile/profile_ns_bad8.sd was added in r3376
(trunk) / r3312 (2.10 branch) and contains the profile name ':ns/t'
which misses the terminating ':' for the namespace.
Unfortunately the tools don't understand namespaces yet and just use the
full profile name. This also means this test doesn't fail as expected
when tested against the utils code.
This patch adds profile_ns_bad8.sd to the exception list of
test-parser-simple-tests.py.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.10.
On Debian and Ubuntu it's possible to have multiple ruby interpreters
installed, and the default to use is handled by the ruby-defaults
package, which includes a symlink from /usr/bin/ruby to the versioned
ruby interpreter.
This patch makes aa.py:get_interpreter_and_abstraction() take that into
account by using a regex to match possible versions of ruby. Testcases
are included. (I noticed this lack of support because on Ubuntu the ruby
test was failing because get_interpreter_and_abstraction() would get the
complete path, which on my 16.04 laptop would get /usr/bin/ruby2.2.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
PtraceRule 'access' and SignalRule 'access' and 'signal' can contain
more than one value. Therefore adjust is_covered_localvars() in both
to use the list (subset) instead of the plain (exactly equal) check.
Also add a testcase for each to ensure the list/subset check works as
expected.
Acked-by: Steve Beattie <steve@nxnw.org>
Thanks to http://bugs.python.org/issue10076, we need to implement this
ourself :-/
Also add some tests to ensure __deepcopy__() works as expected.
I found this bug while testing the dbus patch series, which crashed
aa-cleanprof with
TypeError: cannot deepcopy this pattern object
Acked-by: John Johansen <john.johansen@canonical.com>
Change aa.py to use PtraceRule and PtraceRuleset in profile_storage(),
parse_profile_data() and write_ptrace(). This also means we can drop the
now unused parse_ptrace_rule() and write_ptrace_rules() functions.
Raw_Ptrace_Rule in rules.py is now also unused and can be dropped.
Also adjust logparser.py to include the peer in the result, and shorten
the list of known-failing tests in test-parser-simple-tests.py.
Acked-by: John Johansen <john.johansen@canonical.com>
As usual, we have 100% test coverage :-)
Those tests include all tests from test-ptrace_parse.py, therefore
delete this file.
Acked-by: John Johansen <john.johansen@canonical.com>
The tests in test-ptrace_parse.py used aa.parse_ptrace_rule(), which is
based on Raw_Ptrace_Rule (= regex check + "just store it").
This patch changes the tests to test against PtraceRule.get_clean().
Since get_clean does some cleanups, the expected result slightly differs
from the original rule.
Finally switch to the AATest class and setup_all_loops() we use in most
tests.
Also change test-regex_matches.py to import RE_PROFILE_SIGNAL directly
from apparmor.regex instead of apparmor.aa (where it will vanish soon).
Acked-by: John Johansen <john.johansen@canonical.com>
As a preparation for the PtraceRule class, add a <details> match group
to RE_PROFILE_PTRACE.
Also adjust test-regex_matches.py for the added group.
Note: RE_PROFILE_PTRACE is only used in aa.py, and only matches[0..2]
are used. 0 and 1 are audit and allow/deny and 2 is and stays the whole
rule (except audit and allow/deny). Therefore no aa.py changes are
needed.
Acked-by: John Johansen <john.johansen@canonical.com>
r2637 added support for parsing unix rules, but forgot to add write
support. The result was that a profile lost its unix rules when it was
saved.
This patch adds the write_unix_rules() and write_unix() functions (based
on the write_pivot_root() and write_pivot_root_rules() functions) and
makes sure they get called at the right place.
The cleanprof testcase gets an unix rule added to ensure it's not
deleted when writing the profile. (Note that minitools_test.py is not
part of the default "make check", however I always run it.)
References: https://bugs.launchpad.net/apparmor/+bug/1522938https://bugzilla.opensuse.org/show_bug.cgi?id=954104
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk, 2.10 and 2.9.
This means:
- expect unicode (instead of str) when reading from a file in py2
- convert keys() result to a set to avoid test failures because of
dict_keys type
After this change, all tests work for both py2 and py3.
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk and 2.10.
python 3 uses only the 'str' type, while python 2 also uses 'unicode'.
This patch adds a type_is_str() function to common.py - depending on the
python version, it checks for both. This helper function is used to keep
the complexity outside of the rule classes.
The rule classes get adjusted to use type_is_str() instead of checking
for type(x) == str, which means they support both python versions.
As pointed out by Tyler, there are also some type(...) == str checks in
aare.py and rule/__init__.py which should get the same change.
Finally, add test-common.py with some tests for type_is_str().
References: https://bugs.launchpad.net/apparmor/+bug/1513880
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk and 2.10
Note: 2.10 doesn't contain SignalRule, therefore it doesn't get that
part of the patch.
Parsing variables was broken in several ways:
- empty quotes (representing an intentionally empty value) were lost,
causing parser failures
- items consisting of only one letter were lost due to a bug in RE_VARS
- RE_VARS didn't start with ^, which means leading garbage (= syntax
errors) was ignored
- trailing garbage was also ignored
This patch fixes those issues in separate_vars() and changes
var_transform() to write out empty quotes (instead of nothing) for empty
values.
Also add some tests for separate_vars() with empty quotes and adjust
several tests with invalid syntax to expect an AppArmorException.
var_transform() gets some tests added.
Finally, remove 3 testcases from the "fails to raise an exception" list
in test-parser-simple-tests.py.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
(which also implies 2.10)
Note: 2.9 doesn't have test-parser-simple-tests.py, therefore it won't
get that part of the patch.
Also adjust test-signal.py for AARE (it needs a change in _compare_obj())
and enable the regex-based tests.
Acked-by: John Johansen <john.johansen@canonical.com>
The AARE class is meant to handle the internals of path AppArmor regexes
at various places / rule types (filename, signal peer etc.). The goal is
to use it in rule classes to hide all regex magic, so that the rule
class can just use the match() method.
If log_event is given (which means handing over a raw path, not a regex),
the given path is converted to a regex in convert_expression_to_aare().
(Also, the raw path is used in match().)
BTW: The reason for delaying re.compile to match() is performance - I'd
guess a logprof run calls match() only for profiles with existing log
events, so we can save 90% of the re.compile() calls.
The patch also includes several tests.
Acked-by: John Johansen <john.johansen@canonical.com>
In detail, this means:
- handle signal events in logparser.py
- "translate" those events in aa.py - from log (logparser.py readlog())
to prelog (handle_children()) to log_dict (collapse_log()) to
log_obj (ask_the_questions())
(yes, really! :-/ - needless to say that this is ugly...)
- finally ask the user about the signal in ask_the_questions()
Also add a logparser test to test-signal.py to ensure the logparser step
works as expected.
Note that the aa.py changes are not covered by tests, however they
worked in a manual test.
Acked-by: John Johansen <john.johansen@canonical.com>
As Kshitij mentioned, abstract methods should use NotImplementedError
instead of AppArmorBug.
While changing this, I noticed that __repr__() needs to be robust against
NotImplementedError because get_raw() is not available in BaseRule.
Therefore the patch changes __repr__() to catch NotImplementedError.
Of course the change to NotImplementedError also needs several
adjustments in the tests.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
(long before branching off 2.10, therefore I'll also commit to 2.10)
It's pointless to keep a separate file for those tests - they integrate
well in test-signal.py.
After the move, test-signal_parse.py is empty and will be deleted.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This means:
- import the classes instead of RE_PROFILE_SIGNAL
- simplify signal rule parsing a lot
- drop the (now unused) functions parse_signal_rule() and write_signal_rules()
- change write_signal() to use the SignalRuleset class
Also drop the now unused Raw_Signal_Rule from rules.py.
Finally, drop most parser signal tests from the "known wrong results"
blacklist in test-parser-simple-tests.py because those tests succeed
with SignalRule.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
The tests in test-signal_parse.py used aa.parse_signal_rule(), which is
based on Raw_Signal_Rule (= regex check + "just store it").
This patch changes the tests to test against SignalRule.get_clean().
Since get_clean() does some cleanups, the expected result slightly
differs from the original rule.
Finally switch to the AATest class and setup_all_loops() we use in most
tests.
Also change test-regex_matches.py to import RE_PROFILE_SIGNAL directly
from apparmor.regex instead of apparmor.aa (where it will vanish soon).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Those classes will be used to parse and handle signal rules.
They understand the (surprisingly complex) syntax of signal rules.
Note that get_clean() doesn't output superfluos things, so
signal ( send ) set = ( int ),
will become
signal send set=int,
Also add a set of tests (100% coverage :-) to make sure everything works
as expected.
This is a merged commit of the following patches:
- 07-add-SignalRule-and-SignalRuleset.diff
- 13-test-signal-compare_obj.diff
- 17-signal-rule-cleanup.diff
- 21-test-signal-rename-tests.diff
- 22-signal-rule-adjustments.diff
- 24-signal-rule-fix-error-message.diff
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
(all patches in this commit)
As a preparation for the SignalRule class, add a <details> match group
to RE_PROFILE_SIGNAL.
Also adjust test-regex_matches.py for the added group.
Note: RE_PROFILE_SIGNAL is only used in aa.py, and only matches[0..2]
are used. 0 and 1 are audit and allow/deny and 2 is and stays the whole
rule (except audit and allow/deny). Therefore no aa.py changes are
needed.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.10
The last utils/test/Makefile change switched to using the in-tree
libapparmor by default (unless USE_SYSTEM=1 is given). However, I missed
to add the swig/python parts of libapparmor to PYTHONPATH, so the
system-wide LibAppArmor/__init__.py was always used.
This patch adds the in-tree libapparmor python module to PYTHONPATH.
I'm sorry for the interesting[tm] way to find out that path, but
a) I don't know a better / less ugly way and
b) a similar monster already works in libapparmor/swig/python/test/ ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9 and trunk
(that also implies 2.10 ;-)
This makes print()ing a class object much more helpful - instead of
<apparmor.rule.network.NetworkRule object at 0x7f416b239e48>
we now get something like
<NetworkRule> network inet stream,
(based on get_raw())
A NetworkRuleset will be printed as (also based on get_raw())
<NetworkRuleset>
network inet stream,
allow network inet stream, # comment
</NetworkRuleset>
Also add tests to test-network.py to ensure that __repr__() works as
expected.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
If a script contains a hashbang like
#! /usr/bin/perl -w
aa-autodep created a profile entry like
"/usr/bin/perl -w" ix,
which is obviously incorrect.
This patch fixes this (by using only the first part of the hashbang line)
and also adds some tests for it.
References: https://bugs.launchpad.net/apparmor/+bug/1505775
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Bug: https://launchpad.net/bugs/1393979
Both create_new_profile() and handle_children() check if the given exec
target is a script and add permissions for the interpreter and a
matching abstraction.
This patch merges that into the get_interpreter_and_abstraction()
function and changes create_new_profile() and handle_children() to use
this function.
A nice side effect is that handle_children() now knows more abstractions
(its original list was incomplete).
The behaviour of create_new_profile() doesn't change.
Also add tests for get_interpreter_and_abstraction() to make sure it
does what we expect.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Bug: https://launchpad.net/bugs/1505775
These tests ensure that create_new_profile() sets the expected basic
permissions for scripts and non-script files.
Acked-by: John Johansen <john.johansen@canonical.com>
Also add support for the USE_SYSTEM variable, which means:
- test against the in-tree libapparmor and python modules by default
- test against the system libapparmor and python modules if USE_SYSTEM
is set
The old behaviour was a mix of both - it always used the in-tree python
modules and the system libapparmor.
For obvious reasons, you'll need to build libapparmor before running the
tests (unless you specify USE_SYSTEM=1 as parameter to make check).
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
Add a testcase that parses all tests in the parser/tst/simple_tests/
directory with parse_profile_data() to ensure that everything with valid
syntax is accepted, and that all tests marked as FAIL raise an
exception.
This already resulted in
- several patches to fix low-hanging fruits (including some bugs in the
parser simple_tests itsself)
- a list of tests that don't behave as expected. Those files get their
expected result reverted to make sure we notice any change in the
tools behaviour, especially changing to the really expected resulted.
This method also makes sure that the testcase doesn't report any of
the known failures.
- a 5% improvement in test coverage - mostly caused by nearly completely
covering parse_profile_data.
- addition of some missing testcased (as noticed by missing coverage),
for example several "rule outside of a profile" testcases.
As indicated above, the tools don't work as expected on all test
profiles - most of the failures happen on expected-to-fail tests that
pass parse_profile_data() without raising an exception. There are also
some tests failing despite valid syntax, often with rarely used syntax
like if conditions and qualifier blocks.
Most of the failing (generated) tests are caused by features not
implemented in the tools yet:
- validating dbus rules (currently we just store them without any parsing)
- checks for conflicting x permissions
- permissions before path ("r /foo,")
- 'safe' and 'unsafe' keywords for *x rules
- 'Pux' and 'Cux' permissions (which actually mean PUx and CUx, and get
rejected by the tools - ideally the generator script should create
PUx and CUx tests instead)
skip_startswith excludes several generated tests from being run. I know
that skip_startswith also excludes tests that would not fail, but the
generated filenames (especially generated_x/exact-*) don't have a
pattern that I could easily use to exclude less tests - and I'm not too
keen to add a list with 1000 single filenames ;-)
Acked-by: John Johansen <john.johansen@canonical.com>
With this addition, all globbing styles (as documented in apparmor.d(5))
are covered in the convert_regexp() tests.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The tests for convert_regexp() were hidden in common_test.py, where they
were never executed.
This patch moves them to the new file test-aare.py and also converts the
regex_tests.ini to a tests[] array to have the test data inside the test
file. (All tests from regex_tests.ini are in test-aare.py, and two tests
with prepended and appended path segments were added.)
Also add some tests that check the raw behaviour of convert_regexp() -
the tests "by example" are probably more useful and for sure more
readable ;-) but I want to have some examples of the converted regexes
available.
Acked-by <timeout>
logparser.py does a regex check on log lines as performance improvement
so that it only hands over lines that look like AppArmor events to
LibAppArmor parsing. Those regexes were incomplete and didn't cover all
log formats LibAppArmor accepts, with the end result of "overlooking"
events.
This patch splits off common parts of the regex, adds more regexes for
several log types and finally merges everything into one regex.
test-libapparmor-test_multi.py now also checks all test_multi log lines
against the regex to ensure logparser.py doesn't silently ignore events.
test-logparser.py gets adjusted to the merged RE_LOG_ALL regex.
Finally, add a new test that was posted on IRC to the test_multi set.
As already threatened nearly a month ago,
Acked by <timeout> for trunk and 2.9
This testcase will parse all libraries/libapparmor/testsuite/test_multi
tests and compare the result with the *.out files.
It also include a "ToDo list" of keywords that are not yet supported in
the python code - those are typically related to rule types not
supported in the tools yet (dbus, signal etc.).
An interesting special case are exec events with network details:
testcase01.in, testcase12.in, testcase13.in
which might be hand-made, invalid logs, but nobody remembers ;-)
Acked-by <timeout>
Profile name and attachment can contain variables, so the
RE_PROFILE_START regex should accept it.
(Note: the variable content isn't checked.)
Also add some tests with variables.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
- allow only a specific set of time units
- optionally allow whitespace between rlimit value and unit
- move check for invalid time units to time_to_int()
Also update the tests:
- add several tests with whitespace between value and unit
- change a test that used the (now invalid) "1m" to "1min"
- change the time_to_int() tests to use 'us' as default unit, and add
a test with 'seconds' as default unit
Acked-by: Steve Beattie <steve@nxnw.org>
