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
v8 network permissions extend into the range used by exec mapping
so it is important to not blindly do execmapping on both the
file dfa and policydb dfa any more.
Track what type of dfa and its permissions we are building so
we can properly apply exec mapping only when building the
file dfa.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Signed-off-by: John Johansen <john.johansen@canonical.com>
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 warn_once() function is duplicated in 6 different places. A common,
reusable version has been added to parser_common.c.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
-Wimplicit-fallthrough only recognizes specic comment patterns
switch to a comment it recognizes.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
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>
The current encoding makes every xattr optional and uses this to
propogate the permission from the tail to the individual rule match
points.
This however is wrong. Instead change the encoding so that an xattr
(unless optional) is required to be matched before allowing moving
onto the next xattr match.
The permission is carried on the end on each rule portion file match,
xattr 1, xattr 2, ...
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make sure we can support optional xattrs distinct from optional xattr
values in the encoding.
Currently all xattrs specified are required to be present even
if there value is not specified. However under the old encoding there
was no way to distinguish if the presence of the xattr vs. the
xattr having a null length value.
Fix this so that if we decide to support optional xattrs it is possible
without having to change the abi.
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs are a byte string that can contain all input characters including
the null character. Allow * ** and ? glob patterns to match the null
character while retaining their apparmor characteristics for '/'.
That is * and ? won't traverse a '/' treating it as a path element.
While ** will match anything.
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs can contain NULL characters in their values which means we can
not user regular NULL transitions to separate values. To fix this
use out of band transition instead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the NULL character is used as an out of band transition
for string/path elements. This works for them as the NULL character
is not valid for this data. However this does not work for binary
data that can contain a NULL character.
So far we have only dealt with fixed length fields of binary data
making the NULL separator either unnecessary.
However binary data like in the xattr match and mount data field are
variable length and can contain NULL characters. To deal with this
add the ability to specify out of band transitions, that can only
be triggered by code not input data.
The out of band transition can be used to separate variable length
data fields just as the NULL transition has been used to separate
variable length strings.
In the compressed hfa out of band transitions are expressed as a
negative offset from the states base. This leaves us room to expand
the character match range in the future if desired and on average
makes the range between the out of band transition and the input
transitions smaller than would be had if the out of band transition
had been stored after the valid input transitions.
Out of band transitions in the dfa will not break old kernels
that don't know about them, but they won't be able to trigger
the out of band transition match. So they should not be used unless
the kernel indicates that it supports them.
It should be noted that this patch only adds support for a single
out of band transition. If multiple out of band transitions are
required. It is trivial to extend.
- Add a tag indicating support in the kernel
- add a oob max range field to the dfa header so the kernel knows
what the max range that needs verifying is.
- extend oob generation fns to generate oob based on value instead
of a fixed -1.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
https://launchpad.net/bugs/1584069
This patch allows policy authors to specify how exec transitions should
be handled with respect to setting AT_SECURE in the new process'
auxiliary vector and, ultimately, having libc scrub (or not scrub) the
environment.
An exec mode of 'safe' means that the environment will be scrubbed and
this is the default in kernels that support AppArmor profile stacking.
An exec mode of 'unsafe' means that the environment will not be scrubbed
and this is the default and only supported change_profile exec mode in
kernels that do not support AppArmor profile stacking.
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>
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>
The first entry in the grouping_count array is never initialized to 0;
subsequent depths are. This patch initializes the whole array.
Issue found with valgrind.
Signed-off-by: Steve Beattie <steve@nxnw.org> (with improvement from Seth)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch fixes the unit test memory leaks found
by intrigeri using AddressSanitizer in the following email thread:
https://lists.ubuntu.com/archives/apparmor/2015-August/008491.html
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The default change_onexec id is slightly wrong, it allows matching
'/' as an executable but it really should be anything under /
This results in the equality tests for change_profile failing as it
is different than what specifying /** in a rule does.
We could define rules need to be {/,}** to be equivalent but since
/ can not be an executable change the default value to match what
/** is converted in to.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
While change_profile rules are always created separately from file
rules. The merge phase can result in change_profile rules merging
with file rules, resulting in the change_profile permission being
set when a file rule is created.
Make sure to screen off the change_profile permission, when creating
a file rule.
Note: the proper long term fix is to split file, link and change_profile
rules into their own classes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Note: this patch currently overlays onexec with link_name to take
advantage of code already being used on link_name. Ideally what needs
to happen is entry needs to be split into file, link and change_profile
entry classes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The parser currently is still using the old permission layout, the kernel
uses a newer layout that allows for more permission bits. The newer
newer permission layout is needed by the library to query the kernel,
however that causes some of the permission bits to be redefined.
Rename the permission bits that cause redefination warnings to use
AA_OLD_MAY_XXX
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
BugLink: http://bugs.launchpad.net/bugs/1433829
The apparmor_parser fails to compile deny rules with only link
permissions.
Eg.
deny /f l,
deny l /f,
deny link /f -> /d,
Will all fail to compile with the following assert
apparmor_parser: aare_rules.cc:99: Node* convert_file_perms(int, uint32_t, uint32_t, bool): Assertion `perms != 0' failed.
NOTE: this is a minimal patch a bigger patch that cleans-up and separates
and reorganizes file, link, exec, and change_profile rules is needed
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Abstract af_unix socket names can contain a null character, however the
aare to pcre conversion explicitly disallows null characters because they
are not valid characters for pathnames. Fix this so that they type of
globbing is selectable.
this is a partial fix for
Bug: http://bugs.launchpad.net/bugs/1413410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch implements parsing of fine grained mediation for unix domain
sockets, that have abstract and anonymous paths. Sockets with file
system paths are handled by regular file access rules.
The unix network rules follow the general fine grained network
rule pattern of
[<qualifiers>] af_name [<access expr>] [<rule conds>] [<local expr>] [<peer expr>]
specifically for af_unix this is
[<qualifiers>] 'unix' [<access expr>] [<rule conds>] [<local expr>] [<peer expr>]
<qualifiers> = [ 'audit' ] [ 'allow' | 'deny' ]
<access expr> = ( <access> | <access list> )
<access> = ( 'server' | 'create' | 'bind' | 'listen' | 'accept' |
'connect' | 'shutdown' | 'getattr' | 'setattr' |
'getopt' | 'setopt' |
'send' | 'receive' | 'r' | 'w' | 'rw' )
(some access modes are incompatible with some rules or require additional
parameters)
<access list> = '(' <access> ( [','] <WS> <access> )* ')'
<WS> = white space
<rule conds> = ( <type cond> | <protocol cond> )*
each cond can appear at most once
<type cond> = 'type' '=' ( <AARE> | '(' ( '"' <AARE> '"' | <AARE> )+ ')' )
<protocol cond> = 'protocol' '=' ( <AARE> | '(' ( '"' <AARE> '"' | <AARE> )+ ')' )
<local expr> = ( <path cond> | <attr cond> | <opt cond> )*
each cond can appear at most once
<peer expr> = 'peer' '=' ( <path cond> | <label cond> )+
each cond can appear at most once
<path cond> = 'path' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
<label cond> = 'label' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')')
<attr cond> = 'attr' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
<opt cond> = 'opt' '=' ( <AARE> | '(' '"' <AARE> '"' | <AARE> ')' )
<AARE> = ?*[]{}^ ( see man page )
unix domain socket rules are accumulated so that the granted unix
socket permissions are the union of all the listed unix rule permissions.
unix domain socket rules are broad and general and become more restrictive
as further information is specified. Policy may be specified down to
the path and label level. The content of the communication is not
examined.
Some permissions are not compatible with all unix rules.
unix socket rule permissions are implied when a rule does not explicitly
state an access list. By default if a rule does not have an access list
all permissions that are compatible with the specified set of local
and peer conditionals are implied.
The 'server', 'r', 'w' and 'rw' permissions are aliases for other permissions.
server = (create, bind, listen, accept)
r = (receive, getattr, getopt)
w = (create, connect, send, setattr, setopt)
In addition it supports the v7 kernel abi semantics around generic
network rules. The v7 abi removes the masking unix and netlink
address families from the generic masking and uses fine grained
mediation for an address type if supplied.
This means that the rules
network unix,
network netlink,
are now enforced instead of ignored. The parser previously could accept
these but the kernel would ignore anything written to them. If a network
rule is supplied it takes precedence over the finer grained mediation
rule. If permission is not granted via a broad network access rule
fine grained mediation is applied.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
ptrace rules currently take the form of
ptrace [<ptrace_perms>] [<peer_profile_name>],
ptrace_perm := read|trace|readby|tracedby
ptrace_perms := ptrace_perm | '(' ptrace_perm+ ')'
After having used the cross check (permission needed in both profiles)
I am not sure it is correct for ptrace.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Add signal rules and make sure the parser encodes support for them
if the supported feature set reports supporting them.
The current format of the signal rule is
[audit] [deny] signal [<signal_perms>] [<signal_set>] <target_profile>,
signal_perm := 'send'|'receive'|'r'|'w'|'rw'
signal_perms := <signal_perm> | '(' <signal_perm> ([,]<signal_perm>)* ')'
signal := ("hup"|"int"|"quit"|"ill"|"trap"|"abrt"|"bus"|"fpe"|"kill"|
"usr1"|"segv"|"usr2"|"pipe"|"alrm"|"term"|"tkflt"|"chld"|
"cont"|"stop"|"stp"|"ttin"|"ttou"|"urg"|"xcpu"|"xfsz"|"vtalrm"|
"prof"|"winch"|"io"|"pwr"|"sys"|"emt"|"exists")
signal_set := set=<signal> | '(' <signal> ([,]<signal>)* ')'
it does not currently follow the peer=() format, and there is some question
as to whether it should or not. Input welcome.
Signed-off-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>
This cleans things up a bit and fixes a bug where not all rules are
getting properly counted so that the addition of policy_mediation
rules fails to generate the policy dfa in some cases.
Because the policy dfa is being generated correctly now we need to
fix some tests to use the new -M flag to specify the expected features
set of the test.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Policy enforcement needs to be able to support older userspaces and
compilers that don't know about new features. The absence of a feature
in the policydb indicates that feature mediation is not present for
it.
We add stub rules, that provide a none 0 start state for features that
are supported at compile time. This can be used by the kernel to
indicate that it should enforce a given feature. This does not indicate
the feature is allowed, in an abscence of other rules for the feature
the feature will be denied.
Note: this will break the minimize tests when run with kernels that
support mount or dbus rules. A patch to specify these features to
the parser is needed to fix this.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This will simplify add new features as most of the code can reside in
its own class. There are still things to improve but its a start.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
It was never used, never supported, and we are doing it differently now.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Seth Arnold noticed an ugly string.clear(); convert_entry(string,
NULL) pattern occurred frequently following the conversion to using
std::string. This patch replaces that by using a static pointer to
a constant string matching pattern, and also converts other uses of
that pattern. It also adds a function wrapper that will clear the
passed buffer before calling convert_entry().
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
As noted by Seth Arnold, e_buffer_overflow is no longer set in
convert_aaregex_to_pcre(), so remove it and the sole check for it.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch converts a stack allocated buffer into an std::ostringstream
object. The stringstream interface for specifying the equivalent of
a printf %02x conversion is a bit of an awkward construction, however.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Based on feedback from Seth Arnold, the convert_aaregex_to_pcre()'s
first argument is const char *, and thus the unit test macros don't need
to pass a copy of the input string to it, as it's guaranteed to be
unmodified by the function.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
As noted by Seth Arnold, there's now only one failure case in the
function and thus does not warrant a goto target (especially since
there's no cleanup to occur).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
By default, statically link against the in-tree libapparmor. If the
in-tree libapparmor is not yet built, print a helpful error message. To
build against the system libapparmor, the USE_SYSTEM make
variable can be set on the command line like so:
$ make USE_SYSTEM=1
This patch also fixes issues around the inclusion of the apparmor.h
header. Previously, the in-tree apparmor.h was always being included
even if the parser was being linked against the system libapparmor.
It modifies the apparmor.h include path based on the previous patch
separating them out in the libapparmor source. This was needed because
header file name collisions were already occurring.
For source files needing to include apparmor.h, the make targets were
also updated to depend on the local apparmor.h when building against
the in-tree libapparmor. When building against the system libapparmor,
the variable used in the dependency list is empty. Likewise, a
libapparmor.a dependency is added to the apparmor_parser target when
building against the in-tree apparmor.
Patch history:
v1: from Tyler Hicks <tyhicks@canonical.com>
- initial version
v2: revert to altering the include search path rather than including
the apparmor.h header directly via cpp arguments, alter the
include statements to <sys/apparmor.h> which will work against
either in-tree or (default) system paths.
v3: convert controlling variable to USE_SYSTEM from SYSTEM_LIBAPPARMOR
to unify between the parser and the regression tests.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: Steve Beattie <steve@nxnw.org>
This patch addresses the FIXMEs from the last patch by converting
process_mnt_entry's typebuf from a char[] to std::string. As a side
effect, the code in build_list_val_expr() is greatly simplified.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
