BugLink: http://bugs.launchpad.net/bugs/1534405
Patch -r 2952 switched over to using the library kernel interface, and
added a kernel_interface parameter to the dir_cb struct, that is used
to process directories.
Unfortunately kernel_interface parameter of the dir_cb struct is not being
properly initialized resulting in odd failures and sefaults when the parser
is processing directories.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
If parse_event_for_tree() raises an AppArmorException (for example
because of an invalid/unknown request_mask), catch it in read_log() and
re-raise it together with the log line causing the Exception.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk, 2.10 and 2.9.
handle_children() has some special code for handling link events with
denied_mask = 'l'. Unfortunately this special code depends on a regex
that matches the old, obsolete log format - in a not really parsed
format ("^from .* to .*$").
The result was that aa-logprof did not ask about events containing 'l'
in denied_mask.
Fortunately the fix is easy - delete the code with the special handling
for 'l' events, and the remaining code that handles other file
permissions will handle it :-)
References: Bugreport by pfak on IRC
Testcase (with hand-tuned log event):
aa-logprof -f <( echo 'Jan 7 03:11:24 mail kernel: [191223.562261] type=1400 audit(1452136284.727:344): apparmor="ALLOWED" operation="link" profile="/usr/sbin/smbd" name="/foo" pid=10262 comm=616D617669736420286368362D3130 requested_mask="l" denied_mask="l" fsuid=110 ouid=110 target="/bar"')
should ask to add '/foo l,' to the profile.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
allow read on /run/systemd/resolve/resolv.conf for systems using networkd
(LP: #1529074)
Signed-Off-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
r2637 added support for parsing unix rules, but forgot to add write
support. The result was that a profile lost its unix rules when it was
saved.
This patch adds the write_unix_rules() and write_unix() functions (based
on the write_pivot_root() and write_pivot_root_rules() functions) and
makes sure they get called at the right place.
The cleanprof testcase gets an unix rule added to ensure it's not
deleted when writing the profile. (Note that minitools_test.py is not
part of the default "make check", however I always run it.)
References: https://bugs.launchpad.net/apparmor/+bug/1522938https://bugzilla.opensuse.org/show_bug.cgi?id=954104
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk, 2.10 and 2.9.
This means:
- expect unicode (instead of str) when reading from a file in py2
- convert keys() result to a set to avoid test failures because of
dict_keys type
After this change, all tests work for both py2 and py3.
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk and 2.10.
python 3 uses only the 'str' type, while python 2 also uses 'unicode'.
This patch adds a type_is_str() function to common.py - depending on the
python version, it checks for both. This helper function is used to keep
the complexity outside of the rule classes.
The rule classes get adjusted to use type_is_str() instead of checking
for type(x) == str, which means they support both python versions.
Finally, add test-common.py with some tests for type_is_str().
References: https://bugs.launchpad.net/apparmor/+bug/1513880
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk and 2.10
Note: 2.10 doesn't contain SignalRule and aare.py, and rule/__init__.py
doesn't have check_and_split_list(), therefore it doesn't get those
parts of the patch.
Don't catch AppArmorExceptions in aa-easyprof any longer and rely on
apparmor.fail to print the exception to stderr.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The patch also switches to using error() instead of a plain print() for
AppArmorException, which means prefixing the error message with 'ERROR: '
References: https://bugs.launchpad.net/apparmor/+bug/1521400
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk and 2.10.
https://launchpad.net/bugs/1526085
Revno 2934 'Add fns to handle profile removal to the kernel interface'
introduced a regression in the parser's namespace support by causing the
--namespace-string option to be ignored. This resulted in the profile(s)
being loaded into the global namespace rather than the namespace
specified on the command line.
This patch fixes the bug by setting the Profile object's ns member, if
the --namespace-string option was specified, immediately after the
Profile object is allocated.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
'change_hat' events have the target profile in 'name2', not in 'name'
(which is None and therefore causes a crash when checking if it contains
'//')
Also add the log event causing this crash to the libapparmor testsuite.
References: https://bugs.launchpad.net/apparmor/+bug/1523297
Acked-by: John Johansen <john.johansen@canonical.com> for trunk, 2.10 and 2.9.
Parsing variables was broken in several ways:
- empty quotes (representing an intentionally empty value) were lost,
causing parser failures
- items consisting of only one letter were lost due to a bug in RE_VARS
- RE_VARS didn't start with ^, which means leading garbage (= syntax
errors) was ignored
- trailing garbage was also ignored
This patch fixes those issues in separate_vars() and changes
var_transform() to write out empty quotes (instead of nothing) for empty
values.
Also add some tests for separate_vars() with empty quotes and adjust
several tests with invalid syntax to expect an AppArmorException.
var_transform() gets some tests added.
Finally, remove 3 testcases from the "fails to raise an exception" list
in test-parser-simple-tests.py.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
(which also implies 2.10)
Note: 2.9 doesn't have test-parser-simple-tests.py, therefore it won't
get that part of the patch.
As Kshitij mentioned, abstract methods should use NotImplementedError
instead of AppArmorBug.
While changing this, I noticed that __repr__() needs to be robust against
NotImplementedError because get_raw() is not available in BaseRule.
Therefore the patch changes __repr__() to catch NotImplementedError.
Of course the change to NotImplementedError also needs several
adjustments in the tests.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
(long before branching off 2.10, therefore I also commit to 2.10)
Note: 2.10 doesn't have test-signal.py, which means it can't be patched ;-)
Creating a file is in theory covered by the 'a' permission, however
discussion on IRC brought up that depending on the open flags it might
not be enough (real-world example: creating the apache pid file).
Therefore change the mapping to 'w' permissions. That might allow more
than needed in some cases, but makes sure the profile always works.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for 2.9, 2.10 and trunk
For debugging, it's helpful to know which part of the code initialized a
profile_storage and for which profile and hat this was done.
This patch adds an 'info' array with that information, adds the
corresponding parameters to profile_storage() and changes the callers to
deliver some useful content.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.10
We replaced parse_audit_allow() with parse_modifiers() in r2833, but
overlooked that parse_modifiers() returns allow/deny as boolean. This
resulted in storing bare file rules in aa[profile][hat]['path'][False]
instead of aa[profile][hat]['path']['allow'] (or True instead of 'deny'
for 'deny file,' rules), with the user-visible result of loosing bare
file rules when saving the profile.
This patch converts the boolean value from parse_modifiers back to a
string.
Note: 2.9 is not affected because the old parse_audit_allow() returns
'allow' or 'deny' as string, not as boolean.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk and 2.10
The last utils/test/Makefile change switched to using the in-tree
libapparmor by default (unless USE_SYSTEM=1 is given). However, I missed
to add the swig/python parts of libapparmor to PYTHONPATH, so the
system-wide LibAppArmor/__init__.py was always used.
This patch adds the in-tree libapparmor python module to PYTHONPATH.
I'm sorry for the interesting[tm] way to find out that path, but
a) I don't know a better / less ugly way and
b) a similar monster already works in libapparmor/swig/python/test/ ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9 and trunk
(that also implies 2.10 ;-)
To make things more interesting, /usr/bin/python and /usr/bin/python[23]
are symlinks to /usr/bin/python[23].[0-9], so we have to explicitely
list several versions.
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com> for 2.9, 2.10 and trunk
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>
If a script contains a hashbang like
#! /usr/bin/perl -w
aa-autodep created a profile entry like
"/usr/bin/perl -w" ix,
which is obviously incorrect.
This patch fixes this (by using only the first part of the hashbang line)
and also adds some tests for it.
References: https://bugs.launchpad.net/apparmor/+bug/1505775
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Bug: https://launchpad.net/bugs/1393979
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>
