Ideally we would have a flag or something so the caller could choose
to handle symlinks, or traverse them. But since all callers currently
don't handle symlinks just handle them in the iterator.
Beyond fixing the early termination due to a failed symlink this also
fixes another case of failure in one job cause dir based loads to
terminate early. Which can result in partial loads.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/215
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/850
Signed-off-by: John Johansen <john.johansen@canonical.com>
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
The parser is broken on RLIMIT parsing when receiving unexpected input
because the shared state for this specifies RLIMIT_MODEINCLUDE which
is an unknown start condition resulting in the following warning
parser_lex.l:745: undeclared start condition RLIMIT_MODEINCLUDE
and also means RLIMIT and INCLUDE are not properly handled
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently for directory includes the directory timestamp is ignored.
This is wrong as operations like removing a file from the dir won't
be considered in the timestamp check.
Fix this by updating the timestamp check to include the included
directories timestamp.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/760
Signed-off-by: John Johansen <john@jjmx.net>
Acked-by: Georgia Garcia <georgia.garcia@canonical.com>
Profile includes can be setup to loop and expand in a pathalogical
manner that causes build failures. Fix this by caching which includes
have already been seen in a given profile context.
In addition this can speed up some profile compiles, that end up
re-including common abstractions. By not only deduping the files
being included but skipping the need to reprocess and dedup the
rules within the include.
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/743
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
yyno_top_state was set as part of getting rid of the need to link against
libfl.
However we actually need to ability when the scanner is built with
debugging. Fix it so the option and libfl linking are conditionally
used based on whether DEBUG is defined.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/609
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Mike Salvatore <mike.salvatore@canonical.com>
Newlines should generally be treated as whitespace. Expand the list
of states using the newline rule to include almost all rules that
eat WS.
There are two exceptions assign and comment which have special handling
of newlines.
this fixes the failures
not ok 71543 - ./simple_tests//vars/vars_simple_assignment_13.sd: quoted commas should not trigger an error
not ok 71544 - ./simple_tests//vars/vars_simple_assignment_14.sd: quoted commas should not trigger an error
found by introducing nodefault
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Split the newline processing into a separate rule block so that it can
be shared with states that need to process newlines without processing
end of rule conditions.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
There were several states missing from the default rule which catches
unexpected input in a state.
Update the default rule to catch all input including newlines and
update its error message to include information about which state the
failure occured in. Also update the comment about what to do when
adding new states.
While the lexer now has the "nodefault" option set, it doesn't provide
as much information as the default rule does, so we prefer states
to use our provided default rule.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
... (using `%option nodefault`) instead of echoing the unknown parts to
stdout, and ignoring the error.
This will cause the parser to error out with
flex scanner jammed
and $?=2 if a profile contains unknown/invalid parts. That's not really
a helpful error message, but still better than ignoring errors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add basic support for policy to specify a feature abi. Under the
current implementation the first feature abi specified will be
used as the policy abi for the entire profile.
If no feature abi is defined before rules are processed then the
default policy abi will be used.
If multiple feature abi rules are encountered and the specified
abi is different then a warning will be issued, and the initial abi
will continue to be used. The ability to support multiple policy
feature abis during a compile will be added in a future patch.
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>
Support profiles that choose to match the presence of an extended
attribute without validating its value. This lets AppArmor target xattrs
with binary data, such as security.ima and security.evm values. For
example, it's now possible to write a profile such as:
profile signed_binaries /** xattrs=(security.ima) {
# ...
}
Both presence and value matches can be used in the same profile. To
match a signed xattr, target both the xattr and the security.ima value:
profile python_script /** xattrs=(
security.evm
security.apparmor="python"
) {
# ...
}
Updated to work using out of band matching instead of separate data
array.
Signed-off-by: Eric Chiang <ericchiang@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
AppArmor 3.0 requires policy to use a feature abi rule for access to
new features. However some policy may start using abi rules even if
they don't have rules that require new features. This is especially
true for out of tree policy being shipped in other packages.
Add enough support to older releases that the parser will ignore the
abi rule and warn that it is falling back to the apparmor 2.x
technique of using the system abi.
If the profile contains rules that the older parser does not
understand it will fail policy compilation at the unknown rule instead
of the abi rule.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce the INCLUDE/INCLUDE_EXISTS code dup by using a
variable for whether the name was enclosed by '<' and using
processid() to handle the whether the id is quoted or not.
In addition using processid allows include names to contain
escaoe sequences like \n and have them handled correctly.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce code duplication by checking the current state to
determine the single parameter difference between include and
include if exists
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This is a minimal patch to add conditional includes to the profile
language.
The syntax for conditional includes is similar to regular includes
except with the addition of "if exists" after "include"
include if exists <foo/bar>
include if exists "foo/bar"
include if exists "/foo/bar"
include if exists foo/bar
Note: The patch is designed to be backportable with minimum
effort. Cleanups and code refactoring are planned for follow up
patches that won't be back ported.
Signed-off-by: John Johansen <john.johansen@canonical.com>
includes were not handling WS in path names correctly. Allow WS within
quotes. Eg
include "foo bar"
include <"foo bar">
BugLink: http://bugs.launchpad.net/bugs/1738880
Signed-off-by: John Johansen <john.johansen@canonical.com>
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>
Allow for a leading '&' character to be present in the named transition
target strings to indicate that the transition should stack the current
profile with the specified profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
https://launchpad.net/bugs/1544387
Don't split namespaces from profile names using YACC grammar. Instead,
treat the entire string as a label in the grammer. The label can then be
split into a namespace and a profile name using the new parse_label()
function.
This fixes a bug that caused the profile keyword to not be used with a
label containing a namespace in the profile declaration.
Fixing this bug uncovered a bad parser test case at
simple_tests/profile/profile_ns_ok1.sd. The test case mistakenly
included two definitions of the :foo:unattached profile despite being
marked as expected to pass. I've adjusted the name of one of the
profiles to :foo:unattached2.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
currently the parser supports ambiguous units like m for time,
which could mean minutes or milliseconds. Fix this and refactor the
time parsing into a single routine.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Fix the regression that caused using 'include' instead of '#include' for
includes to stop working.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
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>
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>
allow specifying the change_profile keyword
change_profile,
to grant all permissions change_profile permissions
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>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Don't move globals in favor of lifting those out later]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently the cache tracks the most recent timestamp of parsed files
and then compares that to the cache timestamp. This unfortunately
prevents the parser from being able to know which files caused the
cache check failure.
Rework the cache check so that there is a debug option, and that
the cache file timestamp is set first so that we can output
a deug message for each file that causes a cache check failure.
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Forward ported to trunk and minor cleanups]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://bugzilla.novell.com/show_bug.cgi?id=895495
We define the __unused macro as a shortcut for __attribute__((unused))
to quiet compiler warnings for functions where an argument is unused,
for whatever reason. However, on 64 bit architectures, older glibc's
bits/stat.h header defines an array variable with the name __unused
that collides with our macro and causes the parser to fail to build,
because the resulting macro expansion generates invalid C code.
This commit fixes the issue by removing the __unused macro where it's
not needed (mod_apparmor) and renaming it to 'unused' elsewhere. It also
in some instances reorders the arguments so that the unused macro
appears last consistently.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
BugLink: http://bugs.launchpad.net/bugs/1373085
The parser fails to accept certain characters, even when escaped
or quoted as part of the profile or label name in ipc rules. This
is due to the lexer not accepting those characters as part of the
input pattern.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Let unix keyword accept bare send, receive keywords and add more
simple unix acceptance test cases.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
The match
{VARIABLE_NAME}/{WS}*={WS}*\(
is too broad causing mount and dbus rules to fail for sets of values eg.
mount options=(ro bind)
Instead of doing a broad match, for now lets lock it down to just
peer=(...) being the only cond that can cause entry into CONDLISTID
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The match
{VARIABLE_NAME}/{WS}*={WS}*\(
is too broad causing mount and dbus rules to fail for sets of values eg.
mount options=(ro bind)
Instead of doing a broad match, for now lets lock it down to just
peer=(...) being the only cond that can cause entry into CONDLISTID
Signed-off-by: John Johansen <john.johansen@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>
For some rules the output of apparmor_parser -p has a double comma
Eg.
ptrace (tracedby),
dbus (send,receive),
is output as
ptrace (tracedby),,
dbus (send,receive),,
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
apparmor_parser -p is broken. Outputting garbage charcters after every
include statement.
eg.
##included <tunables/multiarch>
^@^@V><A8>^?^@^@<C8>^NV><A8>^?^@^@<A0>^Pu^@# -----------------------------------
-------------------------------
#
This is happening because includes are handled specially and should not
go through the usual preprocessing output dump.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
strlen() assumes that it can read an entire word but when a char array
does not end on a word boundary, it reads past the end of the array.
This results in the following valgrind warning:
Invalid read of size 4
at 0x40A162: yylex() (parser_lex.l:277)
by 0x40FA14: yyparse() (parser_yacc.c:1487)
by 0x40C5B9: process_profile(int, char const*) (parser_main.c:1003)
by 0x404074: main (parser_main.c:1340)
Address 0x578d870 is 16 bytes inside a block of size 18 alloc'd
at 0x4C2A420: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x53E31C9: strdup (strdup.c:42)
by 0x40A145: yylex() (parser_lex.l:276)
by 0x40FA14: yyparse() (parser_yacc.c:1487)
by 0x40C5B9: process_profile(int, char const*) (parser_main.c:1003)
by 0x404074: main (parser_main.c:1340)
This patch quiets the warning by not using strlen(). This can be done
because yyleng already contains the length of string.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
includes come out like
#include
##included <tunables/home>
which is wrong because #include by itself is broken, and since -p is
supposed to be removing includes, it should not be directly echoed
any keyword in the keyword table is double echoed
ownerowner /{run,dev}/shm/pulse-shm* rwk
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Allows for the policy writer to grant permission to eavesdrop on the
specified bus. Some example rules for granting the eavesdrop permission
are:
# Grant send, receive, bind, and eavesdrop
dbus,
# Grant send, receive, bind, and eavesdrop on the session bus
dbus bus=session,
# Grant send and eavesdrop on the system bus
dbus (send eavesdrop) bus=system,
# Grant eavesdrop on any bus
dbus eavesdrop,
Eavesdropping rules can contain the bus conditional. Any other
conditionals are not compatible with eavesdropping rules and the parser
will return an error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch converts the problematic-with-g++ 4.6 state_names array
into a C++ unordered_map type. Using this depends on using the c++0x
(aka c++11) standard, and as we have gnuisms elsewhere (using the
typeof builtin), the patch also adds/converts to using -std=gnu++c0x
in the build rules (which conveniently eliminates some other warnings
we had due to other c++11-isms).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Seth Arnold <seth.arnold@canonical.com>
Allow directories to be passed directly to the parser and handled instead
of needing an initscript to find the files in the directory.
eg. load all profiles in profiles dir
apparmor_parser -r /etc/apparmor.d/
eg. load all binary files in the cache dir
apparmor_parser -Br /etc/apparmor.d/cache/
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch fixes a few more parser memory leaks as identified by the
simple valgrind test script. These mostly occur during cleanup of
structs and classes and as such, don't represent very serious leaks
for common usages of the parser.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
