Callers of aa_features_unref(), aa_kernel_interface_unref(), and
aa_policy_cache_unref() had to store off errno and restore it after
calling those functions in error paths. This patch preserves errno
across those *_unref() functions so that callers don't have to.
Signed-off-by: Tyler Hicks <tyhicks@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>
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>
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>
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>
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>
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>
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>
The libapparmor library is built with gcc, while the parser is built
with g++. The parser code needs to cast pointers returned from the
malloc(3) family of calls. However, code removed from the parser to
libapparmor can drop the casts.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Creates a libapparmor function, _aa_asprintf(), which sets the *strp to
NULL on error. This is needed for all of the users of the _aa_autofree
cleanup attribute because the value of *strp is undefined when
asprintf() fails and that could result in _aa_autofree() being passed a
pointer value that it should not free.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
As code is moved from the parser to libapparmor, the libapparmor code
base will need to have the "unused" macro defined. This macro will need
to be duplicated in the parser and libapparmor due to it being a
compiler-specific macro that shouldn't be exported from libapparmor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
