The parser correctly rejects mount make-* options (make-shared,
make-slave, make-private, make-unbindable) when a device is specified
(the source argument of mount(2)). However, it was not rejecting the
recursive make-* options (make-rshared, make-rslave, make-rprivate,
make-runbindable) when a device was specified.
This patch adds the MS_REC bit, which is used to indicate a recursive
option, to the MS_CMDS macro. Without this change, the recursive options
are treated as normal mount options.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser should not indicate success when mount rules contain
unknown mount options:
$ echo "/t { mount options=(XXX) -> **, }" | apparmor_parser -qQ
$ echo $?
0
This patch modifies the parser so that it prints an error message and
exits with 1:
$ echo "/t { mount options=(XXX) -> **, }" | apparmor_parser -qQ
unsupported mount options
$ echo $?
1
Bug: https://bugs.launchpad.net/bugs/1401621
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
There are a number of differences between what the apparmor.d(5) man
page lists as valid AppArmor mount rule options and what apparmor_parser
looks for when parsing mount rules. There are also typos in the man page
and parser around mount options. Here's the breakdown of problems and
fixes made in this patch:
* The apparmor.d(5) man page improperly documented a "nodirsync"
option.
- That mount option does not exist and the parser did not honor it.
Remove the mention from the apparmor.d(5) man page.
* The loud option was typoed as "load" in both the man page and parser
- There's no sense in preserving backwards compatibility. "load" is
simply wrong and should not be honored. The man page and parser are
updated to only use "loud".
* The rbind option wasn't listed in the man page.
- Add rbind to the man page. No change needed for the parser.
* The documented unbindable, private, slave, and shared options were
not correctly parsed. The parser expected
make-{unbindable,private,slave,shared}.
- The parser is updated to accept both the documented
{unbindable,private,slave,shared} options and their variants
prefixed with "make-". The man page will not document the "make-"
variants.
* The recursive {runbindable,rprivate,rslave,rshared} options were not
documented and were only recognized by the parser if they were
prefixed with "make-".
- The man page is updated to document the option strings that are not
prefixed with "make-". The parser still accepts the "make-"
variants.
* The man page documented a "rec" option but the parser didn't honor
it. The MS_REC macro is used by the mount utility to be bitwise OR'ed
with MS_{UNBINDABLE,PRIVATE,SLAVE,SHARED} to indicate the
corresponding recursive mount options.
- This is not an option that should be exposed in the AppArmor policy
since we already allow have the
{runbindable,rprivate,rslave,rshared} options.
* The man page typoed the {no,}relatime options as {no,}relative.
- The man page is updated to document the correct option strings. The
parser requires no change.
Bug: https://bugs.launchpad.net/bugs/1401619
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
When using recursive_print for debugging, RawRules objects weren't
reporting detailed information. This patch fixes that, as well as fixing
some indenting issues in the output.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Those *.spec{,.in} files were not updated for years (last change
2006/2007) and don't fit the current "one tarball for everything" model.
Acked-by: Steve Beattie <steve@nxnw.org>
The check-logprof target was not updated to use the python tools, when
they were merged in. This patch fixes the issue.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
LOG_MODE_RE (used in validate_log_mode() in aamode.py) just checked if
the given parameter contains one of the possible matches. This resulted
in "invalid" being a valid log mode (from audit.log requested_mask or
denied_mask) because it contains 'a', which is a valid file mode.
This patch wraps the regex into ^(...)+$ to make sure the full
string contains only allowed file modes.
The patch also adds some tests for validate_log_mode().
Acked-by: Steve Beattie <steve@nxnw.org>
aa.py uses profile_data[profile][hat]['change_profile'] at various
places. However, there are also two places that use 'changes_profile'
(note the additional 's'), which should also be 'change_profile'.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch converts a ValueError raised when parsing of a permission
mode fails into an AppArmorBug with better diagnostic information, and
adds a test case to confirm that the exception is raised.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Also change check_for_apparmor() to allow easier testing by optionally
specifying alternative locations for /proc/filesystems and /proc/mounts
as parameter.
Note that the code in check_for_apparmor() differs from what the comment
says - valid_path() only does syntax checks, but doesn't check if the
directory exists. I added a comment saying exactly that.
Acked-by: Steve Beattie <steve@nxnw.org>
- replace MODE_MAP_RE regex with MODE_MAP_SET set
- change sub_str_to_mode() to use MODE_MAP_SET set instead of MODE_MAP_RE
- change split_log_mode to use split() instead of a regex
Patch by Peter Maloney <peter.maloney@brockmann-consult.de>
Acked-by: Christian Boltz <apparmor@cboltz.de>
split_log_mode() change also
Acked-by: Seth Arnold <seth.arnold@canonical.com>
performance improvement
Patch by Peter Maloney <peter.maloney@brockmann-consult.de>
Acked-by: Christian Boltz <apparmor@cboltz.de>
(previous patch version (with minor difference) also
Acked-by: Seth Arnold <seth.arnold@canonical.com>)
interpreters, it used
aa[profile][hat]['path'][interpreter_path]['mode']
instead of
aa[profile][hat]['allow']['path'][interpreter_path]['mode']
The ['allow'] part was missing.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch pulls out all the common processing for writing out
each of the prior segments that need to be written before writing
the current segment into a function called 'write_prior_segments',
reducing a bunch of ugly duplication.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The assignment for setting segments['include'] = True was wrong,
it occured inside the 'if not segments['include'] and True in
segments.values():' block, whereas it needed to always get set outside
of that if test.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch
- fixes a check that used if "aa[profile][hat][incname]:" instead of
"if aa[profile][hat]['include'].get(incname, False):" ("['include']"
was missing) which means the performance shortcut was never hit
- avoids auto-created empty and superfluous hashers in
aa[profile][hat]['allow']['path'] and
include[incfile][incfile][allow]['path']
- adds the filename to the "Can't find system log" exception
Patch by Peter Maloney <peter.maloney@brockmann-consult.de>
Changes compared to the original patch:
- change back quoting in the exception message to '...'
Acked-By: Christian Boltz <apparmor@cboltz.de>
Modify the build instructions and project requirements to reflect the
utils rewrite from Perl to Python.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch adds the --enable-man-pages option, defaulting to "yes", to
libapparmor's configure stage so that libapparmor can be built on
systems lacking Perl.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Fail the configure stage if podchecker is not available since man page
generation always happens.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Point to the site for Perl the language and not Perl news.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
There's no need to date ourselves.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The configure script fails if pod2man isn't available since PROG_POD2MAN
is called unconditionally so it is safe to split man page generation out
of the --with-perl configure option.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
present as a function (which it is not in Python3, even though it was
under an if else python version check).
The following patch:
- checks the __builtins__ module for existence of raw_input and sets
it up for Python3
Acked-by: Steve Beattie <steve@nxnw.org>
The diff displayed by (V)iew changes in aa-logprof lacks leading
whitespace because it bases the whitespace on the indention level of the
closing "}".
Besides that, it uses different values for the indention level for newly
added rule types than for existing rule types. (For example, if a
profile already had a network rule, added network rules will get a
different indention level.) However you won't notice this because
currently it doesn't indent the rules at all ;-)
This patch fixes serialize_profile_from_old_profile() in aa.py so that
it always uses the correct indention level.
Also clean up and simplify how the profile is written in the end (when
matching RE_PROFILE_END) - we already have "write_methods", so we can
just re-use it instead of "manually" calling one write_* function after
the other. Unfortunately dicts don't keep their original order,
therefore I had to introduce "default_write_order".
Finally, add some missing rule types to "segments" to avoid key errors.
Acked-by: Steve Beattie <steve@nxnw.org>
When aa-logprof asks for adding capability rules, it also offers the
Audi(t) option. Unfortunately, this option does nothing ;-)
This patch fixes ask_the_question() so that it really ;-) allows to
switch the audit flag on and off. It also initializes the "audit"
variable to make sure the next capability doesn't inherit the audit flag
used for the previous capability.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch for recursive_print() in common.py fixes printing dicts with
py3. It also replaced the tabs() lambda function with a plain string,
and the brace() lambda function with a simple formatstring to make the
code easier to understand.
Also add support for nested lists - for the start and end of each list,
print a [ and ]. Without that, you get a long list of items without an
indicator if/when a new parent list starts.
Acked-by: Steve Beattie <steve@nxnw.org>
When aa-logprof asks for a capability, you'll see something like
WARN: unknown capability: CAP_block_suspend
The reason for the warning and "Severity: unknown" is that severity.db
contains the capability names in uppercase, but ask_the_question() calls
sev_db.rank with the capability in lowercase.
This patch converts the "CAP_$capability" string to uppercase before
doing the lookup.
Acked-by: Steve Beattie <steve@nxnw.org>
Also add a testcase (written by Steve Beattie) to ensure this stays fixed.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
utils/Makefile contains a grep -v "undefined name _". Some manual
testing shows that pyflakes doesn't print any warning about "_", so
this grep is superfluous.
Removing the grep also means we don't need a tempfile for the pyflakes
output anymore, which simplifies the pyflakes call a lot.
Acked-by: Steve Beattie <steve@nxnw.org>
The recent re-work of the severity.db tests were not verified to
pyflakes clean. All but one of pyflakes co are of marginal impact
(assigning to a variable that isn't later referenced); however, one
legitimate issue it detected is that I inadvertently created two test
cases with the same method name, so only one test case would actually
be used.
The following patch fixes the issues.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
These regression tests are for an Ubuntu-specific bug. However, they
should benefit the upstream project, as well. Ubuntu took an incomplete
version of a patch, which introduced the bug. The version of that patch
that landed upstream did not contain the bug.
The bug was in policy compilation of certain combinations of rule types,
conditionals, and conditional values. The easiest such combination to
test is a rule such as:
ptrace peer=ABC,
Buggy parsers will generate binary policy that causes the kernel to deny
a ptrace of a process confined by ABC, despite the presence of the above
rule.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
