table
This patch adds the creation of an implicit set variable
@{profile_name} for use within policy. It expands to:
- a given profile name if specified; e.g. for
'profile flappy_bird /some/pattern/match* { [...] }'
@{profile_name} would expand to 'flappy_bird'
- if no given name, the match pattern; e.g. for
'/usr/bin/doge_bird { [...] }'
@{profile_name} would expand to '/usr/bin/doge_bird'
- hats and child profiles will include the fully qualified name; e.g.
the 'doge' hat in the /usr/bin/flappy_bird profile would cause
@{profile_name} to expand to '/usr/bin/flappy_bird//doge' within the
'doge' hat, and '/usr/bin/flappy_bird' outside of it in the profile.
There are some parsing tests added, but more tests are needed to verify
that expansion occurs properly (I've verified manually using parser
dumps of the added tests, but automated checks are needed).
The @{profile_name} variable is expected to be most useful in the
context of signal and ptrace rules (e.g. for specifying that an app
can send itself signals).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
seenlist bug
This patch:
- refactors the parser_symtab.c unit tests a bit in preparation for
the patch to add an implicit autofilled @{profile_name} variable
- expands coverage of the unit tests such that all code paths that
don't result in an exit() or are due to memory allocation errors are
exercised (this doesn't mean the tests are complete; the
__expand_variable() could use more tests for correctness).
- it fixes a bug where variables were not being removed from the
seenlist when a problem was detected in __expand_variable().
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Seth Arnold <seth.arnold@canonical.com>
This patch adds a bunch of language parsing tests for ptrace rules.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch adds the newer rules class entries (e.g. ptrace, signals)
when dumping profiles (invoking the parser with the -dd argument).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@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>
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>
alone when the labeling is not path dependent.
Some rules will not generate label entries, some will generate only
label entries and some will generate both label and path entries.
This is left to the particular rule encoding.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Using the parser timestamp was a work around to force recompilation of
policy that was built with a buggy parser. There are better ways to
handle this so remove checking of the parser timestamp.
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>
The features file patch broke detection of network support.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This is not the cleanup this code needs, but a quick hack to add the
-M flag so we can specify a feature file (or directory) to use for
the compile.
It mostly just moves around existing code and adds the -M option,
though it does introduce a few changes.
While I didn't do it in this patch I propose we drop support for
the match file without create support. This is several years old
now and would clean things up a lot.
Note: that the manually input -m or -M drop support for it already
I just can't see a good way to support a single input stream indicating
the result/existance of two separate files.
This needs more work but is needed to support tests and the policy_mediates
frame work depends on the policydb getting generated with the special
stub rules to indicate whether policy was compiled expecting a certain
feature. But this can break the current tests, at least once a bug
in the policy rule counting is fixed in a follow on patch.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Another issue with commit 2456 is that with older versions of glibc and
g++, a definition for SIZE_MAX was not being found; e.g. on Ubuntu 12.04
LTS and 12.10, the parser fails to compile with the following error:
g++ -g -O2 -pipe -Wall -Wsign-compare -Wmissing-field-initializers -Wformat-security -Wunused-parameter -std=gnu++0x -D_GNU_SOURCE -DPACKAGE=\"apparmor-parser\" -DLOCALEDIR=\"/usr/share/locale\" -DSUBDOMAIN_CONFDIR=\"/etc/apparmor\" -I../libraries/libapparmor//include -c -o lib.o lib.c
lib.c: In function 'int str_escseq(const char**, const char*)':
lib.c:292:32: error: 'SIZE_MAX' was not declared in this scope
The following patch addresses the issue by explicitly including the C stdint
header which contains the definition for SIZE_MAX.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Trunk commit 2456 broke the builds on i386 with the following compiler
error:
g++ -g -O2 -pipe -Wall -Wsign-compare -Wmissing-field-initializers -Wformat-security -Wunused-parameter -std=gnu++0x -D_GNU_SOURCE -DPACKAGE=\"apparmor-parser\" -DLOCALEDIR=\"/usr/share/locale\" -DSUBDOMAIN_CONFDIR=\"/etc/apparmor\" -I../libraries/libapparmor//include -c -o lib.o lib.c
lib.c: In function 'int strn_escseq(const char**, const char*, size_t)':
lib.c:236:47: error: no matching function for call to 'min(long unsigned int, size_t&)'
tmp = strntol(*pos, &end, 8, 255, min(3ul, n));
^
This is due to size_t differing in size on i386 and amd64. The
following patch addresses the issue by casting the constant values
to size_t (and removing the ul suffix since the constant values are
getting cast anyway), satisfying C++'s types (and the patch removes
the unnecessary min 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>
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>
Fix the octal escape sequence that was broken, so that short escapes \0,
\00 \xa, didn't work and actually resulted in some encoding bugs.
Also we were missing support for the decimal # conversion \d123
Incorporate and update Steve Beattie's unit tests of escape sequences
patch
v2
- unify escape sequence processing, creating lib fns.
- address Steve Beattie's feedback
- incorporate Steve Beattie's feedback
v3
- address Seth's feedback
- add missing strn_escseq tests
- expand strn_escseq to take a 3rd parameter to allow specifying chars to
convert straight across. . eg "+" will cause it to convert \+ as +
- fix libapparmor/parse.y failed escape pass through to match processunqoted
Unit tests by Steve Beattie
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch separates pivot_root rules from mount rules, since the syntax
of the two types of rules is very different. It also documents the
missing "oldroot=" prefix required for the conditional corresponding to
the put_old parameter. Finally, it briefly describes pivot_root rules
and provides some examples.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Make it more generic so that it can be shared with signals.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Yes its seems pointless because these will eventually get replaced by
stl. But until then
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Valgrind is offering complaints like the following when dealing with
profiles with mount rules:
==27919== Conditional jump or move depends on uninitialised value(s)
==27919== at 0x805CDC1: mnt_rule::mnt_rule(cond_entry*, char*, cond_entry*
==27919== by 0x805674E: do_mnt_rule(cond_entry*, char*, cond_entry*, char*
==27919== by 0x8057937: yyparse() (parser_yacc.y:1133)
==27919== by 0x8053916: process_profile(int, char const*) (parser_main.c:1
==27919== by 0x804B20E: main (parser_main.c:1340)
Doing this consistently with the other initializers for the mount
class instead:
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
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>
The valgrind test script would happily chug along even if if valgrind
was not installed, not doing anything of use. This patch fixes that, and
offers up the ability to specify an alternate location for valgrind if
it does not exist in the usual /usr/bin location.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
The parser currently indicates that it exited successfully if invalid
arguments are passed to it, which makes it difficult to detect when
other tools are calling it incorrectly. This patch causes it to return
'1' indicating a failure.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
When passing an include directory on the command line to
apparmor_parser, valgrind emits a warning:
Invalid read of size 4
at 0x404DA6: add_search_dir(char const*) (parser_include.c:152)
by 0x40BB37: process_arg(int, char*) (parser_main.c:457)
by 0x403D43: main (parser_main.c:590)
Address 0x572207c is 28 bytes inside a block of size 29 alloc'd
at 0x4C2A420: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x53E31C9: strdup (strdup.c:42)
by 0x404D94: add_search_dir(char const*) (parser_include.c:145)
by 0x40BB37: process_arg(int, char*) (parser_main.c:457)
by 0x403D43: main (parser_main.c:590)
This patch quiets the warning by removing strlen() calls on the t char
array. Instead, it only calls strlen() on the dir char array. t is a
dupe of dir and strlen(dir) does not trigger the valgrind warning.
Additionally, this patch adds a bit of defensive programming to the
while loop to ensure that index into the t array is never negative.
Finally, the valgrind suppression is removed from valgrind_simple.py.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
With commit 2364 addressing one of valgrind's false positives, we can
remove the related valgrind suppression entry from the test script.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Tyler Hicks <tyhicks@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>
When the --cache-loc option was added in trunk commit 1916, it was
intended that -L would be the short form of the option (based on
documentation and usage changes). However, the commit mistakenly
did not include the short option in the list include in the call
to getopt_long(3). This patch adds it along with the indicator
that it requires an argument (the different cache location) to the
getopt_long() call.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
This patch adds support for the rttime rlimit (aka RLIMIT_RTTIME),
available since the 2.6.25 kernel, according to the getrlimit(2)
man page; see that man page for more details on this rlimit.
An acceptance test is also added, as well as an update to the
apparmor.vim input template.
While reviewing to see what made sense in apparmor.vim for the rttime
rlimit, I discovered that RLIMIT_RTTIME's units are microseconds, not
seconds like RLIMIT_CPU (according to the setrlimit(2) manpage). This
necessitated not sharing the case switch with RLIMIT_CPU. I didn't add
a keyword for microseconds, but I did for milliseconds. I also don't
accept any unit larger than minutes, as it didn't seem appropriate
(and even minutes felt... gratuitous). I would appreciate feedback
on what keywords would be useful here.
Patch History:
v1: initial submission
v2: - add apparmor.vim support for rttime keyword
- adjust RLIMIT_TIME value assignment due to its units being
microseconds, not seconds, and add milliseconds keyword.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Close file handle left opened if parser.cfg is found and read from.
Found by cppcheck.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Fix leaked memory if calloc() fails. Found by cppcheck.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
