As originally applied to trunk in 2667:
The AppArmor parser failed to build on the x32 architecture due to a
missing <sys/sysctl.h> header. This header is included by accident, a
vestige of earlier days, and wasn't removed when the sysctls were
removed. (Think Linux 2.0 or Linux 2.2 days.)
See also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760378
Thanks to Thorsten Glaser for the discovery and initial fix.
In commit rev 2127, backported from trunk, a bit of C++ish style code
leaked in, the use of a bool variable. This is problematic for pure C
code. This commit converts the bool to an int and adjusts the true and
false keywords to their corresponding macros as defined in parser.h.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
backport of dev commit 2510
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>
Merge from trunk revision 2308.
std::max in C++ requires that both arguments be the same type. The
previous fix added std::max comparisons between unsigned long numeric
constants and size_t, this fix casts the numeric constants to size_t.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
patch is needed to fix the build.
patch from: Jan Rękorajski <baggins@pld-linux.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
cache files will be written out even if the '--skip-bad-cache' option
is given and the cached features file differs from the features of
the currently running kernel. The patch below fixes the regression.
From: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
we are writing a new cache .features file the cache dir should be cleared
out.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
--cache-loc= is specified. This results in using the .features file from
/etc/apparmor.d/cache or always recompiling policy.
The former case is particularly bad as the .features file in
/etc/apparmor.d/cache/ may not correspond to the file in the specified
cache location.
bug: launchpad.net/bugs/1229393
Signed-off-by: John Johansen <john.johansen@canonical.com>
(The broken URLs were introduced in r1582.)
for utils/*.pod:
Acked-by: Steve Beattie <steve@nxnw.org>
for the other directories:
Patch by Steve Beattie
Acked-by: Christian Boltz <apparmor@cboltz.de>
Merge from trunk revision 2142
This patch fixes problems in the handling of both the final cache
name location and the temporary cache file when an alternate location
is specified.
The first issue is that if the alternate cache directory location was
specified, the alternate directory name would be used as the final
location for the cache file, rather than the alternate directory +
the basename of the profile.
The second issue is that it would generate the temporary file that it
stores the cache file in [basedir]/cache even if an alternate cache
location was specified on the command line. This causes a problem
if [basedir]/cache is on a separate device than the alternate cache
location, because the rename() of the tempfile into the final location
would fail (which the parser would not check the return code of).
This patch fixes the above by incorporating the basename into the cache
file name if the alternate cache location has been specified, bases the
temporary cache file name on the destination cache name (such that they
end up in the same directory), and finally detects if the rename fails
and unlinks the temporary file if that happens (rather than leave it
around). It also has been updated to add a couple of testcases to verify
that writing and reading from an alternate cache location work.
Patch history:
v1: first draft of patch
v2: add testcases, convert PERROR() to pwarn() if rename() fails for
placing cachefile into place.
For 2.8 branch:
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
file is larger than the feature buffer used for cache version comparison.
Ideally this would be dynamically allocated but for 2.8 just bumping the
buffer size is the quick fix.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
caching tests
Merge from trunk commit 2083
Original message:
This patch modifies the parser's caching test to more accurately detect
whether or not the filesystem has a fine enough timestamp resolution.
Occasionally even on filesystems like ext3, the two files' creation
dates would differ when created less than a second apart, which would
typically cause the 'Cache is used when cache is newer' test to fail
because the cached file would have the same timestamp as the profile.
The fix creates 10 files 0.1 seconds apart and ensures that all ten
have distinct timestamps.
(The occasional failure was caught in testing runs like
https://bugs.launchpad.net/qa-regression-testing/+bug/1087061/ )
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk commit 2076
Original message:
Subject: two fixes to the parser's simple test driver
This patch fixes two issue with the simple test driver. The first is
that child exec that actually ran the parser was located inside the
eval statement. This meant that if the exec failed for some reason
(like the parser didn't exist), the child wouldn't actually die,
but would pop out of the eval and continue running through the loop
of test profiles (while the parent process does the same). This meant
that if the script ran on the full testsuite with a misconfiguration,
it would explode creating O(n^2) processes, where n is the number of
testcase files -- with over 25k testcases, that's a lot. The fis is to
lift the child exec outside the eval{}, then an exec() failure causes
the child process to die correctly.
The second fix is that several of the testcases were added with the
DESCRIPTION field added in lower case (i.e. #=Description blah blah).
This fix makes the regex that pulls out the description not be
case-sensitive.
Signed-off-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-By: John Johansen <john.johansen@canonical.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from just the parser/tst/caching.sh portion of trunk commit 2066.
Original message:
apparmor: abstract out the directory walking routine
The apparmor_parser has 3 different directory walking routines.
Abstract
them out and use a single common routine.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Original message:
apparmor: correct apparmor_parser -N command privilege
Fix the apparmor_parsers -N command (which dumps the list of profile
names found in a policy file) to be available without privilege and
also make it be recognized as a command instead of an option so that
it can conflict with -a -r -R -S and -o.
Currently it can be specified with these commands but will cause the
parser to short circuit just dumping the names and not doing the actual
profile compile or load.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Original message:
apparmor: update apparmor_parser man page
Rework and update the apparmor_parser man page. It reworks some of the
text but mostly just reorganizes the commands and options into logical
grouping to make it easier to sort out how the various commands and
options work.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Original Message:
While integrating 3.4-rc1, I ran into a problem where network rules
weren't being processed. It ultimately boiled down to a kernel
issue but I found it useful to see what the parser thought it was
working with. Since the parser already has a debugging mode that
will show things like capabilities, it was an obvious extension to
add network rules.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Nominated-by: Steve Beattie <sbeattie@ubuntu.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Hats/children profiles are used.
the matchflags in the dfa backend are not getting properly reset, which
results in a previously processed profiles match flags being used. This is
not a problem for most permissions but can result in x conflict errors.
Note: this should not result in profiles with the wrong x transitions loaded
as it causes compilation to file with an x conflict.
This is a minimal patch targeted at the 2.8 release. As such I have just
updated the delete_ruleset routine to clear the flags as it is already
being properly called for every rule set.
Apparmor 2.9/3.0 will have a different approach where it is not possible
to reuse the flags.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add the ability to clear out the binary profile cache. This removes the
need to have a separate script to handle the logic of checking and
removing the cache if it is out of date.
The parser already does all the checking to determine cache validity
so it makes sense to allow the parser to clear out inconsistent cache
when it has been instructed to update the cache.
Signed-off-by: John Johnansen <john.johansen@canonical.com>
serious flaw. The test for the network flag was being applied against both
the kernel flags and the cache flags. This means that if either the kernel
or the cache did not have the flag set then network mediation would be
turned off.
Thus if a kernel was booted without the flag, and a cache was generated
based on that kernel and then the system was rebooted into a kernel with
the network flag present, the parser on generating the new policy would
detect the old cache did not support network and turn it off for the
new policy as well.
This can be fixed by either removing the old cache first or regenerating
the cache twice. As the first generation will write that networking is
supported in the cache (even though the policy will have it disabled), and
the second generation will generate the correct policy.
The following patch moves the test so that it is only applied to the kernel
flags set.
Signed-off-by: John Johansen <john.johansen@canonical.com>
compatibility interface. Previously it was assuming that if the compatibility
interface was present that network rules where also present, this is not
necessarily true and causes apparmor to break when only the compatibility
patch is applied.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
- make table of contents, footnotes etc. clickable hyperlinks
- use timestamp of techdoc.tex (instead of build time) as creationdate
in the PDF metadata
- don't include build date on first page of the PDF
- make clean:
- delete techdoc.out (created by pdftex)
- fix deletion of techdoc.txt (was techdo_r_.txt)
The initial target was to get reproduceable PDF builds (therefore the
timestamp-related changes), the other things came up during discussing
this patch with David Haller.
The only remaining difference in the PDF from build to build is the /ID
line. This line can't be controlled in pdflatex and is now filtered
out by build-compare in the openSUSE build service (bnc#760867).
Credits go to David Haller for writing large parts of this patch
(but he didn't notice the techdo_r_.txt ;-)
Signed-Off-By: Christian Boltz <apparmor@cboltz.de>
http://bugs.launchpad.net/bugs/979135
Currently a change_profile rule does not grant access to the
/proc/<pid>/attr/{current,exec} interfaces that are needed to perform
a change_profile or change_onexec, requiring that an explicit rule allowing
access to the interface be granted.
Make it so change_profile implies the necessary
/proc/@{PID}/attr/{current,exec} w,
rule just like the presence of hats does for change_hat
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
http://bugs.launchpad.net/bugs/968956
The parser is incorrectly generating network rules for kernels that can
not support them. This occurs on kernels with the new features directory
but not the compatibility patches applied.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
This fix is needed for the userspace portion of both
BugLink: http://bugs.launchpad.net/bugs/963756
BugLink: http://bugs.launchpad.net/bugs/978038
change_onexec fails for profiles that don't have an attachment specification
eg. unconfined
This is because change_onexec goes through 2 permission checks. The first
at the api call point, which is a straight match of the profile name
eg.
/bin/foo
unconfined
and a second test at exec time, tying the profile to change to to the
exec. This allows restricting the transition to specific execs. This
is mapped as a two entry check
/executable/name\x00profile_name
where the executable name must be marked with the change_onexec permission
and the subsequent profile name as well.
The previous "fix" only covered adding onexec to executable names and
also works for the initial change_onexec request when the profile is
an executable.
However it does not fix the case for when the profile being transitioned
to is not an executable.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Bug #963756
The kernel has an extended test for change_profile when used with
onexec, that allows it to only work against set executables.
The parser is not correctly mapping change_profile for this test
update the mapping so change_onexec will work when confined.
Note: the parser does not currently support the extended syntax
that the kernel test allows for, this just enables it to work
for the generic case.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This patch abstracts out the generation of the lists of capabilities
and network protocol names to the common Make.rules file that is
included in most locations in the build tree, to allow it to be
re-used in the utils/ tree and possibly elsewhere.
It provides the lists in both make variables and as make targets.
It also sorts the resulting lists, which causes it to output differently
than the before case. I did confirm that the results for the generated
files used in the parser build were the same after taking the sorting
into account.
Remount should not be screening off the set of flags it is. They are
the set of flags that the kernel is masking out for make_type and
should not be used on remount. Instead just screen off the other cmds
that can have their own rules generated.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The deny information is not used as valid accept state information,
so remove it from the is_null test. This does not change the dfa
generated but does result in the dumped information changing,
as states that don't have any accept information are no longer
reported as accepting. This is what changes the number of states
reported in the minimize tests.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The same mappings routine had two bugs in it, that in practice haven't
manifested because of partition ordering during minimization. The
result is that some states may fail comparison and split, resulting
in them not being eliminated when they could be.
The first is that direct comparison to the nonmatching state should
not be done as it is a candiate for elimination, instead its partion
should be compared against. This simplifies the first test
The other error is the comparison
if (rep->otherwise != nonmatching)
again this is wrong because nomatching should not be directly
compared against. And again can result in the current rep->otherwise
not being eliminated/replaced by the partion. Again resulting in
extra trap states.
These tests where original done the way they were because
->otherwise could be null, which was used to represent nonmatching.
The code was cleaned up a while ago to remove this, ->otherwise is
always a valid pointer now.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Also make sure the perms method properly switches to hex and back to dec
as some of the previous perm dump code did not.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
The changes are around how user data is handled.
1. permissions are mapped before data is matched
2. If data is to be mapped a AA_CONT_MATCH flag is set in the permissions
which allows data matching to continue.
3. If data auditing is to occur the AA_AUDIT_MNT_DATA flag is set
This allows better control over matching and auditing of data which can
be binary and should not be matched or audited
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
file, should grant access to all files paths on the system but it does
not currently allow access to /
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Rename the pivotroot rule to pivot_root to match the command and the fn
and fix it to support named transition correctly leveraging the parsing
action used for exec transitions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
Currently the backend doesn't like it (blows up) when the a vector entry is
empty. For the case where no flags match build_mnt_flags generates an
alternation of an impossible entry and nothing
(impossible|)
This provides the effect of a null entry without having an empty vector
entry. Unfortunately the impossible entry is not correct.
Note: how this is done needs to be changed and fixed in the next release
this is just a minimal patch to get it working for 2.8
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
When generating the flag set the parser was not generating the complete
set when flags where not consecutive. This is because the len value
was not being reset for each flag considered, so once it was set for
a flag, then the next flag would have to be set to reset it else the
output string was still incremented by the old len value.
Eg.
echo "/t { mount options=rbind, }" | apparmor_parser -QT -D rule-exprs
results in
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d ->
however \x0d only covers the bind and not the recursive flag
This is fixed by adding a continue to the flags generation loop for the
else case.
resulting the dump from above generating
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00\x0d\x0f ->
\x0d\x0f covers both of the required flags
Also fix the flags output to allow for the allow any flags case. This
was being screened out. By masking the flags even when no flags where
specified.
this results in a difference of
echo "/t { mount, }" | apparmor_parser -QT -D rule-exprs
rule: \x07[^\000]*\x00[^\000]*\x00[^\000]*\x00(\x01|)(\x02|)(\x03|)(\x04|)(\x05|)\x00[^\000]*
becoming
\x07[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*\x00[^\000]*
which is simplified and covers all permissions vs. the first rule output
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-By: Steve Beattie <sbeattie@ubuntu.com>
