2.9.x and 2.10 had some time stamp bugs around cache handling that
result in the cache getting a wrong time stamp, and then not getting
correctly updated when policy changes.
Force cache recompiles for these versions by bumping the parser abi
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The parser is incorrectly screening off the bind flags on remount. The
following patch by Ash Wilson fixes this issue
BugLink: http://bugs.launchpad.net/bugs/1272028
Signed-off-by: Ash Wilson <smashwilson@gmail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Drop the reference to the libapparmor policy_cache pseudo object when
the parser is done.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
In recent commits, Tyler fixed some problems with the caching behavior
of the parser, as well as adjusting and improving the caching test
script to verify these behaviors.
In doing so, the test script adjusts the mtime of various
files and ensures that the written files have the expected mtime
timestamp. Unfortunately, the os.utime() function used to adjust mtime
in python 3.2 (as included in Ubuntu 12.04 LTS) does not update with
nanosecond precision, even though the timestamps returned by os.stat()
do have precision to nanoseconds. This causes the tests to fail when
running under python 3.2 with errors like the following:
======================================================================
FAIL: test_abstraction_newer_rewrites_cache (__main__.AAParserCachingTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/��PKGBUILDDIR��/parser/tst/testlib.py", line 50, in new_unittest_func
return unittest_func(self)
File "./caching.py", line 424, in test_abstraction_newer_rewrites_cache
self._set_mtime(self.abstraction, abstraction_mtime)
File "./caching.py", line 238, in _set_mtime
self.assertEquals(os.stat(path).st_mtime, mtime)
AssertionError: 1440337039.40212 != 1440337039.4021206
The following patch creates a new time stamp equality assertion
function that detects if it's running on python 3.2 or earlier, and
loosens the equality bounds when comparing the passed timestamps. On
python 3.3 and newer, where writing timestamps with nanosecond precision
is supported, the strict equality assertion is used.
(Note: I did not convert all time stamp comparisons, just ones where
the timestamp written and checked could be based on a timestamp
derived from os.stat().)
Reference: https://bugs.python.org/issue12904
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The contents of the policy cache files varies based on kernel feature
support found in apparmorfs but the caching tests are mostly about
whether or not a cache file was generated and with the right timestamps.
This patch makes it so that the tests are not entirely skipped when
apparmorfs is not available. Instead, a flat features file will be used
in most cases and only the specific tests that require apparmorfs will
be skipped.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This makes several improvements to the parser caching tests to verify
that the parser is properly consuming the mtime of profiles and
abstractions when dealing with the policy cache.
It introduces a simple abstraction file and tests the mtime handling by
changing the mtime on the profile, abstraction, apparmor_parser, and
cache file in various combinations to check the parser's behavior.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch fixes a regression in setting the cache file's timestamp
handling that was introduced in r3079:
Set cache file tstamp to the mtime of most recent policy file tstamp
The previously used utimes(2) syscall requires a two element timeval
array. The first element in the array is the atime to be used and the
second element is the mtime. The equivalent of a one element timeval
array was being passed to it, resulting in garbage being used for the
mtime value. The utimes(2) syscall either failed, due to the invalid
input, or set mtime to an unexpected value. The return code wasn't being
checked so the failure went unknown.
This patch switches to utimensat(2) for a couple reasons. The UTIME_OMIT
special value allows us to preserve the inode's atime without calling
stat(2) to fetch the atime. It also allows for nanosecond precision
which better aligns with what stat(2) returns on the input profile and
abstraction files. That means that we can have the exact same mtime on
the input profile or abstraction file and the output cache file.
https://launchpad.net/bugs/1484178
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
"rcapparmor kill" results in a funny error message:
/lib/apparmor/rc.apparmor.functions: line 441: return: -v: invalid option
return: usage: return [n]
SLE12 includes a patch that prevents this error message, but also
prevents that $? is handed over correctly to rc_status. This means that
"rcapparmor kill" will happily display "done" even with a compiled-in
apparmor module that can't be unloaded.
This patch is the improved version - it adds a small helper function to
set $? (as handed over to aa_log_end_msg()) and then calls rc_status -v.
This means that "rcapparmor kill" now shows "failed" because it's
impossible to unload something that is compiled directly into the
kernel.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=862170 (non-public)
Acked-by: Seth Arnold <seth.arnold@canonical.com> for 2.9 and trunk
Some of the newly added simple_tests contain lines like
profile foo@{FOO} { }
which are not supported by the tools because the '}' is in the same line,
while the tools expect \n as rule separator.
This patch changes those tests to
profile foo@{FOO} {
}
Acked-by: John Johansen <john.johansen@canonical.com>
Some of the include files added to simple_tests recently don't live in
one of the main include directories (includes/, includes-preamble/ or
include_tests/) which lets test-parser-simple-tests.py fail because
those files don't contain EXRESULT.
Instead of adding more exceptions to test-parser-simple-tests.py, this
patch adds DESCRIPTION and EXRESULT to those include files.
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>
When @{profile_name} is used within a rule matching expression any
aare expressions should be matched literally and not be interpreted as
aare.
That is
profile /foo/** { }
needs /foo/** to expand into a regular expression for its attachment
but, /foo/** is also the profiles literal name. And when trying to
match @{profile_name} in a rule, eg.
ptrace @{profile_name},
the variable needs to be expaned to
ptrace /foo/\*\*,
not
ptrace /foo/**,
that is currently happening.
BugLink: http://bugs.launchpad.net/bugs/1317555
equality tests by
Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The @{profile_name} is incorrectly expanded as a fully qualified path
including its namespace if one was specified in the profile declaration.
ie.
profile :ns://a {
ptrace @{profile_name},
# expands to
# ptrace :ns://a,
}
This is wrong however because within a profile if a rule refers
to a namespace it will be wrt a sub-namespace. That is in the above
example the ptrace rule is refering to a profile in a subnamespace
"ns".
Or from the current profile declaration scope
:ns//ns://a
Instead @{profile_name} should expand into the hname (hierarchical name),
which is the profile hierarchy specification within the namespace the
profile is part of.
In this case
a
or for a child profile case
profile :ns://a {
profile b {
ptrace @{profile_name},
}
}
the hname expansion would be
a//b
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
allow
@{FOO}=bar
/foo@{FOO} { }
to be expanded into
/foobar { }
and
@{FOO}=bar baz
/foo@{FOO} { }
to be expanded into
/foo{bar,baz} { }
which is used as a regular expression for attachment purposes
Further allow variable expansion in attachment specifications
profile foo /foo@{FOO} { }
profile name (if begun with profile keyword) and attachments to begin
with a variable
profile @{FOO} { }
profile /foo @{FOO} { }
profile @{FOO} @{BAR} {}
hats
^@{FOO}
hat @{FOO}
and for subprofiles as well
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-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>
Errors include typos ("DESCRIPT__ON"), missing value after #=EXRESULT
and #=EXRESULT=PASS (= instead of space).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
It's allowed to only specify a TYPE without specifying a DOMAIN.
Also add a missing "]" for QUALIFIERS.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The current rule simplification algorithm has issues that need to be
addressed in a rewrite, but it is still often a win, especially for
larger profiles.
However doing rule simplification as a single pass limits what it can
do. We default to right simplification first because this has historically
shown the most benefits. For two reasons
1. It allowed better grouping of the split out accept nodes that we
used to do (changed in previous patches)
2. because trailing regexes like
/foo/**,
/foo/**.txt,
can be combined and they are the largest source of node set
explosion.
However the move to unique node sets, eliminates 1, and forces 2 to
work within only the single unique permission set on the right side
factoring pass, but it still incures the penalty of walking the whole
tree looking for potential nodes to factor.
Moving tree simplification into the construction phases gets rid of
the need for the right side factoring pass to walk other node sets
that will never combine, and since we are doing simplification we can
do it before the cat and permission nodes are added reducing the
set of nodes to look at by another two.
We do loose the ability to combine nodes from different sets during
the left factoring pass, but experimentation shows that doing
simplification only within the unique permission sets achieve most of
the factoring that a single global pass would achieve.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Currently rules are added to the expression tree in order, and then
tree simplification and factoring is done. This forces simplification
to "search" through the tree to find rules with the same permissions
during right factoring, which dependent on ordering of factoring may
not be able to group all rules of the same permissions.
Instead of having tree factoring do the work to regroup rules with the
same permissions, pregroup them as part of the expr tree construction.
And only build the full tree when the dfa is constructed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
accept nodes per perm bit where done from the very begining in a
false belief that they would help produce minimized dfas because
a nfa states could share partial overlapping permissions.
In reality they make tree factoring harder, reduce in longer nfa
state sets during dfa construction and do not result in a minimized
dfa.
Moving to unique permission sets, allows us to minimize the number
of nodes sets, and helps reduce recreating each set type multiple
times during the dfa construction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
parser_regex.c includes libapparmor_re/aare_rules.h and thus it should
depend on it in the Makefile.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When is_blacklisted() was internal to the parser, it would print an
error message when encountering some file names. If the path parameter
was non-null, the error message would include the file path instead of
the file name.
Now that the function has been moved to libapparmor, callers are
expected to print the appropriate error message if _aa_is_blacklisted()
returns -1. Since the error message printing no longer occurs inside of
_aa_is_blacklisted(), the path parameter can be removed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The errno values libapparmor's aa_policy_cache_new() uses to indicate
when the cache directory does not exist and when an existing, invalid
cache already exists needed to be separated out. They were both ENOENT
but now the latter situation uses EEXIST.
libapparmor also needed to be updated to not print an error message to
the syslog from aa_policy_cache_new() when the max_caches parameter is
0, indicating that a new cache should not be created, and the cache
directory does not exist. This is an error situation but a debug message
is more appropriate.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The aa_policy_cache_new() and aa_policy_cache_remove() functions are
changed to accept a dirfd parameter.
The cache dirfd (by default, /etc/apparmor.d/cache) is opened earlier in
aa_policy_cache_new(). Previously, the directory wasn't accessed until
later in the following call chain:
aa_policy_cache_new() -> init_cache_features() -> create_cache()
Because of this change, the logic to create the cache dir must be moved
from create_cache() to aa_policy_cache_new().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Instead of only accepting a path in the aa_features API, accept a
directory file descriptor and a path like then openat() family of
syscalls. This type of interface is better since it can operate exactly
like a path-only interface, by passing AT_FDCWD or -1 as the dirfd.
However, using the dirfd/path combination, it can eliminate string
allocations needed to open files in subdirectories along with the
even more important benefits mentioned in the open(2) man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
This patch changes the aa_policy_cache_new() prototype and gets rid of
aa_policy_cache_is_valid() and aa_policy_cache_create().
The create bool of aa_policy_cache_new() is replaced with a 16 bit
unsigned int used to specify the maximum number of caches that should be
present in the specified cache directory. If the number is exceeded, the
old cache directories are reaped. The definition of "old" is private to
libapparmor and only 1 cache directory is currently supported. However,
that will change in the near future and multiple cache directories will
be supported.
If 0 is specified for the max_caches parameter, no new caches can be
created and only an existing, valid cache can be used. An error is
returned if no valid caches exist in that case.
If UINT16_MAX is specified, an unlimited amount of caches can be created
and reaping is disabled.
This means that 0 to (2^16)-2, or infinite, caches will be supported in
the future.
This change allows for the parser to continue to support the
--skip-bad-cache (by passing 0 for max_caches) and the --write-cache
option (by passing 1 or more for max_caches) without confusing
libapparmor users with the aa_policy_cache_{is_valid,create}()
functions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@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>
bison isn't properly handling the 3 options of
TOK_CHANGE_PROFILE opt_id TOK_END_OF_RULE
TOK_CHANGE_PROFILE opt_id TOK_ARROW TOK_ID TOK_END_OF_RULE
TOK_CHANGE_PROFILE opt_id TOK_ARROW TOK_COLON TOK_ID TOK_COLON TOK_END_OF_RULE
specifying
change_profile /exec,
results in an unexpected TOK_ID error
refactor so that they share the 3 options share a common head which fixes
the problem.
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>
