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.
BugLink: http://bugs.launchpad.net/bugs/1534405
Patch -r 2952 switched over to using the library kernel interface, and
added a kernel_interface parameter to the dir_cb struct, that is
used to process directories.
Unfortunately kernel_interface parameter of the dir_cb struct is not being
properly initialized resulting in odd failures and sefaults when the parser
is processing directories.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This adds a basic support for parallel compiles. It uses a fork()/wait
model due to the parsers current dependence on global variables and
structures. It has been setup in a similar manner to how cilk handles
multithreading to make it easy to port to a managed thread model once
the parser removes the dependence on global compute structures in the
backend.
This patch adds two new command line flags
-j <n> or --jobs <n>
which follows the make syntax of specifying parallel jobs currently
defaults to -jauto
-j8 or --jobs=8 allows for 8 parallel jobs
-jauto or --jobs=auto sets the jobs to the # of cpus
-jx4 or --jobs=x4 sets the jobs to # of cpus * 4
-jx1 is equivalent to -jauto
Note: unlike make -j must be accompanied by an option
--max-jobs=<n>
allows setting hard cap on the number of jobs that can be specified
by --jobs. It defaults to the number of processors in the system * 8.
It supports the "auto" and "max" keywords, and using x<n> for a
multiple of the available cpus.
additionally the -d flag has been modified to take an optional parameter
and
--debug=jobs
will output debug information for the job control logic.
In light testing on one machine the job control logic provides a nice
performance boost. On an x86 test machine with 60 profiles in the
/etc/apparmor.d/ directory, for the command
time apparmor_parser -QT /etc/apparmor.d/
old (equiv of -j1):
real 0m10.968s
user 0m10.888s
sys 0m0.088s
ubuntu parallel load using xargs:
real 0m8.003s
user 0m21.680s
sys 0m0.216s
-j:
real 0m6.547s
user 0m17.900s
sys 0m0.132s
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
https://launchpad.net/bugs/1526085
Revno 2934 'Add fns to handle profile removal to the kernel interface'
introduced a regression in the parser's namespace support by causing the
--namespace-string option to be ignored. This resulted in the profile(s)
being loaded into the global namespace rather than the namespace
specified on the command line.
This patch fixes the bug by setting the Profile object's ns member, if
the --namespace-string option was specified, immediately after the
Profile object is allocated.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
