CVE-2017-6507
https://launchpad.net/bugs/1668892
The common AppArmor 'restart' code used by some init scripts, upstart
jobs, and/or systemd units contained functionality that is no longer
appropriate to retain. Any profiles not found /etc/apparmor.d/ were
assumed to be obsolete and were unloaded. That behavior became
problematic now that there's a growing number of projects that maintain
their own internal set of AppArmor profiles outside of /etc/apparmor.d/.
It resulted in the AppArmor 'restart' code leaving some important
processes running unconfined. A couple examples are profiles managed by
LXD and Docker.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The latex based techdoc in the parser/ tree adds a number of build
dependencies for downstreams to create it; it also is the primary
element to make the builds unrepeatable. Creating the techdoc and other
documentation when generating a tarball for distribution avoids all
that.
* Makefile: build documentation as part of the tarball creation. Skip
the libraries/libapparmor directory as it needs to have configure run
before the manpages can be made.
* changehat/mod_apparmor/Makefile, changehat/mod_apparmor/Makefile,
utils/Makefile, profiles/Makefile: create separate docs target,
some of them dummies.
* parser/Makefile: pull the techdoc out of the default build target, add
an extra_docs target to create it.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The apparmor.d description about alias rules was broken in multiple
ways. The manpage
- didn't include the alias keyword
- listed alias rules in the "COMMA RULES" section - while that's correct
for the comma requirement, it's also wrong because COMMA RULES is
meant to be inside a profile
- didn't list alias rules in the PREAMBLE section
This patch fixes this.
It also moves the definition of VARIABLE, VARIABLE ASSIGNMENT (both
unchanged) and ALIAS RULE next to PREAMBLE.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10
I already did this in the python code a month ago, and now realized that
we should also update the apparmor.d manpage ;-)
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10.
BugLink: https://launchpad.net/bugs/1588069
Currently
change_profile /** -> A,
change_profile unsafe /** -> A,
do not conflict because the safe rules only set the change_profile
permission where the unsafe set unsafe exec. To fix this we have the
safe version set exec bits as well with out setting unsafe exec.
This allows the exec conflict logic to detect any conflicts.
This is safe to do even for older kernels as the exec bits off of the
2nd term encoding in the change_onexec rules are unused.
Test files
tst/simple_tests/change_profile/onx_no_conflict_safe1.sd
tst/simple_tests/change_profile/onx_no_conflict_safe2.sd
by Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
The opt_unsafe token was being used to represent 'safe' and 'unsafe' so
it is renamed to opt_exec_mode. Create helpfully named macros to compare
opt_exec_mode's value against instead of hard-coded '0', '1', and '2'
values.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Simple tests that validate the parser's ability to handle change_profile
rules containing an exec mode.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
https://launchpad.net/bugs/1584069
This patch allows policy authors to specify how exec transitions should
be handled with respect to setting AT_SECURE in the new process'
auxiliary vector and, ultimately, having libc scrub (or not scrub) the
environment.
An exec mode of 'safe' means that the environment will be scrubbed and
this is the default in kernels that support AppArmor profile stacking.
An exec mode of 'unsafe' means that the environment will not be scrubbed
and this is the default and only supported change_profile exec mode in
kernels that do not support AppArmor profile stacking.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Make future modifications to the change_profile grammar rules easier by
simplifying things. First, the change_profile rule handling is collapsed
into a single grammar rule. The inputs to the grammar rule are given
helpful variable names to make it harder to mix up which variable we're
dealing with. Finally, the two separate calls to new_entry() are unified
into a single call.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This commit touches up the .po files that generate warnings
when msgfmt processes them to create .mo files, at least with gettext
0.19.7-2ubuntu3 in Ubuntu 16.04 LTS. Example warning types cleaned up
include:
ce.po:7: warning: header field 'Last-Translator' still has the initial default value
ce.po:7: warning: header field 'Language' missing in header
de.po:6: warning: header field 'Language-Team' still has the initial default value
This commit also fixes up po files where the Report-Msgid-Bugs-To:
field had not been updated, setting it with the email address
'AppArmor list <apparmor@lists.ubuntu.com>'
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
BugLink: http://bugs.launchpad.net/bugs/1551950
The apparmor_parser is incorrectly outputting the names of child profiles
and hats, by adding a : between the parent and the child profile name
Eg.
/usr/sbin/httpd{,2}-prefork
/usr/sbin/httpd{,2}-prefork://DEFAULT_URI
/usr/sbin/httpd{,2}-prefork://HANDLING_UNTRUSTED_INPUT
instead of what it should be
/usr/sbin/httpd{,2}-prefork
/usr/sbin/httpd{,2}-prefork//DEFAULT_URI
/usr/sbin/httpd{,2}-prefork//HANDLING_UNTRUSTED_INPUT
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
BugLink: http://bugs.launchpad.net/bugs/1566490
This patch enables to parser to scale the max jobs if new resources are
being brought online by the scheduler.
It only enables the scaling check if there is a difference between the
maximum number of cpus (CONF) and the number of online (ONLN) cpus.
Instead of checking for more resources regardless, of whether the online
cpu count is increasing it limits its checking to a maximum of
MAX CPUS + 1 - ONLN cpus times. With each check coming after fork spawns a
new work unit, giving the scheduler a chance to bring new cpus online
before the next check. The +1 ensures the checks will be done at least
once after the scheduling task sleeps waiting for its children giving
the scheduler an extra chance to bring cpus online.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
parser 'make install' failed if 'make' wasn't run before. This patch
adds the missing dependency 'install-indep: indep'.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
https://launchpad.net/bugs/1561939
The debugedit program is unable to cope with the extra slash in the
LIBAPPARMOR_SRC variable.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Suggested-by: Christian Svensson <blue@cmd.nu>
The debugging code for profile entries contains a check to ensure that
it's not NULL, but the list iterator macro already ensures that the
iteration will stop if the item is NULL, making the check redundant.
Coverity CID #55983
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This makes some of the references to functions in the aa_query_label(2)
manpage more consistent and fixes a couple of grammar issues. It also
tries to make the qualifying statements in apparmor.d(5) more distinct,
and also fixes some typos there as well.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
The audit_read capability, mpls address family, and profile stacking are
all new features advertised by the latest AppArmor kernel features file.
Without this change, the parser tests will fail because parsing profiles
that utilize stacking results in an error when the features file
indicates that stacking is not supported by the kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Check if the current kernel supports stacking. If not, ensure that named
transitions (exec, change_profile, etc.) do not attempt to stack their
targets.
Also, set up the change_profile vector according to whether or not the
kernel supports stacking. Earlier kernels expect the policy namespace to
be in its own NUL-terminated vector element rather than passing the
entire label (namespace and profile name) as a single string to the
kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Allow for a leading '&' character to be present in the named transition
target strings to indicate that the transition should stack the current
profile with the specified profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser was splitting up the namespace and profile name from named
transition targets only to rejoin it later when creating the binary
policy. This complicated the changes needed to support the stacking
identifier '&' in named transition targets.
To keep the stacking support simple, this patch keeps the entire named
transition target string intact from initial profile parsing to writing
out the binary.
All of these changes are straightforward except the hunk that removes
the namespace string addition to the vector in the process_dfa_entry()
function. After speaking with John, kernels with stacking have support
for consuming the namespace with the profile name.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch separates the label parsing functionality from the program
termination and memory allocation duties of parse_label(). This will
ultimately help in creating simple helper functions that simply need to
check if a label contains a namespace.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The add_named_transition function was written in a way that is difficult
to understand while attempting to read the function. This patch attempts
to clean it up.
First, this patch removes this confusing code flow issue:
if (!entry->ns) { ... }
if (entry->ns) { ... } else { ... }
It then unifies the way that the ns and nt_name strings of the cod_entry
struct are handled prior to calling add_entry_to_x_table() and/or
returning. ns and nt_name are now guaranteed to be NULL before
performing either of those actions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The copy_cod_entry() function was not copying the nt_name field of the
cod_entry struct.
This was discovered during code review and I'm not certain if it causes
any real world bugs.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Instead of reusing opt_named_transition and be forced to reconstruct the
target path when is looks like ":odd:target", create simpler grammer
rules that have nothing to do with named transitions and namespaces.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
https://launchpad.net/bugs/1540666
Reuse the new parse_label() function to initialize named_transition
structs so that transition targets, when used with change_profile, are
properly seperated into a profile namespace and profile name.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Bug: https://launchpad.net/bugs/1379874
https://launchpad.net/bugs/1544387
Don't split namespaces from profile names using YACC grammar. Instead,
treat the entire string as a label in the grammer. The label can then be
split into a namespace and a profile name using the new parse_label()
function.
This fixes a bug that caused the profile keyword to not be used with a
label containing a namespace in the profile declaration.
Fixing this bug uncovered a bad parser test case at
simple_tests/profile/profile_ns_ok1.sd. The test case mistakenly
included two definitions of the :foo:unattached profile despite being
marked as expected to pass. I've adjusted the name of one of the
profiles to :foo:unattached2.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
https://launchpad.net/bugs/1546455
Don't filter out AF_UNSPEC from the list of valid protocol families so
that the parser will accept rules such as 'network unspec,'.
There are certain syscalls, such as socket(2), where the LSM hooks are
called before the protocol family is validated. In these cases, AppArmor
was emitting denials even though socket(2) will eventually fail. There
may be cases where AF_UNSPEC sockets are accepted and we need to make
sure that we're mediating those appropriately.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Suggested-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
[cboltz: Add 'unspec' to the network domain keywords of the utils]
deny rules don't allow ix, Px, Ux etc. - only 'deny /foo x,' is allowed.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.10
Note: Seth mentioned in the mail that he doesn't like the 'deny x'
section too much, but we didn't find a better solution when discussing
it on IRC. Therefore I keep the patch unchanged, but will happily
review a follow-up patch if someone sends one ;-)
This test causes `make check` to fail but it is known bug so mark it as
a TODO test.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch frees some leaked memory that occur when errors are
detected while adding variables to the parser's symbol table. While not
a significant issue currently due to the parser exiting on failures, as
the process of library-ifying the parser continues, these need to be
addressed. It also makes it easier to use tools like Address Sanitizer
on the parser against our test suite.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
The first entry in the grouping_count array is never initialized to 0;
subsequent depths are. This patch initializes the whole array.
Issue found with valgrind.
Signed-off-by: Steve Beattie <steve@nxnw.org> (with improvement from Seth)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch fixes the unit test memory leaks found
by intrigeri using AddressSanitizer in the following email thread:
https://lists.ubuntu.com/archives/apparmor/2015-August/008491.html
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The capnames list missed a comma, which lead to the funny
"mac_overridesyslog" capability name.
__debug_capabilities() seems to be the only user of capnames, which
might explain why this bug wasn't noticed earlier.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
