As for %s when the pointer is null: even if gcc prints (null) this is still undefined behavior, so we should do this explicitly
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
Older kernels do not support an xtable grouped with the policy dfa.
The presence of a policy.dfa does not indicate whether we should create
an xtable with the policy dfa.
Instead the check should be if the kernel supports the extended
permstable32 format.
Signed-off-by: John Johansen <john.johansen@canonical.com>
v1 of permstable32 has some broken verification checks. By using two
copies of a merged dfa and an xtable the same size of the permstable
we can work around the issue.
Signed-off-by: John Johansen <john.johansen@canonical.com>
If extended permissions are supported use them. We need to build a
permission table and set the accept state of the chfa up as an index
into the table.
For now map the front end permission layout into the old format and
then convert that to the perms table just as the kernel does.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add a flag that allows setting the error code AppArmor will send when
an operation is denied. This should not be used normally.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Given the following profile:
profile foo {
profile bar {
profile baz {
}
}
}
The parser would correctly serialize the "foo" profile and the
"foo//bar" profile, but it would incorrectly name "bar//baz" when it
should be "foo//bar//baz". This would cause issues loading the profile
in certain kernels causing a "parent does not exist" error.
Partially addresses #346.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Add a flag that allows setting the signal used to kill the process.
This should not be normally used but can be very useful when
debugging applications, interaction with apparmor.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for specifying the path prefix used when attach disconnected
is specified. The kernel supports prepending a different value than
/ when a path is disconnected. Expose through a profile flag.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The check isn't correct, it should be checking for capability
MAC_ADMIN, but in the future that won't be correct either. Instead
rely on the kernel to check permission to load policy, which it alread
does as it is possible to by-pass the parser to load policy.
Also improve the error message when the kernel does deny
loading policy due to failed permission checks.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The kernel will allow for a couple of debug flags on a profile that
can be used to trigger debug messages for only profiles/labels that
have the flag set. Add basic support for these to the parser.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With the exception of the documentation fixes, these should all be
invisible to users.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/687
Make all warnings that go through pwarn() controllable by warning
flags. This adds several new warning control flags, documented in
--help=warn
Convert --debug-cache to be unified with warning flags. So it can be
set by either
--debug-cache
or
--warn=debug-cache
Also add an "all" option to be able to turn on all warnings.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
The enforce profile mode is the default but specifying it explicitly
has not been supported. Allow enforce to be specified as a mode. If
no mode is specified the default is still enforce.
The kernel has supported kill and unconfined profile modes for a
long time now. And support to the parser so that profiles can make
use of these modes.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/440
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/7
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The features abi adds the ability to track the policy abi separate
from the kernel. This allow the compiler to determine whether policy
was developed with a certain feature in mind, eg. unix rules.
This allows the compiler to know whether it should tell the kernel to
enforce the feature if the kernel supports the rule but the policy
doesn't use it.
To find if a feature is supported we take the intersection of what is
supported by the policy and what is supported by the kernel.
Policy encoding features like whether to diff_encode policy are not
influenced by policy so these remain kernel only features.
In addition to adding the above intersection of policy rename
--compile-features to --policy-features as better represents what it
represents. --compile-features is left as a hidden item for backwards
compatibility.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
'case OPTION_OFILE' missed the 'break', which means if did fallthrough
to the default case.
Adding the 'break' means no longer executing another PERROR, and no
longer executing the 'exit(1)' in the default branch.
References: coverity #55994
Create new, ref, and unref functions for aa_kernel_interface. The "new"
function allows for the caller to pass in an aa_features object that is
then used to check if the kernel supports set load operations.
Additionally, the "new" function allows for the apparmorfs path to be
discovered once instead of during every policy load.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
__sd_serialize_profile() had a duplicated implementation for writing to
apparmorfs interface files after a profile compilation. This patch
migrates it to the new aa_kernel_interface API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This is the start of the kernel_interface API that allows callers to
specify a buffer, a file path, or a file descriptor that should be
copied to the proper kernel interface for loading, replacing, or
removing in-kernel policies.
Support exists for reading from a file path or file descriptor into a
buffer and then writing that buffer to the appropriate apparmorfs
interface file.
An aa_kernel_interface_write_policy() function is also provided for
callers that want to route a buffer to an arbitrary file descriptor
instead of to an apparmorfs file. This is useful when an admin instructs
apparmor_parser to write to stdout or a file.
Additionally, it removes some parser-specific globals from the
kernel_interface.c file, such as OPTION_{ADD,REPLACE,REMOVE}, in
preparation for moving the code into a library.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Fixed build failures]
[tyhicks: Fixed bug where a warning was being printed when it shouldn't]
[tyhicks: Forward ported to trunk]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Forward ported patch to trunk]
[tyhicks: remove commented out code]
[tyhicks: fix use after free]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Handle inverted return from find_subdomainfs_mountpoint()]
[tyhicks: Link test progs to libapparmor to fix make check build fail]
[tyhicks: Migrate from opendir() to open() for opening apparmorfs]
[tyhicks: Make some of the split out functions static]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
While some of these allocations will go away as we convert to C++,
some of these need to stay C as the are going to be moved into a
library to support loading cache from init daemons etc.
For the bits that will eventually be C++ this helps clean things up,
in the interim.
TODO: apply to libapparmor as well
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
While some of these allocations will go away as we convert to C++,
some of these need to stay C as the are going to be moved into a
library to support loading cache from init daemons etc.
For the bits that will eventually be C++ this helps clean things up,
in the interim.
TODO: apply to libapparmor as well
Signed-off-by: John Johansen <john.johansen@canonical.com>
Both valgrind and strace report the parser doing
close(-1) = -1 EBADF (Bad file descriptor)
This happens the skip kernel load argument is specified in combination
with any of --add, --replace, or --remove arguments (the default
is --add if no other option is specified).
This happens when the parser is not processing profiles but not
writing them out (eg. no kernel load, dump to stdout, file ...)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Currently the apparmor parser warns about rules that are not enforced or
downgraded. This is a problem for distros that are not carrying the out of
tree kernel patches, as most profile loads result in warnings.
Change the behavior to not output a message unless a warn flag is passed.
This patch adds 2 different warn flags
--warn rule-downgraded # warn if a rule is downgraded
--warn rule-not-enforced # warn if a rule is not enforced at all
If the warnings are desired by default the flags can be set in the
parser.conf file.
v2 of patch
- update man page
- add --warn to usage statement
- make --quiet clear warn flags
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
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>
This patch makes use of the htoleXX() functions (see endian(3))
defined as part of endian.h (already included in parser_interface.c),
instead of defining a function differently based on the detection of
endian related macros.
This fixes a build failure experienced on powerpc with John's patch
set applied. This patch has been updated with John's feedback to use
letoh16() in the le16_to_cpu() macro.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This will allow for the parser to invalidate its caches separate of whether
the kernel policy version has changed. This can be desirable if a parser
bug is discovered, a new version the parser is shipped and we need to
force cache files to be regenerated.
Policy current stores a 32 bit version number in the header binary policy.
For newer policy (> v5 kernel abi) split this number into 3 separate
fields policy_version, parser_abi, kernel_abi.
If binary policy with a split version number is loaded to an older
kernel it will be correctly rejected as unsupported as those kernels
will see it as a none v5 version. For kernels that only support v5
policy on the kernel abi version is written.
The rules for policy versioning should be
policy_version:
Set by text policy language version. Parsers that don't understand
a specified version may fail, or drop rules they are unaware of.
parser_abi_version:
gets bumped when a userspace bug is discovered that requires policy be
recompiled. The policy version could be reset for each new kernel version
but since the parser needs to support multiple kernel versions tracking
this is extra work and should be avoided.
kernel_abi_version:
gets bumped when semantic changes need to be applied. Eg unix domain
sockets being mediated at connect.
the kernel abi version does not encapsulate all supported features.
As kernels could have different sets of patches supplied. Basic feature
support is determined by the policy_mediates() encoding in the policydb.
As such comparing cache features to kernel features is still needed
to determine if cached policy is best matched to the kernel.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Tag start of entries in the policydb as being mediated. This makes
the start state for any class being mediated be none 0. The kernel
can detect this to determine whether the parser expected mediation
for the class.
This is just a way of encoding what features expect mediation within
the policydb it self so that a separate table isn't needed.
This is also used to indicate the new unix semantics for mediation of
unix domain sockets on connect should be applied.
Note: this does cause a fail open on situation on Ubuntu Saucy, which
did not properly indicate support. That is if a kernel using this patch
is installed on an Ubuntu Saucy system, unix domain socket mediation
on connect won't happen, instead the older behavior will be applied.
This won't cause policy failures as it is less strict than what
Ubuntu Saucy applies.
This is necessary so that AppArmor can properly function on older
userspaces without a compile time configuration on the kernel to determine
behavior. A kernel expecting this behavior will function correctly
with all old userspaces expect it will not enforce connect time mediation
on Ubuntu Saucy. However Ubuntu does not support Trusty (or newer)
kernels as backports to Saucy, so this does not break them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
With the conversion to c++, the use of void* pointers for the parser
interface buffers generates several warnings. This patch converts the
types from void* to u8* for the buffer pointers, to clean up those
warnings.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch addresses a bunch of the compiler string conversion warnings
that were introduced with the C++-ification patch.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Convert the codomain to a class, and the policy lists that store
codomains to stl containers instead of glibc twalk.
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Merge with dbus changes and process_file_entries() cleanup]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This conversion is nothing more than what is required to get it to
compile. Further improvements will come as the code is refactored.
Unfortunately due to C++ not supporting designated initializers, the auto
generation of af names needed to be reworked, and "netlink" and "unix"
domain socket keywords leaked in. Since these where going to be added in
separate patches I have not bothered to do the extra work to replace them
with a temporary place holder.
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: merged with dbus changes and memory leak fixes]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Remove use of AARE_DFA as the alternate pcre matching engine was removed
years ago.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
policydb is the new matching format, that combines the matching portions
of different rules into a single dfa/hfa. This patch only lays some ground
work it does not add encoding of any rules into the policydb
Signed-off-by: John Johansen <john.johansen@canonical.com>
The ability to set capabilities from a profile has been removed from the
kernel for several releases. Remove it from the parser as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The module interface calls for names with namespaces to be in the format of
:namespace:profile or :namespace://profile
but the parser was generating
namespace:profile
causing profile lookup to fail, or removal of the wrong profile as it was
done against the current namespace, instead of the specified namespace
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
