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>
This patch changes the aa_policy_cache_new() prototype and gets rid of
aa_policy_cache_is_valid() and aa_policy_cache_create().
The create bool of aa_policy_cache_new() is replaced with a 16 bit
unsigned int used to specify the maximum number of caches that should be
present in the specified cache directory. If the number is exceeded, the
old cache directories are reaped. The definition of "old" is private to
libapparmor and only 1 cache directory is currently supported. However,
that will change in the near future and multiple cache directories will
be supported.
If 0 is specified for the max_caches parameter, no new caches can be
created and only an existing, valid cache can be used. An error is
returned if no valid caches exist in that case.
If UINT16_MAX is specified, an unlimited amount of caches can be created
and reaping is disabled.
This means that 0 to (2^16)-2, or infinite, caches will be supported in
the future.
This change allows for the parser to continue to support the
--skip-bad-cache (by passing 0 for max_caches) and the --write-cache
option (by passing 1 or more for max_caches) without confusing
libapparmor users with the aa_policy_cache_{is_valid,create}()
functions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Adjust the internal splitcon() function to strip a single trailing
newline character when the bool strip_newline argument is true.
aa_getprocattr_raw(2) needs to set strip_newline to true since the
kernel appends a newline character to the end of the AppArmor contexts
read from /proc/>PID>/attr/current.
aa_splitcon(3) also sets strip_newline to true since it is unknown
whether the context is originated from a location that appends a newline
or not.
aa_getpeercon_raw(2) does not set strip_newline to true since it is
unexpected for the kernel to append a newline to the the buffer returned
from getsockopt(2).
This patch also creates tests specifically for splitcon() and updates
the aa_splitcon(3) man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Test confinement context splitting, using aa_splitcon(3), with and
without a valid mode pointer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Create a new libapparmor public function that allows external code to
split an AppArmor confinement context.
This is immediately useful for code that retrieves a D-Bus peer's
AppArmor confinement context using the
org.freedesktop.DBus.GetConnectionCredentials bus method.
https://launchpad.net/bugs/1430532
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The parse_confinement_mode() function returned NULL when a confinement
mode was not present (unconfined) and when it could not properly parse
the confinement context. The two situations should be differentiated
since the latter should be treated as an error.
This patch reworks parse_confinement_mode() to split a confinement
context and, optionally, assign the mode string. If a parsing error is
encountered, NULL is returned to indicate error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Use the passed in confinement context string size to improve the
comparison by only doing the string comparison if the size matches and
removing the possibility of reading past the end of the buffer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When passing the size of the confinement context to
parse_confinement_mode(), don't include the NUL terminator byte in the
size.
It is confusing to count the NUL terminator as part of the string's
length. This change makes it so that, after a few additional changes,
parse_confinement_mode() can be exposed as part of libapparmor's public
API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Allow libapparmor to build on older systems where secure_getenv() is
still named __secure_getenv(). This snippet was taken from the glibc
wiki:
https://sourceware.org/glibc/wiki/Tips_and_Tricks/secure_getenv
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The two internal aa_features objects weren't being unreferenced when the
aa_policy_cache object was being freed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
realloc() returns NULL when it fails. Using the same pointer to specify
the buffer to reallocate *and* to store realloc()'s return value will
result in a leak of the previously allocated buffer upon error.
These issues were discovered by cppcheck.
Note that 'buffer' in write_policy_fd_to_iface() has the autofree
attribute so it must not be manually freed if the realloc(3) fails as
it'll be automatically freed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
