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>
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>
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>
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>
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>
Only present when building with DEBUG=1.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
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>
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>
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>
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>
- 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>
This patch creates expected pass tests for all known mount options as
well as expected fail tests for some known bad mount options.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Bug: https://launchpad.net/bugs/1399027
This patch restricts MS_REC to only be used while defining the MS_RBIND,
MS_RUNBINDABLE, MS_RPRIVATE, MS_RSLAVE, and MS_RSHARED macros.
The MS_R* macros are simply an OR of the corresponding non-recursive
macro and MS_REC:
#define MS_RBIND (MS_BIND | MS_REC)
Previously, a shortcut was taken when needing to specify the
non-recursive and recursive macros:
(MS_BIND | MS_UNBINDABLE | MS_PRIVATE | MS_SLAVE | MS_SHARED | MS_REC)
By using MS_REC above, it is not immediately clear that
MS_R{BIND,UNBINDABLE,PRIVATE,SLAVE,SHARED} are also included.
By restricting the use of MS_REC, this patch improves readability by
forcing the use of the MS_R{BIND,UNBINDABLE,PRIVATE,SLAVE,SHARED} macros
instead of relying on the MS_REC shortcut.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
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>
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>
Both valgrind and strace report the parser doing
close(-1) = -1 EBADF (Bad file descriptor)
This happens the skip kernel load argument is specified in combination
with any of --add, --replace, or --remove arguments (the default
is --add if no other option is specified).
This happens when the parser is not processing profiles but not
writing them out (eg. no kernel load, dump to stdout, file ...)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
