Starting with python 3.6, the re.LOCALE flag can only be used with byte
patterns, and errors out if used with str. This patch removes the flag
in get_translated_hotkey().
References: https://bugs.launchpad.net/apparmor/+bug/1661766
Acked-by: Steve Beattie <steve@nxnw.org> for trunk, 2.10 and 2.9
aa.py has a global variable "pid", but it also has several functions
that use "pid" as a local variable name. do_logprof_pass() even uses
both - first, it passes the global variable to ReadLog, and then it
creates a local variable in the "for pid in ..." loop.
This patch renames the global variable to log_pid to get rid of the
confusion.
Note that the global variable is only handed over to ReadLog, and the
only case where its previous content _might_ be used is aa-genprof which
does multipe do_logprof_pass() runs.
Maybe we could even get rid of this variable in aa.py and make it local
to the ReadLog class, but I'm not sure if that would affect aa-genprof
in interesting[tm] ways.
Acked-by: John Johansen <john.johansen@canonical.com>
Grepping through the code shows that running_under_genprof,
unimplemented_warning, ALL, t, seen and skip are unused, so drop them.
Acked-by: Steve Beattie <steve@nxnw.org>
Also drop a '# t = hasher()" comment, as noticed by Steve.
Replace most of aa-mergeprof ask_merge_questions() with a call to
aa.py ask_the_questions() (which is, besides some small exceptions that
are not relevant for aa-mergeprof, in sync with the dropped code).
The remaining part gets renamed to ask_merge_questions() to avoid
confusion with the function name in aa.py. Also drop the (now
superfluous) parameter.
aa.py ask_the_questions() needs to allow 'merge' as aamode.
While on it, replace the fatal_error() call for unknown aamode with
raising an AppArmorBug.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This allows to hand over any source instead of using the global variable.
Now that the function expects its input as parameter, get rid of the
global log_dict, which means
- change collapse_log() to initialize log_dict as local variable and
return it
- change do_logprof_pass() to catch collapse_log()'s return value and
hand it over to ask_the_questions()
- drop all references to the global log_dict variable
- update test-libapparmor-test_multi to follow the changes
Also fix an if condition that would fail if aa[profile][hat] does not
exist - get() defaults to None if the requested item doesn't exist, and
None.get('file') will raise an Exception.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The function is an exact copy of the code in aa-mergeprof (except
removing the 'self' function parameter and changing the whitespace
level)
Also add a ask_conflict_mode() call to aa.py ask_the_questions().
This is needed for aa-mergeprof, and won't hurt in aa-logprof mode
because handle_children() already handles all exec events.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Bug: https://launchpad.net/bugs/1522938
Everything below "if aamode == 'merge':" is an exact copy of the code in
aa-mergeprof (with whitespace changed).
aa-logprof and aa-mergeprof will continue to ignore events from unknown
hats and subprofiles.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Set log_dict['merge'] = other.aa and aamode = 'merge', and use
log_dict[aamode] everywhere.
This brings aa-mergeprof ask_the_questions() closer to the code in aa.py.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
3-way-merge was never really implemented.
This patch drops all traces of it to make the code more readable and
easier to maintain.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
ldd exits with $? == 1 if a file is 'not a dynamic executable'.
This is correct behaviour of ldd, so we should handle it instead of
raising an exception ;-)
Also extend fake_ldd and add a test to test-aa.py to cover this.
Note that 2.10 and 2.9 don't have tests for get_reqs() nor fake_ldd,
so those branches will only get the aa.py changes.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk, 2.10 and 2.9.
This patch allows a user to specify a specific location for ss or
netstat in the invocations of get_pids_ss() or get_pids_netstat().
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch adjusts aa-unconfined to avoid using cat(1) to read
/proc/PID/cmdline entries, and instead opens them for reading directly.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@caanonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
It was reported that converting the netstat command to examine
processes bound to ipv6 addresses broke on OpenSUSE due to the version
of nettools not supporting the short -4 -6 arguments.
This patch switches to use the ss(8) utility from iproute2 by default
(if ss is found) as netstat/net-tools is deprecated. Unfortunately,
ss's '--family' argument does not accept multiple families, nor
does passing '--family' multiple times with different arguments work
either, so aa-unconfined invokes ss multiple times to gather the
different socket families.
It also fixes the invocation of netstat to use the "--protocol
inet,inet6" arguments instead, which should return the same results
as the short options.
This patch provides command line arguments to manually switch using
one tool or the other, as well as converting the invocations of ss
and netstat to not use a shell, and documents these options in the
aa-unconfined man page.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: John Johansen <john.johansen@canonical.com>
The latex based techdoc in the parser/ tree adds a number of build
dependencies for downstreams to create it; it also is the primary
element to make the builds unrepeatable. Creating the techdoc and other
documentation when generating a tarball for distribution avoids all
that.
* Makefile: build documentation as part of the tarball creation. Skip
the libraries/libapparmor directory as it needs to have configure run
before the manpages can be made.
* changehat/mod_apparmor/Makefile, changehat/mod_apparmor/Makefile,
utils/Makefile, profiles/Makefile: create separate docs target,
some of them dummies.
* parser/Makefile: pull the techdoc out of the default build target, add
an extra_docs target to create it.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Sometimes network events come with an operation keyword looking like
file_perm which makes them look like file events. Instead of ignoring
these events (which was a hotfix to avoid crashes), improve the type
detection.
In detail, this means:
- replace OPERATION_TYPES (which was basically a list of network event
keywords) with OP_TYPE_FILE_OR_NET (which is a list of keywords for
file and network events)
- change op_type() parameters to expect the whole event, not only the
operation keyword, and rebuild the type detection based on the event
details
- as a side effect, this simplifies the detection for file event
operations in parse_event_for_tree()
- remove workaround code from parse_event_for_tree()
Also add 4 new testcases with log messages that were ignored before.
References:
a) various bugreports about crashes caused by unexpected operation keywords:
https://bugs.launchpad.net/apparmor/+bug/1466812https://bugs.launchpad.net/apparmor/+bug/1509030https://bugs.launchpad.net/apparmor/+bug/1540562https://bugs.launchpad.net/apparmor/+bug/1577051https://bugs.launchpad.net/apparmor/+bug/1582374
b) the summary bug for this patch
https://bugs.launchpad.net/apparmor/+bug/1613061
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.10.
netstat -nlp46 output:
raw6 0 0 :::58 :::* 7 1326/NetworkManager
which when asking netstat to display name resolution ends up being:
raw6 0 0 [::]:ipv6-icmp [::]:* 7 1326/NetworkManager
Of course, aa-unconfined doesn't show this, the following patch adds
that, by adding the raw keyword as an alternative to tcp|udp and
accepting a number as an alternative to LISTEN.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Storing these event details depending on the operation type only makes
things more difficult because it's hard to differenciate between file
and network events.
Note that this happens at the first log parsing stage (libapparmor log
event -> temporary python array) and therefore doesn't add a serious
memory footprint. The event tree will still only contain the elements
relevant for the actual event type.
This change means that lots of testcases now get 3 more fields (all
None) when testing parse_event(), so update all affected testcases.
(test-network doesn't need a change for probably obvious reasons.)
Also rename a misnamed test in test-change_profile.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10.
Some conditions in RlimitRule can never be hit under normal
circumstances, so this patch adds some "pragma: no cover" and
"pragma: no branch" comments to beautify the coverage report.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds profiles for all log sniplets that are expected to
result in a profile rule.
This also means some changes in test-libapparmor-test_multi.py are
needed:
- split off log_to_profile_skip from log_to_profile_known_failures to
- only skip tests in log_to_profile_skip (causing a crash or requiring
user interaction)
- run tests in log_to_profile_known_failures, but expect a non-equal
result (caused by not added rules etc.)
- add quite some tests to log_to_profile_known_failures - they were
skipped before because they didn't have a *.profile file.
- add handling for hats to shorten list of known failures
This fixes testcase24 and testcase33 (after adjusting the profiles)
and lots of the new *.profile files.
- since we now have *.profile files for all log events that should result
in a profile rule, no longer ignore FileNotFoundError
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds TestLogToProfile to test-libapparmor-test_multi.py which
"translates" the test_multi log sniplets to a profile, and checks if it
matches the expected profile.
The expected profile for one log event will obviously contain only one
rule, and gets added as *.profile to the test_multi directory.
This patch includes 33 test_multi profiles - which means 83 more need to
be created. Whenever you have some time, add one or two! (Please write
those test_multi profiles manually, without using the tools.)
I know some parts of the test code looks complicated. Unfortunately this
is how things work - compare it with do_logprof_pass() in aa.py...
While on it, set tests = 'invalid' which ensures a failure in case
parse_test_profiles() doesn't set the tests array, and move printing
the test name out of parse_test_profiles() to avoid printing it twice.
A nice side effect of this patch is increased test coverage:
- 30% -> 40% in aa.py (= 250 more lines)
- 52% -> 78% in aamode.py (= 23 more lines)
- 26% -> 68% in logparser.py (= 120 more lines)
- total coverage increases from 57% to 62%
Acked-by: Seth Arnold <seth.arnold@canonical.com>
seen_events is a global variable in aa.py that gets increased at several
places, but isn't used (read or printed) anywhere. Since I can't imagine
how it could become useful, simply drop it.
Also drop an outdated comment in handle_children that lived next to a
seen_events line.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This little change means that the tests will run as part of 'make check'.
This commit is only a 'bzr mv utils/test/config_test.py utils/test/test-config.py'
without any changes in the file content.
Acked-by: Steve Beattie <steve@nxnw.org>
aa_test.py doesn't run in 'make check' because its filename doesn't
match the 'test-*.py' pattern, so this move means the tests now actually
get run.
While on it, migrate test-aamode.py to use the AATest base class, and
migrate the str_to_mode() tests to a tests[] array.
After this move, aa_test.py doesn't do anything anymore, so delete it.
Acked-by: Steve Beattie <steve@nxnw.org>.
Also add another test proposed by Steve:
(None, set()),
aa_test.py doesn't run in 'make check' because its filename doesn't
match the 'test-*.py' pattern.
mode_to_str() was dropped as part of the FileRule series, so it's
pointless to keep its tests. (The replacement is totally different and
has full test coverage already.)
loadincludes() still exists, but only testing if the function runs
without errors is not really helpful, so drop this test.
Also drop unused imports and add an explicit import for apparmor.aamode.
Acked-by: Steve Beattie <steve@nxnw.org>
aa_test.py doesn't run in 'make check' because its filename doesn't
match the 'test-*.py' pattern.
Move tests for globbing ("plain" globbing and globbing with ext) to
test-aare.py to make sure those tests actually run.
Note: This isn't an exact move - I adjusted some of the tests to make
them more useful, and added some more tests.
Also, glob_path() and glob_path_withext() no longer exist in aa.py.
They moved to the AARE class as part of the FileRule patch series.
Acked-by: Steve Beattie <steve@nxnw.org>
Add a testcase with exec-only permissions (which get ignored by
get_perms_for_path()) to increase FileRule test coverage to 100%.
Acked-by: Steve Beattie <steve@nxnw.org>
The latest version of pyflakes (1.3.0 / python 3.5) complains that
CMD_CONTINUE is defined twice in ui.py (with different texts).
Funnily CMD_CONTINUE isn't used anywhere, so we can just drop both.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9
As discussed a while ago, switch the utils (including their tests) to
use python3 by default. While on it, drop usage of "env" to always get
the system python3 instead of a random one that happens to live
somewhere in $PATH.
In practise, this patch doesn't change much - AFAIK openSUSE, Debian and
Ubuntu already patch aa-* to use python3.
Also add a note to README to officially deprecate Python 2.x.
(I won't break Python 2.x support intentionally - unless some future
change gives me a very good reason to finally drop Python 2.x support.)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(since 2016-08-23, but the commit had to wait for the FileRule series
because it touches test-file.py)
After looking at matchliteral(), I found out that it's only user is
rematchfrag(), which is only called in a) an "if False:" block and
b) match_include_to_path() - and that is only called by the also unused
match_prof_incs_to_path() function.
This patch drops some dead code (like the mentioned "if False:" block)
and the now unused functions
- matchliteral()
- rematchfrag()
- match_include_to_path()
- match_prof_incs_to_path()
This patch is also THE ANSWER to the question when I'll finally consider
this patch series complete.
42. It can't become better than that! ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
If a merged profile contains additional hats or subprofiles, the "old"
aa-mergeprof silently created them as additional hasher elements (partly
buggy, because subprofiles would end up as '^/subprofile' instead of
'profile /subprofile'). After switching to FileRule, aa-mergeprof crashes
on new hats or subprofiles.
This patch adds code to ask the user if the new hat or subprofile should
be added - which means this patch replaces two bugs (crash + silently
adding subprofiles and hats) with a new feature ;-)
The new questions also add a new text CMD_ADDSUBPROFILE in ui.py.
Finally, the new "button" combinations get added to test-translations.py.
If you want to test, try to aa-mergeprof this profile (the subprofile
and hat are dummies, nothing ping would really require):
#include <tunables/global>
/{usr/,}bin/ping {
#include <abstractions/base>
#include <abstractions/consoles>
#include <abstractions/nameservice>
capability net_raw,
capability setuid,
network inet raw,
network inet6 raw,
/{,usr/}bin/ping mixr,
/etc/modules.conf r,
^hat {
/bin/hat r,
/bin/bash px,
}
profile /subprofile {
/bin/subprofile r,
/bin/bash px,
}
# Site-specific additions and overrides. See local/README for details.
#include <local/bin.ping>
}
Note that this patch is not covered by unittests, but it passed all my
manual tests.
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://launchpad.net/bugs/1507469
aa-mergeprof empties 'includes' when running reset_aa(). The result is
KeyError: 'abstractions/newly_added_abstraction'
if an include file gets added because it isn't part of 'includes' at
this time. Note that you'll need to add another rule after adding the
include to trigger checking the includes for superfluous rules.
This fixes the regression found by Steve - which isn't really a
regression, "just" one more thing that got more visible with the new
code. Before, it was just an ill-addressed hasher that didn't complain ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
The switch to FileRule made some bugs visible that survived unnoticed
with hasher for years.
If aa-logprof sees an exec event for a non-existing profile _and_ a
profile file matching the expected profile filename exists in
/etc/apparmor.d/, it asks for the exec mode nevertheless (instead of
being silent). In the old code, this created a superfluous entry
somewhere in the aa hasher, and caused the existing profile to be
rewritten (without changes).
However, with FileRule it causes a crash saying
File ".../utils/apparmor/aa.py", line 1335, in handle_children
aa[profile][hat]['file'].add(FileRule(exec_target, file_perm, exec_mode, rule_to_name, owner=False, log_event=True))
AttributeError: 'collections.defaultdict' object has no attribute 'add'
This patch makes sure exec events for unknown profiles get ignored.
Reproducer:
python3 aa-logprof -f <(echo 'type=AVC msg=audit(1407865079.883:215): apparmor="ALLOWED" operation="exec" profile="/sbin/klogd" name="/does/not/exist" pid=11832 comm="foo" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0 target="/sbin/klogd//null-1"')
This causes a crash without this patch because
/etc/apparmor.d/sbin.klogd exists, but has
profile klogd /{usr/,}sbin/klogd {
References: https://bugs.launchpad.net/bugs/1379874
Acked-by: Steve Beattie <steve@nxnw.org> for trunk, 2.10 and 2.9
FileRule uses RE_PROFILE_FILE_ENTRY, which also means
RE_PROFILE_PATH_ENTRY, RE_PROFILE_BARE_FILE_ENTRY and RE_OWNER are now
unused.
This patch drops these regexes and their tests in test-regex_matches.py.
Acked-by: Steve Beattie <steve@nxnw.org>
rank() in severity.py is a dispatcher that calls the needed function
(rank_path(), rank_capability()) based on the parameter. Since all
calling code knows what rule type it is handling, this dispatcher is
superfluous - the calling code can call rank_path() or rank_capability()
directly.
This patch drops rank() and switches the remaining users of rank() to
call the rank_*() functions directly. For the tests, this means to drop
the CAP_ prefix because rank_capability doesn't expect this prefix.
Acked-by: Steve Beattie <steve@nxnw.org>
After switching to FileRule, several functions in aamode.py are no
longer used and can be deleted:
- print_mode()
- sub_mode_to_str()
- is_user_mode()
- split_mode()
- mode_to_str()
- flatten_mode()
- owner_flatten_mode()
- mode_to_str_user()
- log_str_to_mode()
The AA_EXEC_TYPE and ALL_AA_EXEC_TYPE variables are also unused now.
Acked-by: Steve Beattie <steve@nxnw.org>
When an user adds a new rule to a profile, cleanup / delete existing
rules that are covered by the new rule, and report the number of deleted
rules.
Acked-by: Steve Beattie <steve@nxnw.org>
Adding a rule to *Ruleset means it simply gets added. This also means
that then-superfluous rules will be kept.
This patch adds an optional cleanup flag to add(). If set, rules covered
by the new rule will be deleted. The difference to delete_duplicates()
is that cleanup only deletes rules that are covered by the new rule, but
keeps other, unrelated superfluous rules.
Also return the number of deleted rules to give the UI a chance to
report this number.
Finally, adjust the existing tests for FileRuleset to ensure default
mode (without cleanup) doesn't delete any rules, and add a test using
the cleanup flag.
Acked-by: Steve Beattie <steve@nxnw.org>
Replace the old (hasher-based) conflict_mode() with the new
(FileRule-based) ask_conflict_mode() function. If it detects conflicting
exec rules, it asks the user which one to keep.
Also call ask_conflict_mode() from ask_the_questions() so that it is
actually used.
Note: This patch isn't covered by unittests, but I did some manual
testing to make sure it works as expected.
Acked-by: Steve Beattie <steve@nxnw.org>
get_exec_rules_for_path() returns a FileRuleset with all rules matching
the given path.
get_exec_conflict_rules() returns a FileRuleset with all exec rules that
conflict with the given oldrule. This will be used by aa-mergeprof to
ask the user which rule he wants to keep.
Also add tests for both functions.
Acked-by: Steve Beattie <steve@nxnw.org>
The clear_common() call was disabled because it crashed in
delete_path_duplicates(). With the switch to FileRule, this function
no longer exists and therefore it can't crash ;-)
This patch re-enables the clear_common() call to avoid asking
superfluous questions.
References: https://bugs.launchpad.net/apparmor/+bug/1382236
Acked-by: Steve Beattie <steve@nxnw.org>
This is the correct way of doing AARE matches. However, this check is
more strict when matching against an AARE containing wildcards etc.
(which can "by luck" match when doing str matching)
To avoid breaking DbusRule, PtraceRule and SignalRule (especially their
tests), introduce _is_covered_aare_compat() which keeps the previous
behaviour of doing str matching, and use it in these classes.
On the long term, _is_covered_aare_compat() needs to go away, but doing
the changes needed in DbusRule, PtraceRule and SignalRule (or ideally
just in AARE) are out of scope for the FileRule patch series.
Acked-by: Steve Beattie <steve@nxnw.org>
When matching an AARE against another AARE, most AARE objects don't
contain orig_regex (only AARE instances originating from a log event
contain orig_regex).
In this case, match() will use is_equal() to error out on the safe side.
Unfortunately this also means that there are lots of false negative
cases where match() returns False errornously.
With this patch, match() checks the given AARE regex and, if it doesn't
contain any special characters (wildcards, alternations or variables),
handles it as plain path. This avoids most of the false negatives.
Also extend the AARE tests to check a bunch of plain path regexes using
AARE matching instead of only str matching.
Acked-by: Steve Beattie <steve@nxnw.org>
Merge the existing and requested permissions into a nice set of headers
that can be displayed by aa-logprof. This will look like:
Path: /foo
Old Mode: r + owner w
New Mode: rw
Also split off a _join_given_perms() function off _joint_perms() so that
we can use the permission string merging for things not stored in self.*.
Finally add some tests for logprof_header().
Acked-by: Steve Beattie <steve@nxnw.org>
