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>
syslog-ng needs to access both the permanent /var/log/journal/ and the
non-permanent /run/journal/.
I also included /var/run/journal/ to stay consistent with supporting
both /run/ and /var/run/.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
This makes print()ing a class object much more helpful - instead of
<apparmor.rule.network.NetworkRule object at 0x7f416b239e48>
we now get something like
<NetworkRule> network inet stream,
(based on get_raw())
A NetworkRuleset will be printed as (also based on get_raw())
<NetworkRuleset>
network inet stream,
allow network inet stream, # comment
</NetworkRuleset>
Also add tests to test-network.py to ensure that __repr__() works as
expected.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
aa-logprof is able to parse all profiles, so there is no longer a
reason to skip this test.
This patch reverts r2097 and r2098 from 2013-01-02.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com>
(and now that the tests work even if logprof.conf doesn't exist,
Steve's NACK is no longer valid)
This patch checks if the cfg object is empty (happens if logprof.conf
doesn't exist). If so, it adds some empty sections to prevent various
failures in code that expects those sections to exist.
Another source of failures was using cfg['section']['setting']. The
patch changes various places to cfg['section'].get('setting') to prevent
those failures. (Those places all have a 'or ...' fallback.)
Finally, find_first_file() in config.py crashed if file_list was Null.
This is fixed by adding an "if file_list:" check before trying to
split() it.
With all those changes applied, 'make check' will work even if
/etc/apparmor/logprof.conf doesn't exist.
The patch also fixes the default value for inactive_profiledir
(I missed aa.py when I changed it to /usr/share/apparmor/extra-profiles/)
References: https://bugs.launchpad.net/apparmor/+bug/1393979
Acked-by: John Johansen <john.johansen@canonical.com>
Both create_new_profile() and handle_children() check if the given exec
target is a script and add permissions for the interpreter and a
matching abstraction.
This patch merges that into the get_interpreter_and_abstraction()
function and changes create_new_profile() and handle_children() to use
this function.
A nice side effect is that handle_children() now knows more abstractions
(its original list was incomplete).
The behaviour of create_new_profile() doesn't change.
Also add tests for get_interpreter_and_abstraction() to make sure it
does what we expect.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Bug: https://launchpad.net/bugs/1505775
These tests ensure that create_new_profile() sets the expected basic
permissions for scripts and non-script files.
Acked-by: John Johansen <john.johansen@canonical.com>
oftc_ftw reported on IRC that Arch Linux has a symlink /bin -> /usr/bin.
This means we have to update paths for /bin/ in several profiles to also
allow /usr/bin/
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
Also add support for the USE_SYSTEM variable, which means:
- test against the in-tree libapparmor and python modules by default
- test against the system libapparmor and python modules if USE_SYSTEM
is set
The old behaviour was a mix of both - it always used the in-tree python
modules and the system libapparmor.
For obvious reasons, you'll need to build libapparmor before running the
tests (unless you specify USE_SYSTEM=1 as parameter to make check).
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
Add a testcase that parses all tests in the parser/tst/simple_tests/
directory with parse_profile_data() to ensure that everything with valid
syntax is accepted, and that all tests marked as FAIL raise an
exception.
This already resulted in
- several patches to fix low-hanging fruits (including some bugs in the
parser simple_tests itsself)
- a list of tests that don't behave as expected. Those files get their
expected result reverted to make sure we notice any change in the
tools behaviour, especially changing to the really expected resulted.
This method also makes sure that the testcase doesn't report any of
the known failures.
- a 5% improvement in test coverage - mostly caused by nearly completely
covering parse_profile_data.
- addition of some missing testcased (as noticed by missing coverage),
for example several "rule outside of a profile" testcases.
As indicated above, the tools don't work as expected on all test
profiles - most of the failures happen on expected-to-fail tests that
pass parse_profile_data() without raising an exception. There are also
some tests failing despite valid syntax, often with rarely used syntax
like if conditions and qualifier blocks.
Most of the failing (generated) tests are caused by features not
implemented in the tools yet:
- validating dbus rules (currently we just store them without any parsing)
- checks for conflicting x permissions
- permissions before path ("r /foo,")
- 'safe' and 'unsafe' keywords for *x rules
- 'Pux' and 'Cux' permissions (which actually mean PUx and CUx, and get
rejected by the tools - ideally the generator script should create
PUx and CUx tests instead)
skip_startswith excludes several generated tests from being run. I know
that skip_startswith also excludes tests that would not fail, but the
generated filenames (especially generated_x/exact-*) don't have a
pattern that I could easily use to exclude less tests - and I'm not too
keen to add a list with 1000 single filenames ;-)
Acked-by: John Johansen <john.johansen@canonical.com>
The global variable 'logger' in aa.py is only used by aa-genprof.
This patch changes aa_genprof to use the (new) logger_path() function,
and moves the code for finding the logger path to that function.
Also make the error message more helpful if logger can't be found.
Acked-by: John Johansen <john.johansen@canonical.com>
The 'ldd' variable in aa.py is only used by get_reqs(), therefore move
setting it (based on the configfile) into the function.
get_reqs() doesn't run too often (only called by create_new_profile(),
which means aa-genprof or when adding a Px or Cx rule to a non-existing
profile). This might even lead to a minor performance win - on average,
I'd guess not every aa-logprof run will lead to a completely new profile
or child profile. And, more important, we get rid of a global variable.
Acked-by: John Johansen <john.johansen@canonical.com>
create_new_profile() didn't init missing required_hats as
profile_storage(), which might lead to crashes when creating a profile
for an application listed in the required_hats config option (= in very
rare cases).
This patch adds the missing profile_storage() call.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This also means the duplicate detection can use the hat's filename instead
of the (possibly wrong) main profile's filename.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This patch is based on a SLE12 patch to allow executing the
--dhcp-script. We already have most parts of that patch since r2841,
except /dev/tty rw which is needed for the shell's stdout and stderr.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=940749 (non-public)
Acked by Seth Arnold on IRC (with "owner" added)
With this addition, all globbing styles (as documented in apparmor.d(5))
are covered in the convert_regexp() tests.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
load_include() used a custom os.listdir call instead of
include_dir_filelist() for directory includes, which means it also read
skippable files like *.rpmnew or README. (It seems nobody created a
README inside an included directory, otherwise we'd have seen a
bugreport ;-)
This patch changes load_include() to use include_dir_filelist(). This
function is used in some more places already and removes skippable files
from the file list.
Acked-by <timeout>
load_include() has a "if not incdata:" block which would be entered if
parse_profile_data() returns None. However, parse_profile_data() always
returns a hasher with [incfile][incfile] = profile_storage(), so that
"if not incdata:" never matches.
Acked-by <timeout>
The "already loaded?" check in load_include() was done at the beginning
of the function, before entering the loop and before the individual
files of directory includes were added to the filelist. This resulted in
a (wrong) "Conflicting profiles" error for directory includes.
This patch moves the "alreay loaded?" check inside the loop, so that
it's executed for all files, including those of directory includes.
Acked-by <timeout>
TL;DR: aa-genprof crashes with a wrong 'Conflicting profiles' error.
aa-genprof uses autodep() to create a basic profile, which is then
stored in aa and original_aa. After that, read_profiles() is called,
which reads all profiles (including the new one) from disk, causing a
(wrong) 'Conflicting profiles' error in attach_profile_data() because
the autodep()-generated profile is already there.
Therefore this patch resets aa and original_aa in read_profiles() to
avoid that problem.
Acked-by <timeout>
The tests for convert_regexp() were hidden in common_test.py, where they
were never executed.
This patch moves them to the new file test-aare.py and also converts the
regex_tests.ini to a tests[] array to have the test data inside the test
file. (All tests from regex_tests.ini are in test-aare.py, and two tests
with prepended and appended path segments were added.)
Also add some tests that check the raw behaviour of convert_regexp() -
the tests "by example" are probably more useful and for sure more
readable ;-) but I want to have some examples of the converted regexes
available.
Acked-by <timeout>
logparser.py does a regex check on log lines as performance improvement
so that it only hands over lines that look like AppArmor events to
LibAppArmor parsing. Those regexes were incomplete and didn't cover all
log formats LibAppArmor accepts, with the end result of "overlooking"
events.
This patch splits off common parts of the regex, adds more regexes for
several log types and finally merges everything into one regex.
test-libapparmor-test_multi.py now also checks all test_multi log lines
against the regex to ensure logparser.py doesn't silently ignore events.
test-logparser.py gets adjusted to the merged RE_LOG_ALL regex.
Finally, add a new test that was posted on IRC to the test_multi set.
As already threatened nearly a month ago,
Acked by <timeout> for trunk and 2.9
This patch is based on a SLE12 patch to allow executing the
--dhcp-script. We already have most parts of that patch since r2841,
however the SLE bugreport indicates that /bin/sh is executed (which is
usually a symlink to /bin/bash or /bin/dash), so we should also allow
/bin/sh
References: https://bugzilla.opensuse.org/show_bug.cgi?id=940749 (non-public)
Acked-by: Seth Arnold <seth.arnold@canonicalc.com> for trunk and 2.9
Add some permissions that I need on my system:
- execute nm-dhcp-helper
- read and write /var/lib/dhcp6/dhclient.leases
- read /var/lib/NetworkManager/dhclient-*.conf
- read and write /var/lib/NetworkManager/dhclient-*.conf
Looks-good-by: Steve Beattie <steve@nxnw.org>
Acked-by: <timeout> for trunk and 2.9
This testcase will parse all libraries/libapparmor/testsuite/test_multi
tests and compare the result with the *.out files.
It also include a "ToDo list" of keywords that are not yet supported in
the python code - those are typically related to rule types not
supported in the tools yet (dbus, signal etc.).
An interesting special case are exec events with network details:
testcase01.in, testcase12.in, testcase13.in
which might be hand-made, invalid logs, but nobody remembers ;-)
Acked-by <timeout>
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>
Fix memory leaks when parsing dmesg timestamps as well as when handling
message the library does not understand.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@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>
/usr/share/locale-bundle/ contains translations packaged in
bundle-lang-* packages in openSUSE.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
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>
In testing against the 4.1 kernel, the syscall_sysctl testcase started
failing even in the unconfined case. What the test program does is
attempt to adjust the kernel.threads-max sysctl to be slightly larger
and see if the operation succeeds by reading the value back out. It
also attempts to save the original value and restore it. The test
was failing because (in VMs at least) the default value chosen by
the kernel for the kernel.threads-max setting was high enough that
attempts to increase it would be ignored (likely to prevent too much
use of kernel memory by threads), helpfully without any message being
report to dmesg. Thus, the initial read of the current value would
succeed, the write of that value + 1024 would appear to succeed,
but then reading the value back out and comparing it to the expected
value would fail, as it would still be the original value, not the
expected new value.
This patch attempts to address this by first attempting to raise
the value, and if that does not appear to work, to then attempt
to lower it. It also refactors the code a bit by creating helper
functions to perform the actual sysctl(2) calls to make the code a
bit easier to read.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Having two profiles for the same binary is "technically allowed", but it
leads to interesting[tm] behaviour because one of them "wins" depending
on the load order. To make things even more interesting, the kernel load
order can be different from the tools load order, leading to even more
fun.
Short version: you do _not_ want that situation ;-)
This patch adds a duplicate check to attach_profile_data() so that it
errors out if it finds duplicate profiles or hats, and lists the profile
files that contain them.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both trunk and 2.9.
In some cases, the return value of name_to_prof_filename() is undefined.
This happens when deleting the to-be-confined binary while running
aa-genprof and leads to a not-too-helpful
File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 265, in enforce
prof_filename, name = name_to_prof_filename(path)
TypeError: 'NoneType' object is not iterable
(reported by maslen on IRC)
This patch makes sure name_to_prof_filename() always returns None, None
(instead of undefined aka just None) so that at least the caller can
successfully split it into two None values.
For the exotic aa-genprof usecase given above, this at least improves
the error message to
Can't find $binary_name
(raised by enforce() via fatal_error())
The patch also changes fatal_error() to display the traceback first, and
the human-readable message at the end, which makes it more likely that
the user actually notices the human-readable message.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both trunk and 2.9.
Profile name and attachment can contain variables, so the
RE_PROFILE_START regex should accept it.
(Note: the variable content isn't checked.)
Also add some tests with variables.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
add_event_to_tree() is a hard-to-test function because it hands over its
result to add_to_tree().
This patch converts add_event_to_tree() to a simple wrapper function and
moves the main code into parse_event_for_tree() and map_log_type(). These
two new functions return their results and are therefore easier to test.
Acked-by: Kshitij Gupta <kgupta8592@gmail.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
dconf abstraction: allow reading /etc/dconf/**.
That's needed e.g. for Totem on current Debian Jessie.
Acked-By: Jamie Strandboge <jamie@canonical.com>
The '#!/usr/bin/env python' line in apparmor/rule/*.py is superfluous
and causes "non-executable script" rpmlint warnings on openSUSE.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
TL;DR: the answer is "yes" ;-)
(see the patch for the question...)
Long version:
When creating a new child profile with aa-logprof or aa-genprof, the
child profile wasn't properly initialized in handle_children(), which
lead to a crash in delete_duplicates() later because capability etc.
was not set to a CapabilityRuleset etc. class and therefore
profile['capability'] didn't have a .delete_duplicates() method.
Funnily there was already a comment "do we need to init the profile here?"
This patch replaces the question in the comment with the answer.
Acked-by: Steve Beattie <steve@nxnw.org>
The local defines in the link_subset test collide and result in build
warnings. Replace the defines with a naming that won't collide and
makes it clear a local define for the test is being used.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
1. The test is using the wrong defines: It is using the defines from the
parser for the packed dfa permissions. This set of permissions is not
meant to be exposed to the outside world
2. The kernel is using the wrong mapping function for the permissions
in the file class. This results in partially exposing the packed
permissions, but even then it doesn't fully line up with the packed
permissions, and is not correct for several of the potential permissions.
Attached is a patch that fixes the test, and moves the two tests that
fail due to the kernel to xpass.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
In the commit "Rev 3169: regression tests: have
ptrace use PTRACE_GETREGSET by default", I created
some ifdef magic to use the per arch general purpose
register data structures for various architectures,
including arm64. Unfortunately, in the upstream glibc commit
7d05a8168b
<bits/ptrace.h> is no longer included in the arm64 specific user.h,
which defined the structure as 'struct user_pt_regs'; instead user.h
was converted to define 'struct user_regs_struct'. Because of this, the
ptrace test fails to compile on arm64 when glibc is 2.20 or newer.
This patch adjusts the ptrace test to use the newer structure on arm64
if it's detected that a newer glibc is detected and reverts to using
the older one for older glibcs. It also adds an error when compiling
on architectures that haven't been incorporated yet.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Besides adding this feature, this also fixes a crash in tools.py __init__():
AttributeError: 'Namespace' object has no attribute 'do_reload'
Acked-by: Steve Beattie <steve@nxnw.org>
create_new_profile() created a wrong structure for local_profile, which
resulted in an aa-genprof crash directly at startup (in the autodep
phase).
This patch fixes it to use the correct structure.
Acked-by: Steve Beattie <steve@nxnw.org>
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>
cux and CUx are valid exec permissions, so they should be accepted
by validate_profile_mode() ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
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>
- allow only a specific set of time units
- optionally allow whitespace between rlimit value and unit
- move check for invalid time units to time_to_int()
Also update the tests:
- add several tests with whitespace between value and unit
- change a test that used the (now invalid) "1m" to "1min"
- change the time_to_int() tests to use 'us' as default unit, and add
a test with 'seconds' as default unit
Acked-by: Steve Beattie <steve@nxnw.org>
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>
aa-logprof raises an exception if
- an include file contains a hat
- that file is included in a profile and
- aa-logprof hits an audit log entry for this profile
Reproducer ("works" on 2.9 and trunk):
python3 aa-logprof -f <(echo 'Jun 19 11:50:36 piorun kernel: [4474496.458789] audit: type=1400 audit(1434707436.696:153): apparmor="DENIED" operation="open" profile="/usr/sbin/apache2" name="/etc/gai.conf" pid=2910 comm="apache2" requested_mask="r" denied_mask="r" fsuid=0 ouid=0') -d ../profiles/apparmor.d/
This happens because profiles/apparmor.d/apache2.d/phpsysinfo was
already read when pre-loading the include files.
This patch changes aa.py parse_profile_data() to only raise the
exception if it is not handling includes currently.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
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>
is_known_rule() ignored directory includes, which resulted in asking for
and adding superfluous rules that are already covered by a file in the
included directory.
This patch looks bigger than it is because it moves quite some lines
into the "else:" branch. Everything inside the "else:" just got an
additional whitespace level.
References: https://bugs.launchpad.net/apparmor/+bug/1471425
(however, trunk didn't crash, it "just" ignored directory includes)
Acked-by: Steve Beattie <steve@nxnw.org>
is_known_rule() in aa.py checked only direct includes, but not includes
in the included files. As a result, aa-logprof asked about things that
are already covered by an indirect include.
For example, the dovecot/auth profile includes abstractions/nameservice,
and abstractions/nameservice includes abstractions/nis, which contains
"capability net_bind_service,".
Nevertheless, aa-logprof asked to add capability net_bind_service.
Reproducer: (asks for net_bind_service without this patch, should not
ask for anything after applying the patch):
python3 aa-logprof -d ../profiles/apparmor.d/ -f <(echo 'type=AVC msg=audit(1415403814.628:662): apparmor="ALLOWED" operation="capable" profile="/usr/lib/dovecot/auth" pid=15454 comm="auth" capability=13 capname="net_bind_service"')
The patch adds code to check include files included by other include
files. Note that python doesn't allow to change a list while looping
over it, therefore we have to use "while includelist" as workaround.
This fixes a regression for network rules (this patch is based on the
old match_net_include() code). Funnily it "only" fixes capability rule
handling (without the "regression" part) because the old
match_cap_include() didn't do the recursive include handling.
Acked-by: Steve Beattie <steve@nxnw.org>
For some (not yet known) reason, we get file_perm events without
request_mask set, which causes an aa-logprof crash.
Reproducer log entry:
Jun 19 12:00:55 piorun kernel: [4475115.459952] audit: type=1400 audit(1434708055.676:19629): apparmor="ALLOWED" operation="file_perm" profile="/usr/sbin/apache2" pid=3512 comm="apache2" laddr=::ffff:193.0.236.159 lport=80 faddr=::ffff:192.168.103.80 fport=61985 family="inet6" sock_type="stream" protocol=6
This patch changes logparser.py to ignore those events.
References: https://bugs.launchpad.net/apparmor/+bug/1466812/
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
According to the parser test profiles (which are the only
"documentation" I found about this), definition of boolean variables
is only allowed outside profiles, not inside them.
parse_profile_data() got it the wrong way round, therefore this patch
fixes the condition and updates the error message.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Thanks to a bug in the apparmor.d manpage, NetworkRule rejected rules
that contained only TYPE (for example "network stream,"). A bugreport on
IRC and some testing with the parser showed that this is actually
allowed, so NetworkRule should of course allow it.
Note: not strip()ing rule_details is the easiest way to ensure we have
whitespace in front of the TYPE in TYPE-only rules, which is needed by
the RE_NETWORK_DETAILS regex.
Also adjust the tests to the correct behaviour.
Acked-by: Steve Beattie <steve@nxnw.org>
Instead of always showing a backtrace,
- for AppArmorException (used for profile syntax errors etc.), print only
the exceptions value because a backtrace is superfluous and would
confuse users.
- for other (unexpected) exceptions, print backtrace and save detailed
information in a file in /tmp/ (including variable content etc.) to
make debugging easier.
This is done by adding the apparmor.fail module which contains a custom
exception handler (using cgitb, except for AppArmorException).
Also change all python aa-* tools to use the new exception handler.
Note: aa-audit did show backtraces only if the --trace option was given.
This is superfluous with the improved exception handling, therefore this
patch removes the --trace option. (The other aa-* tools never had this
option.)
If you want to test the behaviour of the new exception handler, you can
use this script:
#!/usr/bin/python
from apparmor.common import AppArmorException, AppArmorBug
from apparmor.fail import enable_aa_exception_handler
enable_aa_exception_handler()
# choose one ;-)
raise AppArmorException('Harmless example failure')
#raise AppArmorBug('b\xe4d bug!')
#raise Exception('something is broken!')
Acked-by: Seth Arnold <seth.arnold@canonical.com>
As shown in parser/tst/simple_tests/profile/flags/flags_ok_whitespace.sd,
the parser is quite tolerant to additional or missing whitespace around
flags=, while the tools are more strict.
This patch updates the RE_PROFILE_START regex to follow this tolerance.
Acked-by: Steve Beattie <steve@nxnw.org>.
The only difference between PROFILE_MODE_RE and PROFILE_MODE_NT_RE
was that the latter one additionally allowed 'x', which looks wrong.
(Standalone 'x' is ok for deny rules, but those are handled by
PROFILE_MODE_DENY_RE.)
This patch completely drops PROFILE_MODE_NT_RE and the related code in
validate_profile_mode().
Also wrap the two remaining regexes in '^(...)+$' instead of doing it
inside validate_profile_mode(). This makes the code more readable and
also results in a 2% performance improvement when parsing profiles.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Add the missing "pux" to PROFILE_MODE_RE and PROFILE_MODE_NT_RE.
Also move those regexes and PROFILE_MODE_DENY_RE directly above
validate_profile_mode() which is the only user.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Parsing of boolean assignments failed with
TypeError: '_sre.SRE_Match' object is not subscriptable
because of a missing ".groups()"
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
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
Bug: https://bugs.launchpad.net/apparmor/+bug/1470985
The ptrace regression test fails to compile on the arm64 platform,
because it uses PTRACE_GETREGS and not the newer PTRACE_GETREGSET
interface for getting access to arch-specific register information[0].
However, fixing it is complicated by the fact that the struct name
for for the general purpose registers is not named consistently
across architectures. This patch attempts to address those issues,
and compiles at least on i386, amd64, arm64, arm (armhf), ppc64,
and ppc64el. The test is verified to continue to function correctly
on i386 and amd64.
[0] https://sourceware.org/ml/archer/2010-q3/msg00193.html
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
RlimitRule accidently used 'ms' (milliseconds) as default unit for
rttime rules, but rttime without unit means 'us' (microseconds). This
patch fixes this.
Also add some tests with 'us' as unit, and two more to cover terribly
invalid corner cases (and to improve test coverage by 2 lines ;-)
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Change minitools tests to use AATest and work inside a tmpdir.
This results in lots of changes ('./profiles' -> self.profile_dir,
local_profilename -> self.local_profilename etc.) and also moves some
code from the global area to AASetup().
Also drop the no longer needed clean_profile_dir() and add linebreaks
in assert* calls with a long error message specified.
Acked-by: Steve Beattie <steve@nxnw.org>
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>
LSMs, such as AppArmor, aren't consulted when a program calls access(2).
This can result in access(2) returning 0 but a subsequent open(2)
failing.
The aa-status utility was doing the access() -> open() sequence and we
became aware of a large number of tracebacks due to open() failing for
lack of permissions. This patch catches any IOError exceptions thrown by
open(). It continues to print the same error message as before when
access() failed but also prints that error message when AppArmor blocks
the open of the apparmorfs profiles file.
https://launchpad.net/bugs/1466768
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The function is basically a wrapper around a regex, so regex.py is a
much better home.
While on it, rename the regex to RE_INCLUDE, change it to named matches,
use RE_EOL to handle comments and compile it outside the function, which
should result in a (small) performance improvement.
Also rewrite re_match_include(), let it check for empty include
filenames ("#include <>") and let it raise AppArmorException in that
case.
Finally, adjust code calling it to the new location, and add some tests
for re_match_include()
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
profile_storage() returns an empty, properly initialized profile.
It doesn't explicitly init all keys (yet) and will be extended over
time, with the final goal to get rid of hasher().
Also change various places in aa.py to use it (instead of an empty
hasher or sub-hasher), and remove various "init rule class (if not done
yet)" cases.
This also avoids a crash in aa-cleanprof remove_duplicate_rules().
Hats weren't properly initialized in aa.py parse_profile_data()
(especially rule classes were missing), which caused a crash because
hasher doesn't support the delete_duplicates() method.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Change hat declarations ("^hat,") are no longer supported (see previous
patch for details). Therefore remove support for writing them.
This also means to completely remove the 'declared' flag, which was only
needed for hat declarations, and was (after the previous patch) always
set to False.
Also add a hat to the cleanprof_test.{in,out} test profile to make sure
aa-cleanprof doesn't break hats, and a hat declaration with the same
name to make sure it gets removed and doesn't break the "real" hat.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Hat declarations ("^hat,") were added in 2.3 for declaring external
hats, but in the meantime aren't supported by the parser anymore (tested
with 2.9.2 parser).
Additionally, if a profile contains both a hat declaration and the hat
("^hat { ...}"), the hat declaration can overwrite the content of the
hat on a "last one wins" base.
This is caused by setting 'declared' to True, which means write_piece()
will only write the "^hat," line, but not the "^hat { ... }" block.
Therefore no longer set 'declared' to True, print a warning that hat
declarations are no longer supported, and ignore the rule. This also
means that running aa-cleanprof can make the profile valid again :-)
Also no longer change 'hat' when hitting a profile declaration, which
also looks wrong.
Note: This change removes the only usage of 'declared'. A follow-up
patch (trunk only) will completely remove the 'declared' handling.
Reproducer profile (run aa-cleanprof on it):
(will crash in remove_duplicate_rules() 80% of the time - if so, try
multiple times. One of the next patches will fix that. Or just try 2.9,
which doesn't have the crash in remove_duplicate_rules().)
/usr/bin/true {
^FOO {
capability setgid,
}
# deletes the content of ^FOO when saving the profile! (last one wins)
# additionally, the parser says this is invalid syntax
^FOO,
}
See also the "Hat declarations" thread on the ML,
https://lists.ubuntu.com/archives/apparmor/2015-June/008107.html
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both 2.9 and trunk.
Change aa.py to use RlimitRule and RlimitRuleset instead of a sub-hasher
to store and write rlimit rules. In detail:
- drop all rlimit rule parsing from parse_profile_data() and
serialize_profile_from_old_profile() - instead, just call
RlimitRule.parse()
- change write_rlimits() to use RlimitRuleset
- add removal of superfluous/duplicate change_profile rules (the old
code didn't do this)
- update the comment about aa[profile][hat] usage - rlimit and
change_profile are no longer dicts.
Also cleanup RE_PROFILE_RLIMIT in regex.py - the parenthesis around
'<=' are no longer needed.
Note: This patch is quite small because aa-logprof doesn't ask for
rlimit rules.
I tested all changes manually with aa-cleanprof and aa-logprof (adding
some file rules, rlimit rules kept unchanged)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
check-logprof in profiles/Makefile needs the local/* files.
Add a dependency to make sure they are generated.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Only use the special %exception directive for functions that return a
negative int and set errno upon error.
This prevents, for example, _aa_is_blacklisted() from raising an
exception when it returns -1. This is important because it doesn't set
errno so an exception based on the value of errno would be
unpredictable.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@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>
Ignore README files when performing an operation on a list of files.
This matches the behavior of the is_skipped_file() function in aa.py.
The hope is that is_skippable_file() can reuse _aa_is_blacklisted().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
It looks odd to access the first character of a string before checking
to see if the string's length is zero. This is actually fine, in
practice, since strlen() looks at the first character of the string for
the presence of '\0' which means this is entirely a cosmetic change.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Prepend the function prototypes with extern to match the style of the
existing prototypes.
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>
Create a section 3 man page for the aa_policy_cache family of functions.
Additionally, update the in-code descriptions to match the descriptions
in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create a section 3 man page for the aa_kernel_interface family of
functions. Additionally, update the in-code descriptions to match the
descriptions in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create a section 3 man page for the aa_features family of functions.
Additionally, update the in-code descriptions to match the descriptions
in the man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johanse@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>
Make the function prototype for reading a features directory the same
as the function prototype for reading a features file.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Two different implementations were in use for reading features files.
One for reading a single file and another for reading a single file
after walking a directory. This patch creates a single function that is
used in both cases.
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>
The most common case when creating an aa_kernel_interface object will be
to do so while using the current kernel's feature set for the
kernel_features parameter. Rather than have callers instantiate their
own aa_features object in this situation, aa_kernel_interface_new()
should do it for them if they specify NULL for the kernel_features
parameter.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The most common case when creating an aa_policy_cache object will be to
do so while using the current kernel's feature set for the
kernel_features parameter. Rather than have callers instantiate their
own aa_features object in this situation, aa_policy_cache_new() should
do it for them if they specify NULL for the kernel_features parameter.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The aa_features object that is passed to aa_policy_cache_new() does not
have to represent the currently running kernel. It may represent a
different kernel, such as a kernel that was just installed, that is not
currently running.
This patch adjusts the function comments to remove mentions of
"... the currently running kernel".
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
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>
Add two variable references (aa and changed) in aa-mergeprof
ask_the_questions() so that the code can use the short name and be more
in sync with aa.py ask_the_questions().
With this patch applied, the "for ruletype in ['capability', 'network']:"
block is in sync, with the exception of the sections that intentionally
differ:
- the check for the profile mode
- the default button selection based on profile mode
- the seen_events counter
The patch also includes some minor whitespace fixes.
Acked-by: Steve Beattie <steve@nxnw.org>
Applying patches often creates *.orig files, and those files are quite
annoying in the "bzr status" output and also in the "unknown" file list
when commiting.
Note: I intentionally don't want to add *.rej files - while those files
should never end up in bzr, they are important enough to be listed in
bzr status output.
Acked-by: Steve Beattie <steve@nxnw.org>
flags_bad.sd contains multiple failures. Split the file into multiple
files with one failure in each and, while on it, using more helpful
filenames.
Acked-by: Steve Beattie <steve@nxnw.org>
The following patch:
- removes re import
- uses apparmor.re_match_include instead of the regex
which also means to use the correct regex instead of
the slightly wrong one cleanprofile.py had
Acked-by: Christian Boltz <apparmor@cboltz.de>
The cleanprofile.py has an apparmor import, this patch modifies the import to make it consistent with the rest of modules.
Acked-by: Christian Boltz <apparmor@cboltz.de>
The following patch:
- Brings the return to the correct indentation
- Adds a sorted call over the set keys of hat in the profile
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.9.
After switching to winbindd as test profile, comments about the ntpd
profile don't make sense anymore ;-)
The patch also includes some whitespace fixes.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This time we only have 98% coverage (some missing and partial) because
I didn't find corner cases that raise some exceptions ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
The class comes with the usual set of features, so I'll only mention a
special feature: the is_covered() and is_equal() functions can even
compare limits with different units (for example they recognize that
2minutes == 120seconds).
Also change RE_PROFILE_RLIMIT:
- make it a bit more strict (the old one accepted any chars, including
spaces, for rlimit and value)
- convert it to named matches
- '<=' isn't optional - remove the '?' (but keep the parenthesis to
avoid breaking parsing in aa.py)
- allow rules with no spaces around '<='
Acked-by: Steve Beattie <steve@nxnw.org>
aa-cleanprof (actually clean_profile() in tools.py) used reload_base()
from aa.py which sends the parser output to /dev/null. This had two
effects:
- aa-cleanprof ignored the --no-reload parameter
- there was no error message because reload_base() /dev/null's the
parser output
This patch changes clean_profile() to use reload_profile() from tools.py
(which honors the --no-reload option).
Also add a TODO note to aa.py reload_base(), the (AFAIK only) winner of
the 'useless use of cat' award in the AppArmor code.
We should really change it to use reload_profile(), even if that means
moving the function from tools.py to aa.py or common.py. And it should
not /dev/null the apparmor_parser output. ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1443637
Acked-by: Steve Beattie <steve@nxnw.org>
aa-complain is part of the enforce/complain/disable triple. Therefore
I expect it to actually load a profile in complain mode.
To do this, it has to delete the 'disable' symlink, but set_complain()
in aa.py didn't do this (and therefore kept the profile disabled).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Users might expect that setting a profile into audit mode also activates
it (which shouldn't happen IMHO because the audit flag is not part of
the enforce/complain/disable triple), so we should at least tell them.
References: https://bugs.launchpad.net/apparmor/+bug/1429448
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
aa-complain, aa-enforce, aa-disable and aa-audit refused to change
profiles for non-existing binaries. This patch also allows paths
starting with /. This also makes it possible to use
aa-complain '/{usr/,}bin/ping'
and
aa-complain /etc/apparmor.d/bin.ping
This patch fixes https://bugs.launchpad.net/apparmor/+bug/1416346
Well, mostly - we still need to decide how we handle wildcards in
profile names:
aa-complain ping
aa-complain /usr/bin/ping
will still error out with "Profile not found" because it isn't an exact
match (and matching the wildcard would change more than the user wants).
Oh, and this patch also fixes the last failure in minitools_test.py.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Change minitools_test.py to use the winbind instead of the ntpd profile
for testing. The tests broke because the ntpd profile has the
attach_disconnected flag set now, and therefore didn't match the
expected flags anymore.
Also replace the usage of filecmp.cmp() in the cleanprof test with
reading the file and using assertEqual - this has the advantage that we
get a full diff instead of just "files differ".
Note: The aa-cleanprof test is still failing because of a bug in
tools.py, but will be fixed by the next patch.
See https://bugs.launchpad.net/apparmor/+bug/1416346 for details.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This allows to run minitools_test.py as non-root user.
Also add a check that only creates the force-complain directory if it
doesn't exist yet.
Note: With this patch applied, there are still 4 failing tests, probably
caused by changes in the profiles that are used in the tests.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add a --no-reload parameter to aa-audit, aa-cleanprof, aa-complain,
aa-disable and aa-enforce. This makes it possible to change the
profile flags without reloading the profile.
Also change tools.py to honor the --no-reload parameter.
References: https://bugs.launchpad.net/apparmor/+bug/1458480
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
--fixes lp:1458480
The function will return the 'Exec Condition' and the 'Target Profile'
as nice list to use in aa-logprof (once we have support for
change_profile in logparser.py) and aa-mergeprof.
Also add some tests to ensure the correct result.
Acked-by: Steve Beattie <steve@nxnw.org>
This allows to drop the "apparmor.aa." prefix in ask_the_question() to
get the code more in sync with aa.py ask_the_question().
Acked-by: Steve Beattie <steve@nxnw.org>
Replace the code in aa.py ask_the_questions() that handles network rules
with the ask_the_questions() code initially copied from aa-mergeprof.
This means to convert the network/netdomain log events to a
NetworkRuleset stored in the log_obj hasher, and then let the code from
aa-mergeprof operate on this hasher.
The user interface is mostly unchanged, with two exceptions:
- options always displayed, even if there is only one option
- some slightly changed texts
If you didn't understand why there's a need for the previous patch, this
one should explain it :-)
This also ends up fixing at least one bug where the 'audit' keyword
wasn't listed as a separate qualifier, but instead showed up smooshed
into the Network Family header.
Acked-by: Steve Beattie <steve@nxnw.org>
Replace the code in aa.py ask_the_questions() that handles capabilities
with the ask_the_questions() code from aa-mergeprof.
This means to convert the capability log events to a CapabilityRuleset
stored in the (new) log_obj hasher, and then let the code from
aa-mergeprof operate on this hasher.
Most of the code after the "aa-mergeprof also has this code" comment is
a direct copy of the aa-mergeprof code, with the following changes:
- filter for profile mode (enforce/complain)
- set default button (allow or deny) based on profile mode
- keep seen_events counter happy (even if it isn't displayed anywhere)
- replace apparmor.aa.foo with just foo
The user interface is mostly unchanged, with two exceptions:
- options always displayed, even if there is only one option
- some slightly changed texts
Acked-by: Steve Beattie <steve@nxnw.org>
When switching the audit flag for network events in aa-logprof
(technically, it happens in aa.py ask_the_question()), the "(I)gnore"
button gets "lost".
This patch fixes the list of available buttons.
I propose this patch for trunk and 2.9.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Move the code to set q.headers, q.functions and q.default for network
and capability rules inside the "while not done" loop. This ensures to
always have valid headers (for example, after changing the audit
qualifier, the severity was "lost" before) and avoids some duplicated
code.
Also drop a useless "if True:" condition and change the whitespace of
the following lines.
Acked-by: Steve Beattie <steve@nxnw.org>
Now that the handling for capability and network rules is the same,
wrap the former network rule-only code with
for ruletype in ['capability', 'network']:
and delete the superfluous ;-) capabiltiy code block.
Needless to say that future updates for other rule types will be
quite easy ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
BaseRule:
- add logprof_header() - sets the 'Qualifier' (audit, allow/deny) header
if a qualifier is specified, calls logprof_header_localvars() and then
returns an array of headers to display in aa-logprof and aa-mergeprof
- add logprof_header_localvars() - dummy function that needs to be
implemented in the child classes
NetworkRule: add logprof_header_localvars() - adds 'Network Family'
and 'Socket Type' to the headers
CapabilityRule: add logprof_header_localvars() - adds 'Capability' to
the headers
Also change aa-mergeprof to use rule_obj.logprof_header() for network
and capability rules. This means deleting lots of lines (that moved to
the *Rule classes) and also deleting the last differences between
capabiltiy and network rules.
Finally add tests for the newly added functions.
Acked-by: Steve Beattie <steve@nxnw.org>
This means:
a) for capability rules:
- move audit and deny to a new "Qualifier" header (only displayed if
non-empty)
- always display options, even if only one is available
- use available_buttons(), which means to add the CMD_AUDIT_* button
- add handling for CMD_AUDIT_* button
- CMD_ALLOW: only add rule_obj if the user didn't select a #include
- move around some code to get it in sync with network rule handling
b) for network rules
- move audit and deny to a new "Qualifier" header (only displayed if
non-empty)
- call rule_obj.severity() (not implemented for network rules, does
nothing)
- change messages to generic 'Adding %s to profile.'
- move around some code to get it in sync with capability rule
handling
The only remaining difference is in q.headers[] and the variables
feeding it:
- capability rules show "Capability: foo"
- network rules show "Network Family: foo" and "Socket type: bar"
Acked-by: Steve Beattie <steve@nxnw.org>
Note: the != sev_db.NOT_IMPLEMENTED: check in aa-mergeprof is
superfluous for capabilities, but will become useful once this code
block is used for other rule types.
Acked-by: Steve Beattie <steve@nxnw.org>
Also implement handling for the special capability value '__ALL__' in
severity.py, which is used for 'capability,' rules (aa-mergeprof might
need to display the severity for such a rule).
Finally, add some tests for severity() in test-capability.py and a test
for '__ALL__' in test-severity.py.
Acked-by: Steve Beattie <steve@nxnw.org>
severity() will, surprise!, return the severity of a rule, or
sev_db.NOT_IMPLEMENTED if a *Rule class doesn't implement the severity()
function.
Also add the NOT_IMPLEMENTED constant to severity.py, and a test to
test-baserule.py that checks the return value in BaseRule.
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>
The parser currently is still using the old permission layout, the kernel
uses a newer layout that allows for more permission bits. The newer
newer permission layout is needed by the library to query the kernel,
however that causes some of the permission bits to be redefined.
Rename the permission bits that cause redefination warnings to use
AA_OLD_MAY_XXX
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Currently the cache file has its mtime set at creation time, but this
can lead to cache issues when a policy file is updated separately from
the cache. This makes it possible for an update to ship a policy file
that is newer than the what the cache file was generated from, but
result in a cache hit because the cache file was local compiled after
the policy file was package into an update (this requires the update
to set the mtime of the file when locally installed to the mtime of
the file in its update archive but this is commonly done, especially
in image based updates).
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When caching was converted to use mtime instead of ctime, the cache
file timestamp did not get switched over. This means we are comparing
the cache file's ctime against the policy file's mtime. Which can make
the cache look newer than it really is.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
- missing formatting code prefixes, usually I for BNFish arguments
- added blank lines before preformatted sections as the html formatter
wasn't treating them as seperate from the preceding text (also, they
generated podchecker warnings)
- fixed a grammar issue
- fixed link description text block that was mistakenly indented and
thus treated as preformatted text
- moved the "Qualifier Blocks" subsection out of the =over/=back as
all the pod tools did not like this and it caused podchecker to exit
with an error, breaking builds that ran make check on the parser
tree.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Rename require_features to require_kernel_features and
have_features to kernel_features
to indicate they are tests for kernel features, as now there are tests
for parser features and in the future there might be library features
as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
has not been updated. The issue is that the regression tests detect the
kernel features set and generate policy that the parser may not be able
to compile.
Augment the regressions tests with a couple simple functions to test what
is supported by the parser, and update the test conditionals to use them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This option was previously only documented in the --help output.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
I decided to use a "small" solution for now, which basically means
s/unittest.TestCase/AATest/, cleanup of some setUp() and renaming the
remaining setUp() functions to AASetup().
This doesn't mean an instant win (like in test-severity.py), but allows
to add tests with a tests[] array.
Acked-by: Steve Beattie <steve@nxnw.org>
To be able to distinguish between severity 10 and unknown severity,
change AASetup to specify 'unknown' as default rank, and change the
expected result to 'unknown' where it's expected.
Also change the "expected rank %d" to "%s" because it can be a string
now, and add a test that contains directories with different severity
in one variable.
After these changes, handle_variable_rank() errors out with
TypeError: unorderable types: str() > int()
so fix it by
- initializing rank with the default rank (instead of none)
- explicitely check that rank and rank_new are != the default rank before
doing a comparison
A side effect is another bugfix - '@{HOME}/sys/@{PROC}/overcommit_memory'
is severity 4, not 10 or unknown (confirmed by reading severity.db).
Acked-by: Steve Beattie <steve@nxnw.org>
This simplifies test-severity.py a lot:
- lots of test functions are replaced with tests[] arrays
- tempdir handling and cleanup is now done automagically
Even if test-severity.py shrunk by 65 lines, all tests are still there.
There's even an addition - SeverityTestCap now additionally verifies the
result of rank_capability().
Acked-by: Steve Beattie <steve@nxnw.org>
Change rank_capability() so that it doesn't expect the CAP_ prefix.
This makes usage easier because callers can simply hand over the
capability name.
Also change rank() to call rank_capability() without the CAP_ prefix.
Acked-by: Steve Beattie <steve@nxnw.org>
Replace rule-specific names with generic names:
- s/'capability'/ruletype/
- s/cap_obj/rule_obj/
- s/'network'/ruletype/
- s/net_obj/rule_obj/
Also set ruletype at the beginning of each block.
The long-term goal is to have
for ruletype in ['capability', 'network', ...]:
with common code to handle all rule types, and having common names makes
it easier to compare the blocks.
Acked-by: Steve Beattie <steve@nxnw.org>
aa-mergeprof has some sections where it first resets the 'deleted'
variable, and then overwrites it again a line or two later.
This patch removes the superfluous variable resets.
Acked-by: Steve Beattie <steve@nxnw.org>
Add a check to parse_profile_data() to detect if a file contains two
profiles with the same name.
Note: Two profiles with the same name, but in different files, won't be
detected by this check.
Also add basic tests to ensure that a valid profile gets parsed, and two
profiles with the same name inside the same file raise an exception.
(Sidenote: these simple tests improve aa.py coverage from 9% to 12%,
which also confirms the function is too long ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Add writeTmpfile() to AATest to write a file into the tmpdir. If no
tmpdir exists yet, automatically create one.
createTmpdir() is a separate function so that it's possible to manually
create the tmpdir (for example, if a test needs an empty tmpdir).
Also add a tearDown() function to delete the tmpdir again. This function
calls self.AATeardown() to avoid the need for super() in child classes.
Finally, simplify AaTestWithTempdir in test-aa.py to use createTmpdir()
and add an example for AATeardown() to test-example.py.
Acked-by: Steve Beattie <steve@nxnw.org>
aa-mergeprof no longer calls match_net_includes(), which means the
function can be dropped.
After that, match_net_include() is also unused, so also drop it.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
aa-mergeprof still used the old aa[profile][hat][allow]['netdomain']
which no longer gets populated. This resulted in not asking for merging
any network rules.
This patch changes ask_the_question() to the NetworkRule(set) layout.
Besides that,
- don't ask for network rules that are already covered.
Using is_known_rule() also fixes
https://bugs.launchpad.net/apparmor/+bug/1382241
- include the audit keyword in the "Network Family" headline
(I'd prefer to just use the get_clean() rule, but that's another topic)
- hide "(A)llow" when merging a deny rule
- as a side effect of using NetworkRule, fix crashes for 'network,' and
'network foo,' rules
To avoid having to repeat the list of available "buttons" and the logic
to update that list, add a available_buttons() function that returns the
list of available buttons depending on rule_obj.deny and rule_obj.audit
to aa.py, and import it into mergeprof.
I tested all changes manually.
Acked-by: Steve Beattie <steve@nxnw.org>
aa-mergeprof still used the old aa[profile][hat][allow]['capability']
which no longer gets populated - which resulted in not asking for
merging any capabilities.
Actually (and funnily),
- if other.aa[profile][hat].get(allow, False):
- continue
resulted in never merging capability rules even before the change to
CapabilityRule(set) - this was meant as optimization, but a "not" was
missing in the condition ;-) so it always skipped capability rules.
The patch changes ask_the_question to the CapabilityRule(set) layout.
Besides that,
- include the audit and deny keywords in the "Capability" headline
(I'd prefer to just use the get_clean() rule, but that's another topic)
- hide "(A)llow" when merging a deny rule
- don't ask for capabilities that are already covered
Also delete match_cap_includes() from aa.py, which is no longer used.
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://launchpad.net/bugs/1382241
Also rename RE_PROFILE_CHANGE_PROFILE_2 to RE_PROFILE_CHANGE_PROFILE
and update apparmor/rule/change_profile.py to use the changed name.
Acked-by: Steve Beattie <steve@nxnw.org>
Change aa.py to use ChangeProfileRule and ChangeProfileRuleset instead
of a sub-hasher to store and write change_profile rules. In detail:
- drop all the change_profile rule parsing from parse_profile_data() and
serialize_profile_from_old_profile() - instead, just call
ChangeProfileRule.parse()
- change write_change_profile to use ChangeProfileRuleset
- add removal of superfluous/duplicate change_profile rules (the old
code didn't do this)
Note that this patch is much smaller than the NetworkRule and
CapabilityRule patches because aa-logprof doesn't ask for adding
change_profile rules - adding that is something for a later patch.
Acked-by: Steve Beattie <steve@nxnw.org>
Add utils/apparmor/rule/change_profile.py with the ChangeProfileRule and
ChangeProfileRuleset classes. These classes are meant to handle
change_profile rules.
In comparison to the current code in aa.py, ChangeProfileRule has some
added features:
- support for audit and allow/deny keywords (for which John promised a
parser patch really soon)
- support for change_profile rules with an exec condition
Also add the improved regex RE_PROFILE_CHANGE_PROFILE_2 to regex.py.
Acked-by: Steve Beattie <steve@nxnw.org>
It did this in the old 2.8 code, but didn't in 2.9.x (first there was a
broken hat regex, then I commented out the hat handling to avoid
breakage caused by the broken regex).
This patch makes sure the hat flags get set when setting the flags for
the main profile.
Also change RE_PROFILE_HAT_DEF to use more named matches
(leadingwhitespace and hat_keyword). Luckily all code that uses the
regex uses named matches already, which means adding another (...) pair
doesn't hurt.
Finally adjust the tests:
- change _test_set_flags to accept another optional parameter
expected_more_rules (used to specify the expected hat definition)
- add tests for hats (with '^foobar' and 'hat foobar' syntax)
- add tests for child profiles, one of them commented out (see below)
Remaining known issues (also added as TODO notes):
- The hat and child profile flags are *overwritten* with the flags used
for the main profile. (That's well-known behaviour from 2.8 :-/ but we
have more flags now, which makes this more annoying.)
The correct behaviour would be to add or remove the specified flag,
while keeping other flags unchanged.
- Child profiles are not handled/changed if you specify the 'program'
parameter. This means:
- 'aa-complain smbldap-useradd' or 'aa-complain /usr/sbin/smbldap-useradd'
_will not_ change the flags for the nscd child profile
- 'aa-complain /etc/apparmor.d/usr.sbin.smbldap-useradd' _will_ change
the flags for the nscd child profile (and any other profile and
child profile in that file)
Even with those remaining issues (which need bigger changes in
set_profile_flags() and maybe also in the whole flags handling), the
patch improves things and fixes the regression from the 2.8 code.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
The test program was querying its own profile. Adjust the profile
generation so that a separate profile is generated and have query_label
query the separate profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Adjust the internal splitcon() function to strip a single trailing
newline character when the bool strip_newline argument is true.
aa_getprocattr_raw(2) needs to set strip_newline to true since the
kernel appends a newline character to the end of the AppArmor contexts
read from /proc/>PID>/attr/current.
aa_splitcon(3) also sets strip_newline to true since it is unknown
whether the context is originated from a location that appends a newline
or not.
aa_getpeercon_raw(2) does not set strip_newline to true since it is
unexpected for the kernel to append a newline to the the buffer returned
from getsockopt(2).
This patch also creates tests specifically for splitcon() and updates
the aa_splitcon(3) man page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Test confinement context splitting, using aa_splitcon(3), with and
without a valid mode pointer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Create a new libapparmor public function that allows external code to
split an AppArmor confinement context.
This is immediately useful for code that retrieves a D-Bus peer's
AppArmor confinement context using the
org.freedesktop.DBus.GetConnectionCredentials bus method.
https://launchpad.net/bugs/1430532
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The parse_confinement_mode() function returned NULL when a confinement
mode was not present (unconfined) and when it could not properly parse
the confinement context. The two situations should be differentiated
since the latter should be treated as an error.
This patch reworks parse_confinement_mode() to split a confinement
context and, optionally, assign the mode string. If a parsing error is
encountered, NULL is returned to indicate error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Use the passed in confinement context string size to improve the
comparison by only doing the string comparison if the size matches and
removing the possibility of reading past the end of the buffer.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
When passing the size of the confinement context to
parse_confinement_mode(), don't include the NUL terminator byte in the
size.
It is confusing to count the NUL terminator as part of the string's
length. This change makes it so that, after a few additional changes,
parse_confinement_mode() can be exposed as part of libapparmor's public
API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch modifies the socketpair.c test to verify the return value of
aa_getpeercon() based upon the expected label and expected mode lengths.
The test had to be changed slightly so that the returned mode, from
aa_getpeercon(), was preserved. It was being overwritten with the
special NO_MODE value.
This change helps to make sure that future changes to the code behind
aa_getpeercon() does not unintentionally change the function's return
value.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Samba 4.2 needs some more permissions for nmbd and winbindd.
To avoid overcomplicated profiles, change abstractions/samba to allow
/var/lib/samba/** rwk, (instead of **.tdb rwk) - this change already
fixes the nmbd profile.
winbindd additionally needs some more write permissions in /etc/samba/
(and also in /var/lib/samba/, which is covered by the abstractions/samba
change and also results in some profile cleanup)
References: https://bugzilla.opensuse.org/show_bug.cgi?id=921098 and
https://bugzilla.opensuse.org/show_bug.cgi?id=923201
Acked-by: Seth Arnold <seth.arnold@canonical.com>
I noticed "disconnected path" (run/nscd/*) events for ntpd while
updating to the latest openSUSE Tumbleweed.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9.
aa-mergeprof failed to fail ;-) when it should raise an AppArmorException.
Instead, it failed with
AttributeError: 'module' object has no attribute 'AppArmorException'
I confirmed this bug in trunk and 2.9.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
quote_if_needed() will be used by the upcoming ChangeProfileRule class,
which means it must be moved out of aa.py to avoid an import loop.
rule/__init__.py looks like a better place.
Also re-import quote_if_needed() into aa.py because it's still needed
there by various functions.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(might get re-used later ;-)
Also add two tests for profile names not starting with / - the quoted
version wasn't catched as invalid before, so this change is actually
also a bugfix.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Change aa-notify parse_message() to also honor complain mode log events.
This affects both modes - desktop notifications and the summary report.
Acked-by: Steve Beattie <steve@nxnw.org>
Add setUp() to AATest that sets "self.maxDiff = None" (unlimited).
This gives us unlimited array diffs everywhere where AATest is used.
Also rename several setUp() functions in test-regex_matches.py to
AASetup() to avoid that the shiny new AATest setUp() gets overwritten.
Acked-by: Steve Beattie <steve@nxnw.org>
As requested by Steve, also add an example AASetup() to test-example.py.
This ignores the sniplets generated by profiles/Makefile, but doesn't
ignore local/README because it doesn't have a dot in its name.
Acked-by: John Johansen <john.johansen@canonical.com>
Add several missing network DOMAINs to the apparmor.d manpage.
The list is based on the list that utils/vim/Makefile generates.
Acked-by: John Johansen <john.johansen@canonical.com>
reported by darix on IRC. This is needed if you have a bigger setup with
dovecot on a different (or multiple) machines
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Replace usage of RE_PROFILE_CAP and RE_PROFILE_NETWORK with
CapabilityRule.match() and NetworkRule.match() calls.
This also means aa.py doesn't need to import those regexes anymore.
As a side effect of this change, test-regex_matches.py needs a small
fix because it imported RE_PROFILE_CAP from apparmor.aa instead of
apparmor.regex.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Add match() and _match() class methods to rule classes:
- _match() returns a regex match object for the given raw_rule
- match() converts the _match() result to True or False
The primary usage is to get an answer to the question "is this raw_rule
your job?". (For a moment, I thought about naming the function
*Rule.myjob() instead of *Rule.match() ;-)
My next patch will change aa.py to use *Rule.match() instead of directly
using RE_*, which will make the import list much shorter and hide
another implementation detail inside the rule classes.
Also change _parse() to use _match() instead of the regex, and add some
tests for match() and _match().
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Change aa.py to use NetworkRule and NetworkRuleset instead of a
sub-hasher to store, check and write network rules. In detail:
- drop profile_known_network() and use is_known_rule() instead
- replace match_net_includes() usage with match_includes() calls
- drop delete_net_duplicates(), use the code in NetworkRule and
NetworkRuleset instead
- make match_net_includes() (still used by aa-mergeprof) a wrapper for
match_includes()
- drop all the network rule parsing from parse_profile_data() and
serialize_profile_from_old_profile() - instead, just call
NetworkRule.parse()
- now that write_net_rules() got fixed, drop it ;-)
- change write_netdomain to use NetworkRuleset
- drop netrules_access_check() - that's is_covered() now
- use 'network' instead of 'netdomain' as storage keyword (log events
still use 'netdomain')
Also update cleanprofile.py to use the NetworkRuleset class.
This also means to delete the (now superfluous) delete_net_duplicates()
function.
Finally, there are some changes in regex.py:
- change RE_PROFILE_NETWORK in regex.py to named matches and to use
RE_COMMA_EOL (not only RE_EOL)
- drop the no longer needed RE_NETWORK_FAMILY and RE_NETWORK_FAMILY_TYPE
(rule/network.py has regexes that check against the list of available
keywords)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add utils/test/test-network.py with tests for NetworkRule and
NetworkRuleset.
The tests are hopefully self-explaining, so let me just mention the most
important things:
- I started to play with namedtuple, which looks very useful (see "exp")
- the test loops make the tests much more readable (compare with
test-capability.py!) and make it easy to add some more tests
- 100% coverage :-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add utils/apparmor/rule/network.py with the NetworkRule and
NetworkRuleset classes. These classes are meant to handle network rules.
In comparison to the existing code in aa.py, relevant news are:
- the keywords are checked against a list of allowed domains, types and
protocols (these lists are based on what the utils/vim/Makefile
generates - on the long term an autogenerated file with the keywords
for all rule types would be nice ;-)
- there are variables for domain and type_or_protocol instead of
first_param and second_param. (If someone is bored enough to map the
protocol "shortcuts" to their expanded meaning, that shouldn't be too
hard.)
- (obviously) more readable code because we have everything at one place
now
- some bugs are fixed along the way (for example, "network foo," will now
be kept, not "network foo bar," - see my last mail about
write_net_rules() for details)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
CleanProf.remove_duplicate_rules() didn't call
$profile['capability'].delete_duplicates()
because aa-cleanprof sets same_file=True.
Fix this by calling delete_duplicates(None) so that it
only checks the profile against itsself.
Note: this is only needed if the to-be-cleaned profile doesn't
contain any include rules - with includes present, the
"for inc in includes:" block already called delete_duplicates()
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Implement in-profile de-duplication in BaseRuleset (currently affects
"only" CapabilityRuleset, but will also work for all future *Ruleset
classes).
Also change 'deleted' to be a simple counter and add some tests that
verify the in-profile deduplication.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
test_parse_modifiers_invalid() uses a hand-broken ;-) regex to parse
only the allow/deny/audit keywords. This test applies to all rule types
and doesn't contain anything specific to capability or other rules,
therefore it should live in test-baserule.py
Moving that test also means to move the imports for parse_modifiers and
re around (nothing else in test-capability.py needs them).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Add some tests for the Baserule class to cover the 3 functions that must
be re-implemented in each rule class. This means we finally get 100%
test coverage for apparmor/rule/__init__.py ;-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Ensure nosetests sees all tests in the tests[] tuples. This requires
some name changes because nosetests thinks all function names containing
"test" are tests. (A "not a test" docorator would be an alternative, but
that would require some try/except magic to avoid a dependency on nose.)
To avoid nosetests thinks the functions are a test,
- rename setup_all_tests() to setup_all_loops()
- rename regex_test() to _regex_test() (in test-regex_matches.py)
Also add the module_name as parameter to setup_all_loops and always run
it (not only if __name__ == '__main__').
Known issue: nosetests errors out with
ValueError: no such test method in <class ...>: stub_test
when trying to run a single test generated out of tests[].
(debugging hint: stub_test is the name used in setup_test_loop().)
But that's still an improvement over not seeing those tests at all ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Assume you have a profile like
/bin/foo {
/etc/ r,
network,
/usr/ r,
}
(important: there must be be a non-path rule between the two path blocks)
Then run aa-logprof and add another path event. When choosing (V)iew changes,
it will crash with a misleading
File ".../utils/apparmor/aamode.py", line 205, in split_mode
other = mode - user
TypeError: unsupported operand type(s) for -: 'collections.defaultdict' and 'set'
The reason for this is our beloved hasher, which is playing funny games
another time.
The patch wraps the hasher usage with a check for the parent element to
avoid auto-creation of empty childs, which then lead to the above crash.
BTW: This is another issue uncovered by the LibreOffice profile ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Update the postfix-common abstraction to cope with signal and unix
socket mediation, update the access to the sasl library locations
in a multiarch compliant way, and allow access to limited bits
of the filesystem paths under which postfix chroots itself to
(/var/spool/postfix/ on Ubuntu).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
serialize_profile_from_old_profiles() calls store_list_var() with an
empty hasher. This fails for "+=" because in this case store_list_var()
expects a non-empty hasher with the variable already defined, and raises
an exception because of the empty hasher.
This patch sets "correct = False" if a "+=" operation appears, which
means the variable will be written in "clean" mode instead.
Adding proper support for "add to variable" needs big changes (like
storing a variable's "history" - where it was initially defined and what
got added where).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
the LibreOffice profile uncovered that handling of @{var} += is broken:
File ".../utils/apparmor/aa.py", line 3272, in store_list_var
var[list_var] = set(var[list_var] + vlist)
TypeError: unsupported operand type(s) for +: 'set' and 'list'
This patch fixes it:
- change separate_vars() to use and return a set instead of a list
(FYI: separate_vars() is only called by store_list_var())
- adoptstore_list_var() to expect a set
- remove some old comments in these functions
- explain the less-intuitive parameters of store_list_var()
Also add some tests for separate_vars() and store_list_var().
The tests were developed based on the old code, but not all of them
succeed with the old code.
As usual, the tests uncovered some interesting[tm] behaviour in
separate_vars() (see the XXX comments and tell me what the really
expected behaviour is ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Move the code that does the c -> a and d -> w replacement in denied_mask
and requested_mask so that it only runs for path and exec events, but not
for other events (like dbus and ptrace). The validate_log_mode() and
log_str_to_mode() calls are also moved.
Technically, this means moving code from parse_event() to the path
and exec sections in add_event_to_tree().
This also means aa-logprof no longer crashes if it hits a ptrace or
dbus event in the log.
The "if dmask:" and "if rmask:" checks are removed - if a path event
doesn't have these two, it is totally broken and worth a aa-logprof
crash ;-)
Also adjust the parse_event() tests to expect the "raw" mask instead of
a set.
This patch fixes
https://bugs.launchpad.net/apparmor/+bug/1426651 and
https://bugs.launchpad.net/apparmor/+bug/1243932
I manually tested that
- c and d log events are still converted to a and w
- aa-logprof handles exec events correctly
- ptrace events no longer crash aa-logprof
Note: add_event_to_tree() is not covered by tests.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
"capability foo".is_covered("deny capability foo") should return False
even if check_allow_deny is False.
Also add some tests with check_allow_deny=False.
Acked-by: Steve Beattie <steve@nxnw.org>
Also add libraries/libapparmor/swig/perl/Makefile.perle (noticed and
proposed by Steve)
With these changes, "bzr status" is clean again after "make distclean"
Acked-by: Steve Beattie <steve@nxnw.org>.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Thanks to the used data structure, write_net_rules() replaces bare
'network,' rules with the invalid 'network all,' when saving a profile.
This patch makes sure a correct 'network,' rule is written.
Also reset 'audit' to avoid all (remaining) rules get the audit flag
after writing an audit network rule.
Note: The first section of the function (that claims to be responsible
for bare 'network,' rules) is probably never hit - but I'm not too keen
to remove it and try it out ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
When parsing a profile with named exec rules, the exec target included
the arrow. This resulted in two arrows when writing the profile (and one
more each time the profile was updated).
Fix this by using the match group that only contains the exec target
without the arrow in parse_profile_data() and
serialize_profile_from_old_profile().
References: https://bugs.launchpad.net/apparmor/+bug/1437901
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
When evince opens a dvi file, it updates the user fonts using
texlive commands in /usr/share/texlive/texmf-dist/web2c/ (or possibly
/usr/share/texlive/texmf/web2c/ in older releases). This patch adjusts
the sanitized_helper profile to allow these tools to run.
Bug: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1010909
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Jamie Strandboge <jamie@canonical.com>
write_net_rules() doesn't add a space after 'audit' in two of three
cases, leading to invalid network rules.
This patch adds the missing spaces.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9
write_net_rules() creates invalid rules for network rules with one
parameter (for example "network bluetooth").
Add a trailing comma to create valid rules.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9.
Change serialize_parse_profile_start() to use parse_profile_start()
instead of using duplicated code.
The behaviour is mostly kept, with the exception that the function is
more strict now and raises exceptions instead of ignoring errors.
In practise, this won't change anything because the profiles are parsed
with parse_profile() (which calls parse_profile_start()) - and that
already errors out.
The tests are updated to match the more strict behaviour.
The next step would be to drop serialize_parse_profile_start()
completely, but this isn't urgent and can/should be done when we have
test coverage for serialize_profile_from_old_profile() one day ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Fix is_skippable_dir() - the regex also matched things like
/etc/apparmor.d/dont_disable, while it should match on the full
directory name.
Also add some tests based on a real-world aa-logprof run (with "print (path)"
in is_skippable_dir()) and some additional "funny"[tm] dirs.
Needless to say that the tests
('dont_disable', False),
('/etc/apparmor.d/cache_foo', False),
will fail with the old is_skippable_dir().
Acked-by: Steve Beattie <steve@nxnw.org>
Replace RE_PROFILE_START with RE_PROFILE_START_2 and adjust all
code sections that used RE_PROFILE_START_2.
The only real change is that test_get_flags_invalid_01 and
test_get_flags_invalid_02 now expect AppArmorException instead of
AppArmorBug.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk
This patch implements attachment handling - aa-logprof now works with
profiles that have an attachment defined, instead of ignoring audit.log
entries for those profiles.
Changes:
- parse_profile_start_line(): remove workaround that merged the
attachment into the profile name
- parse_profile_data(): store attachment when parsing a profile
- update test_parse_profile_start_03, test_serialize_parse_profile_start_03,
test_set_flags_nochange_09 and some parse_profile_start_line() tests -
they now expect correct attachment handling
Acked-by: Steve Beattie <steve@nxnw.org>
this patch makes set_profile_flags more strict:
- raise AppArmorBug if newflags contains only whitespace
- raise AppArmorBug if the file doesn't contain the specified profile or
no profile at all
The tests are adjusted to expect AppArmorBug instead of a silent
failure. Also, some tests are added for profile=None, which means to
change the flags for all profiles in a file.
- test_set_flags_08 is now test_set_flags_invalid_04
- test_set_flags_invalid_03 is changed to only contain one reason for a
failure, not two ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Changes in set_profile_flags():
- rewrite set_profile_flags to use parse_profile_start_line() and
write_header().
- replace the silent failure for non-existing files with a proper
exception (using lazy programming - the check is done by removing the
"if os.path.isfile()" check, open_file_read then raises the
exception ;-)
- comment out regex_hat_flag and the code that was supposed to handle
hat flags, which were totally broken. We'll need another patch to fix
it, and we also need to decide if we want to do that because it
introduces a behaviour change (currently, aa-complain etc. don't
change hat flags).
The tests for set_profile_flags() are also updated:
- prepend a space to comments because write_header always adds a space
between '{' and the comment
- remove a test with superfluous quotes that are no longer kept (that's
just a profile cleanup, so dropping that test is the easiest way)
- update test_set_flags_10 and test_set_flags_12 to use the correct
profile name
- enable the tests for invalid (empty) flags
- update the test for a non-existing file
Note: test_set_flags_10, test_set_flags_12 and test_set_flags_nochange_09
will fail with this patch applied. The next patch will fix that.
Acked-by: Steve Beattie <steve@nxnw.org>
The Makefiles don't create/need the 'common' symlinks since some time,
which also means we no longer need to have them in .bzrignore.
Acked-by: Steve Beattie <steve@nxnw.org>
if 3/2 == 1:
print("python2 inside")
Add "from __future__ import division" so that python2 returns the
correct result (if needed, as float)
On related news: At least python3 knows how to calculate correctly.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
aa-logprof doesn't ask anything for
type=AVC msg=audit(1427633461.202:281): apparmor="DENIED" operation="chmod" profile="/usr/lib64/firefox/plugin-container" name="/home/cb/.config/ibus/bus/" pid=7779 comm="plugin-containe" requested_mask="w" denied_mask="w" fsuid=1000 ouid=1000
This patch fixes this by adding 'chmod' to the list of file operation
types in logparser.py.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9.
Rewrite parse_profile_start() in aa.py to a more readable version.
The behaviour remains unchanged (and is covered by tests).
The patch also updates the comment about the internal struct of
aa[profile][hat] - initial_comment was missing.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Change the write_header tests so that the 'profile_keyword' and
'header_comment' parameters can be (and are) tested:
- add a None for both to the existing tests
- add some tests that come with the profile keyword and/or a comment
Acked-by: Steve Beattie <steve@nxnw.org>
- add support for prof_data['header_comment'] (comment after '{')
and prof_data['profile_keyword'] (to force the 'profile' keyword, even
if it isn't needed) to write_header().
(set_profile_flags() will be the only user of these two for now)
- fix a crash if depth is not an integer - for example,
len(' ')/2 # 3 spaces = 1.5
would cause a crash.
Also add a test for 1.5 and 1.3 spaces.
- rewrite the handling of flags to avoid we have to maintain two
different template lines.
- update the tests to set 'profile_keyword' and 'header_comment' to None.
This avoids big changes in the test code. I'll send another patch that
makes sure profile_keyword and header_comment are tested ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Add the attachment to the parse_profile_start() and
serialize_parse_profile_start() return values, and adjust the functions
calling the *parse_profile_start() functions to save the attachment in
the "attachment" variable (which isn't used yet).
Also adjust the tests for the added return value.
(Sorry for not getting the resultset right from the beginning!)
Acked-by: Steve Beattie <steve@nxnw.org>
Also fix a little bug that added the profile keyword if the path needed
quotes (profile "/foo bar" - but "/foo bar" is enough). This was caused
by a regex that always matched on quoted paths (hint: "/ matches
^[^/] ;-)
Also add some tests with attachments and update the test for the bugfix
mentioned above.
Now the remaining part is to make sure that prof_data['attachment'] gets
set when parsing the profiles :-)
Acked-by: Steve Beattie <steve@nxnw.org>
Also add loop support to test-aa.py.
BTW: In case you wonder - the need to replace unittest.TestCase with
AATest is intentional. It might look annoying, but it makes sure that
a test-*.py file doesn't contain a test class where tests = [...] is
ignored because it's still unittest.TestCase.
(Technically, setup_all_tests() will error out if a test class doesn't
contain tests = [...] - either explicit or via its parent AATest.)
Acked-by: Steve Beattie <steve@nxnw.org>
Add various tests for set_profile_flags, and document various
interesting[tm] things I discovered while writing the tests (see
the inline comments for details).
Also adds a read_file() function to common_test.py.
The most interesting[tm] thing I found is:
regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$')
which matches various unexpected things - but not a hat :-/
(see mailinglist for all funny details)
Acked-by: Steve Beattie <steve@nxnw.org>
Convert serialize_parse_profile_start() to use
parse_profile_start_line(), and adjust a test to expect an AppArmorBug
instead of an AttributeError exception.
Also add two tests (they succeed with the old and the new code).
Note that these tests document interesting[tm] behaviour - I tend to
think that those cases should raise an exception, but I'm not sure about
this because serialize_profile_from_old_profile() is a good example for
interesting[tm] code :-/
I couldn't come up with a real-world test profile that would hit those
cases without erroring out aa-logprof earlier - maybe the (more
sane-looking) parse_profiles() / serialize_parse_profile_start()
protects us from hitting this interesting[tm] behaviour.
Acked-by: Steve Beattie <steve@nxnw.org>
The commit message for r2976 says:
[...]
The patch also adds test-example.py, which is
- a demo of the code added to common_test.py
- a template file that we can copy for future test-*.py
Acked-by: Steve Beattie <steve@nxnw.org>
but I forgot to add test-example.py to bzr, which I hereby do.
The previous patch slightly changed the behaviour of parse_profile_start()
and get_profile_flags() - they raise AppArmorBug instead of
AppArmorException when specifying a line that is not the start of a
profile and therefore doesn't match RE_PROFILE_START_2.
This patch updates test-aa.py to expect the correct exceptions, and adds
another test with quoted profile name to ensure that stripping the
quotes works as expected.
Acked-by: Steve Beattie <steve@nxnw.org>
Add the parse_profile_start_line() function to regex.py, which is a
wrapper for RE_PROFILE_START_2 and returns an array with named matches.
Also change some places in aa.py from using RE_PROFILE_START to the
parse_profile_start_line() function.
Notes:
- until everything is migrated to the new function, I'll keep the old
RE_PROFILE_START unchanged - that's the reason to add the new regex
as RE_PROFILE_START_2
- the patch changes only aa.py sections that are covered by tests already
(which means some users of RE_PROFILE_START are remaining)
- parse_profile_start_line() merges 'profile' and 'attachment' into
'profile' (aka the old, broken behaviour) until aa.py can handle the
attachment properly. The alternative would be to ignore 'attachment',
which would be worse.
Acked-by: Steve Beattie <steve@nxnw.org>
Add better support for looping over a tests[] array to common_test.py:
- class AATest - a base class we can use for all tests, and that will
probably get more features in the future (for example tempdir
handling)
- setup_all_tests() - a function that iterates over all classes in the
given file and calls setup_test_loops() for each of them
- setup_tests_loop() - a function that creates tests based on tests[]
in the given class. Those tests call the class' _run_test() method for
each test specified in tests[] (inspired by setup_regex_tests() ;-)
This means we can get rid of the manually maintained tests list in
test-regex_matches.py and just need to call setup_all_tests() once in
each file.
The patch also adds test-example.py, which is
- a demo of the code added to common_test.py
- a template file that we can copy for future test-*.py
Acked-by: Steve Beattie <steve@nxnw.org>
The following patch addresses two issues on older releases:
1) In trunk commit 2911, the line 'undefine VERBOSE' was added to
parser/tst/Makefile so that the equality tests would not generate
verbose output when $VERBOSE != 1. Unfortunately, the 'undefine'
keyword was not introduced in GNU Make until version 3.82. On
distro releases like Ubuntu 12.04 LTS that include versions of Make
older than that, make check and make clean abort when VERBOSE is
not set to 1. The patch fixes that by setting VERBOSE to a zero
length string if does not already equal 1.
2) In trunk commit 2923, a workaround for systemd as init was added
to the pivot_root regression test. The workaround included a
call to ps(1) to determine if systemd is pid 1. Unfortunately,
in older versions of the procps package (such as the version in
Ubuntu 12.04 LTS), 'ps -hp1' emits the warning
Warning: bad ps syntax, perhaps a bogus '-'? See http://procps.sf.net/faq.html
The patch below converts the ps call to 'ps hp1' which does not
generate the warning.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from Cameron Norman <camerontnorman@gmail.com> based on a patch
from Christian Boltz <apparmor@cboltz.de>.
This patch allows /var/lib/misc/dnsmasq.*.leases rw and
/{,var/}run/lxc/dnsmasq.pid rw for LXC networking setup.
Acked-by: Steve Beattie <steve@nxnw.org>
The two internal aa_features objects weren't being unreferenced when the
aa_policy_cache object was being freed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The aa_features and aa_kernel_interface APIs get a little bit of
testing, as well.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
realloc() returns NULL when it fails. Using the same pointer to specify
the buffer to reallocate *and* to store realloc()'s return value will
result in a leak of the previously allocated buffer upon error.
These issues were discovered by cppcheck.
Note that 'buffer' in write_policy_fd_to_iface() has the autofree
attribute so it must not be manually freed if the realloc(3) fails as
it'll be automatically freed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The libapparmor library is built with gcc, while the parser is built
with g++. The parser code needs to cast pointers returned from the
malloc(3) family of calls. However, code removed from the parser to
libapparmor can drop the casts.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Creates a libapparmor function, _aa_asprintf(), which sets the *strp to
NULL on error. This is needed for all of the users of the _aa_autofree
cleanup attribute because the value of *strp is undefined when
asprintf() fails and that could result in _aa_autofree() being passed a
pointer value that it should not free.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser no longer has a need for the atomic operations since all
callers have been moved to libapparmor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
With create_cache() headed for libapparmor, we can't use the show_cache
or write_cache globals.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
As code is moved from the parser to libapparmor, the libapparmor code
base will need to have the "unused" macro defined. This macro will need
to be duplicated in the parser and libapparmor due to it being a
compiler-specific macro that shouldn't be exported from libapparmor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The existing kernel_interface.c file collides with the expected file
name of the implementation of the aa_kernel_interface API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Remove the use of the "_" macro, which translates into gettext(3), from
code that will be used from the parser to libapparmor since libapparmor
will not support gettext(3) for debug messages and syslog messages.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser's copy of the two atomic operations will be removed once the
new API's (aa_features, aa_policy_cache, aa_kernel_interface) are moved
from the parser to libapparmor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The function names must be prepended with "_aa_" since they're going to
be exported from libapparmor. The code bases using the _aa_autofree(),
_aa_autoclose(), and _aa_autofclose() will need to internally alias
those functions to the previously used autofree, autoclose, and
autofclose names.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch adds equivalents of the parser's PDEBUG() and PERROR()
functions to libapparmor.
It does not add gettext(3) support to libapparmor since these are
messages that only developers will see (debug builds with
LIBAPPARMOR_DEBUG=1) or messages that go to the syslog.
PDEBUG() does nothing unless libapparmor is built with --enable-debug.
It prints to stderr if libapparmor is built with --enable-debug and the
LIBAPPARMOR_DEBUG environment variable is set.
PERROR() uses syslog(LOG_ERR, ...) by default. The message is sent to
the syslog and to stderr if libapparmor is built with --enable-debug and
the LIBAPPARMOR_DEBUG environment variable is set.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This is needed for some of the parser functionality that will be moved
to libapparmor. In the short term, only the 'bool' type is needed but it
makes sense to simply require a C99 compliant compiler for libapparmor
since the parser is being rewritten in C++. The use of C99 will reduce
future headaches when moving code between the two code bases.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch creates a private API in libapparmor in which upstream
provides no guarantees in regards to ABI stability.
A new header file, <sys/apparmor_private.h>, is created. The "_aa"
prefix will be used for symbols belonging to the private API.
To kick things off, a library friendly version of is_blacklisted() is
moved into libapparmor.
The purpose of a private libapparmor API is to prevent duplicated code
between the parser and libapparmor. This becomes an issue as we prepare
to move chunks of the parser into libapparmor.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This may be useful for something like an init daemon that simply wants
to load all cached binaries without worrying about any sort of policy
compilation.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Create new, ref, and unref functions for aa_kernel_interface. The "new"
function allows for the caller to pass in an aa_features object that is
then used to check if the kernel supports set load operations.
Additionally, the "new" function allows for the apparmorfs path to be
discovered once instead of during every policy load.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
__sd_serialize_profile() had a duplicated implementation for writing to
apparmorfs interface files after a profile compilation. This patch
migrates it to the new aa_kernel_interface API.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This is the start of the kernel_interface API that allows callers to
specify a buffer, a file path, or a file descriptor that should be
copied to the proper kernel interface for loading, replacing, or
removing in-kernel policies.
Support exists for reading from a file path or file descriptor into a
buffer and then writing that buffer to the appropriate apparmorfs
interface file.
An aa_kernel_interface_write_policy() function is also provided for
callers that want to route a buffer to an arbitrary file descriptor
instead of to an apparmorfs file. This is useful when an admin instructs
apparmor_parser to write to stdout or a file.
Additionally, it removes some parser-specific globals from the
kernel_interface.c file, such as OPTION_{ADD,REPLACE,REMOVE}, in
preparation for moving the code into a library.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This function allows for a policy cache to be removed without having a
previously instatiated aa_policy_cache object. It simply works off of a
path.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This API has the same look-and-feel of the previous aa_features API.
The cache setup code was heavily dependent on globals set by CLI
options. Options such as "skip the read cache", or "skip the write
cache", or "don't clear the cache if it isn't valid", won't be useful
for all aa_policy_cache API users so some of that logic was lifted out
of the API. The constructor function still provides a bool parameter
that specifies if the cache should be created or not.
If the policy cache is invalid (currently meaning that the cache
features file doesn't match the kernel features file), then a new
aa_policy_cache object is still created but a call to
aa_policy_cache_is_valid() will return false. The caller can then decide
what to do (create a new valid cache, stop, etc.)
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This option adds unneeded complexity to the parser CLI and the upcoming
aa_policy_cache API. Get rid of it and simply create the cache dir if
--write-cache is specified.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch removes the final dependency on callers needing access to the
features string so aa_features_get_string() can go away.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Defines a function that can be called to test features support. It is
string based which allows the support tests to work with new kernel
features without any changes.
The use of global variables in the parser to store and check features
support is still preserved. The parser should probably move over to
passing the aa_features object around but that's left for later.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This is a simple aa_features equality test. Placing it behind a function
call allows us to do something more complex than a simple string
comparison later.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The aa_features_new_*() functions create an aa_features object. They can
be thought of as the constructor of aa_features objects. A number of
constructors are available depending on whether the features are coming
from a file in the policy cache, a string specified on the command line,
or from apparmorfs.
The aa_features_ref() and aa_features_unref() functions are used to grab
and give up references to an aa_features. When the ref count hits zero,
all allocated memory is freed. Like with free(), aa_features_unref() can
be called with a NULL pointer for convenience.
Pre-processor macros are hidden behind functions so that they don't
become part of our ABI when we move this code into libapparmor later on.
A temporary convenience function, aa_features_get_string(), is provided
while code that uses aa_features is migrated from expecting raw features
string access to something more abstract. The function will be removed
in an upcoming patch.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
snprintf_buffer() needed to be modified in order to properly return error
conditions up the stack, instead of exiting, but there were some other
cleanups that it could use.
It was obviously implemented with the features_struct in mind so this
patch simplifies the input parameters by directly accepting a
features_struct pointer. Also, the name is changed to reflect that it is
intended to work on a features_struct instead of an arbritrary buffer.
A quick sanity check is added to make sure that the features_struct.pos
value isn't pointing past the end of the buffer.
The printf(3) family of functions can return a negative value upon error
so a check of the return value of vsnprintf(3) is added.
Finally, the return values of the function are simplified to 0 on
success or -1, with errno set, on error. This is possible since
features_struct.pos can be internally updated after a successful
vsnprintf(3).
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
These operations will be used for grabbing and releasing references to
objects. They leverage the GCC builtins for atomic operations.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Require the caller of setup_cache() to pass in a valid cache location
string. This removes the use of the basedir global from the
policy_cache.c file.
Additionally, it is no longer necessary to return the "cache dir" path
from setup_cache() since it will always be the same as the input path.
The return value is changed to an int so an error code can be returned
instead of using exit().
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Modify setup_cache() to accept the user-supplied cacheloc and return the
validated or created cache directory. The caller must then track that
variable and pass it into any parser/policy_cache.c functions that need
it.
The main reason for this change is that the cache location and the cache
directory will soon be two different paths. The cache location will
typically be the parent of the cache directory.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch moves the logic that sets up the policy into a new function
in policy_cache.c
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Fixed build failures]
[tyhicks: Fixed bug where a warning was being printed when it shouldn't]
[tyhicks: Forward ported to trunk]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Forward ported patch to trunk]
[tyhicks: remove commented out code]
[tyhicks: fix use after free]
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
[tyhicks: Handle inverted return from find_subdomainfs_mountpoint()]
[tyhicks: Link test progs to libapparmor to fix make check build fail]
[tyhicks: Migrate from opendir() to open() for opening apparmorfs]
[tyhicks: Make some of the split out functions static]
Signed-off-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>
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>
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>
Update the file rule pattern to show it is possible to specify a bare
file rule. Eg.
file,
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Refactor FILEGLOB so that it means both quoted and unquoted file globs.
Also
FILEGLOB was uncorrectly referenced in a few places where it should have
allowed for quoting.
There were also a few places that provided a parameter description with
FILEGLOB without defining that that is full equivalent to FILEGLOB.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Add miss ix and ux fallback permission modes, named profile transitions.
Also fix the file access modes and rule pattern to properly reflect
what is allowed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Consolidate and update the qualifier information in the man page.
Most of the rule qualifiers where duplicated instead of being pulled
into a common section.
Also the rule qualifiers where missing the 'allow' qualifier.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
- verify audit and audit allow is equal
- verify audit differs from deny and audit deny
- verify deny differs from audit deny
- make the verbose text a little more useful for some cases
- correct overlap exec tests to substitute in looped perms
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
- make the verbose output of equality.sh honor whether or not
the environment variable VERBOSE is set
- thereby making the output verbose when 'make check V=1' or 'make
check VERBOSE=1' is given from within the parser/ directory. This
will make distribution packagers happy when diagnosing build
failures caused by test failures.
- if verbose output is not emitted and the tests were successful, emit
a newline before printing PASS.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
This adds several new equality tests and turned up a couple of more
bugs
https://launchpad.net/bugs/1433829https://launchpad.net/bugs/1434018
- add link/link subset tests
- add pix, Pix, cix, Cix, pux, Pux, cux, Cux and specified profile
transitions (/f px -> b ...)
- test equality of leading and trailing permission file rules
ie. /foo rw, == rw /foo,
- test that specific x match overrides generic x rule. ie.
/** ix, /foo px, is different than /** ix, /foo ix,
- test that deny removes permission
/f[abc] r, deny /fb r, is differnt than /f[abc] r,
In addition to adding the new tests, it changes the output of the
equality tests, so that if the $verbose variable is not set successful
tests only output a period, with failed tests outputing the full
info. If verbose is set the full test info is output as before.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
BugLink: http://bugs.launchpad.net/bugs/1433829
The apparmor_parser fails to compile deny rules with only link
permissions.
Eg.
deny /f l,
deny l /f,
deny link /f -> /d,
Will all fail to compile with the following assert
apparmor_parser: aare_rules.cc:99: Node* convert_file_perms(int, uint32_t, uint32_t, bool): Assertion `perms != 0' failed.
NOTE: this is a minimal patch a bigger patch that cleans-up and separates
and reorganizes file, link, exec, and change_profile rules is needed
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Darix' guess is that this is needed by libpq because he uses a postgresql
database with dovecot and has ssl enabled in postgresql.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9
This patch fixes the equality test script and the valgrind wrapper
script to make the parser under test use the features.all features file
from the features_files/ subdirectory. Otherwise, the equality tests
will fail on systems where the not all of the current language features
are supported. The equality fix does so in a way to make the script work
correctly regardless of the directory it is run from.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
The fix to prevent the compiler from SEGV'ing when dumping network
rules in commit 2888 introduced the following compiler warning:
network.c: In function ‘const char* net_find_af_name(unsigned int)’:
network.c:331:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
for (i = 0; i < sizeof(network_mappings) / sizeof(*network_mappings); i++) {
The problem is that the counter i is an int, but sizeof returns size_t
which is unsigned. The following patch fixes the issue by converting the
type of i to size_t.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Previously, we only had the ability to test that binary policy files
were equal. This patch allows for the testing of binary policy files
that are not equal.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This fixes the incorrect compilation of audit modifiers for exec and
pivot_root as detailed in
https://launchpad.net/bugs/1431717https://launchpad.net/bugs/1432045
The permission accumulation routine on the backend was incorrectly setting
the audit mask based off of the exec type bits (info about the exec) and
not the actual exec permission.
This bug could have also caused permissions issues around overlapping exec
generic and exact match exec rules, except the encoding of EXEC_MODIFIERS
ensured that the
exact_match_allow & AA_USER/OTHER_EXEC_TYPE
test would never fail for a permission accumulation with the exec permission
set.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
serialize_profile_from_old_profile() in aa.py, as a preparation to add
tests and then switch to the upcoming RE_PROFILE_START wrapper function.
Besides moving the code, I replaced write_prof_data[profile][hat]['profile']
and write_prof_data[profile][hat]['external'] with function parameters
to avoid that I have to pass around the full write_prof_data.
Note: The "lineno" parameter is technically superfluous - I kept it to
have the parameters as close to parse_profile_start() as possible and
hope that I can merge those functions later (when we have test coverage).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
The parser.conf example statement for Include statements used
/etc/apparmor.d/abstractions which is unlikely to make anyone enabling
it happy as our shipped and example policies all include the
'abstractions/' directory in the relative paths. This patch adjusts the
example and provides a second example, based on an enabled entry as
shipped in Ubuntu.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
and change the code to use them.
Also add a comment to act() that it's only used by aa-cleanprof.
Note: The new functions add the --base parameter to the apparmor_parser
calls, which also means the disable directory inside the given profile
dir (and not always /etc/apparmor.d/disable) is now honored.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
logparser.py / add_event_to_tree() has 5 places to handle 'path' events.
This patch merges most if conditions to reduce that to 2 places.
It also makes the matching a bit more strict - instead of using 'in',
'xattr' has to be an exact match and 'file_' is matched with startswith().
Also, 'getattr' is added to the list of file events.
Acked-by: Steve Beattie <steve@nxnw.org>
---------- trunk only, unclear for 2.9 --------------
Without it, aa-disable
- didn't error out when hitting a broken profile directory
- didn't find a profile if it doesn't use the default naming scheme
(for example /bin/true profile hiding in bin.false)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
As mir has come into use in Ubuntu touch and is available for testing on
Ubuntu desktop, confined apps need access to a few mir specific things.
This patch adds a mir abstraction.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
aa-status was crashing when parsing through /proc/mounts looking to see
if and where the securityfs synthetic file system is mounted if there
was a mount point that contained characters outside of the charset in
use in the environment of aa-status. This patch fixes the issue by
converting the read of /proc/mounts into a binary read and then uses
decode on the elements.
Patch by Alain BENEDETTI.
Acked-by: Steve Beattie <steve@nxnw.org>
The error path was being taken when openat() return 0 but openat()
returns -1 on error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
As a follow-up to the logparser.py change that converts disconnected
path events to an error, add a testcase to test-logparser.py.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Parts of the regression tests that use the do_open() inline function
from changehat.h fail to build under gcc-5 like so:
cc -g -O0 -Wall -Wstrict-prototypes changeprofile.c -lapparmor -o changeprofile
/tmp/ccT6GE6k.o: In function `main':
/home/ubuntu/bzr/apparmor/tests/regression/apparmor/changeprofile.c:43: undefined reference to `do_open'
collect2: error: ld returned 1 exit status
<builtin>: recipe for target 'changeprofile' failed
This patch converts the do_open function declaration to be static
inline, which apparently keeps gcc-5 from getting confused.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The upcoming function parse_profile_start() (which is a wrapper around
the updated RE_PROFILE_START, and will live in regex.py) needs
strip_profile(), but importing it from aa.py fails with an import loop.
Therefore this patch moves strip_quotes() from aa.py to regex.py and
re-imports it into aa.py.
As a bonus, the patch also adds some tests for strip_quotes() ;-)
Also add TestStripQuotes to the test_suite list because it won't run
otherwise.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9
Move the code for parsing the profile start ("/foo {") from aa.py
parse_profile_data() to a separate function parse_profile_start().
Most of the changes are just moving around code, with some small
exceptions:
- instead of handing over profile_data to parse_profile_start() to
modify it, it sets two variables (pps_set_profile and
pps_set_hat_external) as part of its return value, which are then
used in parse_profile_data() to set the flags in profile_data.
- existing_profiles[profile] = file is executed later, which means
it used the strip_quotes() version of profile now
- whitespace / tab level changes
The patch also adds some tests for the parse_profile_start() function.
Acked-by: Steve Beattie <steve@nxnw.org>
flags_bad5.sd contains tests to ensure the debug flag is no longer
accepted.
However, the file contains multiple expected failures, which means that
it will still fail as long as at least one of them fails. This patch
splits each test into its own file to ensure each of them fails.
Acked-by: Steve Beattie <steve@nxnw.org>
Also adds a check to get_profile_flags() to catch an invalid syntax:
/foo ( ) {
was accepted by get_profile_flags, while
/foo () {
failed.
When testing with the parser, both result in a syntax error, therefore
the patch makes sure it also fails in get_profile_flags().
Acked-by: Steve Beattie <steve@nxnw.org>
Seth pointed out that dirat_for_each() didn't correctly handle the
return value from readdir_r(). On error, it directly returns a positive
errno value. This would have resulted in that positive errno value being
returned, with an undefined errno value set, from dirat_for_each().
However, the dirat_for_each() documentation states that -1 is returned,
with errno set, on error.
This patch results in readdir_r()'s return value being handled
appropriately. In addition, it ensures that 0 is always returned on
success.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The smbd profile contains /{,var/}run/cups/cups.sock rw, which is
covered by abstractions/cups-client and therefore superfluous.
Acked-by: Steve Beattie <steve@nxnw.org>
This means that aa-logprof will ignore the event instead of crashing with
AppArmorException: 'Unexpected rank input: var/run/nscd/passwd'
Note that I made the check as specific as possible to be sure it doesn't
hide other events.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=918787
Acked-by: Steve Beattie <steve@nxnw.org>
Also update test-capability.py - it contains a test that needs
'error_code': 0,
added to avoid a failure.
Patch by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
From: Felix Geyer <debfx@ubuntu.com>
At least Debian/Ubuntu started shipping some aspell files in
/usr/share/aspell/.
For example:
/usr/share/aspell/iso-8859-1.cmap
/usr/share/aspell/iso-8859-1.cset
The abstraction should allow read access to these files.
Acked-by: Steve Beattie <steve@nxnw.org>
Remove the check if the disable directory exists. If it's really
missing, it will be auto-created by create_symlink(), so we
automagically fix things instead of annoying the user with an
error message ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Like net_find_af_name, this assumed that AF_* values were consecutive.
[smcv: split out from a larger patch, added commit message,
removed dead declaration]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
The network_families array is automatically built from AF_NAMES, which is
extracted from the defines in <bits/socket.h>. The code assumes that
network_families is indexed by the AF defines. However, since the
defines are sparse, and the gaps in the array are not packed with
zeroes, the array is shorter than expected, and the indexing is wrong.
When this function was written, the network families that were
covered might well have been consecutive, but this is no longer true:
there's a gap between AF_LLC (26) and AF_CAN (29). In addition,
the code that parses <sys/socket.h> does not recognise AF_DECnet (12)
due to the lower-case letters, leading to a gap betwen AF_ROSE (11)
and AF_NETBEUI (13).
This assumption caused a crash in our testing while parsing the rule
"network raw".
[smcv: split out from a larger patch, added commit message]
Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Don't pass an ostream reference into another ostream via <<.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Make sure most tools (for example aa-complain) don't error out if
no logfile can be found. (For obvious reasons, aa-logprof and
aa-genprof will still require a logfile ;-)
This is done by moving code from the global area in aa.py to the new
function set_logfile(), which is called by aa-logprof and aa-genprof.
While on it,
- rename apparmor.filename to apparmor.logfile
- move the error handling for user-specified logfile from aa-genprof
and aa-logprof to aa.py set_logfile()
Note: I'd have prefered to hand over the logfile as parameter to
do_logprof_pass(), but that would break last_audit_entry_time() in
aa-genprof which requires the log filename before do_logprof_pass()
is called.
References: https://bugs.launchpad.net/apparmor/+bug/1423702
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Abstract af_unix socket names can contain a null character, however the
aare to pcre conversion explicitly disallows null characters because they
are not valid characters for pathnames. Fix this so that they type of
globbing is selectable.
this is a partial fix for
Bug: http://bugs.launchpad.net/bugs/1413410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The lexer front end currently incorrectly processes the \000 \x00 \d00 escape sequence resulting in a null character being embedded in the processed string, this results in the string not being full processed later.
The aare to pcre regex conversion fn also incorrectly strips out the \00, and any other escape sequence it doesn't know about, resulting in incorrect strings being passed to the backend. Fix this by passing through any valid escape sequence that is not handled by the fn.
this is a partial fix for
Bug: http://bugs.launchpad.net/bugs/1413410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Adjust the libapparmor function prototypes, variable names, and comments
that incorrectly used the name "con" when referring to the label.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The correct usage of the terms context and label is not clear in the
aa_getcon(2) man page. The aa_getcon(2) family of functions are also
prototyped incorrectly since the *con parameter represents a label and
not a context.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
libapparmor _aa_is_blacklisted() - some extensions were missing in the
python code.
Also make the code more readable and add some testcases.
Notes:
- the original code additionally ignored *.swp. I didn't include that -
*.swp looks like vim swap files which are also dot files
- the python code ignores README files, but the C code doesn't
(do we need to add README in the C code?)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for 2.9 and trunk
Acked-by: Steve Beattie <steve@nxnw.org>
string or if a mode_char is not in MODE_HASH.
Also update the testcase for "asdf42" (which raises AppArmorBug now)
and add a test that simulates MODE_HASH and MODE_MAP_SET getting out
of sync (tests the second part of the if condition).
Acked-by: Steve Beattie <steve@nxnw.org>
Since the Makefile cleanup, the _clean target is only used to delete
manpages etc. generated from *.pod files.
This patch renames the _clean target to pod_clean to make it obvious
what it does.
Acked-by: John Johansen <john.johansen@canonical.com>
The lexer front end currently incorrectly processes the \000 \x00 \d00 escape sequence resulting in a null character being embedded in the processed string, this results in the string not being full processed later.
The aare to pcre regex conversion fn also incorrectly strips out the \00, and any other escape sequence it doesn't know about, resulting in incorrect strings being passed to the backend. Fix this by passing through any valid escape sequence that is not handled by the fn.
this is a partial fix for
Bug: http://bugs.launchpad.net/bugs/1413410
Signed-off-by: John Johansen <john.johansen@canonical.com>
Get rid of the relics in libapparmor's Makefile.am for generating
tarballs from svn, which is no longer relevant. Also clean generated
manpages during make clean rather than just make maintainer-clean.
This patch removes a bunch of the per-directory tarball and rpm
generation cruft that is no longer needed now that we've been
distributing a unified tarball in our releases.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
- drop the symlink magic of the common/ directory, and just include
files directly from there.
- update comments indicating required steps to take when including
common/Make.rules
- drop make clean steps that refer to no longer generated tarballs,
specfiles, and symlinks to the common directory/Make.rules.
- don't silence clean steps if VERBOSE is set
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian "Ghostbuster" Boltz <apparmor@cboltz.de>
make sure nothing accidently hits the first-best target (well,
first-not-so-good would better describe the rpm target ;-)
Also add a dummy "all:" target to the toplevel Makefile with a short
hint towards README.
(see "[patch] fun with the toplevel Makefile") on the ML for the fun
that lead to this patch)
Acked-by: Steve Beattie <steve@nxnw.org>
journal socket. On Debian and Ubuntu systems, /dev/log is a symlink to
/run/systemd/journal/dev-log, so this access is now required in the base
abstraction to maintain current behavior.
Bug: https://bugs.launchpad.net/apparmor/+bug/1413232
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Split is_covered() in capability.py into
- is_covered_localparts() for rule-specific code
- is_covered() for common code - located in __init__.py
The object type comparison now uses type(self) and a slightly different
error message to make it usable everywhere.
Also rename rule_obj to other_rule which is more self-explaining
(inspired by the parameter name in the is_covered() dummy in __init__.py).
v2:
- remove check_allow_deny and check_audit parameters from
is_covered_localvars()
Acked-by: Steve Beattie <steve@nxnw.org>
If one of the testcases fail, this goes unnoticed in "make coverage".
This patch changes the Makefile so that test failures let
"make coverage" fail.
You can use make COVERAGE_IGNORE_FAILURES=true coverage to build
coverage data even if some tests fail.
Signed-off-by: Steve Beattie <steve@nxnw.org>
(which was most probably meant as an Acked-by)
Also Acked-by: <timeout> ;-)
For reasons that are unclear, python's setuptools doesn't install
recursively from a directory, meaning that on make install, the new
Rules/Ruleset classes were not being installed. This patch causes
the rule subdirectory to be included.
Bug: https://bugs.launchpad.net/bugs/1407437
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
adds some tests for severity.py and improves the test coverage to
nearly 100% (only 3 partial left).
Added tests and details (all in SeverityVarsTest):
- move writing the tunables file from setUp() into _init_tunables() for
more flexibility (allows to specify other file content)
- test adding to a variable (+=)
- test #include
- make sure double definition of a variable fails
- make sure redefinition of non-existing variable fails
BTW: even the comment added to VARIABLE_DEFINITIONS contributes to
the coverage ;-)
severity.py passes all added tests, however I should note that including
a non-existing file is silently ignored.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Adds #include <abstractions/dovecot-common> to the usr.sbin.dovecot
profile. Effectively this adds "deny capability block_suspend," which
is the only missing part from
https://bugs.launchpad.net/apparmor/+bug/1296667/
It also removes "capability setgid," (covered by
abstractions/dovecot-common) and "@{PROC}/filesystems r," (part of
abstractions/base).
Acked-by: John Johansen <john.johansen@canonical.com>
This patch hides raw_rule within the BaseRule class by making parse() be
a class method for all the rule types, implemented via a rule-specific
abstract method _parse() that returns a parsed Rule object.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch integrated the new capability rule class into aa.py and
cleanprof.py.
Patch changes:
v6:
- fix logic around same_file in cleanprofile.py that was causing
capabilities to be deleted when they weren't covered by an
abstraction.
v5:
- merge my changes into Christian's original patches
- use CapabilityRule.parse() for parsing raw capability rules and
getting a CapabilityRule instance back
- cope with move of parse_modifiers back into rule/__init__.py.
Originally-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Patch changes:
v5:
- merge my changes into Christian's original patches
- update to use CapabilityRule.parse() as the entry point for
parsing raw rules and getting a CapabilityRule instance in
return.
Originally-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch adds four classes - two "base" classes and two specific for
capabilities:
utils/apparmor/rule/__init__.py:
class base_rule(object):
Base class to handle and store a single rule
class base_rules(object):
Base class to handle and store a collection of rules
utils/apparmor/rule/capability.py:
class capability_rule(base_rule):
Class to handle and store a single capability rule
class capability_rules(base_rules):
Class to handle and store a collection of capability rules
Changes:
v5:
- flattened my changes into Christian's patches
- pull parse_modifiers into rule/__init__.py
- pull parse_capability into rule/capability.py
- make CapabiltyRule.parse() be the class/static method for parsing
raw capability rules.
- parse_capability: renamed inlinecomment and rawrule to comment
and raw_rule to be consistent with CapabilityRule fields.
Originally-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
B<UNQUOTED PROFILE NAME> = (must start with alphanumeric character (after variable expansion), or '/' B<AARE> have special meanings; see below. May include I<VARIABLE>. Rules with embedded spaces or tabs must be quoted.)
B<ATTACHMENT SPECIFICATION> = I<FILEGLOB>
B<PROFILE FLAG CONDS> = [ 'flags=' ] '(' comma or white space separated list of I<PROFILE FLAGS> ')'
B<HATNAME> = ( must start with alphanumeric character. see aa_change_hat(2) for a description of how this "hat" is used. IF '^' is used to start a hat then there is no space between the '^' and I<HATNAME>)
B<FILEGLOB> = (must start with '/' (after variable expansion), B<AARE> have special meanings; see below. May include I<VARIABLE>. Rules with embedded spaces or tabs must be quoted. Rules must end with '/' to apply to directories.)
B<UNQUOTED FILEGLOB> = (must start with '/' (after variable expansion), B<AARE> have special meanings; see below. May include I<VARIABLE>. Rules with embedded spaces or tabs must be quoted. Rules must end with '/' to apply to directories.)
B<ACCESS> = ( 'r' | 'w' | 'a' | 'l' | 'k' | 'm' | I<EXEC TRANSITION> )+ (not all combinations are allowed; see below.)
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
