This patch improves the error messages in aa.py store_list_var() to make
debugging of profile syntax problems easier. It also adds an additional
parameter for the profile filename (used in the error message)
Acked-by: Steve Beattie <steve@nxnw.org>
This patch fixes a crash in aa-complain when a profile name is quoted.
It also makes sure aa-complain actually adds the complain flag in such
cases. (aa-enforce etc. will also benefit from this fix.)
Note: superfluous quotes will be removed when saving the profile (for
example with aa-cleanprof), but they are kept if needed, like in
profile "/bin/foo bar"
(tested with aa-complain and aa-cleanprof - and also with "rcapparmor
reload", where the initscript bailed out because my profile filename
contained a space...)
The patch also adds some TODO notes.
References: https://bugs.launchpad.net/apparmor/+bug/1296218
Acked-by: Steve Beattie <steve@nxnw.org>.
When creating a child profile while using genprof, I get a backtrace:
Traceback (most recent call last):
File "aa-genprof", line 160, in <module>
lp_ret = apparmor.do_logprof_pass(logmark, passno)
File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2291, in do_logprof_pass
save_profiles()
File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2309, in save_profiles
for prof_name in changed.keys():
RuntimeError: dictionary changed size during iteration
(See https://bugs.launchpad.net/apparmor/+bug/1014304 for more details.)
After digging into the code, it seems for some reason the child profile
is added to "changed" - I doubt this is correct (guess why it's removed
later... ;-)
After digging a bit more, I found out that create_new_profile() is
(ab)used to create a new stub profile to be used as child profile.
create_new_profile then adds the new child (which looks like a normal
profile to it) to "changed".
This patch most probably makes the cleanup round in save_profile()
superfluous by adding a is_stub parameter to create_new_profile(). If
this parameter is set, the new (child) profile is not added to "created"
and "changed".
I intentionally added the two print() lines in safe_profile because
a) I think they will never be displayed
b) I want to know if a) is wrong ;-)
c) it's always nice to have a "nice" error message before displaying
a backtrace ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
(unlimited) because the "if not value:" check matches 0.
This patch replaces the check with "... is None".
It also prints a warning if the old value is None (could in theory
happen if reading the old value failed).
Acked-by: Steve Beattie <steve@nxnw.org>. Thanks.
preprocessor and is not as thorough as -QTK (--skip-kernel-load,
--skip-read-cache, --skip-cache). Like with '-p', '-QTK' can be run without
privilege but it will catch things like conflictings 'x' modifiers.
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Bug: https://bugs.launchpad.net/bugs/1325109
The parser will accept rules with either umount or unmount rule types.
The utils should follow suite.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
- convert "tail" result from byte to string to avoid TypeError crash
- use apparmor.filename instead of hardcoded /var/log/audit/audit.log
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
(in a more serious version: add some tests for dbus, *mount, signal,
ptrace and pivot_root and make sure a space after the keyword is enforced.
The tools shouldn't accept a "dbusdriver" or "pivot_rootbeer" rule. ;-)
Acked-by: Tyler Hicks <tyhicks@canonical.com>
(dbus, *mount, signal, ptrace, pivot_root) except if the line only
contains the bare keyword.
Note that in most cases (except *mount) I used an alternation - this has
the advantage that it doesn't change the match group numbering, with the
small disadvantage of having to mention the keyword twice in the regex.
I chose this way to avoid that I have to change lots of other places and
possibly introduce bugs by overlooking something.
For the *mount rules, I read the code - it shouldn't need any changes
because it uses only matches[0..2]
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Remove duplicated test code by adding a simple way for regex test
classes to declare a regex to use and a list of tuples consisting of
(line, expected_result). The setup_regex_tests() method generates test
methods for each tuple in a classes list. The test methods are based on
the regex_test() method, which performs the regex search and compares
the results to the expected_result.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The regexes for signal, ptrace, and pivot_root rules each had an extra
grouping around the terms 'signal', 'ptrace', and 'pivot_root'. Those
extra groupings can be safely removed.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch backs out most of the changes from r2448 in favor of a better
approach.
The optional "file" keyword is handled under the pre-existing
RE_PROFILE_PATH_ENTRY regex and a new regex, RE_PROFILE_BARE_FILE_ENTRY,
is created for handling bare file rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The first conditional around string being set is not needed. If string
is not set, the while loop will be skipped and mode will be returned.
The variable tmp was being overloaded by being the regex search result
and then being reassigned to be the first match group in the regex
search result. This patch keeps tmp as the regex search result and then
uses mode_char to represent the first match group of the search.
Group the search and replace actions together at the beginning of the
loop and group the mode character processing at the end of the loop.
Finally, remove the unnecessary check of tmp (now mode_char) before
calling MODE_HASH.get(tmp, False). If tmp is None or '', get() will
do the right thing and return False.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1307665
When str_to_mode() was given a string containing unknown mode
characters, it entered an infinite loop. The case of the MODE_MAP_RE
regex string not matching the mode string was being ignored.
This patch makes it so that the loop breaks when MODE_MAP_RE no longer
matches the mode string. This occurs when all of the valid mode
characters have been processed and only invalid mode characters remain.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1298678
This patch does bare bones parsing of pivot_root rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering pivot_root rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1300317
This patch does bare bones parsing of ptrace rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering ptrace rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1300316
This patch does bare bones parsing of signal rules and stores the raw
strings for writing them out later. It is meant to be a simple change to
prevent aa.py from emitting a traceback when encountering signal rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-By: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1295346
Add the ability to read and write path rules containing the file prefix.
This also includes bare "file," rules.
The ALL global is updated to include a preceding NUL char to eliminate
possibilities of a real file path colliding with the ALL global.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
With the conversion of the python utils, aa-easyprof got added to the
list of tools to be installed (in /usr/sbin/), but is already installed
(in /usr/bin) by the python-tools-setup.py distutils script, leaving two
copies of the tool in place. This patch filters out aa-easyprof from the
list of tools for the makefile to install itself, leaving it to
(continue to) be installed by the distutils script.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds some simple tests of the capability regex in
apparmor/aa.py.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Bug: https://bugs.launchpad.net/bugs/1294819
This patch as minimal support for bare capability rules ("capability,").
It prevents aa.py from emitting a traceback when encountering such a
rule.
It only adds the ability to parse and write the bare rule. It doesn't
attempt to be clever when deleting duplicate rules, such as realizing
that "capability audit_control," can be deleted if "capability," is also
present.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1294825
This patch is inspired by sbeattie's patch to add limited dbus rule
support. It adds does very dumb parsing of mount rules. Basically, it
stores mount, remount, and umount rules as raw strings wrapped in a
class.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1294848
This patch fixes the testsuite for aa-easyprof when the easyprof
utilities and configuration files are not installed in the system.
What was happening was that verify_manifest was calling parse_args()
without the synthetic arguments created by the test case and passing
the result to AppArmorEasyProfile(). Because parse_args() didn't
have the synthetic arguments, it would parse the actual command line
arguments passed to the testscript, which of course didn't specify the
alternate configuration file location. This would work when easyprof
had been installed in the system, because the fallback configuration
file in /etc/apparmor/easyprof.conf would exist and specify template
and policy group locations. Without that, though, the tests would abort
due to not knowing the location of the templates and policy groups.
This patch fixes the issue by passing the synthetic argument list
to verify_manifest, which uses that when calling parse_args(). A
debugging statement that states which conffile is being used when
AppArmorEasyProfile is being instantiated.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
last_audit_entry_time() was waiting forever because
subprocess.check_output() started tail without any parameters.
Fixed by removing shell=True (default is shell=False).
Also fix the regex ("^.*", the dot was missing)
Acked-by: Steve Beattie <steve@nxnw.org>
- added beginning of utils translations for Polish and Swedish
- Some rejiggering of existing utils translations; I don't think any
existing translations got lost, but there are new missing entries
- A whole bunch of comment updating for the parser translations
Acked-by: Steve Beattie <steve@nxnw.org>
D-Bus rules in particular seem to get written as multi-line rules. This
patch adds very simple hackish support for multiple lines. Essentially,
what it does is if the parsing of a line doesn't match anything and
falls all the way through, it saves the line and prepends it to the next
line that occurs in the profile, but *only* if the line does not have a
trailing comma to indicate the end of a rule. If the trailing comma
exists, then it assumes that it's a rule that it doesn't understand and
aborts.
With this patch, the simpler tools (aa-enforce, aa-complain, etc.) can
parse policies containing multi-line rules to an extent and continue to
function correctly. Again, aa-logprof and aa-genprof may have issues on
the writing back of profiles, so some assistance testing here would be
appreciated.
Some testcases are added to exercise the regex that looks for a rule
with a trailing comma but can still handle rules that have (,) or {,}
in them.
Patch history:
v1 - initial version
v2 - simplify and rearrange rule-ending comma search regex, since
we only care about the trailing comma
- add a new regex to search for trailing comments to filter out
- simplify reset of lastline variable
- restructure tests into a new script, and add more tests
v3 - add additional testcases, most of which are problematic and thus
commented out :(
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch adds very limited support for very dumb parsing of dbus
rules. Basically, it stores dbus rules as raw strings wrapped in
a class.
There's class structure to indicate how I'd like to see fuller future
support for dbus rules to be implemented and act as a guidepost for
how to handle most rules, moving away from the giant structure of
nested dictionaries. A stub test script is included as well, with a
modification to the make check target to set the PYTHONPATH to point
in the right place.
With this patch, aa-audit, aa-autodep, aa-complain, aa-disable,
and aa-enforce all function for me. aa-logprof and aa-genprof have
functionality issues for me at the moment (one of them dumps a
backtrace even without this patch), and I'm not sure the writing out
of dbus rules is completely implemented for modified profiles.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch splits out the genprof tool functionality into a separate
command function, merging with the use_autodep function that already
existed.
Patch history:
v1 - initial revision
v2 - mark strings for translation and modify message when a profile
name is passed to aa-autodep, rather than a program name/path.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch moves the audit functionality to an audit specific command
function.
As an aside, the -r option is left in place here, because aa-audit
is a bit orthogonal to aa-enforce, aa-complain, and aa-disable.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch creates a separate tool.cmd_complain function, as well as
removes the -r remove option, to match aa-enforce and aa-disable.
It also cleans up some bits in aa-enforce now that aa-complain and
aa-enforce have been separated.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch splits out the aa-enforce functionality into a separate
method in the aa_tools class. It also removes one last reference to
the no-longer-existent -r option in the aa-enforce manpage.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch modifies the aa-disable tool implementation to allow it to
take a profile name (rather than a program name) as the argument(s)
for what to disable, as this was supported behavior in the perl
tools. (The rest of the commands that make use of the aa_tools.act()
method have not been exercised with this patch in place, as further
patches will separate those out.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch removes a debugging print statement accidentally left in,
as well as a duplicated initialization to a variable, and moves the
variable init closer to the declaration that the variable is a global.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
It's not useful to report the location of the temporary directory for
each test if you're going to immediately delete it.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
aa-genprof was incorrectly trying to refer to UI_xxx functions in
apparmor.aa rather than the correct apparmor.ui. This patch fixes the
issue.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The relative directory tests in test-aa-easyprof.py were failing when
TMPDIR pointed to a directory that was a symlink, because the generated
tmpdir path was not the same as the realpath that easyprof resolved to.
This patch sets the tmpdir to the realpath of the result of the
tempfile.mkdtemp() to avoid the issue.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch removes the '--remove' option on aa-enforce as well as from
the man page. It also removes the test entry that contains it, but I
don't think this is entirely correct because I think the second half
of the test is dependent on the (now deleted) first half of the test.
(It also removes a missed reference to --revert in the aa-disable man
page.)
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
