Move postfix-common from program-chunks/ to abstractions/; remove
program-chunks directory since postfix-common was the last resident of
that directory (and had been since 2007), and adjust the includes of all
the profiles that include postfix-common.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
- Allow reciprocal ptrace readby to everyone (requires peer unconfined or to
ptrace read to us)
- same for ptrace tracedby
- allow us to ptrace read ourselves
- receive all signals from unconfined
- allow us to signal ourselves
- allow sending and receiving "exists" (for pid existence)
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch improves the error messages in aa.py store_list_var() to make
debugging of profile syntax problems easier. It also adds an additional
parameter for the profile filename (used in the error message)
Acked-by: Steve Beattie <steve@nxnw.org>
Earlier fixes to the parser's handling of escape sequences involving '\'
caused a behavioral change that profiles no longer needed to contain
'\\' before an octal escape sequence. However, the regression tests were
never modified to take this change into account, and thus the i18n.sh
octal tests would fail. This patch fixes that.
Also, with the changes, the parser no longer accepts _\_ as a valid
sequence, so we skip this character.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com> (on IRC)
The change to processing escape sequences in trunk commit r2537 requires
a corresponding change to the unit tests in parser_misc.c.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
escape sequences that result in special character that will be interpreted
by later processing need to be passed through as well.
Eg. previously \\ was fixed to be passed through, but other chars
get interpretted as well.
*?[]{}
and ^, in character classes
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Also for characters that are not recognized as a valid escape seq
make sure that the character is emitted.
previously
\$ resulted in \
where it should have been \$ if $ wasn't recognized
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
r2456 unified escape sequence processing but it results in the \\
sequence being processed multiple times (lexer, regex conversion,
backend pcre parsing).
What used to happen was the lexer would only convert octal sequences
and a few special escapes, \\ would be passed through the lexer and
the regex conversion, thus only being handled in the pcre backend.
r2456 changed that so that \\ is handled by the lexer, converting it
to \, which is handled as an escape sequence in both the regex
conversion and the pcre backend.
This means
\\001 instead of being treated as the literal \001 is treated
as an octal escape sequence which is rejected by the regex conversion
(it only allows for certain special chars).
etc.
Fix this by ensuring the lexer does not processes \\ and passes it
through so it is only handled in the backend as was done in the past.
Also fix front end escape sequence processing of octals etc from resulting
in a later escape sequence. That is \134, \d92, .. would get converted
to \ in the lexer and then treated as an escape sequence in the regex
conversion or pcre processing.
We fix this by converting them to the equivalent \\ sequence in the
lexer and letting the backend processes it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This patch fixes a crash in aa-complain when a profile name is quoted.
It also makes sure aa-complain actually adds the complain flag in such
cases. (aa-enforce etc. will also benefit from this fix.)
Note: superfluous quotes will be removed when saving the profile (for
example with aa-cleanprof), but they are kept if needed, like in
profile "/bin/foo bar"
(tested with aa-complain and aa-cleanprof - and also with "rcapparmor
reload", where the initscript bailed out because my profile filename
contained a space...)
The patch also adds some TODO notes.
References: https://bugs.launchpad.net/apparmor/+bug/1296218
Acked-by: Steve Beattie <steve@nxnw.org>.
For some strange reason our caching use ctime instead of mtime.
However this can lead to odd cases of the cache missing even though
neither the profile data nor cache data have changed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Update the apache2 profile so that the parent apache process can kill
worker processes inside of hats. Update the example comments and the
DEFAULT_URI and HANDLING_UNTRUSTED_INPUT hats to include the
apache2-common abstraction to allow them to receive signals from the
parent process.
Author: Kees Cook <kees@ubuntu.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://bugs.launchpad.net/apparmor/+bug/1322764
Update the apache2-common abstraction so that the parent apache process
can kill worker processes inside of hats, as well as handle the updated
mod_apparmor behavior that invokes aa_change_hatv() and then checks
which hat it ended up in via aa_getconn() (which reads from
{PROC}/@{pid}/attr/current).
Author: Kees Cook <kees@ubuntu.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://bugs.launchpad.net/apparmor/+bug/1322764
The child process changes into a hat while the parent process stays in
the main profile.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Add two tests that verify AppArmor denials when one end of the pipe has
bad access permissions to the pipe.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The named_pipe parent process kills the child process at exit. A
"signal," rule must be added to all confinement profiles when the test
is running under a kernel that performs signal mediation.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Allow for the parent and child processes to change into separate hats to
verify named pipe communications between hats with varying permissions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Add debugging info to test binaries and disable optimizations.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
When creating a child profile while using genprof, I get a backtrace:
Traceback (most recent call last):
File "aa-genprof", line 160, in <module>
lp_ret = apparmor.do_logprof_pass(logmark, passno)
File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2291, in do_logprof_pass
save_profiles()
File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2309, in save_profiles
for prof_name in changed.keys():
RuntimeError: dictionary changed size during iteration
(See https://bugs.launchpad.net/apparmor/+bug/1014304 for more details.)
After digging into the code, it seems for some reason the child profile
is added to "changed" - I doubt this is correct (guess why it's removed
later... ;-)
After digging a bit more, I found out that create_new_profile() is
(ab)used to create a new stub profile to be used as child profile.
create_new_profile then adds the new child (which looks like a normal
profile to it) to "changed".
This patch most probably makes the cleanup round in save_profile()
superfluous by adding a is_stub parameter to create_new_profile(). If
this parameter is set, the new (child) profile is not added to "created"
and "changed".
I intentionally added the two print() lines in safe_profile because
a) I think they will never be displayed
b) I want to know if a) is wrong ;-)
c) it's always nice to have a "nice" error message before displaying
a backtrace ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
(unlimited) because the "if not value:" check matches 0.
This patch replaces the check with "... is None".
It also prints a warning if the old value is None (could in theory
happen if reading the old value failed).
Acked-by: Steve Beattie <steve@nxnw.org>. Thanks.
preprocessor and is not as thorough as -QTK (--skip-kernel-load,
--skip-read-cache, --skip-cache). Like with '-p', '-QTK' can be run without
privilege but it will catch things like conflictings 'x' modifiers.
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Commit r2456 fixes a bug in the parsers compilation that can result
policy failures. Unfortunately this Bug slipped into the wild and
shipped in at least one distro.
Bump the parser abi so that parsers that have the fix will invalid
existing cache files, and recompile policy to ensure the fix is applied.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://bugs.launchpad.net/bugs/1325109
The parser will accept rules with either umount or unmount rule types.
The utils should follow suite.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Bug: https://bugs.launchpad.net/bugs/1235478
This is a test to check the label on file descriptors returned from
socketpair().
In its simple form, it simply calls socketpair() and checks the
labels on both fds.
In its complex form, it has the ability to do the simple test, then set
up an exec transition using aa_change_onexec(), and re-exec itself to
check the labeling after the file descriptors have been passed across an
exec transition.
The complex form is meant to test revalidation at exec. AppArmor
currently keeps the original labeling in place across the exec
transition.
Note that this test does not currently test read/write access to the
file descriptors. It only checks the label, as returned by
aa_getpeercon(2).
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Allow for the regression tests to specify arbitrary profile names
without hitting fatal errors or getting warnings from mkprofile.pl.
This allows for a test to have a line like this:
genprofile change_profile->':arbitrary_name -- \
image=arbitrary_name addimage:$test
In the example above, $test can call aa_change_onexec("arbitrary_name")
and then re-exec itself to test behavior across exec transitions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Ubuntu 14.04's chromium-browser has changed paths in a way that prevents
evince from opening clicked links in chromium-browser windows.
This patch adds a new path for the chrome-sandbox executable to the
sanitized_helper profile, so chromium will get its own tailored profile if
necessary.
The reporter who said this patch helped included some further DENIED lines
for signals that indicates this is probably not sufficient but did make
the links work as expected.
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1282314
Signed-off-by: Seth Arnold <seth.arnold@canonical.com>
Acked-By: Jamie Strandboge <jamie@canonical.com>
- convert "tail" result from byte to string to avoid TypeError crash
- use apparmor.filename instead of hardcoded /var/log/audit/audit.log
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Jamie Strandboge <jamie@canonical.com>
Description: man page updates for signals, ptrace and new variables
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
v3: fix freeing of filename when undefined
v2: address tyhicks feedback
refactor to have a common write routine
fix issue with set profile load being done even if !kernel_load
Profile loads from cache files that contain multiple profiles can
result in multiple reloads of the same profile or error messages about
failure to load profiles if the --add option is used. eg.
apparmor="STATUS" operation="profile_load"
name="/usr/lib/apache2/mpm-prefork/apache2" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058388] type=1400 audit(1395415826.937:616):
apparmor="STATUS" operation="profile_load" name="DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.058391] type=1400 audit(1395415826.937:617):
apparmor="STATUS" operation="profile_load"
name="HANDLING_UNTRUSTED_INPUT" pid=8631 comm="apparmor_parser"
<sth0R> [82932.058394] type=1400 audit(1395415826.937:618):
apparmor="STATUS" operation="profile_load" name="phpsysinfo" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059058] type=1400 audit(1395415826.937:619):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//DEFAULT_URI" pid=8631
comm="apparmor_parser"
<sth0R> [82932.059574] type=1400 audit(1395415826.937:620):
apparmor="STATUS" operation="profile_replace" info="profile can not be
replaced" error=-17
name="/usr/lib/apache2/mpm-prefork/apache2//HANDLING_UNTRUSTED_INPUT"
pid=8631 comm="apparmor_parser"
The reason this happens is that the cache file is a container that
can contain multiple profiles in sequential order
profile1
profile2
profile3
The parser loads the entire cache file to memory and the writes the
whole file to the kernel interface. It then skips foward in the file
to the next profile and reloads the file from that profile into
the kernel.
eg. First load
profile1
profile2
profile3
advance to profile2, do second load
profile2
profile3
advance to profile3, do third load
profile3
With older kernels the interface would stop after the first profile and
return that it had processed the whole file, thus while wasting compute
resources copying extra data no errors occurred. However newer kernels
now support atomic loading of multipe profiles, so that all the profiles
passed in to the interface get processed.
This means on newer kernels the current parser load behavior results
in multiple loads/replacements when a cache file contains more than
one profile (note: loads from a compile do not have this problem).
To fix this, detect if the kernel supports atomic set loads, and load
the cache file once. If it doesn't only load one profile section
from a cache file at a time.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Mention, in the apparmor.d man page, that pivot_root arguments must end
with a '/' character since they are directories.
The parser currently allows pivot_root arguments that do not end in '/',
but those rules will always fail to match.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
It may not be obvious that the peer label can be "unconfined". Provide
an example rule, in the apparmor.d man page, demonstrating the
peer=(label=unconfined) conditional.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
(in a more serious version: add some tests for dbus, *mount, signal,
ptrace and pivot_root and make sure a space after the keyword is enforced.
The tools shouldn't accept a "dbusdriver" or "pivot_rootbeer" rule. ;-)
Acked-by: Tyler Hicks <tyhicks@canonical.com>
(dbus, *mount, signal, ptrace, pivot_root) except if the line only
contains the bare keyword.
Note that in most cases (except *mount) I used an alternation - this has
the advantage that it doesn't change the match group numbering, with the
small disadvantage of having to mention the keyword twice in the regex.
I chose this way to avoid that I have to change lots of other places and
possibly introduce bugs by overlooking something.
For the *mount rules, I read the code - it shouldn't need any changes
because it uses only matches[0..2]
Acked-by: Tyler Hicks <tyhicks@canonical.com>
This patch extends the coverage of the parser's simple dbus language
tests.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
This patch adds basic signal tests to the parser's simple language
test suite.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
The mount.sh script mixes calls to the regression test 'mount' binary
and /sbin/mount. This can result in stale mtab entries being left around
after a test run because /sbin/mount adds an mtab entry but the test
'mount' binary, which is also used for unmounting, does not remove mtab
entries.
To solve this problem, the -n option is passed to /sbin/mount so that it
doesn't add an mtab entry when mounting.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
