cache files will be written out even if the '--skip-bad-cache' option
is given and the cached features file differs from the features of
the currently running kernel. The patch below fixes the regression.
From: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Fix Makefile to rebuild dbus object when yacc definitions change.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@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 is a very minor optimization to the search to determine
whether a given rule is an exact match or not. If a wildcard rule
(i.e. an inexact match) is discovered, exact_match is set to 0,
so we don't need to continue the tree traversal.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
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>
Our simple language tests did not include any file deny rule tests. This
patch adds a few simple ones.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The README in the parser directory was woefully out of date; this patch
updates the information to contain the current mail list, wiki, and bug
tracking locations.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
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>
Drop support for the old subdomainfs mountpoint and use the fn exported
by libapparmor.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
we are writing a new cache .features file the cache dir should be cleared
out.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Fix this and unify creation and use of cacheloc so that we can hopefully
avoid these bugs.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
--cache-loc= is specified. This results in using the .features file from
/etc/apparmor.d/cache or always recompiling policy.
The former case is particularly bad as the .features file in
/etc/apparmor.d/cache/ may not correspond to the file in the specified
cache location.
bug: launchpad.net/bugs/1229393
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
AppArmor dbus rules are split into two classes. The first is
(send receive) rules and the second in bind rules. When the parser was
creating its internal representation of dbus rules, it wasn't separating
the overlapping bitmasks for (send receive) perms and bind perms.
(send receive) perms are 0x06 and bind perms are 0x40. Here's the old
parser output for an audit dbus rule that has accept states for
(send receive) and for bind:
$ dbus="/t { audit dbus, }"
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{3} (0x 40/0/40/0)
{7} (0x 46/0/46/0)
The {3} state is the accept state for the bind perms. The {7} state is
the accept state for the (send receive) perms. Note that the bind perm
mask bled over into the (send receive) accept state's mask.
With this patch, the masks for the two accept states do not overlap:
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{3} (0x 40/0/40/0)
{7} (0x 6/0/6/0)
Additionally, this patch makes the rule creation for (send receive)
perms more strict to keep any future perm bits from unintentionally
slipping into the (send receive) accept states.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Rules that have the audit and deny modifiers are to be explicitly denied
and audited. However, accept states were incorrectly being generated
with the deny and quiet masks set. This resulted in actions being denied
but not audited.
Here's the old parser output for audit deny dbus and mount rules:
$ dbus="/t { audit deny dbus, }"
$ mount="/t { audit deny mount, }"
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{3} (0x 0/40/0/40)
{7} (0x 0/46/0/46)
$ $ echo $mount | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{5} (0x 0/2/0/2)
With this patch, no accept states are generated which means that actions
will be denied and audited:
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
$ echo $mount | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
When using the deny rule modifier, accept states were not being
generated for dbus and mount rules. This means that the actions were
being denied, but it was not possible to quiet the auditing of the
actions.
The problem is that the deny and audit members of the dbus_entry and
mnt_entry structs were being used incorrectly. The deny member is a
boolean, not a bitmask. When the deny modifier is exclusively used in a
rule, the deny boolean should be true and the audit mask should be equal
to the perm mask.
Here's the old parser output for denied dbus and mount rules:
$ dbus="/t { deny dbus, }"
$ mount="/t { deny mount, }"
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
$ echo $mount | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
With this patch, the accept states are generated correctly with deny and
quiet masks:
$ echo $dbus | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{3} (0x 0/40/0/40)
{7} (0x 0/46/0/46)
$ echo $mount | apparmor_parser -qQD dfa-states 2>&1 | sed '/^$/,$d'
{1} <== (allow/deny/audit/quiet)
{5} (0x 0/2/0/2)
https://launchpad.net/bugs/1226356
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@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>
change_hat 1.4 was an experiement is more directly controlling change_hat
by adding hat rulles to the profile. It has not been used since the
original experiment (4 years). So remove it
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
remove old dead code that used to fail compilation if regular expressions
where detected in the rules and the apparmor kernel module did not support
regular expression matching.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@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>
From: John Johansen <john.johansen@canonical.com>
let allow be used as a prefix in place of deny. Allow is the default
and is implicit so it is not needed but some user keep tripping over
it, and it makes the language more symmetric
eg.
/foo rw,
allow /foo rw,
deny /foo rw,
Patch history:
v1: - initial revision
v2: - rename yacc target rule from opt_deny to opt_perm_mode to
reflect
that it can be either an allow or deny modifier
- break apart tests into more digestible chunks and to clarify
their purpose
- fix some tests to exercise 'audit allow'
- add negative tests for 'allow' and 'deny' in the same rule
- add support for 'allow' keyword to apparmor.vim
- fix a bug in apparmor.vim to let it recognize multiple
capability entries in a single line.
v3: - add support for optional keywords on capability rules in
regression tests, as well as the bare capability keyword (via
'cap:ALL')
- add allow, deny, and conflicting capability behavioral
regression tests
- fix vim syntax modeline to refer to apparmor in parser tests
- adjust FILE regex in vim syntax file creator script
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(The broken URLs were introduced in r1582.)
for utils/*.pod:
Acked-by: Steve Beattie <steve@nxnw.org>
for the other directories:
Patch by Steve Beattie
Acked-by: Christian Boltz <apparmor@cboltz.de>
- Make indenting consistent
- Move common match + fn patterns into a single shared entry with mulitstate
headers
- add names table to convert lexer state #s to state names used in the code
- Create/use macros for common patterns of DUMP, DEBUG, return ...
this fixes a few places where preprocess or DEBUG output was not
available
- update RE patterns for bugs eg. {WS} inside a character class [] does
not match whitespace
all told despite adding code to provide better support to debug and
preprocessing, the code is about 150 lines shorter, and has few corner
cases cleaned up.
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Added small fix for D-Bus parsing bug]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Removes an unnecessary variable, simplifies and unifies some of the loop
logic, and removes commented out code.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
When merging file entries in process_file_entries(), an error condition
can leave the entries list in a bad state which can cause invalid reads
and/or double frees when freeing the codomain and entries list memory.
The problem comes from the need to sort the entries linked list. An
array of pointers is created to represent the linked list, then the
array is sorted, then the linked list and the array coexist while the
entries are merged, then the linked list is reconstructed and the array
is freed. While the entries are being merged, an error condition can
occur and the function can return while the linked list is partially
modified.
The solution is to complete the sorting, reconstruct the linked list,
and free the array immediately. Once the linked list is in a good state,
the entries can be merged. Care is taken to adjust the linked list
pointers as entries are merged. An error condition can occur but the
linked list is always in a good state and proper cleanup can be
performed without any memory access issues.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
If a parser test case causes the parser to produce a core dump, the
simple.pl test runner incorrectly treats the test as a success.
This patch treats tests that cause core dumps as failures, even when the
tests are marked as #=TODO. The only way to ignore tests that fail in
this manner is with #=DISABLED.
Note that this patch changes the meaning of the $result variable.
Previously, it held a true or false status returned when closing the
apparmor_parser pipe. Now, it holds the exit status of apparmor_parser.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch fixes a few memory leaks found by valgrind. Most of these
occur in error cases and as such, are not a big deal. The dbus TOK_MODE
and flags TOK_CONDID leaks in parser_yacc.y are legitimate leaks,
if of very small amounts of memory.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
The function new_dbus_entry() free()s the conds argument but not the
peer_conds argument.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This patch moves the DUP_STRING macro to parser.h and modifies
it to accept a goto error target, that will be jumped to if the
call to strdup(3) fails. It also uses it in additional locations
where copying structures occurs, as well as detecting additional
cases where a structure duplication might have failed but not been
propagated outward.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Bug: https://bugs.launchpad.net/bugs/1218099
This patch adds support for expanding variables with dbus rules.
Specifically, they can expanded within the bus, name, path, member,
interface, and peer label fields.
Parser test cases and regression test cases are added as well.
Patch history:
v1: initial version of patch
v2: add equality.sh tests to verify that the results of using
variable expansion is the same as what should be equivalent rules
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This test is to verify that a list of profiles compile down into the
same binary representation. This is useful, for example, when testing a
rule syntax that includes permission aliases, as well as implied and
explicit accesses.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This is a test in the style of gen-xtrans.pl that attempts to run
through the most commonly constructed DBus rules. It also attempts to
run through some common mistakes to ensure that the parser fails
appropriately.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch implements the parsing of DBus rules.
It attempts to catch all corner cases, such as specifying a bind
permission with an interface conditional or specifying a subject name
conditional and a peer name conditional in the same rule.
It introduces the concept of conditional lists to the lexer and parser
in order to handle 'peer=(label=/usr/bin/foo name=com.foo.bar)', since
the existing list support in the lexer only supports a list of values.
The DBus rules are encoded as follows:
bus,name<bind_perm>,peer_label,path,interface,member<rw_perms>
Bind rules stop matching at name<bind_perm>. Note that name is used for
the subject name in bind rules and the peer name in rw rules. The
function new_dbus_entry() is what does the proper sanitization to make
sure that if a name conditional is specified, that it is the subject
name in the case of a bind rule or that it is the peer name in the case
of a rw rule.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Document the DBus rule syntax and provide several examples.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch fixes problems in the handling of both the final cache
name location and the temporary cache file when an alternate location
is specified.
The first issue is that if the alternate cache directory location was
specified, the alternate directory name would be used as the final location for
the cache file, rather than the alternate directory + the basename of
the profile.
The second issue is that it would generate the temporary file that it
stores the cache file in [basedir]/cache even if an alternate cache
location was specified on the command line. This causes a problem
if [basedir]/cache is on a separate device than the alternate cache
location, because the rename() of the tempfile into the final location
would fail (which the parser would not check the return code of).
This patch fixes the above by incorporating the basename into the cache
file name if the alternate cache location has been specified, bases the
temporary cache file name on the destination cache name (such that they
end up in the same directory), and finally detects if the rename fails
and unlinks the temporary file if that happens (rather than leave it
around). It also has been updated to add a couple of testcases to verify
that writing and reading from an alternate cache location work.
Patch history:
v1: first draft of patch
v2: add testcases, convert PERROR() to pwarn() if rename() fails for
placing cachefile into place.
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Using --subdomainfs without an argument triggers a segfault. This was due
to the long option missing the "has_arg" flag.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
file is larger than the feature buffer used for cache version comparison.
Ideally this would be dynamically allocated but for 2.8 just bumping the
buffer size is the quick fix.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
was addressed (however temporarily) in commit 2085.
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
fix a nasty little bug that can surface in apparmor 2.8 when
Hats/children profiles are used.
the matchflags in the dfa backend are not getting properly reset, which
results in a previously processed profiles match flags being used. This is
not a problem for most permissions but can result in x conflict errors.
Note: this should not result in profiles with the wrong x transitions loaded
as it causes compilation to file with an x conflict.
This is a minimal patch targeted at the 2.8 release. As such I have just
updated the delete_ruleset routine to clear the flags as it is already
being properly called for every rule set.
Apparmor 2.9/3.0 will have a different approach where it is not possible
to reuse the flags.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
whether or not the filesystem has a fine enough timestamp resolution.
Occasionally even on filesystems like ext3, the two files' creation
dates would differ when created less than a second apart, which would
typically cause the 'Cache is used when cache is newer' test to fail
because the cached file would have the same timestamp as the profile.
The fix creates 10 files 0.1 seconds apart and ensures that all ten
have distinct timestamps.
(The occasional failure was caught in testing runs like
https://bugs.launchpad.net/qa-regression-testing/+bug/1087061/ )
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch fixes two issue with the simple test driver. The first is
that child exec that actually ran the parser was located inside the
eval statement. This meant that if the exec failed for some reason
(like the parser didn't exist), the child wouldn't actually die,
but would pop out of the eval and continue running through the loop
of test profiles (while the parent process does the same). This meant
that if the script ran on the full testsuite with a misconfiguration,
it would explode creating O(n^2) processes, where n is the number of
testcase files -- with over 25k testcases, that's a lot. The fis is to
lift the child exec outside the eval{}, then an exec() failure causes
the child process to die correctly.
The second fix is that several of the testcases were added with the
DESCRIPTION field added in lower case (i.e. #=Description blah blah).
This fix makes the regex that pulls out the description not be
case-sensitive.
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-By: John Johansen <john.johansen@canonical.com>
This patch replaces the hardcoded path to the in-tree apparmor parser
in several of the script based test scripts with the APPARMOR_PARSER
environment variable, keeping the hardcoded location as the default.
It also adds support for overriding the location of the parser via the
same environment variable. The make infrastructure is updated to use
this, though uses a different variable (PARSER) to drive it.
Thus 'make check PARSER=/some/path/to/an/alternate/apparmor_parser'
will run all the parser tests on that binary. This is useful for
running the testsuite in an automated post-install environment.
(It should be noted that doing so will still build and run the unit
test binaries based on the source tree.)
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-By: John Johansen <john.johansen@canonical.com>
The apparmor_parser has 3 different directory walking routines. Abstract
them out and use a single common routine.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
