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>
The features_struct.size variable is used to hold a buffer size and it
is also passed in as the size parameter to read(). It should be a size_t
instead of an int.
A new helper function, features_buffer_remaining(), is created to handle
the two places where the remaining bytes in the features buffer are
calculated.
This patch also changes the size parameter of load_features_dir() to a
size_t to match the same parameter of load_features_file() as well as
the features_struct.size change described above.
Two casts were needed when comparing signed types to unsigned types.
These casts are safe because the signed value is checked for "< 0"
immediately before the casts.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The load_features_file() function returned an int but calculated the
value by subtracting two pointers. On 64 bit systems, that results in a
64 bit value being represented as a 32 bit type.
Coverity CID #55992
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Fixes build error when attempting to build and test the 2.10.95 release
on Ubuntu 14.04:
$ (cd libraries/libapparmor/ && ./autogen.sh && ./configure && \
make && make check) > /dev/null
...
libtool: Version mismatch error. This is libtool 2.4.6 Debian-2.4.6-0.1, but the
libtool: definition of this LT_INIT comes from libtool 2.4.2.
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.6 Debian-2.4.6-0.1
libtool: and run autoconf again.
make[2]: *** [grammar.lo] Error 63
make[1]: *** [all] Error 2
make: *** [all-recursive] Error 1
The --force option is needed to regenerate the libtool file in
libraries/libapparmor/.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
We already ignore network events that look like file events (based on
the operation keyword) if they have a request_mask of 'send' or
'receive' to avoid aa-logprof crashes because of "unknown" permissions.
It turned out that both can happen at once, so we should also ignore
this case.
Also add the now-ignored log event as test_multi testcase.
References: https://bugs.launchpad.net/apparmor/+bug/1577051#13
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk, 2.10 and 2.9.
Some people have the full hostname in their syslog messages, so
libapparmor needs to accept hostnames that contain dots.
References: https://bugs.launchpad.net/apparmor/+bug/1453300 comments
#1 and #2 (the log samples reported by scrx in #apparmor)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
for trunk, 2.10 and 2.9.
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>
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>
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>
The variable was only referenced by commented section of code so move
the declaration into the comment.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
I suspect that the incorrect description of EPERM was copied from
the aa_change_hat man page, where it is possible to see EPERM if the
application is not confined by AppArmor.
This patch corrects the description by documenting that the only
possible way to see EPERM is if a confined application has the
no_new_privs bit set.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
It is possible that file descriptors will be revalidated after an
aa_change_profile() but there is a lot of complexity involved that
doesn't need to be spelled out in the man page. Instead, mention that
revalidation is possible but the only way to ensure that file
descriptors are not passed on is to close them.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The statement was meant to convey the difference between aa_change_hat()
and aa_change_profile(). Unfortunately, it read as if there was
something preventing a program from using aa_change_profile() twice to
move from profile A to profile B and back to profile A, even if profiles
A and B contained the necessary rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Remove extra leading parenthesis from some of the function prototypes.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Swap aa_query_link_path_len() and aa_query_link_path() to match the
order of aa_query_file_path() and aa_query_file_path_len().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Doing manual line wraps resulted in an unreadable SYNOPSIS section.
Allow man to handle line wrapping the function prototypes itself.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
aa_query_file_path, aa_query_file_path_len, aa_query_link_path, and
aa_query_link_path_len were omitted from the NAME section.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
'change_hat' events have the target profile in 'name2', not in 'name'
(which is None and therefore causes a crash when checking if it contains
'//')
Also add the log event causing this crash to the libapparmor testsuite.
References: https://bugs.launchpad.net/apparmor/+bug/1523297
Acked-by: John Johansen <john.johansen@canonical.com> for trunk, 2.10 and 2.9.
This patch adds a check-local target to libapparmor/testsuite/Makefile.am
that checks the logfile generated by the test_multi tests
(libaalogparse.log) and errors out if
- the logfile doesn't exist (which might mean that dejagnu isn't installed
- the logfile contains 'ERROR'
This isn't the best solution I can imagine, but it's the only/easiest
way I found that doesn't need changing of autogenerated files.
Also extend clean-local to delete libaalogparse.{log,sum}
Finally, add test_multi/testcase_syslog_read.err (empty file) to avoid
make check fails.
Acked-by: John Johansen <john.johansen@canonical.com>
Fix memory leaks when parsing dmesg timestamps as well as when handling
message the library does not understand.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Only use the special %exception directive for functions that return a
negative int and set errno upon error.
This prevents, for example, _aa_is_blacklisted() from raising an
exception when it returns -1. This is important because it doesn't set
errno so an exception based on the value of errno would be
unpredictable.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
When is_blacklisted() was internal to the parser, it would print an
error message when encountering some file names. If the path parameter
was non-null, the error message would include the file path instead of
the file name.
Now that the function has been moved to libapparmor, callers are
expected to print the appropriate error message if _aa_is_blacklisted()
returns -1. Since the error message printing no longer occurs inside of
_aa_is_blacklisted(), the path parameter can be removed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Ignore README files when performing an operation on a list of files.
This matches the behavior of the is_skipped_file() function in aa.py.
The hope is that is_skippable_file() can reuse _aa_is_blacklisted().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
It looks odd to access the first character of a string before checking
to see if the string's length is zero. This is actually fine, in
practice, since strlen() looks at the first character of the string for
the presence of '\0' which means this is entirely a cosmetic change.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Prepend the function prototypes with extern to match the style of the
existing prototypes.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The errno values libapparmor's aa_policy_cache_new() uses to indicate
when the cache directory does not exist and when an existing, invalid
cache already exists needed to be separated out. They were both ENOENT
but now the latter situation uses EEXIST.
libapparmor also needed to be updated to not print an error message to
the syslog from aa_policy_cache_new() when the max_caches parameter is
0, indicating that a new cache should not be created, and the cache
directory does not exist. This is an error situation but a debug message
is more appropriate.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create a section 3 man page for the aa_policy_cache family of functions.
Additionally, update the in-code descriptions to match the descriptions
in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create a section 3 man page for the aa_kernel_interface family of
functions. Additionally, update the in-code descriptions to match the
descriptions in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create a section 3 man page for the aa_features family of functions.
Additionally, update the in-code descriptions to match the descriptions
in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johanse@canonical.com>
The aa_policy_cache_new() and aa_policy_cache_remove() functions are
changed to accept a dirfd parameter.
The cache dirfd (by default, /etc/apparmor.d/cache) is opened earlier in
aa_policy_cache_new(). Previously, the directory wasn't accessed until
later in the following call chain:
aa_policy_cache_new() -> init_cache_features() -> create_cache()
Because of this change, the logic to create the cache dir must be moved
from create_cache() to aa_policy_cache_new().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Instead of only accepting a path in the aa_features API, accept a
directory file descriptor and a path like then openat() family of
syscalls. This type of interface is better since it can operate exactly
like a path-only interface, by passing AT_FDCWD or -1 as the dirfd.
However, using the dirfd/path combination, it can eliminate string
allocations needed to open files in subdirectories along with the
even more important benefits mentioned in the open(2) man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Make the function prototype for reading a features directory the same
as the function prototype for reading a features file.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Two different implementations were in use for reading features files.
One for reading a single file and another for reading a single file
after walking a directory. This patch creates a single function that is
used in both cases.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The _aa_dirat_for_each() function used the DIR * type for its first
parameter. It then switched back and forth between the directory file
descriptors, retrieved with dirfd(), and directory streams, retrieved
with fdopendir(), when making syscalls and calling the call back
function.
This patch greatly simplifies the function by simply using directory
file descriptors. No functionality is lost since callers can still
easily use the function after calling dirfd() to retrieve the underlying
file descriptor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The most common case when creating an aa_kernel_interface object will be
to do so while using the current kernel's feature set for the
kernel_features parameter. Rather than have callers instantiate their
own aa_features object in this situation, aa_kernel_interface_new()
should do it for them if they specify NULL for the kernel_features
parameter.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The most common case when creating an aa_policy_cache object will be to
do so while using the current kernel's feature set for the
kernel_features parameter. Rather than have callers instantiate their
own aa_features object in this situation, aa_policy_cache_new() should
do it for them if they specify NULL for the kernel_features parameter.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The aa_features object that is passed to aa_policy_cache_new() does not
have to represent the currently running kernel. It may represent a
different kernel, such as a kernel that was just installed, that is not
currently running.
This patch adjusts the function comments to remove mentions of
"... the currently running kernel".
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
