https://launchpad.net/bugs/1569316
When Ubuntu made the jump from network-manager 1.0.4 to 1.1.93, the
dnsmasq process spawned from network-manager started hitting a
disconnected path denial:
audit: type=1400 audit(1460463960.943:31702): apparmor="ALLOWED"
operation="connect" info="Failed name lookup - disconnected path"
error=-13 profile="/usr/sbin/dnsmasq"
name="run/dbus/system_bus_socket" pid=3448 comm="dnsmasq"
requested_mask="wr" denied_mask="wr" fsuid=65534 ouid=0
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Since the latest openSUSE Tumbleweed update (dovecot 2.2.21 -> 2.2.22),
dovecot/auth writes to /var/run/dovecot/stats-user.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
acmetool is an alternative client for Let's Encrypt.
(https://github.com/hlandau/acme/)
It stores the certificates etc. in the following directory layout:
/var/lib/acme/live/<domain> -> ../certs/<hash>
/var/lib/acme/certs/<hash>/cert
/var/lib/acme/certs/<hash>/chain
/var/lib/acme/certs/<hash>/privkey -> ../../keys/<hash>/privkey
/var/lib/acme/certs/<hash>/url
/var/lib/acme/certs/<hash>/fullchain
/var/lib/acme/keys/<hash>/privkey
This patch adds the needed permissions to the ssl_certs and ssl_keys
abstractions so that the certificates can be used.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
In /etc/nscd.conf there is an option allowing to restart nscd after a
certain time. However, this requires reading /proc/self/cmdline -
otherwise nscd will disable paranoia mode.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=971790
Acked-By: Jamie Strandboge <jamie@canonical.com> for trunk, 2.10 and 2.9
Since 2.9 r2978, test-aa.py fails thanks to a missing import of
'var_transform'. This patch adds the missing import.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
write_prof_data[hat] is correct (it only contains one profile, see bug 1528139),
write_prof_data[profile][hat] is not and returns an empty (sub)hasher.
This affects RE_PROFILE_START and RE_PROFILE_BARE_FILE_ENTRY.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.9 and 2.10
dovecot-lda needs to read and write /tmp/dovecot.lda.*.
It also needs to be able to execute sendmail to send sieve vacation
mails.
For now, I'm using a child profile for sendmail to avoid introducing a
new profile with possible regressions. This child profile is based on
the usr.sbin.sendmail profile in extras and should cover both postfix'
and sendmail's sendmail.
I also mixed in some bits that were needed for (postfix) sendmail on my
servers, and dropped some rules that were obsolete (directory rules not
ending with a /) or covered by an abstraction.
In the future, we might want to provide a stand-alone profile for
sendmail (based on this child profile) and change the rule in the
dovecot-lda profile to Px.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=954959https://bugzilla.opensuse.org/show_bug.cgi?id=954958
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
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>
If a profile file contains multiple profiles, aa-mergeprof crashes on
saving in write_profile() because the second profile in the file is not
listed in 'changed'. (This happens only if the second profile didn't
change.)
This patch first checks if 'changed' contains the profile before
pop()ing it.
Reproducer: copy utils/test/cleanprof_test.in to your profile directory
and run aa-mergeprof utils/test/cleanprof_test.out. Then just press
's' to save the profile.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9
If autodep() is called with a pname starting with / (which can happen
for (N)amed exec depending on the user input), this pname is mapped to
bin_name.
This might look like a good idea, however if the given pname doesn't
exist as file on-disk, autodep() returns None instead of a (mostly
empty) profile. (Reproducer: choose (N)amed, enter "/foo/bar")
Further down the road, this results in two things:
a) the None result gets written as empty profile file (with only a "Last
modified" line)
b) a crash if someone chooses to add an abstraction to the None, because
None doesn't support the delete_duplicates() method for obvious
reasons ;-)
Unfortunately this patch also introduces a regression - aa-logprof now
fails to follow the exec and doesn't ask about the log events for the
exec target anymore. However this doesn't really matter because of a) -
asking and saving to /dev/null vs. not asking isn't a real difference
;-)
Actually the patch slightly improves things - it creates a profile for
the exec target, but only with the depmod() defaults (abstractions/base)
and always in complain mode.
I'd prefer a patch that also creates a complete profile for the exec
target, but that isn't as easy as fixing the issues mentioned above and
therefore is something for a future fix. To avoid we forget it, I opened
https://bugs.launchpad.net/apparmor/+bug/1545155
Note: 2.9 "only" writes an empty file and doesn't crash - but writing
an empty profile is still an improvement.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9
Most probably-file log events can also be network events. Therefore
check for request_mask in all events, not only file_perm, file_inherit
and (from the latest bugreport) file_receive.
References: https://bugs.launchpad.net/apparmor/+bug/1540562
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for trunk, 2.10 and 2.9.
If reading /dev/urandom failed, the corresponding file descriptor was
leaked through the error path.
Coverity CID #56012
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
I suspect that the incorrect description of EPERM was copied from
the aa_change_hat man page, where it is possible to see EPERM if the
application is not confined by AppArmor.
This patch corrects the description by documenting that the only
possible way to see EPERM is if a confined application has the
no_new_privs bit set.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
It is possible that file descriptors will be revalidated after an
aa_change_profile() but there is a lot of complexity involved that
doesn't need to be spelled out in the man page. Instead, mention that
revalidation is possible but the only way to ensure that file
descriptors are not passed on is to close them.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The statement was meant to convey the difference between aa_change_hat()
and aa_change_profile(). Unfortunately, it read as if there was
something preventing a program from using aa_change_profile() twice to
move from profile A to profile B and back to profile A, even if profiles
A and B contained the necessary rules.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
When hitting an unknown line while parsing a profile, it's a good idea
to include that line in the error message ;-)
Note: 2.9 would print a literal \n because it doesn't have apparmor.fail,
so it will get a slightly different patch with spaces instead of \n.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk, 2.10 and 2.9.
'!' is a reserved symbol and needs to be escaped in AARE.
Note: aare.py only exists in trunk, therefore this part is trunk-only.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9 as needed.
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.
If parse_event_for_tree() raises an AppArmorException (for example
because of an invalid/unknown request_mask), catch it in read_log() and
re-raise it together with the log line causing the Exception.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk, 2.10 and 2.9.
Note: 2.9 can't handle \n in exception messages, therefore I'm using spaces.
handle_children() has some special code for handling link events with
denied_mask = 'l'. Unfortunately this special code depends on a regex
that matches the old, obsolete log format - in a not really parsed
format ("^from .* to .*$").
The result was that aa-logprof did not ask about events containing 'l'
in denied_mask.
Fortunately the fix is easy - delete the code with the special handling
for 'l' events, and the remaining code that handles other file
permissions will handle it :-)
References: Bugreport by pfak on IRC
Testcase (with hand-tuned log event):
aa-logprof -f <( echo 'Jan 7 03:11:24 mail kernel: [191223.562261] type=1400 audit(1452136284.727:344): apparmor="ALLOWED" operation="link" profile="/usr/sbin/smbd" name="/foo" pid=10262 comm=616D617669736420286368362D3130 requested_mask="l" denied_mask="l" fsuid=110 ouid=110 target="/bar"')
should ask to add '/foo l,' to the profile.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk, 2.10 and 2.9.
allow read on /run/systemd/resolve/resolv.conf for systems using networkd
(LP: #1529074)
Signed-Off-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
r2637 added support for parsing unix rules, but forgot to add write
support. The result was that a profile lost its unix rules when it was
saved.
This patch adds the write_unix_rules() and write_unix() functions (based
on the write_pivot_root() and write_pivot_root_rules() functions) and
makes sure they get called at the right place.
The cleanprof testcase gets an unix rule added to ensure it's not
deleted when writing the profile. (Note that minitools_test.py is not
part of the default "make check", however I always run it.)
References: https://bugs.launchpad.net/apparmor/+bug/1522938https://bugzilla.opensuse.org/show_bug.cgi?id=954104
Acked-by: Tyler Hicks <tyhicks@canonical.com> for trunk, 2.10 and 2.9.
'change_hat' events have the target profile in 'name2', not in 'name'
(which is None and therefore causes a crash when checking if it contains
'//')
Also add the log event causing this crash to the libapparmor testsuite.
References: https://bugs.launchpad.net/apparmor/+bug/1523297
Acked-by: John Johansen <john.johansen@canonical.com> for trunk, 2.10 and 2.9.
Parsing variables was broken in several ways:
- empty quotes (representing an intentionally empty value) were lost,
causing parser failures
- items consisting of only one letter were lost due to a bug in RE_VARS
- RE_VARS didn't start with ^, which means leading garbage (= syntax
errors) was ignored
- trailing garbage was also ignored
This patch fixes those issues in separate_vars() and changes
var_transform() to write out empty quotes (instead of nothing) for empty
values.
Also add some tests for separate_vars() with empty quotes and adjust
several tests with invalid syntax to expect an AppArmorException.
var_transform() gets some tests added.
Finally, remove 3 testcases from the "fails to raise an exception" list
in test-parser-simple-tests.py.
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
(which also implies 2.10)
Note: 2.9 doesn't have test-parser-simple-tests.py, therefore it won't
get that part of the patch.
Creating a file is in theory covered by the 'a' permission, however
discussion on IRC brought up that depending on the open flags it might
not be enough (real-world example: creating the apache pid file).
Therefore change the mapping to 'w' permissions. That might allow more
than needed in some cases, but makes sure the profile always works.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for 2.9, 2.10 and trunk
After switching to winbindd as test profile, comments about the ntpd
profile don't make sense anymore ;-)
The patch also includes a whitespace fix.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
Change minitools_test.py to use the winbind instead of the ntpd profile
for testing. The tests broke because the ntpd profile has the
attach_disconnected flag set now, and therefore didn't match the
expected flags anymore.
Also replace the usage of filecmp.cmp() in the cleanprof test with
reading the file and using assertEqual - this has the advantage that we
get a full diff instead of just "files differ".
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
This allows to run minitools_test.py as non-root user.
Also add a check that only creates the force-complain directory if it
doesn't exist yet.
Note: With this patch applied, there are still 4 failing tests, probably
caused by changes in the profiles that are used in the tests.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
Change aa-notify parse_message() to also honor complain mode log events.
This affects both modes - desktop notifications and the summary report.
Acked-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
aa-complain is part of the enforce/complain/disable triple. Therefore
I expect it to actually load a profile in complain mode.
To do this, it has to delete the 'disable' symlink, but set_complain()
in aa.py didn't do this (and therefore kept the profile disabled).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
Users might expect that setting a profile into audit mode also activates
it (which shouldn't happen IMHO because the audit flag is not part of
the enforce/complain/disable triple), so we should at least tell them.
References: https://bugs.launchpad.net/apparmor/+bug/1429448
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9
The last utils/test/Makefile change switched to using the in-tree
libapparmor by default (unless USE_SYSTEM=1 is given). However, I missed
to add the swig/python parts of libapparmor to PYTHONPATH, so the
system-wide LibAppArmor/__init__.py was always used.
This patch adds the in-tree libapparmor python module to PYTHONPATH.
I'm sorry for the interesting[tm] way to find out that path, but
a) I don't know a better / less ugly way and
b) a similar monster already works in libapparmor/swig/python/test/ ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for 2.9 and trunk
(that also implies 2.10 ;-)
To make things more interesting, /usr/bin/python and /usr/bin/python[23]
are symlinks to /usr/bin/python[23].[0-9], so we have to explicitely
list several versions.
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com> for 2.9, 2.10 and trunk
syslog-ng needs to access both the permanent /var/log/journal/ and the
non-permanent /run/journal/.
I also included /var/run/journal/ to stay consistent with supporting
both /run/ and /var/run/.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Also add support for the USE_SYSTEM variable, which means:
- test against the in-tree libapparmor and python modules by default
- test against the system libapparmor and python modules if USE_SYSTEM
is set
The old behaviour was a mix of both - it always used the in-tree python
modules and the system libapparmor.
For obvious reasons, you'll need to build libapparmor before running the
tests (unless you specify USE_SYSTEM=1 as parameter to make check).
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
oftc_ftw reported on IRC that Arch Linux has a symlink /bin -> /usr/bin.
This means we have to update paths for /bin/ in several profiles to also
allow /usr/bin/
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
This patch is based on a SLE12 patch to allow executing the
--dhcp-script. We already have most parts of that patch since r2841,
except /dev/tty rw which is needed for the shell's stdout and stderr.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=940749 (non-public)
Acked by Seth Arnold on IRC (with "owner" added)
logparser.py does a regex check on log lines as performance improvement
so that it only hands over lines that look like AppArmor events to
LibAppArmor parsing. Those regexes were incomplete and didn't cover all
log formats LibAppArmor accepts, with the end result of "overlooking"
events.
This patch splits off common parts of the regex, adds more regexes for
several log types and finally merges everything into one regex.
test-logparser.py gets adjusted to the merged RE_LOG_ALL regex.
Finally, add a new test that was posted on IRC to the test_multi set.
As already threatened nearly a month ago,
Acked by <timeout> for trunk and 2.9
Note: 2.9 doesn't have test-libapparmor-test_multi.py, therefore I can't
add the check to verify all test_multi log lines against the regex to
ensure logparser.py doesn't silently ignore events.
Bug: https://launchpad.net/bugs/1569316
This patch is based on a SLE12 patch to allow executing the
--dhcp-script. We already have most parts of that patch since r2841,
however the SLE bugreport indicates that /bin/sh is executed (which is
usually a symlink to /bin/bash or /bin/dash), so we should also allow
/bin/sh
References: https://bugzilla.opensuse.org/show_bug.cgi?id=940749 (non-public)
Acked-by: Seth Arnold <seth.arnold@canonicalc.com> for trunk and 2.9
Add some permissions that I need on my system:
- execute nm-dhcp-helper
- read and write /var/lib/dhcp6/dhclient.leases
- read /var/lib/NetworkManager/dhclient-*.conf
- read and write /var/lib/NetworkManager/dhclient-*.conf
Looks-good-by: Steve Beattie <steve@nxnw.org>
Acked-by: <timeout> for trunk and 2.9
/usr/share/locale-bundle/ contains translations packaged in
bundle-lang-* packages in openSUSE.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk commit rev 3219
In testing against the 4.1 kernel, the syscall_sysctl testcase started
failing even in the unconfined case. What the test program does is
attempt to adjust the kernel.threads-max sysctl to be slightly larger
and see if the operation succeeds by reading the value back out. It
also attempts to save the original value and restore it. The test
was failing because (in VMs at least) the default value chosen by
the kernel for the kernel.threads-max setting was high enough that
attempts to increase it would be ignored (likely to prevent too much
use of kernel memory by threads), helpfully without any message being
report to dmesg. Thus, the initial read of the current value would
succeed, the write of that value + 1024 would appear to succeed,
but then reading the value back out and comparing it to the expected
value would fail, as it would still be the original value, not the
expected new value.
This patch attempts to address this by first attempting to raise
the value, and if that does not appear to work, to then attempt
to lower it. It also refactors the code a bit by creating helper
functions to perform the actual sysctl(2) calls to make the code a
bit easier to read.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
In some cases, the return value of name_to_prof_filename() is undefined.
This happens when deleting the to-be-confined binary while running
aa-genprof and leads to a not-too-helpful
File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 265, in enforce
prof_filename, name = name_to_prof_filename(path)
TypeError: 'NoneType' object is not iterable
(reported by maslen on IRC)
This patch makes sure name_to_prof_filename() always returns None, None
(instead of undefined aka just None) so that at least the caller can
successfully split it into two None values.
For the exotic aa-genprof usecase given above, this at least improves
the error message to
Can't find $binary_name
(raised by enforce() via fatal_error())
The patch also changes fatal_error() to display the traceback first, and
the human-readable message at the end, which makes it more likely that
the user actually notices the human-readable message.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both trunk and 2.9.
"rcapparmor kill" results in a funny error message:
/lib/apparmor/rc.apparmor.functions: line 441: return: -v: invalid option
return: usage: return [n]
SLE12 includes a patch that prevents this error message, but also
prevents that $? is handed over correctly to rc_status. This means that
"rcapparmor kill" will happily display "done" even with a compiled-in
apparmor module that can't be unloaded.
This patch is the improved version - it adds a small helper function to
set $? (as handed over to aa_log_end_msg()) and then calls rc_status -v.
This means that "rcapparmor kill" now shows "failed" because it's
impossible to unload something that is compiled directly into the
kernel.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=862170 (non-public)
Acked-by: Seth Arnold <seth.arnold@canonical.com> for 2.9 and trunk
Merge from trunk commit 3201
In the commit "Rev 3169: regression tests: have
ptrace use PTRACE_GETREGSET by default", I created
some ifdef magic to use the per arch general purpose
register data structures for various architectures,
including arm64. Unfortunately, in the upstream glibc commit
7d05a8168b
<bits/ptrace.h> is no longer included in the arm64 specific user.h,
which defined the structure as 'struct user_pt_regs'; instead user.h
was converted to define 'struct user_regs_struct'. Because of this,
the ptrace test fails to compile on arm64 when glibc is 2.20 or newer.
This patch adjusts the ptrace test to use the newer structure on arm64
if it's detected that a newer glibc is detected and reverts to using
the older one for older glibcs. It also adds an error when compiling
on architectures that haven't been incorporated yet.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
cux and CUx are valid exec permissions, so they should be accepted
by validate_profile_mode() ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
aa-logprof raises an exception if
- an include file contains a hat
- that file is included in a profile and
- aa-logprof hits an audit log entry for this profile
Reproducer ("works" on 2.9 and trunk):
python3 aa-logprof -f <(echo 'Jun 19 11:50:36 piorun kernel: [4474496.458789] audit: type=1400 audit(1434707436.696:153): apparmor="DENIED" operation="open" profile="/usr/sbin/apache2" name="/etc/gai.conf" pid=2910 comm="apache2" requested_mask="r" denied_mask="r" fsuid=0 ouid=0') -d ../profiles/apparmor.d/
This happens because profiles/apparmor.d/apache2.d/phpsysinfo was
already read when pre-loading the include files.
This patch changes aa.py parse_profile_data() to only raise the
exception if it is not handling includes currently.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
For some (not yet known) reason, we get file_perm events without
request_mask set, which causes an aa-logprof crash.
Reproducer log entry:
Jun 19 12:00:55 piorun kernel: [4475115.459952] audit: type=1400 audit(1434708055.676:19629): apparmor="ALLOWED" operation="file_perm" profile="/usr/sbin/apache2" pid=3512 comm="apache2" laddr=::ffff:193.0.236.159 lport=80 faddr=::ffff:192.168.103.80 fport=61985 family="inet6" sock_type="stream" protocol=6
This patch changes logparser.py to ignore those events.
References: https://bugs.launchpad.net/apparmor/+bug/1466812/
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
According to the parser test profiles (which are the only
"documentation" I found about this), definition of boolean variables
is only allowed outside profiles, not inside them.
parse_profile_data() got it the wrong way round, therefore this patch
fixes the condition and updates the error message.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Ignore include files that were not read before (= don't exist in
include[], which typically happens for #include <directory>) so that
the profile_known_*() functions don't crash.
Note: Since the 2.9 code is too different, this patch only avoids the
crash, but doesn't ensure that the files in the included directory are
honored (which would need in a rewrite of the profile_known_*()
functions).
BTW: I tested with a network log entry and hope the best for
profile_known_capability() ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1471425
Acked-by: Steve Beattie <steve@nxnw.org>
The only difference between PROFILE_MODE_RE and PROFILE_MODE_NT_RE
was that the latter one additionally allowed 'x', which looks wrong.
(Standalone 'x' is ok for deny rules, but those are handled by
PROFILE_MODE_DENY_RE.)
This patch completely drops PROFILE_MODE_NT_RE and the related code in
validate_profile_mode().
Also wrap the two remaining regexes in '^(...)+$' instead of doing it
inside validate_profile_mode(). This makes the code more readable and
also results in a 2% performance improvement when parsing profiles.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Add the missing "pux" to PROFILE_MODE_RE and PROFILE_MODE_NT_RE.
Also move those regexes and PROFILE_MODE_DENY_RE directly above
validate_profile_mode() which is the only user.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Parsing of boolean assignments failed with
TypeError: '_sre.SRE_Match' object is not subscriptable
because of a missing ".groups()"
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Errors include typos ("DESCRIPT__ON"), missing value after #=EXRESULT
and #=EXRESULT=PASS (= instead of space).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk revision 3169
Bug: https://bugs.launchpad.net/apparmor/+bug/1470985
The ptrace regression test fails to compile on the arm64 platform,
because it uses PTRACE_GETREGS and not the newer PTRACE_GETREGSET
interface for getting access to arch-specific register information[0].
However, fixing it is complicated by the fact that the struct name
for for the general purpose registers is not named consistently
across architectures. This patch attempts to address those issues,
and compiles at least on i386, amd64, arm64, arm (armhf), ppc64,
and ppc64el. The test is verified to continue to function correctly
on i386 and amd64.
[0] https://sourceware.org/ml/archer/2010-q3/msg00193.html
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk commit 3159
parser_regex.c includes libapparmor_re/aare_rules.h and thus it
should depend on it in the Makefile.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
LSMs, such as AppArmor, aren't consulted when a program calls access(2).
This can result in access(2) returning 0 but a subsequent open(2)
failing.
The aa-status utility was doing the access() -> open() sequence and we
became aware of a large number of tracebacks due to open() failing for
lack of permissions. This patch catches any IOError exceptions thrown by
open(). It continues to print the same error message as before when
access() failed but also prints that error message when AppArmor blocks
the open of the apparmorfs profiles file.
https://launchpad.net/bugs/1466768
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Hat declarations ("^hat,") were added in 2.3 for declaring external
hats, but in the meantime aren't supported by the parser anymore (tested
with 2.9.2 parser).
Additionally, if a profile contains both a hat declaration and the hat
("^hat { ...}"), the hat declaration can overwrite the content of the
hat on a "last one wins" base.
This is caused by setting 'declared' to True, which means write_piece()
will only write the "^hat," line, but not the "^hat { ... }" block.
Therefore no longer set 'declared' to True, print a warning that hat
declarations are no longer supported, and ignore the rule. This also
means that running aa-cleanprof can make the profile valid again :-)
Also no longer change 'hat' when hitting a profile declaration, which
also looks wrong.
Note: This change removes the only usage of 'declared'. A follow-up
patch (trunk only) will completely remove the 'declared' handling.
Reproducer profile (run aa-cleanprof on it):
(will crash in remove_duplicate_rules() 80% of the time - if so, try
multiple times. One of the next patches will fix that. Or just try 2.9,
which doesn't have the crash in remove_duplicate_rules().)
/usr/bin/true {
^FOO {
capability setgid,
}
# deletes the content of ^FOO when saving the profile! (last one wins)
# additionally, the parser says this is invalid syntax
^FOO,
}
See also the "Hat declarations" thread on the ML,
https://lists.ubuntu.com/archives/apparmor/2015-June/008107.html
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both 2.9 and trunk.
Currently the cache file has its mtime set at creation time, but this
can lead to cache issues when a policy file is updated separately from
the cache. This makes it possible for an update to ship a policy file
that is newer than the what the cache file was generated from, but
result in a cache hit because the cache file was local compiled after
the policy file was package into an update (this requires the update
to set the mtime of the file when locally installed to the mtime of
the file in its update archive but this is commonly done, especially
in image based updates).
http://bugs.launchpad.net/bugs/1460152
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Merged from trunk revision 3117
Some parts of the AppArmor build system don't respect $CPPFLAGS.
The attached patch fixes this.
Acked-by: Steve Beattie <steve@nxnw.org>
The following patch:
- Brings the return to the correct indentation
- Adds a sorted call over the set keys of hat in the profile
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.9.
aa-complain, aa-enforce, aa-disable and aa-audit refused to change
profiles for non-existing binaries. This patch also allows paths
starting with /. This also makes it possible to use
aa-complain '/{usr/,}bin/ping'
and
aa-complain /etc/apparmor.d/bin.ping
This patch fixes https://bugs.launchpad.net/apparmor/+bug/1416346
Well, mostly - we still need to decide how we handle wildcards in
profile names:
aa-complain ping
aa-complain /usr/bin/ping
will still error out with "Profile not found" because it isn't an exact
match (and matching the wildcard would change more than the user wants).
Oh, and this patch also fixes the last failure in minitools_test.py.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Add a --no-reload parameter to aa-audit, aa-cleanprof, aa-complain,
aa-disable and aa-enforce. This makes it possible to change the
profile flags without reloading the profile.
Also change tools.py to honor the --no-reload parameter.
References: https://bugs.launchpad.net/apparmor/+bug/1458480
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
When switching the audit flag for network events in aa-logprof
(technically, it happens in aa.py ask_the_question()), the "(I)gnore"
button gets "lost".
This patch fixes the list of available buttons.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Thanks to a broken optimization (which missed a "not" in the if
condition), aa-mergeprof never asked to merge capability rules.
Also fix a syntax / parameter error uncovered after fixing the
condition.
This patch is only meant for the 2.9 branch.
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://launchpad.net/bugs/1210514
It did this in the old 2.8 code, but didn't in 2.9.x (first there was a
broken hat regex, then I commented out the hat handling to avoid
breakage caused by the broken regex).
This patch makes sure the hat flags get set when setting the flags for
the main profile.
Also change RE_PROFILE_HAT_DEF to use more named matches
(leadingwhitespace and hat_keyword). Luckily all code that uses the
regex uses named matches already, which means adding another (...) pair
doesn't hurt.
Finally adjust the tests:
- change _test_set_flags to accept another optional parameter
expected_more_rules (used to specify the expected hat definition)
- add tests for hats (with '^foobar' and 'hat foobar' syntax)
- add tests for child profiles, one of them commented out (see below)
Remaining known issues (also added as TODO notes):
- The hat and child profile flags are *overwritten* with the flags used
for the main profile. (That's well-known behaviour from 2.8 :-/ but we
have more flags now, which makes this more annoying.)
The correct behaviour would be to add or remove the specified flag,
while keeping other flags unchanged.
- Child profiles are not handled/changed if you specify the 'program'
parameter. This means:
- 'aa-complain smbldap-useradd' or 'aa-complain /usr/sbin/smbldap-useradd'
_will not_ change the flags for the nscd child profile
- 'aa-complain /etc/apparmor.d/usr.sbin.smbldap-useradd' _will_ change
the flags for the nscd child profile (and any other profile and
child profile in that file)
Even with those remaining issues (which need bigger changes in
set_profile_flags() and maybe also in the whole flags handling), the
patch improves things and fixes the regression from the 2.8 code.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Bug: https://launchpad.net/bugs/1501913
Samba 4.2 needs some more permissions for nmbd and winbindd.
To avoid overcomplicated profiles, change abstractions/samba to allow
/var/lib/samba/** rwk, (instead of **.tdb rwk) - this change already
fixes the nmbd profile.
winbindd additionally needs some more write permissions in /etc/samba/
(and also in /var/lib/samba/, which is covered by the abstractions/samba
change)
References: https://bugzilla.opensuse.org/show_bug.cgi?id=921098 and
https://bugzilla.opensuse.org/show_bug.cgi?id=923201
Note: this commit differs from trunk r3038 - the 2.9 version keeps the
/var/lib/samba/ lines in the winbindd profile avoid problems if for
some reason abstractions/samba isn't updated (*.rpmnew etc.)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
I noticed "disconnected path" (run/nscd/*) events for ntpd while
updating to the latest openSUSE Tumbleweed.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9.
aa-mergeprof failed to fail ;-) when it should raise an AppArmorException.
Instead, it failed with
AttributeError: 'module' object has no attribute 'AppArmorException'
I confirmed this bug in trunk and 2.9.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
(might get re-used later ;-)
Also add two tests for profile names not starting with / - the quoted
version wasn't catched as invalid before, so this change is actually
also a bugfix.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
reported by darix on IRC. This is needed if you have a bigger setup with
dovecot on a different (or multiple) machines
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Ensure nosetests sees all tests in the tests[] tuples. This requires
some name changes because nosetests thinks all function names containing
"test" are tests. (A "not a test" docorator would be an alternative, but
that would require some try/except magic to avoid a dependency on nose.)
To avoid nosetests thinks the functions are a test,
- rename setup_all_tests() to setup_all_loops()
- rename regex_test() to _regex_test() (in test-regex_matches.py)
Also add the module_name as parameter to setup_all_loops and always run
it (not only if __name__ == '__main__').
Known issue: nosetests errors out with
ValueError: no such test method in <class ...>: stub_test
when trying to run a single test generated out of tests[].
(debugging hint: stub_test is the name used in setup_test_loop().)
But that's still an improvement over not seeing those tests at all ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Assume you have a profile like
/bin/foo {
/etc/ r,
network,
/usr/ r,
}
(important: there must be be a non-path rule between the two path blocks)
Then run aa-logprof and add another path event. When choosing (V)iew changes,
it will crash with a misleading
File ".../utils/apparmor/aamode.py", line 205, in split_mode
other = mode - user
TypeError: unsupported operand type(s) for -: 'collections.defaultdict' and 'set'
The reason for this is our beloved hasher, which is playing funny games
another time.
The patch wraps the hasher usage with a check for the parent element to
avoid auto-creation of empty childs, which then lead to the above crash.
BTW: This is another issue uncovered by the LibreOffice profile ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Merge from trunk revision 3012
Update the postfix-common abstraction to cope with signal and unix
socket mediation, update the access to the sasl library locations
in a multiarch compliant way, and allow access to limited bits
of the filesystem paths under which postfix chroots itself to
(/var/spool/postfix/ on Ubuntu).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
serialize_profile_from_old_profiles() calls store_list_var() with an
empty hasher. This fails for "+=" because in this case store_list_var()
expects a non-empty hasher with the variable already defined, and raises
an exception because of the empty hasher.
This patch sets "correct = False" if a "+=" operation appears, which
means the variable will be written in "clean" mode instead.
Adding proper support for "add to variable" needs big changes (like
storing a variable's "history" - where it was initially defined and what
got added where).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
the LibreOffice profile uncovered that handling of @{var} += is broken:
File ".../utils/apparmor/aa.py", line 3272, in store_list_var
var[list_var] = set(var[list_var] + vlist)
TypeError: unsupported operand type(s) for +: 'set' and 'list'
This patch fixes it:
- change separate_vars() to use and return a set instead of a list
(FYI: separate_vars() is only called by store_list_var())
- adoptstore_list_var() to expect a set
- remove some old comments in these functions
- explain the less-intuitive parameters of store_list_var()
Also add some tests for separate_vars() and store_list_var().
The tests were developed based on the old code, but not all of them
succeed with the old code.
As usual, the tests uncovered some interesting[tm] behaviour in
separate_vars() (see the XXX comments and tell me what the really
expected behaviour is ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Move the code that does the c -> a and d -> w replacement in denied_mask
and requested_mask so that it only runs for path and exec events, but not
for other events (like dbus and ptrace). The validate_log_mode() and
log_str_to_mode() calls are also moved.
Technically, this means moving code from parse_event() to the path
and exec sections in add_event_to_tree().
This also means aa-logprof no longer crashes if it hits a ptrace or
dbus event in the log.
The "if dmask:" and "if rmask:" checks are removed - if a path event
doesn't have these two, it is totally broken and worth a aa-logprof
crash ;-)
Also adjust the parse_event() tests to expect the "raw" mask instead of
a set.
Note: the 2.9 branch doesn't contain test-capability.py, therefore I
skipped this part of the patch for obvious reasons ;-)
This patch fixes
https://bugs.launchpad.net/apparmor/+bug/1426651 and
https://bugs.launchpad.net/apparmor/+bug/1243932
I manually tested that
- c and d log events are still converted to a and w
- aa-logprof handles exec events correctly
- ptrace events no longer crash aa-logprof
Note: add_event_to_tree() is not covered by tests.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk revision 3001
Change serialize_parse_profile_start() to use parse_profile_start()
instead of using duplicated code.
The behaviour is mostly kept, with the exception that the function is
more strict now and raises exceptions instead of ignoring errors.
In practise, this won't change anything because the profiles are parsed
with parse_profile() (which calls parse_profile_start()) - and that
already errors out.
The tests are updated to match the more strict behaviour.
The next step would be to drop serialize_parse_profile_start()
completely, but this isn't urgent and can/should be done when we have
test coverage for serialize_profile_from_old_profile() one day ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 3000
Fix is_skippable_dir() - the regex also matched things like
/etc/apparmor.d/dont_disable, while it should match on the full
directory name.
Also add some tests based on a real-world aa-logprof run (with "print (path)"
in is_skippable_dir()) and some additional "funny"[tm] dirs.
Needless to say that the tests
('dont_disable', False),
('/etc/apparmor.d/cache_foo', False),
will fail with the old is_skippable_dir().
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2999
Replace RE_PROFILE_START with RE_PROFILE_START_2 and adjust all
code sections that used RE_PROFILE_START_2.
The only real change is that test_get_flags_invalid_01 and
test_get_flags_invalid_02 now expect AppArmorException instead of
AppArmorBug.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk
to use write_header(), and making set_profile_flags
more strict.
Merge from trunk revisions 2996, 2997, and 2998.
Changes in set_profile_flags():
- rewrite set_profile_flags to use parse_profile_start_line() and
write_header().
- replace the silent failure for non-existing files with a proper
exception (using lazy programming - the check is done by removing the
"if os.path.isfile()" check, open_file_read then raises the
exception ;-)
- comment out regex_hat_flag and the code that was supposed to handle
hat flags, which were totally broken. We'll need another patch to fix
it, and we also need to decide if we want to do that because it
introduces a behaviour change (currently, aa-complain etc. don't
change hat flags).
The tests for set_profile_flags() are also updated:
- prepend a space to comments because write_header always adds a space
between '{' and the comment
- remove a test with superfluous quotes that are no longer kept
(that's
just a profile cleanup, so dropping that test is the easiest way)
- update test_set_flags_10 and test_set_flags_12 to use the correct
profile name
- enable the tests for invalid (empty) flags
- update the test for a non-existing file
this patch makes set_profile_flags more strict:
- raise AppArmorBug if newflags contains only whitespace
- raise AppArmorBug if the file doesn't contain the specified profile or
no profile at all
The tests are adjusted to expect AppArmorBug instead of a silent
failure. Also, some tests are added for profile=None, which means to
change the flags for all profiles in a file.
- test_set_flags_08 is now test_set_flags_invalid_04
- test_set_flags_invalid_03 is changed to only contain one reason for
a failure, not two ;-)
Finally implement attachment handling
This patch implements attachment handling - aa-logprof now works with
profiles that have an attachment defined, instead of ignoring audit.log
entries for those profiles.
Changes:
- parse_profile_start_line(): remove workaround that merged the
attachment into the profile name
- parse_profile_data(): store attachment when parsing a profile
- update test_parse_profile_start_03,
test_serialize_parse_profile_start_03,
test_set_flags_nochange_09 and some parse_profile_start_line() tests -
they now expect correct attachment handling
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2990
Rewrite parse_profile_start() in aa.py to a more readable version.
The behaviour remains unchanged (and is covered by tests).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Merge from trunk revision 2989
Also add AANamedRegexTest class that can be used to test a regex with
named match groups.
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2988
Change the write_header tests so that the 'profile_keyword' and
'header_comment' parameters can be (and are) tested:
- add a None for both to the existing tests
- add some tests that come with the profile keyword and/or a comment
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2987
- add support for prof_data['header_comment'] (comment after '{')
and prof_data['profile_keyword'] (to force the 'profile' keyword, even
if it isn't needed) to write_header().
(set_profile_flags() will be the only user of these two for now)
- fix a crash if depth is not an integer - for example,
len(' ')/2 # 3 spaces = 1.5
would cause a crash.
Also add a test for 1.5 and 1.3 spaces.
- rewrite the handling of flags to avoid we have to maintain two
different template lines.
- update the tests to set 'profile_keyword' and 'header_comment' to None.
This avoids big changes in the test code. I'll send another patch that
makes sure profile_keyword and header_comment are tested ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2986
Add the attachment to the parse_profile_start() and
serialize_parse_profile_start() return values, and adjust the functions
calling the *parse_profile_start() functions to save the attachment in
the "attachment" variable (which isn't used yet).
Also adjust the tests for the added return value.
(Sorry for not getting the resultset right from the beginning!)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2985
Also fix a little bug that added the profile keyword if the path needed
quotes (profile "/foo bar" - but "/foo bar" is enough). This was caused
by a regex that always matched on quoted paths (hint: "/ matches
^[^/] ;-)
Also add some tests with attachments and update the test for the bugfix
mentioned above.
Now the remaining part is to make sure that prof_data['attachment'] gets
set when parsing the profiles :-)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk commit 2983
Add various tests for set_profile_flags, and document various
interesting[tm] things I discovered while writing the tests (see
the inline comments for details).
Also adds a read_file() function to common_test.py.
The most interesting[tm] thing I found is:
regex_hat_flag = re.compile('^([a-z]*)\s+([A-Z]*)\s*(#.*)?$')
which matches various unexpected things - but not a hat :-/
(see mailinglist for all funny details)
Acked-by: Steve Beattie <steve@nxnw.org>
serialize_parse_profile_start() to use parse_profile_start_line();
update test-aa.py to match parse_profile_start() and
get_profile_flags() changes
Merge from trunk commits 2978, 2979, and 2982
Add the parse_profile_start_line() function to regex.py, which is
a wrapper for RE_PROFILE_START_2 and returns an array with named matches.
Also change some places in aa.py from using RE_PROFILE_START to
the parse_profile_start_line() function.
Notes: - until everything is migrated to the new function, I'll
keep the old
RE_PROFILE_START unchanged - that's the reason to add the new
regex as RE_PROFILE_START_2
- the patch changes only aa.py sections that are covered by tests
already (which means some users of RE_PROFILE_START are remaining)
- parse_profile_start_line() merges 'profile' and 'attachment' into
'profile' (aka the old, broken behaviour) until aa.py can handle
the attachment properly. The alternative would be to ignore
'attachment', which would be worse.
Convert serialize_parse_profile_start() to use
parse_profile_start_line(), and adjust a test to expect an AppArmorBug
instead of an AttributeError exception.
Also add two tests (they succeed with the old and the new code).
Note that these tests document interesting[tm] behaviour - I tend to
think that those cases should raise an exception, but I'm not sure about
this because serialize_profile_from_old_profile() is a good example for
interesting[tm] code :-/
I couldn't come up with a real-world test profile that would hit those
cases without erroring out aa-logprof earlier - maybe the (more
sane-looking) parse_profiles() / serialize_parse_profile_start()
protects us from hitting this interesting[tm] behaviour.
The previous patch slightly changed the behaviour of parse_profile_start()
and get_profile_flags() - they raise AppArmorBug instead of
AppArmorException when specifying a line that is not the start of a
profile and therefore doesn't match RE_PROFILE_START_2.
This patch updates test-aa.py to expect the correct exceptions, and adds
another test with quoted profile name to ensure that stripping the
quotes works as expected.
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2984
Also add loop support to test-aa.py.
BTW: In case you wonder - the need to replace unittest.TestCase with
AATest is intentional. It might look annoying, but it makes sure that
a test-*.py file doesn't contain a test class where tests = [...] is
ignored because it's still unittest.TestCase.
(Technically, setup_all_tests() will error out if a test class doesn't
contain tests = [...] - either explicit or via its parent AATest.)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2977
The test behaviour is the same with and without this patch - 166 tests
run, all successful.
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revisions 2976 and 2980
Add better support for looping over a tests[] array to common_test.py:
- class AATest - a base class we can use for all tests, and that will
probably get more features in the future (for example tempdir
handling)
- setup_all_tests() - a function that iterates over all classes in the
given file and calls setup_test_loops() for each of them
- setup_tests_loop() - a function that creates tests based on tests[]
in the given class. Those tests call the class' _run_test() method for
each test specified in tests[] (inspired by setup_regex_tests() ;-)
This means we can get rid of the manually maintained tests list in
test-regex_matches.py and just need to call setup_all_tests() once in
each file.
The patch also adds test-example.py, which is
- a demo of the code added to common_test.py
- a template file that we can copy for future test-*.py
Acked-by: Steve Beattie <steve@nxnw.org>
Thanks to the used data structure, write_net_rules() replaces bare
'network,' rules with the invalid 'network all,' when saving a profile.
This patch makes sure a correct 'network,' rule is written.
Also reset 'audit' to avoid all (remaining) rules get the audit flag
after writing an audit network rule.
Note: The first section of the function (that claims to be responsible
for bare 'network,' rules) is probably never hit - but I'm not too keen
to remove it and try it out ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
When parsing a profile with named exec rules, the exec target included
the arrow. This resulted in two arrows when writing the profile (and one
more each time the profile was updated).
Fix this by using the match group that only contains the exec target
without the arrow in parse_profile_data() and
serialize_profile_from_old_profile().
References: https://bugs.launchpad.net/apparmor/+bug/1437901
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Merge from trunk revision 3004
When evince opens a dvi file, it updates the user fonts using
texlive commands in /usr/share/texlive/texmf-dist/web2c/ (or possibly
/usr/share/texlive/texmf/web2c/ in older releases). This patch adjusts
the sanitized_helper profile to allow these tools to run.
Bug: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1010909
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-By: Jamie Strandboge <jamie@canonical.com>
write_net_rules() doesn't add a space after 'audit' in two of three
cases, leading to invalid network rules.
This patch adds the missing spaces.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9
write_net_rules() creates invalid rules for network rules with one
parameter (for example "network bluetooth").
Add a trailing comma to create valid rules.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9.
aa-logprof doesn't ask anything for
type=AVC msg=audit(1427633461.202:281): apparmor="DENIED" operation="chmod" profile="/usr/lib64/firefox/plugin-container" name="/home/cb/.config/ibus/bus/" pid=7779 comm="plugin-containe" requested_mask="w" denied_mask="w" fsuid=1000 ouid=1000
This patch fixes this by adding 'chmod' to the list of file operation
types in logparser.py.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
for both trunk and 2.9.
if 3/2 == 1:
print("python2 inside")
Add "from __future__ import division" so that python2 returns the
correct result (if needed, as float)
On related news: At least python3 knows how to calculate correctly.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
add --include-templates-dir and --include-policy-groups-dir options to easyprof
to support framework policy on Snappy for Ubuntu Core
Signed-off-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Merge from trunk revision 2975
The following patch addresses two issues on older releases:
1) In trunk commit 2911, the line 'undefine VERBOSE' was added to
parser/tst/Makefile so that the equality tests would not generate
verbose output when $VERBOSE != 1. Unfortunately, the 'undefine'
keyword was not introduced in GNU Make until version 3.82. On
distro releases like Ubuntu 12.04 LTS that include versions of Make
older than that, make check and make clean abort when VERBOSE is
not set to 1. The patch fixes that by setting VERBOSE to a zero
length string if does not already equal 1.
2) In trunk commit 2923, a workaround for systemd as init was added
to the pivot_root regression test. The workaround included a
call to ps(1) to determine if systemd is pid 1. Unfortunately,
in older versions of the procps package (such as the version in
Ubuntu 12.04 LTS), 'ps -hp1' emits the warning
Warning: bad ps syntax, perhaps a bogus '-'? See http://procps.sf.net/faq.html
The patch below converts the ps call to 'ps hp1' which does not
generate the warning.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk revision 2974
Patch from Cameron Norman <camerontnorman@gmail.com> based on a patch
from Christian Boltz <apparmor@cboltz.de>.
This patch allows /var/lib/misc/dnsmasq.*.leases rw and
/{,var/}run/lxc/dnsmasq.pid rw for LXC networking setup.
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk commits 2909, 2910, 2911, and 2912
BugLink: http://bugs.launchpad.net/bugs/1433829
The apparmor_parser fails to compile deny rules with only link
permissions.
Eg.
deny /f l,
deny l /f,
deny link /f -> /d,
Will all fail to compile with the following assert
apparmor_parser: aare_rules.cc:99: Node* convert_file_perms(int, uint32_t, uint32_t, bool): Assertion `perms != 0' failed.
NOTE: this is a minimal patch a bigger patch that cleans-up and separates
and reorganizes file, link, exec, and change_profile rules is needed
parser: Expand Equality tests
This adds several new equality tests and turned up a couple of more
bugs
https://launchpad.net/bugs/1433829https://launchpad.net/bugs/1434018
- add link/link subset tests
- add pix, Pix, cix, Cix, pux, Pux, cux, Cux and specified profile
transitions (/f px -> b ...)
- test equality of leading and trailing permission file rules
ie. /foo rw, == rw /foo,
- test that specific x match overrides generic x rule. ie.
/** ix, /foo px, is different than /** ix, /foo ix,
- test that deny removes permission
/f[abc] r, deny /fb r, is differnt than /f[abc] r,
In addition to adding the new tests, it changes the output of the
equality tests, so that if the $verbose variable is not set successful
tests only output a period, with failed tests outputing the full
info. If verbose is set the full test info is output as before.
It also does:
- make the verbose output of equality.sh honor whether or not
the environment variable VERBOSE is set
- thereby making the output verbose when 'make check V=1' or 'make
check VERBOSE=1' is given from within the parser/ directory. This
will make distribution packagers happy when diagnosing build
failures caused by test failures.
- if verbose output is not emitted and the tests were successful, emit
a newline before printing PASS.
- verify audit and audit allow is equal
- verify audit differs from deny and audit deny
- verify deny differs from audit deny
- make the verbose text a little more useful for some cases
- correct overlap exec tests to substitute in looped perms
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Darix' guess is that this is needed by libpq because he uses a postgresql
database with dovecot and has ssl enabled in postgresql.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9
Merge from trunk commit revision 2907
This patch fixes the equality test script and the valgrind wrapper
script to make the parser under test use the features.all features file
from the features_files/ subdirectory. Otherwise, the equality tests
will fail on systems where the not all of the current language features
are supported. The equality fix does so in a way to make the script work
correctly regardless of the directory it is run from.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk commit 2906
The fix to prevent the compiler from SEGV'ing when dumping network
rules in commit 2888 introduced the following compiler warning:
network.c: In function ‘const char* net_find_af_name(unsigned int)’:
network.c:331:16: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
for (i = 0; i < sizeof(network_mappings) / sizeof(*network_mappings); i++) {
The problem is that the counter i is an int, but sizeof returns size_t
which is unsigned. The following patch fixes the issue by converting the
type of i to size_t.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Previously, we only had the ability to test that binary policy files
were equal. This patch allows for the testing of binary policy files
that are not equal.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
cherry-pick: -r2901
This fixes the incorrect compilation of audit modifiers for exec and
pivot_root as detailed in
https://launchpad.net/bugs/1431717https://launchpad.net/bugs/1432045
The permission accumulation routine on the backend was incorrectly setting
the audit mask based off of the exec type bits (info about the exec) and
not the actual exec permission.
This bug could have also caused permissions issues around overlapping exec
generic and exact match exec rules, except the encoding of EXEC_MODIFIERS
ensured that the
exact_match_allow & AA_USER/OTHER_EXEC_TYPE
test would never fail for a permission accumulation with the exec permission
set.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
serialize_profile_from_old_profile() in aa.py, as a preparation to add
tests and then switch to the upcoming RE_PROFILE_START wrapper function.
Besides moving the code, I replaced write_prof_data[profile][hat]['profile']
and write_prof_data[profile][hat]['external'] with function parameters
to avoid that I have to pass around the full write_prof_data.
Note: The "lineno" parameter is technically superfluous - I kept it to
have the parameters as close to parse_profile_start() as possible and
hope that I can merge those functions later (when we have test coverage).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
and change the code to use them
Also add a comment to act() that it's only used by aa-cleanprof.
Note: The new functions add the --base parameter to the apparmor_parser
calls, which also means the disable directory inside the given profile
dir (and not always /etc/apparmor.d/disable) is now honored.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
logparser.py / add_event_to_tree() has 5 places to handle 'path' events.
This patch merges most if conditions to reduce that to 2 places.
It also makes the matching a bit more strict - instead of using 'in',
'xattr' has to be an exact match and 'file_' is matched with startswith().
Also, 'getattr' is added to the list of file events.
Acked-by: Steve Beattie <steve@nxnw.org> (also for 2.9)
---------- trunk only, unclear for 2.9 --------------
Without it, aa-disable
- didn't error out when hitting a broken profile directory
- didn't find a profile if it doesn't use the default naming scheme
(for example /bin/true profile hiding in bin.false)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk revision 2893
As mir has come into use in Ubuntu touch and is available for testing on
Ubuntu desktop, confined apps need access to a few mir specific things.
This patch adds a mir abstraction.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Merge from trunk revision 2889
Parts of the regression tests that use the do_open() inline function
from changehat.h fail to build under gcc-5 like so:
cc -g -O0 -Wall -Wstrict-prototypes changeprofile.c -lapparmor -o changeprofile /tmp/ccT6GE6k.o: In function `main':
/home/ubuntu/bzr/apparmor/tests/regression/apparmor/changeprofile.c:43: undefined reference to `do_open'
collect2: error: ld returned 1 exit status
<builtin>: recipe for target 'changeprofile' failed
This patch converts the do_open function declaration to be static
inline, which apparently keeps gcc-5 from getting confused.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk revision 2892
aa-status was crashing when parsing through /proc/mounts looking
to see if and where the securityfs synthetic file system is mounted
if there was a mount point that contained characters outside of the
charset in use in the environment of aa-status. This patch fixes the
issue by converting the read of /proc/mounts into a binary read and
then uses decode on the elements.
Patch by Alain BENEDETTI.
Acked-by: Steve Beattie <steve@nxnw.org>
The error path was being taken when openat() return 0 but openat()
returns -1 on error.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
As a follow-up to the logparser.py change that converts disconnected
path events to an error, add a testcase to test-logparser.py.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
remove unused net_find_af_val function, and network_families array
Merge from trunk commit 2888.
net_find_af_name: do not assume that address families are consecutive
The network_families array is automatically built from AF_NAMES, which is
extracted from the defines in <bits/socket.h>. The code assumes that
network_families is indexed by the AF defines. However, since the
defines are sparse, and the gaps in the array are not packed with
zeroes, the array is shorter than expected, and the indexing is wrong.
When this function was written, the network families that were
covered might well have been consecutive, but this is no longer true:
there's a gap between AF_LLC (26) and AF_CAN (29).
This assumption caused a crash in our testing while parsing the rule
"network raw".
Remove unused net_find_af_val function, and network_families array
Like net_find_af_name, this assumed that AF_* values were consecutive.
Patches from Philip Withnall and Simon McVittie.
The upcoming function parse_profile_start() (which is a wrapper around
the updated RE_PROFILE_START, and will live in regex.py) needs
strip_profile(), but importing it from aa.py fails with an import loop.
Therefore this patch moves strip_quotes() from aa.py to regex.py and
re-imports it into aa.py.
As a bonus, the patch also adds some tests for strip_quotes() ;-)
Also add TestStripQuotes to the test_suite list because it won't run
otherwise.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9
This means that aa-logprof will ignore the event instead of crashing with
AppArmorException: 'Unexpected rank input: var/run/nscd/passwd'
Note that I made the check as specific as possible to be sure it doesn't
hide other events.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=918787
Acked-by: Steve Beattie <steve@nxnw.org> for 2.9
(This is a backport of trunk r2877, but without the test-capability.py
adjustment because that file doesn't exist in 2.9)
Move the code for parsing the profile start ("/foo {") from aa.py
parse_profile_data() to a separate function parse_profile_start().
Most of the changes are just moving around code, with some small
exceptions:
- instead of handing over profile_data to parse_profile_start() to
modify it, it sets two variables (pps_set_profile and
pps_set_hat_external) as part of its return value, which are then
used in parse_profile_data() to set the flags in profile_data.
- existing_profiles[profile] = file is executed later, which means
it used the strip_quotes() version of profile now
- whitespace / tab level changes
The patch also adds some tests for the parse_profile_start() function.
Acked-by: Steve Beattie <steve@nxnw.org> for 2.9 as well.
Also adds a check to get_profile_flags() to catch an invalid syntax:
/foo ( ) {
was accepted by get_profile_flags, while
/foo () {
failed.
When testing with the parser, both result in a syntax error, therefore
the patch makes sure it also fails in get_profile_flags().
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Seth pointed out that dirat_for_each() didn't correctly handle the
return value from readdir_r(). On error, it directly returns a positive
errno value. This would have resulted in that positive errno value being
returned, with an undefined errno value set, from dirat_for_each().
However, the dirat_for_each() documentation states that -1 is returned,
with errno set, on error.
This patch results in readdir_r()'s return value being handled
appropriately. In addition, it ensures that 0 is always returned on
success.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2875
From: Felix Geyer <debfx@ubuntu.com>
At least Debian/Ubuntu started shipping some aspell files in
/usr/share/aspell/.
For example:
/usr/share/aspell/iso-8859-1.cmap
/usr/share/aspell/iso-8859-1.cset
The abstraction should allow read access to these files.
Acked-by: Steve Beattie <steve@nxnw.org>
Remove the check if the disable directory exists. If it's really
missing, it will be auto-created by create_symlink(), so we
automagically fix things instead of annoying the user with an
error message ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Merge from trunk revision 2871
Don't pass an ostream reference into another ostream via <<.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
libapparmor _aa_is_blacklisted() - some extensions were missing in the
python code.
Also make the code more readable and add some testcases.
Notes:
- the original code additionally ignored *.swp. I didn't include that -
*.swp looks like vim swap files which are also dot files
- the python code ignores README files, but the C code doesn't
(do we need to add README in the C code?)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for 2.9 and trunk
Acked-by: Steve Beattie <steve@nxnw.org>
journal socket. On Debian and Ubuntu systems, /dev/log is a symlink to
/run/systemd/journal/dev-log, so this access is now required in the base
abstraction to maintain current behavior.
Bug: https://bugs.launchpad.net/apparmor/+bug/1413232
Acked-By: Jamie Strandboge <jamie@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
argument. Also fixed /usr/lib -> /usr/{lib,lib64} to get libvirt
leasehelper script to run even on x86_64.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=911001
Patch by "Cédric Bosdonnat" <cbosdonnat@suse.com>
Note: the original patch used {lib,lib64} - I changed it to lib{,64} to
match the style we typically use.
Acked-by: John Johansen <john.johansen@canonical.com>
(backport of trunk r2841)
Add #include <abstractions/dovecot-common> to the usr.sbin.dovecot
profile. Effectively this adds "deny capability block_suspend," which
is the only missing part from
https://bugs.launchpad.net/apparmor/+bug/1296667/
Also remove "capability setgid," (covered by
abstractions/dovecot-common) and "@{PROC}/filesystems r," (part of
abstractions/base).
Acked-by: John Johansen <john.johansen@canonical.com>
(backport of trunk r2840)
This patch restricts MS_REC to only be used while defining the MS_RBIND,
MS_RUNBINDABLE, MS_RPRIVATE, MS_RSLAVE, and MS_RSHARED macros.
The MS_R* macros are simply an OR of the corresponding non-recursive
macro and MS_REC:
#define MS_RBIND (MS_BIND | MS_REC)
Previously, a shortcut was taken when needing to specify the
non-recursive and recursive macros:
(MS_BIND | MS_UNBINDABLE | MS_PRIVATE | MS_SLAVE | MS_SHARED | MS_REC)
By using MS_REC above, it is not immediately clear that
MS_R{BIND,UNBINDABLE,PRIVATE,SLAVE,SHARED} are also included.
By restricting the use of MS_REC, this patch improves readability by
forcing the use of the MS_R{BIND,UNBINDABLE,PRIVATE,SLAVE,SHARED} macros
instead of relying on the MS_REC shortcut.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser correctly rejects mount make-* options (make-shared,
make-slave, make-private, make-unbindable) when a device is specified
(the source argument of mount(2)). However, it was not rejecting the
recursive make-* options (make-rshared, make-rslave, make-rprivate,
make-runbindable) when a device was specified.
This patch adds the MS_REC bit, which is used to indicate a recursive
option, to the MS_CMDS macro. Without this change, the recursive options
are treated as normal mount options.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The parser should not indicate success when mount rules contain
unknown mount options:
$ echo "/t { mount options=(XXX) -> **, }" | apparmor_parser -qQ
$ echo $?
0
This patch modifies the parser so that it prints an error message and
exits with 1:
$ echo "/t { mount options=(XXX) -> **, }" | apparmor_parser -qQ
unsupported mount options
$ echo $?
1
Bug: https://bugs.launchpad.net/bugs/1401621
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
There are a number of differences between what the apparmor.d(5) man
page lists as valid AppArmor mount rule options and what apparmor_parser
looks for when parsing mount rules. There are also typos in the man page
and parser around mount options. Here's the breakdown of problems and
fixes made in this patch:
* The apparmor.d(5) man page improperly documented a "nodirsync"
option.
- That mount option does not exist and the parser did not honor it.
Remove the mention from the apparmor.d(5) man page.
* The loud option was typoed as "load" in both the man page and parser
- There's no sense in preserving backwards compatibility. "load" is
simply wrong and should not be honored. The man page and parser are
updated to only use "loud".
* The rbind option wasn't listed in the man page.
- Add rbind to the man page. No change needed for the parser.
* The documented unbindable, private, slave, and shared options were
not correctly parsed. The parser expected
make-{unbindable,private,slave,shared}.
- The parser is updated to accept both the documented
{unbindable,private,slave,shared} options and their variants
prefixed with "make-". The man page will not document the "make-"
variants.
* The recursive {runbindable,rprivate,rslave,rshared} options were not
documented and were only recognized by the parser if they were
prefixed with "make-".
- The man page is updated to document the option strings that are not
prefixed with "make-". The parser still accepts the "make-"
variants.
* The man page documented a "rec" option but the parser didn't honor
it. The MS_REC macro is used by the mount utility to be bitwise OR'ed
with MS_{UNBINDABLE,PRIVATE,SLAVE,SHARED} to indicate the
corresponding recursive mount options.
- This is not an option that should be exposed in the AppArmor policy
since we already allow have the
{runbindable,rprivate,rslave,rshared} options.
* The man page typoed the {no,}relatime options as {no,}relative.
- The man page is updated to document the correct option strings. The
parser requires no change.
Bug: https://bugs.launchpad.net/bugs/1401619
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
When using recursive_print for debugging, RawRules objects weren't
reporting detailed information. This patch fixes that, as well as fixing
some indenting issues in the output.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Those *.spec{,.in} files were not updated for years (last change
2006/2007) and don't fit the current "one tarball for everything" model.
Acked-by: Steve Beattie <steve@nxnw.org>
The check-logprof target was not updated to use the python tools, when
they were merged in. This patch fixes the issue.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
LOG_MODE_RE (used in validate_log_mode() in aamode.py) just checked if
the given parameter contains one of the possible matches. This resulted
in "invalid" being a valid log mode (from audit.log requested_mask or
denied_mask) because it contains 'a', which is a valid file mode.
This patch wraps the regex into ^(...)+$ to make sure the full
string contains only allowed file modes.
The patch also adds some tests for validate_log_mode().
Acked-by: Steve Beattie <steve@nxnw.org>
aa.py uses profile_data[profile][hat]['change_profile'] at various
places. However, there are also two places that use 'changes_profile'
(note the additional 's'), which should also be 'change_profile'.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch converts a ValueError raised when parsing of a permission
mode fails into an AppArmorBug with better diagnostic information, and
adds a test case to confirm that the exception is raised.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Also change check_for_apparmor() to allow easier testing by optionally
specifying alternative locations for /proc/filesystems and /proc/mounts
as parameter.
Note that the code in check_for_apparmor() differs from what the comment
says - valid_path() only does syntax checks, but doesn't check if the
directory exists. I added a comment saying exactly that.
Acked-by: Steve Beattie <steve@nxnw.org>
- replace MODE_MAP_RE regex with MODE_MAP_SET set
- change sub_str_to_mode() to use MODE_MAP_SET set instead of MODE_MAP_RE
- change split_log_mode to use split() instead of a regex
Patch by Peter Maloney <peter.maloney@brockmann-consult.de>
Acked-by: Christian Boltz <apparmor@cboltz.de>
split_log_mode() change also
Acked-by: Seth Arnold <seth.arnold@canonical.com>
interpreters, it used
aa[profile][hat]['path'][interpreter_path]['mode']
instead of
aa[profile][hat]['allow']['path'][interpreter_path]['mode']
The ['allow'] part was missing.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch pulls out all the common processing for writing out
each of the prior segments that need to be written before writing
the current segment into a function called 'write_prior_segments',
reducing a bunch of ugly duplication.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The assignment for setting segments['include'] = True was wrong,
it occured inside the 'if not segments['include'] and True in
segments.values():' block, whereas it needed to always get set outside
of that if test.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch
- fixes a check that used if "aa[profile][hat][incname]:" instead of
"if aa[profile][hat]['include'].get(incname, False):" ("['include']"
was missing) which means the performance shortcut was never hit
- avoids auto-created empty and superfluous hashers in
aa[profile][hat]['allow']['path'] and
include[incfile][incfile][allow]['path']
- adds the filename to the "Can't find system log" exception
Patch by Peter Maloney <peter.maloney@brockmann-consult.de>
Changes compared to the original patch:
- change back quoting in the exception message to '...'
Acked-By: Christian Boltz <apparmor@cboltz.de>
Modify the build instructions and project requirements to reflect the
utils rewrite from Perl to Python.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch adds the --enable-man-pages option, defaulting to "yes", to
libapparmor's configure stage so that libapparmor can be built on
systems lacking Perl.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Fail the configure stage if podchecker is not available since man page
generation always happens.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The configure script fails if pod2man isn't available since PROG_POD2MAN
is called unconditionally so it is safe to split man page generation out
of the --with-perl configure option.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
present as a function (which it is not in Python3, even though it was
under an if else python version check).
The following patch:
- checks the __builtins__ module for existence of raw_input and sets
it up for Python3
Acked-by: Steve Beattie <steve@nxnw.org>
The diff displayed by (V)iew changes in aa-logprof lacks leading
whitespace because it bases the whitespace on the indention level of the
closing "}".
Besides that, it uses different values for the indention level for newly
added rule types than for existing rule types. (For example, if a
profile already had a network rule, added network rules will get a
different indention level.) However you won't notice this because
currently it doesn't indent the rules at all ;-)
This patch fixes serialize_profile_from_old_profile() in aa.py so that
it always uses the correct indention level.
Also clean up and simplify how the profile is written in the end (when
matching RE_PROFILE_END) - we already have "write_methods", so we can
just re-use it instead of "manually" calling one write_* function after
the other. Unfortunately dicts don't keep their original order,
therefore I had to introduce "default_write_order".
Finally, add some missing rule types to "segments" to avoid key errors.
Acked-by: Steve Beattie <steve@nxnw.org>
When aa-logprof asks for adding capability rules, it also offers the
Audi(t) option. Unfortunately, this option does nothing ;-)
This patch fixes ask_the_question() so that it really ;-) allows to
switch the audit flag on and off. It also initializes the "audit"
variable to make sure the next capability doesn't inherit the audit flag
used for the previous capability.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch for recursive_print() in common.py fixes printing dicts with
py3. It also replaced the tabs() lambda function with a plain string,
and the brace() lambda function with a simple formatstring to make the
code easier to understand.
Also add support for nested lists - for the start and end of each list,
print a [ and ]. Without that, you get a long list of items without an
indicator if/when a new parent list starts.
Acked-by: Steve Beattie <steve@nxnw.org>
When aa-logprof asks for a capability, you'll see something like
WARN: unknown capability: CAP_block_suspend
The reason for the warning and "Severity: unknown" is that severity.db
contains the capability names in uppercase, but ask_the_question() calls
sev_db.rank with the capability in lowercase.
This patch converts the "CAP_$capability" string to uppercase before
doing the lookup.
Acked-by: Steve Beattie <steve@nxnw.org>
Also add a testcase (written by Steve Beattie) to ensure this stays fixed.
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
utils/Makefile contains a grep -v "undefined name _". Some manual
testing shows that pyflakes doesn't print any warning about "_", so
this grep is superfluous.
Removing the grep also means we don't need a tempfile for the pyflakes
output anymore, which simplifies the pyflakes call a lot.
Acked-by: Steve Beattie <steve@nxnw.org>
The recent re-work of the severity.db tests were not verified to
pyflakes clean. All but one of pyflakes co are of marginal impact
(assigning to a variable that isn't later referenced); however, one
legitimate issue it detected is that I inadvertently created two test
cases with the same method name, so only one test case would actually
be used.
The following patch fixes the issues.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
These regression tests are for an Ubuntu-specific bug. However, they
should benefit the upstream project, as well. Ubuntu took an incomplete
version of a patch, which introduced the bug. The version of that patch
that landed upstream did not contain the bug.
The bug was in policy compilation of certain combinations of rule types,
conditionals, and conditional values. The easiest such combination to
test is a rule such as:
ptrace peer=ABC,
Buggy parsers will generate binary policy that causes the kernel to deny
a ptrace of a process confined by ABC, despite the presence of the above
rule.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
This commit renames the unit test script for the severity db so that it
will be included in the 'make check' and 'make coverage*' targets.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch fixes Severity.__init__() when it is not given an argument to
raise an AppArmor exception rather than returning a Severity object in
an incompletely initialized state. It also adjusts a test case covering
this situation.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This patch is a re-work of the severity_test.py tests, to break them
up into individual unit tests, and to add coverage for detecting
an invalid severity database (it does reduce the coverage for walking
profiles to find variable declarations, but that should be pulled out of
the severity handling code anyway).
Note that the last test case will fail, because even though the code
path in Severity.__init__() looks like it will return None if no path
is given, a Severity object in a half-state of initialization will
actually be returned.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
for r2769, which only checked for "exists")
Also allow everything except directories as logfile argument in
aa-genprof.
Acked-by: Steve Beattie <steve@nxnw.org>
This patch adds support for generating test coverage information for the
python utils.
To view a text based report, in the test subdirectory do:
make coverage-report
To generate detailed html reports, do:
make coverage-html
And then point your web browser at
$(YOUR_CURRENT_WORKING_TREE)/utils/test/htmlcov/index.html .
An alternate output location can be specified by setting the
COVERAGE_OUT variable, e.g.
make coverage-html COVERAGE_OUT=/tmp/coverage/
(the output directory does not need to exist beforehand.)
To generate only the coverage data, do:
make coverage
or
make .coverage
(The coverage data generated by python is stored in the .coverage
file.) This essentially runs make check, using a single python
interpreter, and records which lines and branches of the python code
were exercised.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: John Johansen <john.johansen@canonical.com>
This patch moves the declaration of phony and quieted make targets
to a single section, to avoid repeated lines. It's not so useful
for just two targets, but future patches will add more targets with
similar attributes.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Both valgrind and strace report the parser doing
close(-1) = -1 EBADF (Bad file descriptor)
This happens the skip kernel load argument is specified in combination
with any of --add, --replace, or --remove arguments (the default
is --add if no other option is specified).
This happens when the parser is not processing profiles but not
writing them out (eg. no kernel load, dump to stdout, file ...)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
netrules_access_check() in aa.py checks if
type(netrules['rule'][family]) == dict
however this check always returns false (at least with py3, I didn't
test with py2).
This broken type check is the reason for
https://bugs.launchpad.net/apparmor/+bug/1380368
aa-logprof doesn't propose abstractions for network rules
and
https://bugs.launchpad.net/apparmor/+bug/1380367
aa-logprof asks for already existing network rules
which are both fixed with this patch.
The type check is needed because netrules['rule'][family] can be
boolean True (for rules like "network inet,") - see line 2994.
The sock_type in .... .keys() check is there to ensure the hasher doesn't
automagically add an empty sub-dict, which caused the regression in the
first version of my patch.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
match_net_includes() in aa.py gets "nettype" as parameter, but then uses
"type" when calling valid_include(). "type" is a global variable, but not
what we want to use here ;-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Update the apparmor_parser documentation for the new ability to load
profiles from a specified directory.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Bug: https://bugs.launchpad.net/bugs/1366572
In the move of the apparmor.vim manpage source from the parser
directory to utils/vim/, the creation of the html version of the
manpage was lost. This patch fixes that, as well as fixes the vim
Makefile to use the common/Make.rules _clean target (which clears the
pod2htm*.tmp files created by pod2html as well).
It also fixes a bug in common/Make.rules where the _clean target
would report an error when used in a directory where the Makefile
doesn't set the NAME variable, such as utils/vim/.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Christian Boltz <apparmor@cboltz.de>
- change --help for files - "Profile(s) to merge" instead of "base profile"
- display the profile to save when asking to save it
- disable searching for existing network rules in abstractions because
it crashes. This doesn't hurt too much, see
https://bugs.launchpad.net/apparmor/+bug/1382241
Acked-by: Steve Beattie <steve@nxnw.org>
- remove some debug output (which Kshitij intentionally kept in the
draft patch)
- add a UI_Info to display which profile will be merged
- disable the mergeprofiles.clear_common() call because it crashes
(https://bugs.launchpad.net/apparmor/+bug/1382236)
- disable (M)ore (CMD_OTHER) because it crashes
- make (F)inish work everywhere
- change the help text so that it doesn't mention 3-way-merge until we
implement it
The python utils didn't know about the hat keyword, for example
hat foo {
This patch changes RE_PROFILE_HAT_DEF to add support for the hat keyword.
Note that this patch only changes profile reading - when saving a
profile, the ^foo syntax will always be used.
While on it, also convert the regex to named matches - the result
numbering changed anyway ;-)
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This is the rebased version of the patch by
Kshitij Gupta <kgupta8592@gmail.com>
(mostly) original patch description:
Changes to facilitate 2-way merge (maybe also 3-way) of multiple
profiles as discussed on IRC
This patch
- moves reset method to reset_aa function
- modifies message displayed to user
- allows processing of multiple files in 2-way merge
- disables 3-way merge till new syntax has been decided
The changes reflect the approach of providing arbitrary number of
files using wildcards or explicitly.
The changes map the profiles in the given files to their respective
files in the local directory specified using -d. Then the merges take
place profile-wise.
Acked-by: Steve Beattie <steve@nxnw.org>.
When adding inet rules in aa-logprof, it crashes with
IndexError: list index out of range
The reason is that it doesn't display the options if only the raw rule
is available (aka "no abstraction").
This patch checks if options[] is set and otherwise sets selection to
the raw rule.
Acked-by: Steve Beattie <steve@nxnw.org>
After several hours of debugging on IRC and flooding paste.opensuse.org ;-)
I finally found the reason - reading(!) from log_dict with a wrong
key caused the creation of that strange mode in log_dict.keys().
While finding that bug was very hard, the fix is easy - just replace
"profile" with "aamode". (That probably makes one char per hour of
debugging...)
To improve that ratio, also add a warning to common.py so that this
interesting[tm] behaviour of hasher() is at least documented.
Acked-by: Steve Beattie <steve@nxnw.org>
...
File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/severity.py", line 147, in handle_variable_rank
variable = regex_variable.search(resource).groups()[0]
AttributeError: 'NoneType' object has no attribute 'groups'
handle_variable_rank() checked with if '@' in resource:
and if it finds it, expects it can match a variable, which means @{.....}
If a filename contains a @ this fails.
The patch fixes the if condition so that it does a regex match.
It also adds two testcases for filenames containing @ to make sure they
don't cause a crash and result in the exptected severity rank.
Acked-by: Steve Beattie <steve@nxnw.org>
aa-cleanprof (and others?) duplicate quotes in file rules.
If a profile contains
"/bin/foo bar" mrix,
and I run aa-cleanprof on it several times, I end up with
""""""/bin/foo bar"""""" mrix,
This patch calls strip_quotes on the pathname.
(If needed, the quotes are re-added when writing the profile - tested
with aa-cleanprof.)
References: https://bugs.launchpad.net/apparmor/+bug/1328707
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
This patch changes open_file_read() and open_file_write() to use
errors='surrogateescape' (with fallback to 'replace' for py2).
This avoids a crash when reading a logfile with special characters that
are not utf8-encoded (for example a latin1 "ö"), and also avoids crashes
at several other places we don't know yet ;-)
The patch also changes open_file_read() and open_file_write() to wrapper
functions, and moves the "real" code to the new open_file_anymode()
function.
Also, I removed the try/except - it's superfluous because it throws the
exception without any modifications.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
#=Description basic rules to test the "M" mount option
#=EXRESULT PASS
/usr/bin/foo {
mount options=M /a -> /1,
mount options=(M) /b -> /2,
mount options=(rw,M) /c -> /3,
mount options in (M) /d -> /4,
mount options in (ro,M) /e -> /5,
}
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
