The AppArmor kernel now checks for both read and write permissions when
a process calls connect() on a UNIX domain socket.
The patch updates four abstractions that were found to be needing
changes after the change in AF_UNIX kernel mediation.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The accessibility bus uses an abstract socket, so there hasn't been a
need for an accessibility bus abstraction in the past. Now that D-Bus
mediation is supported, an abstraction becomes a useful place to put
accessibility bus D-Bus rules.
This patch follows the lead of the dbus and dbus-session abstraction by
granting full access to the accessibility bus.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Before D-Bus mediation support was added to AppArmor, the dbus and
dbus-session abstractions granted full access to the system and session
buses, respectively.
In order to continue granting full access to those buses, bus-specific
D-Bus mediation rules need to be added to the abstractions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
With the previous patch to switch to using alternations for variable
expansion, the clone_and_chain set of functions are no longer needed
and no longer need to be passed around. This patch removes them.
(I kept this patch separate to keep the previous patch smaller and more
easily reviewed.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch converts the parser's variable expansion from adding new
entries for each additional variable value to incorporating an
alternation that includes all the values for the variable; e.g. given:
@{BINS}=/bin /usr/bin /sbin /usr/sbin
@{BINS}/binary ix,
rather than expanding to exntries for
/bin/binary
/usr/bin/binary
/sbin/binary
/usr/sbin/binary
one entry would remain that looks like:
{/bin,/usr/bin,/sbin,/usr/sbin}/binary
One complication with this patch is that we try to prevent mistakes for
our users with variable expansion around '/'s; it's common for people to
write profiles that contain things like:
@{BAR}=/bingo/*/ /bango/
/foo/@{BAR}/baz
We already have a post-processing step that walks entries looking
for multiple sequences of '/'s and filters them into single
'/' which worked when creating new entries for each variable
expansion. Converting to alternation expansion breaks this filtering,
so code is added that removes leading and trailing slashes in variable
values in the expansion if the character immediately preceding or
following the variable is also a slash.
The intent behind this is to reduce the amount of memory allocations
and structure walking that needed to occur in when converting from the
entry strings to the back end nodes. Examples with real world profiles
showed performance improvements ranging from 2.5% to 10%. However,
because the back end operations are sensitive to the front end inputs,
it is possible for worse results to occur; for example, it takes the
simple_tests/vars/vars_stress_0[123].sd tests significantly longer to
complete after this patch is applied (vars_stress_03.sd in particular
takes ~23 times longer). An initial analysis of profiling output in
this negative case looks like it causes the tree simplification in
the back end to do more work for unknown reasons.
On the other hand, the test simple_tests/vars/vars_dbus_9.sd
(introduced in "[patch 09/12] parser: more dbus variable testcases")
takes ~1 sec to complete on my laptop before this patch, and roughly
0.01s with this patch applied.
(One option would be to keep the "expand entries" approach as an
alternative, but I couldn't come up with a good heuristic for when
to use it instead.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
This patch removes the string length limit in convert_aaregex_to_pcre()
usage. One of the benefits to moving to C++ is the ability to use
std::strings, which dynamically resize themselves. While it's a large
patch, a non-trivial amount is due to needing to get a char * string
back out via the c_str() method.
The unit tests are modified to include checks to ensure that
convert_aaregex_to_pcre only appends to the passed pcre string,
it never resets it.
As the test case with overlong alternations added in the previous
patch now passes, the TODO status is removed from it.
(Note: there's a couple of FIXME comments related to converting typebuf
to std::string that are added by this patch that are addressed in the
next patch. I kept that conversion separate to try to reduce the size
of this patch a little.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds a test case with an extremely large set of alternations.
It is marked TODO, because it fails with the current parser due to
strings used in convert_aaregex_to_pcre() being limited to (roughly)
PATH_MAX.
While contrived, it is possible to have alternations that are longer
than PATH_MAX that always match paths that are shorter than PATH_MAX.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
At least that's how this patch started ;-)
The updated (and much bigger) patch
- removes the note about can ?not mknod
- also removes mount and umount from the can ?not list which are covered
by mount rules now
- updates the example audit.log lines to the current log format
- updates the description of the log format
Acked-By: Seth Arnold <seth.arnold@canonical.com> (on IRC)
Seth also promised a follow-up patch with the remaining changes.
Patch history:
v1: initial version
v2: based on feedback from cboltz and sarnold:
- fix bad grammar when mentioning *.gcno and *.gcda files
- mention that distros generally don't need other options besides
verbose builds
- fix 'the valgrind' grammar messup.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds more testcases around variables used in dbus rules.
In particular, it
- attempts to verify that variable expansion and alternation
expansion results in identical DFA blobs,
- tests that variables can be expanded within alternations,
- tests that alternations can occur in variable definitions, and
- that having alternations inside variable declarations that are
used inside alternations results in parsing success
Note that vars/vars_dbus_9.sd veers into stress test land, as the
combinatoric expansion results in over 1000 dbus rule entries being
generated, which means that DFA reduction on all the fields takes
noticeable amounts of time (around 1s on my i5 ivy-core laptop).
Patch history:
v1: initial version
v2: based on feedback:
- add more alternation tests for cases where only part of the
alternation is defined within a variable
- mark test with nested alternations as being successful now that
the patch that implements it was accepted
v3: based on feedback from cboltz:
- tst/simple_tests/vars/vars_dbus_9.sd: reference all variables
declared, including a variable that references another variable
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When compiling the parser, g++ currently emits warnings like so:
profile.h: In constructor ‘Profile::Profile()’:
profile.h:177:11: warning: missing initializer for member ‘aa_rlimits::limits’ [-Wmissing-field-initializers]
rlimits = { 0 };
^
This patch fixes the issue.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The parser was not checking for an error when reading from
/proc/sys/kernel/osrelease. Additionally, valgrind was complaining
because of the uninitialized space in the buffer in between where
the read(2) had deposited its data and where the parser was writing
a trailing NUL to close the string. This patch fixes the above by
writing the NUL byte at the position at the end of the read characters
and checks for a negative result from the read() call.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The parser was converting alternation characters ('{', '}', and ',')
to their pcre versions ('(', ')', and '|', respectively) that occurred
inside of character class patterns (i.e. inside '[ ]'). This patch
fixes the issue and adds a few unit tests around character classes.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently, D-Bus rules are the only type of policy that we expect to be
queried from userspace. Therefore, we do not need to export other
mediation types at this time.
This patch removes all AA_CLASS_* macros, except AA_CLASS_DBUS, from
libapparmor's apparmor.h header. These macros are already defined in the
parser's policydb.h header.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Now that the parser links against libapparmor, it makes sense to move
all public permission types and flags to libapparmor's apparmor.h. This
prevents duplication across header files for the parser and libapparmor.
Additionally, this patch breaks the connection between
AA_DBUS_{SEND,RECEIVE,BIND} and AA_MAY_{WRITE,READ,BIND} by using raw
values when defining the AA_DBUS_{SEND,RECEIVE,BIND} macros. This makes
sense because the two sets of permission flags are from two distinctly
different mediation types (AA_CLASS_DBUS and AA_CLASS_FILE). While it is
nice that they share some of the same values, the macros don't need to
be linked together. In other words, when you're creating a D-Bus rule,
it would be incorrect to use permission flags from the AA_CLASS_FILE
type.
The change mentioned above allows the AA_MAY_{WRITE,READ,BIND} macros
to be removed from public-facing apparmor.h header.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Simple regression test that calls AddMatch using a match string that
sets up eavesdropping on all method call messages.
The shell script file runs the test unconfined and under a variety of
confinement profiles to make sure that eavesdropping confinement is
working as intended.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Rules using implied permissions may pick up the eavesdropping
permission, depending on the conditionals present in the rule.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Make the dbus rule generator knowledgeable of the eavesdrop permission.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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 adds a warning when quote characters '\' are added
unnecessarily, generates an error when a single quote is the last
character in a pattern, and uncomments and corrects the relevant unit
test cases.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds a parser make variable and a make target for building
the compiler with coverage compilation flags. With this, coverage
information can be generated by running tests/test suites against the
built parser and run through tools like gcovr.
Patch History:
v1: initial version
v2: refreshed/no change
v3: address feedback from sarnold:
- mark coverage target as phony
- correct missing '.' typo in clean target
- make coverage extensions consistent in clean targets
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
equivalents. (v2)
This patch verifies basic alternation usage.
Patch history:
v1: initial revision
v2: mark nested alternation tests as passing, as it was deemed a bug
that the parser didn't support them.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
This patch adds a test that verifies the parser considers an emty
character class regex as a parse arror.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
This patch annotates that a couple of values emitted on failure are
of type size_t, eliminating a couple of compiler warnings.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This patch adds unit tests and macros for the convert_aaregex_to_pcre()
function.
Patch history:
v1: initial version
v2: - give more verbose output on failures
- free memory used in tests
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
useful for people without the tools installed, and is against Debian
packaging policy (symlink pointing outside the source tree).
Signed-off-by: Kees Cook <kees@ubuntu.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This is patch tries to reduce the number of dynamic_cast<>s needed
during normalization by pushing the operations of normalize_tree()
into the expr-tree classes themselves rather than perform it as
an external function. This eliminates the need for dynamic_cast<>
checks on the current object under inspection and reduces the number
of checks needing to be performed on child Nodes as well.
In non-strict benchmarking, doing the dynamic_cast<> reduction
for just the tree normalization operation resulted in a ~10-15%
improvement in overall time on a couple of different hosts (amd64,
armel), as measured against apparmor_parser -Q. Valgrind's callgrind
tool indicated a reduction in the number of calls to dynamic_cast<>
on the tst/simple_tests/vars/dbus_vars_9.sd test profile from ~19
million calls to ~12 million.
In comparisons with dumped expr trees over both the entire
tst/simple_tests/ tree and from 1000 randomly generated profiles via
stress.rb, the generated trees were identical.
Patch history:
v1: initial version of patch
v2: update patch to take into account the infinite loop fix in
trunk rev 1975 and refresh against current code.
v3: no change
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Enabling the python caching test by default broke the build tests when
running in environments that do not contain the apparmor securityfs
mounted (think build chroots). This is because an initial check from the
shell script version of the tests was not reproduced within the python
version. This patch adds a check in the base class setUp function that
marks each testcase as skipped if apparmor's securityfs cannot be found.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch:
- incorporates the new python caching test into the make check/make
caching target, and removes the older shell based test script
- adjusts the python scripts to give verbose output when the VERBOSE
flag is set
- reorders the tests so that the tests that take a shorter amount of
time to run come first, leaving the language sanity test with its
69000+ testcases last
Patch history:
v1: initial revision
v2: add gen_xtrans/gen_dbus dependency to valgrind test
v3: drop gen_xtrans/gen_dbus as that was committed as a separate fix
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
With the C++-ization of the parser, some functions were renamed or
eliminated; this patch fixes the relevant valgrind false positive
suppression
pattern to match.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
update-ca-certificates (from ca-certificates-1_201310161709-1.1.noarch)
stores certs in this directory now.
References: https://bugzilla.novell.com/show_bug.cgi?id=852018
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>
