preprocessor and is not as thorough as -QTK (--skip-kernel-load,
--skip-read-cache, --skip-cache). Like with '-p', '-QTK' can be run without
privilege but it will catch things like conflictings 'x' modifiers.
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Commit r2456 fixes a bug in the parsers compilation that can result
policy failures. Unfortunately this Bug slipped into the wild and
shipped in at least one distro.
Bump the parser abi so that parsers that have the fix will invalid
existing cache files, and recompile policy to ensure the fix is applied.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://bugs.launchpad.net/bugs/1325109
The parser will accept rules with either umount or unmount rule types.
The utils should follow suite.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1235478
This is a test to check the label on file descriptors returned from
socketpair().
In its simple form, it simply calls socketpair() and checks the
labels on both fds.
In its complex form, it has the ability to do the simple test, then set
up an exec transition using aa_change_onexec(), and re-exec itself to
check the labeling after the file descriptors have been passed across an
exec transition.
The complex form is meant to test revalidation at exec. AppArmor
currently keeps the original labeling in place across the exec
transition.
Note that this test does not currently test read/write access to the
file descriptors. It only checks the label, as returned by
aa_getpeercon(2).
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Allow for the regression tests to specify arbitrary profile names
without hitting fatal errors or getting warnings from mkprofile.pl.
This allows for a test to have a line like this:
genprofile change_profile->':arbitrary_name -- \
image=arbitrary_name addimage:$test
In the example above, $test can call aa_change_onexec("arbitrary_name")
and then re-exec itself to test behavior across exec transitions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Ubuntu 14.04's chromium-browser has changed paths in a way that prevents
evince from opening clicked links in chromium-browser windows.
This patch adds a new path for the chrome-sandbox executable to the
sanitized_helper profile, so chromium will get its own tailored profile if
necessary.
The reporter who said this patch helped included some further DENIED lines
for signals that indicates this is probably not sufficient but did make
the links work as expected.
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1282314
Signed-off-by: Seth Arnold <seth.arnold@canonical.com>
Acked-By: Jamie Strandboge <jamie@canonical.com>
- convert "tail" result from byte to string to avoid TypeError crash
- use apparmor.filename instead of hardcoded /var/log/audit/audit.log
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Jamie Strandboge <jamie@canonical.com>
Description: man page updates for signals, ptrace and new variables
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
v3: fix freeing of filename when undefined
v2: address tyhicks feedback
refactor to have a common write routine
fix issue with set profile load being done even if !kernel_load
Profile loads from cache files that contain multiple profiles can
result in multiple reloads of the same profile or error messages about
failure to load profiles if the --add option is used. eg.
apparmor="STATUS" operation="profile_load"
name="/usr/lib/apache2/mpm-prefork/apache2" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058388] type=1400 audit(1395415826.937:616):
apparmor="STATUS" operation="profile_load" name="DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058391] type=1400 audit(1395415826.937:617):
apparmor="STATUS" operation="profile_load"
name="HANDLING_UNTRUSTED_INPUT" pid=8631 comm="apparmor_parser"
<sth0R> [82932.058394] type=1400 audit(1395415826.937:618):
apparmor="STATUS" operation="profile_load" name="phpsysinfo" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059058] type=1400 audit(1395415826.937:619):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059574] type=1400 audit(1395415826.937:620):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//HANDLING_UNTRUSTED_INPUT"
pid=8631 comm="apparmor_parser"
The reason this happens is that the cache file is a container that
can contain multiple profiles in sequential order
profile1
profile2
profile3
The parser loads the entire cache file to memory and the writes the
whole file to the kernel interface. It then skips foward in the file
to the next profile and reloads the file from that profile into
the kernel.
eg. First load
profile1
profile2
profile3
advance to profile2, do second load
profile2
profile3
advance to profile3, do third load
profile3
With older kernels the interface would stop after the first profile and
return that it had processed the whole file, thus while wasting compute
resources copying extra data no errors occurred. However newer kernels
now support atomic loading of multipe profiles, so that all the profiles
passed in to the interface get processed.
This means on newer kernels the current parser load behavior results
in multiple loads/replacements when a cache file contains more than
one profile (note: loads from a compile do not have this problem).
To fix this, detect if the kernel supports atomic set loads, and load
the cache file once. If it doesn't only load one profile section
from a cache file at a time.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Mention, in the apparmor.d man page, that pivot_root arguments must end
with a '/' character since they are directories.
The parser currently allows pivot_root arguments that do not end in '/',
but those rules will always fail to match.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
It may not be obvious that the peer label can be "unconfined". Provide
an example rule, in the apparmor.d man page, demonstrating the
peer=(label=unconfined) conditional.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(in a more serious version: add some tests for dbus, *mount, signal,
ptrace and pivot_root and make sure a space after the keyword is enforced.
The tools shouldn't accept a "dbusdriver" or "pivot_rootbeer" rule. ;-)
Acked-by: Tyler Hicks <tyhicks@canonical.com>
(dbus, *mount, signal, ptrace, pivot_root) except if the line only
contains the bare keyword.
Note that in most cases (except *mount) I used an alternation - this has
the advantage that it doesn't change the match group numbering, with the
small disadvantage of having to mention the keyword twice in the regex.
I chose this way to avoid that I have to change lots of other places and
possibly introduce bugs by overlooking something.
For the *mount rules, I read the code - it shouldn't need any changes
because it uses only matches[0..2]
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This patch extends the coverage of the parser's simple dbus language
tests.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds basic signal tests to the parser's simple language
test suite.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The mount.sh script mixes calls to the regression test 'mount' binary
and /sbin/mount. This can result in stale mtab entries being left around
after a test run because /sbin/mount adds an mtab entry but the test
'mount' binary, which is also used for unmounting, does not remove mtab
entries.
To solve this problem, the -n option is passed to /sbin/mount so that it
doesn't add an mtab entry when mounting.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The end of the mount.sh regression test script contained cleanup
commands to unmount and detach the loop device used for testing.
However, the second losetup command fails and, with the recent
regression test suite fix to not ignore failed shell commands, an error
is triggered at the end of the test run.
Additionally, these cleanup commands are not ran when the test fails
during the test run and an immediate exit is requested upon failure
(with the -r flag).
This patch fixes and moves the cleanup logic into a function that is
assigned to do_onexit so that the cleanup is always performed at exit
and the test can run successfully.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When there was a test error, such as a shell command failure, the
function used for the ERR trap, error_handler(), was causing the error
to be silently ignored by the test runner.
It was calling exit_handler() directly, before calling fatalerror().
This caused $_fatal to be left unset when exit_handler() was called.
exit_handler() sources epilogue.inc and the last bit of epilogue.inc
exits with $num_testfailures if $_fatal was unset. The fatalerror() call
site in error_hanlder() was never reached. So, as long as there were no
test failures, then an error in a test script would cause the test to
exit early with 0.
It is safe to simply call fatalerror() from error_handler() because
fatalerror() sets $_fatal to true and exits. This causes exit_handler()
to be called and since $_fatal is set to true, prologue.inc exits with
127.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
With the recent addition of features like ptrace and signals that
give warnings and then ignore the subset of rules when the features
directory indicates that the kernel does not support mediating such
features, at least one of the language tests fails in a chroot
environment where the apparmor securityfs tree is not mounted
inside it.
To compensate, a features file containing the current supported features
is included, and the simple.pl test driver is modified to pass it as an
argument to the parser, so that it will act as if the environment
supports all our current features.
A simple python script is included that was used to generate the
features file based on the current feature set.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
table
This patch adds the creation of an implicit set variable
@{profile_name} for use within policy. It expands to:
- a given profile name if specified; e.g. for
'profile flappy_bird /some/pattern/match* { [...] }'
@{profile_name} would expand to 'flappy_bird'
- if no given name, the match pattern; e.g. for
'/usr/bin/doge_bird { [...] }'
@{profile_name} would expand to '/usr/bin/doge_bird'
- hats and child profiles will include the fully qualified name; e.g.
the 'doge' hat in the /usr/bin/flappy_bird profile would cause
@{profile_name} to expand to '/usr/bin/flappy_bird//doge' within the
'doge' hat, and '/usr/bin/flappy_bird' outside of it in the profile.
There are some parsing tests added, but more tests are needed to verify
that expansion occurs properly (I've verified manually using parser
dumps of the added tests, but automated checks are needed).
The @{profile_name} variable is expected to be most useful in the
context of signal and ptrace rules (e.g. for specifying that an app
can send itself signals).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
seenlist bug
This patch:
- refactors the parser_symtab.c unit tests a bit in preparation for
the patch to add an implicit autofilled @{profile_name} variable
- expands coverage of the unit tests such that all code paths that
don't result in an exit() or are due to memory allocation errors are
exercised (this doesn't mean the tests are complete; the
__expand_variable() could use more tests for correctness).
- it fixes a bug where variables were not being removed from the
seenlist when a problem was detected in __expand_variable().
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Seth Arnold <seth.arnold@canonical.com>
Remove duplicated test code by adding a simple way for regex test
classes to declare a regex to use and a list of tuples consisting of
(line, expected_result). The setup_regex_tests() method generates test
methods for each tuple in a classes list. The test methods are based on
the regex_test() method, which performs the regex search and compares
the results to the expected_result.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The regexes for signal, ptrace, and pivot_root rules each had an extra
grouping around the terms 'signal', 'ptrace', and 'pivot_root'. Those
extra groupings can be safely removed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch backs out most of the changes from r2448 in favor of a better
approach.
The optional "file" keyword is handled under the pre-existing
RE_PROFILE_PATH_ENTRY regex and a new regex, RE_PROFILE_BARE_FILE_ENTRY,
is created for handling bare file rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The first conditional around string being set is not needed. If string
is not set, the while loop will be skipped and mode will be returned.
The variable tmp was being overloaded by being the regex search result
and then being reassigned to be the first match group in the regex
search result. This patch keeps tmp as the regex search result and then
uses mode_char to represent the first match group of the search.
Group the search and replace actions together at the beginning of the
loop and group the mode character processing at the end of the loop.
Finally, remove the unnecessary check of tmp (now mode_char) before
calling MODE_HASH.get(tmp, False). If tmp is None or '', get() will
do the right thing and return False.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1307665
When str_to_mode() was given a string containing unknown mode
characters, it entered an infinite loop. The case of the MODE_MAP_RE
regex string not matching the mode string was being ignored.
This patch makes it so that the loop breaks when MODE_MAP_RE no longer
matches the mode string. This occurs when all of the valid mode
characters have been processed and only invalid mode characters remain.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1298678
This patch does bare bones parsing of pivot_root rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering pivot_root rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1300317
This patch does bare bones parsing of ptrace rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering ptrace rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1300316
This patch does bare bones parsing of signal rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering signal rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
This patch adds a bunch of language parsing tests for ptrace rules.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch adds the newer rules class entries (e.g. ptrace, signals)
when dumping profiles (invoking the parser with the -dd argument).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
The previous test patches where done with the hardcoded bypass for
unconfined.
This semantic was changed so that a confined app can now block unconfined
processes from tracing or sending signals to it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Update mkprofile.pl to generate ptrace rules and update test scripts to
test ptrace mediation.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Update mkprofile.pl to generate signal rules and update test scripts to
grant signal permissions when needed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
ptrace rules currently take the form of
ptrace [<ptrace_perms>] [<peer_profile_name>],
ptrace_perm := read|trace|readby|tracedby
ptrace_perms := ptrace_perm | '(' ptrace_perm+ ')'
After having used the cross check (permission needed in both profiles)
I am not sure it is correct for ptrace.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The match
{VARIABLE_NAME}/{WS}*={WS}*\(
is too broad causing mount and dbus rules to fail for sets of values eg.
mount options=(ro bind)
Instead of doing a broad match, for now lets lock it down to just
peer=(...) being the only cond that can cause entry into CONDLISTID
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The match
{VARIABLE_NAME}/{WS}*={WS}*\(
is too broad causing mount and dbus rules to fail for sets of values eg.
mount options=(ro bind)
Instead of doing a broad match, for now lets lock it down to just
peer=(...) being the only cond that can cause entry into CONDLISTID
Signed-off-by: John Johansen <john.johansen@canonical.com>
