parse_profile_data(): Ensure last line in a profile is valid
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/271
Acked-by: John Johansen <john.johansen@canonical.com>
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
Replace "existing_profiles" & fix minitools for named profiles
See merge request apparmor/apparmor!249
Acked-by: John Johansen <john.johansen@canonical.com>
Expr tree simplification makes multiple passes at simplifying the
expression tree trying to use fatoring rules and heuristics to achieve
the minimum tree, so that dfa construction has fewer nodes to deal
with.
Unfortunately expr tree simplification can slow some policy compiles,
dependent on the type of expressions generated, down, and even worse
is currently subject to never terminating on some expressions as the
left and right passes keep undoing each others work.
Limiting the number of passes that expr tree simplification does can
provide most of its benefits (later passes generally have diminishing
returns), reduces the overhead it has on simple policy where it is of
little benefit, and insures that simplifications can not get stuck in
an infinite loop due to the left and right passes ping-ponging on each
others factoring.
Note: This also results in a performance improvement in evince
compiles, and general policy compiles because it achieves a better
balance between time spent on simplifying the tree to remove nodes and
time the dfa build requires to build with extra nodes and then
eliminate with minimization.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.744s
user 0m2.714s
sys 0m0.028s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.992s
user 0m2.979s
sys 0m0.012s
and
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.568s
user 0m14.529s
sys 0m0.152s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.741s
user 0m15.400s
sys 0m0.179s
PR: https://gitlab.com/apparmor/apparmor/merge_requests/246
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Remove traces of aa-eventd
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and didn't get any serious updates for several more years, so it's most probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE variables in rc.apparmor.functions anymore.
(In theory I could move these variables to deprecated/rc.aaeventd.* - but in practise that sounds more than superfluous ;-)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/263
Acked-by: John Johansen <john.johansen@canonical.com>
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and
didn't get any serious updates for several more years, so it's most
probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE
variables in rc.apparmor.functions anymore.
The apparmor kernel "module" has not been a loadable module for more
than a decade, it must be built into the kernel and due configuration
requirements it will never go back to being a loadable module.
Remove the long unfunctioning load_module support from the init script.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/257
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: seth.arnold@canonical.com
Use @{sys} tunable in profiles and abstractions
Commit aa065287 made @{sys} tunable available by default.
Update profiles and abstractions to actually use @{sys} tunable for better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
PR: https://gitlab.com/apparmor/apparmor/merge_requests/262
Acked-by: John Johansen <john.johansen@canonical.com>
Commit aa06528790 made @{sys} tunable
available by default.
Update profiles and abstractions to actually use @{sys} tunable for
better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
Merge branch 'regex-dragon-book' into 'master'
PR: https://gitlab.com/apparmor/apparmor/merge_requests/261
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Elaborate in class comment of firstpos, lastpos, followpos, and nullable
fields beyond just referencing the Dragon book. Also add the section of
the book these are explained in.
If a test is marked as TODO, but matches its EXRESULT, this means the
TODO is superfluous and (probably) a change fixed what the TODO was for.
Instead of more or less ignoring such superfluous TODOs, error out to
make the change visible instantly.
Both result in "superfluous TODO" (for unknown reason), but fail after
removing the TODO.
Disable the tests until we find out why they have this strange
behaviour, to unblock merging the "error out on superfluous TODO" patch.
Remove TODO notes from no-longer-failing tests
See merge request apparmor/apparmor!180
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: intrigeri <intrigeri@debian.org>
replace -1 return codes with 255
Technically "return -1" returns 255, so we should write it that way.
(found by shellcheck)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/256
Acked-by: John Johansen <john.johansen@canonical.com>
Technical stuff first:
Replace existing_profiles (a dict with the filenames for both active and
inactive profiles) with active_profiles and extra_profiles which are
ProfileList()s and store the active profiles and those in the extra
directory separately. Thanks to ProfileList, now also the relation
between attachments and filenames is easily available.
Also replace all usage of existing_profiles with active_profiles and
extra_profiles, and adjust it to the ProfileList syntax everywhere.
With this change, several bugs in aa-complain and the other minitools
get fixed:
- aa-complain etc. never found profiles that have a profile name
(the attachment wasn't checked)
- even if the profile name was given as parameter to aa-complain, it
first did "which $parameter" so it never matched on named profiles
- profile names with alternations (without attachment specification)
also never matched because the old code didn't use AARE.
References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882047#92
(search for "As usual" ;-)
Just for completeness - the matching still doesn't honor/expand
variables in the profile name.
ProfileList is meant to store the list of profiles (both name and
attachment) and in which files they live.
Also add unittests to make sure everything works as expected.
parse_profile_data() returns the parsed profiles, but writes to
existing_profiles directly.
read_profiles() calls parse_profile_data() and already handles adding
the parsed profiles to aa, original_aa or extras, which means updating
existing_profiles there is a much better place.
This commit also includes a hidden change: Previously, when parsing
include files, they were also added to existing_profiles. This is
superfluous, only real profiles need to be stored there.
Split get_profile_filename() into
- get_profile_filename_from_profile_name() (parameter: a profile name)
- get_profile_filename_from_attachment() (parameter: an attachment)
Currently both functions call get_profile_filename_orig() (formerly
get_profile_filename()) so the behaviour doesn't change yet.
The most important part of this commit is changing all
get_profile_filename() calls to use one of the new functions to make
clear if they specify a profile or an attachment/executable as
parameter.
As promised, the is_attachment parameter starts to get used in this
patch ;-)
Note: The get_new parameter (which I'll explain in the patch actually
using it) is set to True in all calls to the new functions.
The long term plan is to get rid of it in most cases (hence defaulting
to False), but that will need more testing.
The minitools call write_profile(), write_profile_feedback_ui() and
serialize_profile() with the _attachment_ as parameter.
However, aa-logprof etc. call them with the _profile name_ as parameter.
This patch adds an is_attachment parameter to write_profile() and
write_profile_feedback_ui(). It also passes it through to
serialize_profile() via the options parameter.
If is_attachment is True, the parameter will be handled as attachment,
otherwise it is expected to be a profile name.
tools.py gets changed to set is_attachment to True when calling the
functions listed above to make clear that the parameter is an attachment.
Note: This patch only adds the is_attachment parameter/option, but
doesn't change any behaviour. That will happen in the next patch.
This "will break with non-glibc libcs on Debian and with glibc headers moved to
multiarch locations" (https://bugs.debian.org/798955). Patch based on the one
proposed by Helmut Grohne <helmut@subdivi.de>, amended to replace hard coded
"gcc" with "$(CC)".
Bug-Debian: https://bugs.debian.org/909966
profiles/Makefile: test abstractions against apparmor_parser
See merge request apparmor/apparmor!237
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.13.
Pre-acked for 2.10..2.12 after removing the --config-file option which is not supported in these branches.
aa-notify man page: update user's configuration file path
See merge request apparmor/apparmor!239
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
Also adjust the signal rules in the dovecot-common and apache2-common
abstractions to match the profile names, and to really do that
(peer=...{bin,sbin}... didn't work, the correct syntax would have been
peer=...\{bin,sbin\}...)
This fixes the regression introduced by !149 / commit
4200932d8f
Add most abi/bad_*.sd tests to "exception not raised" list
Interestingly, abi/bad_6.sd is detected as invalid, and therefore not
added to the list.
I propose this for all branches that g[eo]t support for abi rules and the abi testcases.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/238
Acked-by: John Johansen <john.johansen@canonical.com>
AppArmor 3.0 requires policy to use a feature abi rule for access to
new features. However some policy may start using abi rules even if
they don't have rules that require new features. This is especially
true for out of tree policy being shipped in other packages.
Add enough support to older releases that the parser will ignore the
abi rule and warn that it is falling back to the apparmor 2.x
technique of using the system abi.
If the profile contains rules that the older parser does not
understand it will fail policy compilation at the unknown rule instead
of the abi rule.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The handling for quotedid checked for the first quote but failed
to ensure the trailing quote was present.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce the INCLUDE/INCLUDE_EXISTS code dup by using a
variable for whether the name was enclosed by '<' and using
processid() to handle the whether the id is quoted or not.
In addition using processid allows include names to contain
escaoe sequences like \n and have them handled correctly.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce code duplication by checking the current state to
determine the single parameter difference between include and
include if exists
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently if stdin is used the warning
apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
is always displayed but if caching has been disabled there is no need for
this message.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
