The debugging code for profile entries contains a check to ensure that
it's not NULL, but the list iterator macro already ensures that the
iteration will stop if the item is NULL, making the check redundant.
Coverity CID #55983
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
It's possible to end up unreferencing a kernel_interface object that
has ->dirfd set to -1. This patch avoids calling close(2) on that fd.
(close(-1) will just return EBADF anyway.)
Coverity CIDs #55996 and #55997
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This makes some of the references to functions in the aa_query_label(2)
manpage more consistent and fixes a couple of grammar issues. It also
tries to make the qualifying statements in apparmor.d(5) more distinct,
and also fixes some typos there as well.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This adds support to the profile generator script for change_profile
rules, giving the ability to write the 3 factor version of the rule
(e.g. "change_profile /t -> A_PROFILE") which was significantly more
difficult using straight raw rules, which is why we don't have any 3
factor rule tests.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Now that the onexec test program notices that it failed to send SIGSTOP
to itself, causing a whole bunch of tests to be detected as failing,
grant the ability to send and receive signals to the onexec tests.
(The onexec tests are not tests intended to verify signal mediation.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
The onexec test was ignoring errors from the kill() call, so it didn't
notice when it had failed to send SIGSTOP to itself.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Based on a patch by John Johansen <john.johansen@canonical.com>
(I converted the check to look for the process directory in /proc
rather than sending signal 0 to the task, as John had done in a patch
sent to me, to prevent failures in signal delivery from blocking the
check from working correctly.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Based on a patch by John Johansen <john.johansen@canonical.com>
Add more details to the checks in the regression tests onexec tests,
to make debugging failures easier. Also, use more local variables
to indicate what and how many arguments are expected to the onexec
check_* functions.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
https://launchpad.net/bugs/1210514
It looks plausible to me that prefork_open_logs() ->
ap_proc_mutex_create() -> ap_unixd_set_proc_mutex_perms() -> chown().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The aa_stack_profile() and aa_stack_onexec() functions were added to
libapparmor since 2.10.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The audit_read capability, mpls address family, and profile stacking are
all new features advertised by the latest AppArmor kernel features file.
Without this change, the parser tests will fail because parsing profiles
that utilize stacking results in an error when the features file
indicates that stacking is not supported by the kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The stacking test binary links against libapparmor for
aa_stack_profile() and aa_stack_onexec(), which will be present in 2.11.
This means that regression test builds using the system libapparmor
should not build the stacking test binary unless the libapparmor 2.11 or
newer is present.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Policy namespaces are not well supported in older parsers and kernels.
This is a case where the kernel support doesn't seem to be working.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Stacking is a complex feature and, in order to sufficiently test all
aspects of stacking, a relatively complex test program is needed.
This patch adds a program that can call
aa_stack_onexec()/aa_stack_profile(), perform file IO on a given file
path, verify that the current confinement context is what it is expected
to be, and/or execute itself or another program.
The confinement context verification can handle stacked labels with any
ordering.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Some tidying up is needed in order to reuse do_open(). This patch
eliminates the chance of returning 0 due to errno being not set. It also
adjusts the file string to be const.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The idea is that the $test profile grants $file access and the
$othertest profile grants $subfile access. Both profiles grant
$stacktest access. The tests verify that after changing to the stacked
$othertest//&$test profile, only $stacktest can be accessed.
Similar tests are also added for stacking with a namespaced profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The kernel patches that implement AppArmor profile stacking made changes
that allow the the backed for change_profile to detect if the target
profile does not exist prior to checking if the current profile allows
the change_profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Check if the current kernel supports stacking. If not, ensure that named
transitions (exec, change_profile, etc.) do not attempt to stack their
targets.
Also, set up the change_profile vector according to whether or not the
kernel supports stacking. Earlier kernels expect the policy namespace to
be in its own NUL-terminated vector element rather than passing the
entire label (namespace and profile name) as a single string to the
kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Allow for a leading '&' character to be present in the named transition
target strings to indicate that the transition should stack the current
profile with the specified profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser was splitting up the namespace and profile name from named
transition targets only to rejoin it later when creating the binary
policy. This complicated the changes needed to support the stacking
identifier '&' in named transition targets.
To keep the stacking support simple, this patch keeps the entire named
transition target string intact from initial profile parsing to writing
out the binary.
All of these changes are straightforward except the hunk that removes
the namespace string addition to the vector in the process_dfa_entry()
function. After speaking with John, kernels with stacking have support
for consuming the namespace with the profile name.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch separates the label parsing functionality from the program
termination and memory allocation duties of parse_label(). This will
ultimately help in creating simple helper functions that simply need to
check if a label contains a namespace.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The add_named_transition function was written in a way that is difficult
to understand while attempting to read the function. This patch attempts
to clean it up.
First, this patch removes this confusing code flow issue:
if (!entry->ns) { ... }
if (entry->ns) { ... } else { ... }
It then unifies the way that the ns and nt_name strings of the cod_entry
struct are handled prior to calling add_entry_to_x_table() and/or
returning. ns and nt_name are now guaranteed to be NULL before
performing either of those actions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The copy_cod_entry() function was not copying the nt_name field of the
cod_entry struct.
This was discovered during code review and I'm not certain if it causes
any real world bugs.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Based on the existing implementations of aa_change_profile(2) and
aa_change_onexec(2).
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Modeled after the aa_change_profile(2) man page, this profile defines
the libapparmor and kernel interfaces for the in-progress profile
stacking feature.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Bug: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1480492
If python3-apparmor is not installed, aa-status aborts due to the added
import to handle fancier exception handling failing. This patch makes
aa-status(8) work even in that case, falling back to normal python
exceptions, to keep its required dependencies as small as possible.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
write_prof_data[hat] is correct (it only contains one profile, see bug 1528139),
write_prof_data[profile][hat] is not and returns an empty (sub)hasher.
This affects RE_PROFILE_START and RE_PROFILE_BARE_FILE_ENTRY.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.9 and 2.10
Instead of reusing opt_named_transition and be forced to reconstruct the
target path when is looks like ":odd:target", create simpler grammer
rules that have nothing to do with named transitions and namespaces.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
a) change log_dict to profile_storage()
Change collapse_log() to initialize log_dict[aamode][profile][hat]
as profile_storage() instead of a hasher().
This also means path events need to go into
log_dict[aamode][profile][hat]['allow']['path']
instead of
log_dict[aamode][profile][hat]['path']
to match the profile_storage() layout.
b) Simplify log translation
The translation from logparser.py's output to *Rule events was more ugly
than needed. This patch removes one step.
Instead of translating log_dict to log_obj in ask_the_questions(), add
*Rule objects to log_dict and adjust ask_the_questions() to use log_dict
instead of log_obj.
This also means log_obj in ask_the_questions() is now superfluous and
can be removed.
c) Other small changes:
- use is_known_rule() instead of .is_covered() for capability events,
which means included files are also checked now.
- remove the "if rule_obj.log_event != aamode:" check, because
a) it depends on the content of *Rule.log_event (which means it
ignores events with log_event != 'ALLOWING' or 'REJECTING'
b) it's superfluous because the whole code section is wrapped in a
"for aamode in sorted(log.dict.keys())" which means we have
separate loops for enforce and complain mode already
Acked-by: Kshitij Gupta <kgupta8592@gmail.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>
According to a discussion with John on IRC, denied_mask="x" can only
happen for 'exec' log events. This patch raises an exception if John
is wrong ;-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This should happen rarely, but nevertheless it can happen - and since
AppArmor needs the symlink target in the profile, we have to resolve all
symlinks.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
