libapparmor performs a test for the new stacking interface, however
how it does this test is problematic as it requires all confined
tasks to be given read access to the task introspection interface.
This results in tasks needing to be given read access to the interface
even if they don't need it. Making it possible for tasks to discover
their confinement even if they are not supposed to be able to.
Instead change the check to using state on the parent directory.
This will generate a getattr request instead of read and make it
on the directory instead of on any interface file that could be
used to obtain information.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen john.johansen@canonical.com
Acked-by: Timeout
(cherry-picked from commit 29215b4784)
Signed-off-by: John Johansen john.johansen@canonical.com
Adjust the interface check and fallback. Unfortunately there is no
solution that will fix all failure cases. Instead try to minimize
the failure cases and bias towards failures that don't cause a
regression under an old parser/policy.
Note: In cases where we absolutely know the interface should not
be accessed fail those accesses imediately instead of relying
on what ever LSM active to handle it.
While we are at it document the interfaces and failure cases.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 35e58273e6)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The parameter that is landing upstream in "available" not
"private_enabled".
Also set the correct variable, as previously we were not.
Note: that skipping checking available for the private apparmor
proc interfaces is okay, as the dedicated apparmor interfaces will
fail correctly if available is False.
This just gives a clear way for userspace to query this info without
having to resort to error codes that access to the private interfaces
would return.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 3fb4c4b876)
Signed-off-by: John Johansen <john.johansen@canonical.com>
libapparmor performs a test for the new stacking interface, however
how it does this test is problematic as it requires all confined
tasks to be given read access to the task introspection interface.
This results in tasks needing to be given read access to the interface
even if they don't need it. Making it possible for tasks to discover
their confinement even if they are not supposed to be able to.
Instead change the check to using stat on the parent directory.
This will generate a getattr request instead of read and make it
on the directory instead of on any interface file that could be
used to obtain information.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/150
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/713
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 6c4ed2af8d)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently for directory includes the directory timestamp is ignored.
This is wrong as operations like removing a file from the dir won't
be considered in the timestamp check.
Fix this by updating the timestamp check to include the included
directories timestamp.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/760
Signed-off-by: John Johansen <john@jjmx.net>
Acked-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 3d1232e640)
... and include it in abstractions/base.
This is a backport of
https://gitlab.com/apparmor/apparmor/-/merge_requests/772 which only
adds the crypto abstraction, but doesn't clean up the rules moved from
other abstractions into crypto.
Details:
* @{etc_ro}/gcrypt/random.conf r, (new rule) is possibly needed for all
programs that use libgcrypt.
* @{PROC}/sys/crypto/* r, (from base)
* crypto-policies (from ssl_certs)
* @{PROC}/sys/crypto/fips_enabled (from openssl)
MR 722 contains individual commits with more details.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/773
Acked-by: John Johansen <john.johansen@canonical.com>
... and include it in abstractions/base.
This is a backport of
https://gitlab.com/apparmor/apparmor/-/merge_requests/772 which only
adds the crypto abstraction, but doesn't clean up the rules moved from
other abstractions into crypto.
Details:
* @{etc_ro}/gcrypt/random.conf r, (new rule) is possibly needed for all
programs that use libgcrypt.
* @{PROC}/sys/crypto/* r, (from base)
* crypto-policies (from ssl_certs)
* @{PROC}/sys/crypto/fips_enabled (from openssl)
MR 722 contains individual commits with more details.
If an include file includes itsself (for example if local/foo has
'#include <local/foo>'), print a warning instead of calling
load_include() again and again.
This fixes a crash when hitting such a case:
RecursionError: maximum recursion depth exceeded while calling a Python object
Fixes: https://bugzilla.suse.com/show_bug.cgi?id=1184779 for the tools.
The parser will also need a fix.
This is the 3.0 version of 30323a2ded /
https://gitlab.com/apparmor/apparmor/-/merge_requests/742
Currently the wutmp abstraction has the following rules:
/var/log/lastlog rwk,
/var/log/wtmp wk,
@{run}/utmp rwk,
According to what I see in my apparmor profiles, just a few apps want
to interact with the files listed above, especially with the
/var/log/wtmp . But when the apps do this, they sometimes want the
read access to this file. An example could be the last command. Is
there any reason for not having the r in the rule? The second thing
is the file /var/log/btmp (which isn't included in the
abstracion). Whenever I see an app, which wants to access the
/var/log/wtmp file, it also tries to interact with the /var/log/btmp
file, for instance lightdm/sddm or su . Most of the time they need
just wk permissions, but sometimes apps need also r on this file, an
example could be the lastb command, which is just a link to last.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/152
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/724
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit d4e0a94511)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Rule downgrades are used to provide some confinement when a feature
is only partially supported by the kernel.
Eg. On a kernel that doesn't support fine grained af_unix mediation
but does support network mediation.
unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
network unix type=stream,
Which while more permissive still provides some mediation while
allowing the appication to still function. However making the rule
a deny rule result in tightening the profile.
Eg.
deny unix (connect, receive, send)
type=stream
peer=(addr="@/tmp/.ICE-unix/[0-9]*"),
will be downgraded to
deny network unix type=stream,
and that deny rule will take priority over any allow rule. Which means
that if the profile also had unix allow rules they will get blocked by
the downgraded deny rule, because deny rules have a higher priority,
and the application will break. Even worse there is no way to add the
functionality back to the profile without deleting the offending deny
rule.
To fix this we drop deny rules that can't be downgraded in a way that
won't break the application.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1180766
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/700
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 855dbd4ac8)
Signed-off-by: John Johansen <john.johansen@canonical.com>
On some systems the build of the parser is spitting out
cc: fatal error: no input files
compilation terminated.
This is being caused by the REALLOCARRAY checkfailing due to cpp trying
to check for both input and output files and not correctly falling
back to stdin/stdout if infile and outfile aren't specified.
Fix this by being explicit that infile and outfile are supposed to
use stdin and stdout.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/712
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit b6fbe10d11)
Signed-off-by: John Johansen <john.johansen@canonical.com>
My main user account is managed by systemd-homed. When I enable
AppArmor and have nscd running, I get inconsistent behavior with my
user account - sometimes I can't log in, sometimes I can log in but
not use sudo, etc.
This is the output of getent passwd:
$ getent passwd
root❌0:0::/root:/usr/bin/zsh
bin❌1:1::/:/sbin/nologin
daemon❌2:2::/:/sbin/nologin
mail❌8:12::/var/spool/mail:/sbin/nologin
ftp❌14:11::/srv/ftp:/sbin/nologin
http❌33:33::/srv/http:/sbin/nologin
nobody❌65534:65534:Nobody:/:/sbin/nologin
dbus❌81:81:System Message Bus:/:/sbin/nologin
[...]
rose❌1000:1000:Rose Kunkel:/home/rose:/usr/bin/zsh
But getent passwd rose and getent passwd 1000 both return no output.
Stopping nscd.service fixes these problems. Checking the apparmor
logs, I noticed that nscd was denied access to
/etc/machine-id. Allowing access to that file seems to have fixed the
issue.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/707
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/145
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit ee5303c8a0)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Ubuntu 18.04, Firefox 60.0.1+build2-0ubuntu0.18.04.1
Running firefix, then going to netflix.com and attempting to play a
movie. The widevinecdm plugin crashes, the following is found in
syslog:
Jun 15 19:13:22 xplt kernel: [301351.553043] audit: type=1400 audit(1529046802.585:246): apparmor="DENIED" operation="file_mmap" profile="/usr/lib/firefox/firefox{,*[^s][^h]}" name="/home/xav/.mozilla/firefox/wiavokxk.default-1510977878171/gmp-widevinecdm/1.4.8.1008/libwidevinecdm.so" pid=16118 comm="plugin-containe" requested_mask="m" denied_mask="m" fsuid=1000 ouid=1000
Jun 15 19:13:22 xplt kernel: [301351.553236] audit: type=1400 audit(1529046802.585:247): apparmor="DENIED" operation="ptrace" profile="/usr/lib/firefox/firefox{,*[^s][^h]}" pid=24714 comm="firefox" requested_mask="trace" denied_mask="trace" peer="/usr/lib/firefox/firefox{,*[^s][^h]}"
Jun 15 19:13:22 xplt kernel: [301351.553259] plugin-containe[16118]: segfault at 0 ip 00007fcdfdaa76af sp 00007ffc1ff03e28 error 6 in libxul.so[7fcdfb77a000+6111000]
Jun 15 19:13:22 xplt snmpd[2334]: error on subcontainer 'ia_addr' insert
...
Fixes: https://bugs.launchpad.net/ubuntu/+source/firefox/+bug/1777070
Reported-by: Xav Paice <xav.paice@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/684
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
(cherry picked from commit 656f2103ed)
Signed-off-by: John Johansen <john.johansen@canonical.com>
job scaling allows the parser to resample the number of cpus available
and increase the number of jobs that can be launched if cpu available
increases.
Unfortunately job scaling was being applied even when a fixed number
of jobs was specified. So
--jobs=2
doesn't actually clamp the compile at 2 jobs.
Instead job scaling should only be applied when --jobs=auto or when
jobs are set to a multiple of the cpus.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/703
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
(cherry picked from commit 65ba20b955)
There is currently a case in which proc_attr_base won't get set when
asprintf is able to generate the path, but the file doesn't exist, it
will exit proc_attr_base_init_once() without proc_attr_base having been
set as the fall-through if/else logic will get bypassed when asprintf is
successful.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/701
(cherry picked from commit cc113f4820)
Signed-off-by: John Johansen <john.johansen@canonical.com>
libraries/libapparmor/swig/python/Makefile.am:
Add global LDFLAGS when building the python library.
When only applying the custom PYTHON_LDFLAGS (which are in fact
`python-config --ldflags`) distributions are unable to build the library
with e.g. full RELRO.
Fixes#129
Related to #138
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/689
(cherry picked from commit b646bbf21b)
Signed-off-by: John Johansen <john.johansen@canonical.com>
libapparmor on startup does detection of whether the new stacking
proc interfaces are available and then store a var for which interface
should be used. This avoids libapparmor needing to detect which interface
to use on each proc based api call.
Unfortunately if the domain is changed on the task via change_hat or
change_profile and the proc interface is used after the domain change
it is possible that access to the interface will be denied by policy.
This is not a problem in and of it self except policy may have been
created assuming the old interface.
Fix this by adding a fallback that tries the old interface if we
are using the new interface by default and the failure was due to
an EACCES denial (policy based).
Also refactor the code a bit so this retry is isolated to one function
instead of adding it in two places.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/131
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/681
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
(cherry picked from commit d26da6c42f)
If aa-notify races file rotation it may crash with a trace back to
the log file being removed before the new one is moved into place.
Traceback (most recent call last):
File "/usr/sbin/aa-notify", line 570, in <module>
main()
File "/usr/sbin/aa-notify", line 533, in main
for message in notify_about_new_entries(logfile, args.wait):
File "/usr/sbin/aa-notify", line 145, in notify_about_new_entries
for event in follow_apparmor_events(logfile, wait):
File "/usr/sbin/aa-notify", line 236, in follow_apparmor_events
if os.stat(logfile).st_ino != log_inode:
FileNotFoundError: [Errno 2] No such file or directory: '/var/log/audit/audit.log'
If we hit this situation sleep and then retry opening the logfile.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/130
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/688
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit 7c88f02d6a)
Dovecot is hit with this denial on Debian 10 (buster):
```
type=AVC msg=audit(1603647096.369:24514): apparmor="DENIED"
operation="open" profile="dovecot" name="/usr/share/dovecot/dh.pem"
pid=28774 comm="doveconf" requested_mask="r" denied_mask="r" fsuid=0
ouid=0
```
This results in fatal error:
```
Oct 25 19:31:36 dovecot[28774]: doveconf: Fatal: Error in configuration
file /etc/dovecot/conf.d/10-ssl.conf line 50: ssl_dh: Can't open file
/usr/share/dovecot/dh.pem: Permission denied
```
Add rule to allow reading dh.pem.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/671
(cherry picked from commit 9d8e111abe)
Signed-off-by: John Johansen <john.johansen@canonical.com>
[Fontmatrix](https://github.com/fontmatrix/fontmatrix) [adds \~/.Fontmatrix/Activated to fonts.conf](https://github.com/fontmatrix/fontmatrix/blob/75552e2/src/typotek.cpp#L1081-L1088). This causes programs which use [Fontconfig](https://gitlab.freedesktop.org/fontconfig/fontconfig) (directly or indirectly through libraries such as [Pango](https://pango.gnome.org/)) to include that directory in their font search path, which causes errors such as:
```
audit: type=1400 audit(1602678958.525:53): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/.uuid" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
audit: type=1400 audit(1602678958.525:54): apparmor="DENIED" operation="open" profile="fr.emersion.Mako" name="/home/username/.Fontmatrix/Activated/" pid=48553 comm="mako" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
```
if the program does not explicitly include this directory in its AppArmor profile. As with other common font locations, add `~/.Fontmatrix/Activated` to the fonts abstraction for read-only access.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/657
Acked-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 24855edd11)
On arch
make -C profiles check-logprof
fails with
*** Checking profiles from ./apparmor.d against logprof
ERROR: Can't find AppArmor profiles in /etc/apparmor.d
make: *** [Makefile:113: check-logprof] Error 1
make: Leaving directory '/build/apparmor/src/apparmor-2.13.3/profiles'
because /etc/apparmor.d/ is not available in the build environment
and aa-logprofs --dir argument, is not being passed to init_aa()
but used to update profiles_dir after the fact.
Fix this by passing profiledir as an argument to init_aa()
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/36
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/663
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit 15dc06248c)
When run with the -p flag, aa-notify works fine for 100 seconds and then it exits.
I suspect that the issue arises from the following check on line 259 in utils/aa-notify
if debug_logger.debug_level <= 10 and int(time.time()) - start_time > 100:
debug_logger.debug('Debug mode detected: aborting notification emitter after 100 seconds.')
sys.exit(0)
together with line 301 in utils/apparmor/common.py which initializes debug_logger.debug_level to logging.DEBUG which has the numerical value 10.
A simple solution might be to just remove the check as I'm not quit sure why one would want aa-notify to exit when run in debug mode in the first place.
Alternatively, one could check against debug_logger.debugging (initialized to False) or change the initialization of debug_logger.debug_level to something else, but I don't know how that would affect other consumers of utils/apparmor/common.py.
For now just add dbugger_logger.debugging as an additional check as the
reason for timing out after 100s during debugging are unclear.
Suggested-by: vicvbcun
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/126
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/660
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Otto Kekäläinen <otto@kekalainen.net>
(cherry picked from commit 8ea7630b6d)
This reverts merge request !628. My reason for this proposal is that commit 88c142c6 already provided this change, something I must have missed when I opened the initial merge request. This resulted in duplicate entries in the profile, something that is also potentially confusing to users or other contributors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/659
Acked-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 38c611ed31)
e0b20a4d Revert "Merge dnsmasq: Permit access to /proc/self/fd/"
When using the in tree parser we should not be using the system
parser.conf file, as if the system apparmor is newer than the
tree being tested the parser.conf file could contain options not
understood by the in tree apparmor_parser.
Use --config-file to specify the default in tree parser.conf
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/653
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 5ac368bce7)
In cross build environments, using the hosts cpp gives incorrect
detection of reallocarray. Change cpp to a variable.
fixes:
parser_misc.c: In function 'int capable_add_cap(const char*, int, unsigned int, capability_flags)':
| parser_misc.c:297:37: error: 'reallocarray' was not declared in this scope
| 297 | tmp = (struct capability_table *) reallocarray(cap_table, sizeof(struct capability_table), cap_table_size+1);
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/647
Signed-off-by: Armin Kuster <akuster808@gmail.com>
(cherry picked from commit 0dbcbee700)
Signed-off-by: John Johansen <john.johansen@canonical.com>
It trys to create a symlink before the man pages are installed.
ln -sf aa-status.8 /(path}/apparmor/3.0-r0/image/usr/share/man/man8/apparmor_status.8
| ln: failed to create symbolic link '{path}/apparmor/3.0-r0/image/usr/share/man/man8/apparmor_status.8': No such file or directory
...
install -d /{path}/apparmor/3.0-r0/image/usr/share/man/man8 ; install -m 644 aa-status.8 /{path}/apparmor/3.0-r0/image/usr/share/man/man8;
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/646
Signed-off-by: Armin Kuster <akuster808@gmail.com>
(cherry picked from commit 37b9028499)
Signed-off-by: John Johansen <john.johansen@canonical.com>
While `include/sys/apparmor.h` makes use of `socklen_t`, it doesn't
include the `<sys/socket.h>` header to make its declaration available.
While this works on systems using glibc via transitive includes, it
breaks compilation on musl libc.
Fix the issue by including the header.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/642
Signed-off-by: Patrick Steinhardt <ps@pks.im>
(cherry picked from commit 47263a3a74)
Signed-off-by: John Johansen <john.johansen@canonical.com>
While `_aa_asprintf` is supposed to be of private visibility, it's used
by apparmor_parser and thus required to be visible when linking. This
commit thus adds it to the list of private symbols to make it available
for linking in apparmor_parser.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/643
Signed-off-by: Patrick Steinhardt <ps@pks.im>
(cherry picked from commit 9a8fee6bf1)
Signed-off-by: John Johansen <john.johansen@canonical.com>
With AppArmor release 3.0, a new function `aa_features_new_from_file`
was added, but not added to the list of public symbols. As a result,
it's not possible to make use of this function when linking against
libapparmor.so.
Fix the issue by adding it to the symbol map.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/643
Signed-off-by: Patrick Steinhardt <ps@pks.im>
(cherry picked from commit c9255a0343)
Signed-off-by: John Johansen <john.johansen@canonical.com>
bison change the default text past to yerror in bison 3.6, this
breaks make check as some tests are comparing against the error
output
======================================================================
FAIL: test_modefail (__main__.AAErrorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/jj/apparmor.git/parser/tst/testlib.py", line 50, in new_unittest_func
return unittest_func(self)
File "./errors.py", line 58, in test_modefail
self._run_test(
File "./errors.py", line 40, in _run_test
self.assertIn(message, outerr, report)
AssertionError: 'AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error, unexpected TOK_ID, expecting TOK_MODE' not found in 'AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error\n' :
Command: ../apparmor_parser --config-file=./parser.conf -S -I errors errors/modefail.sd
Exit value:1
STDERR
AppArmor parser error for errors/modefail.sd in profile errors/modefail.sd at line 6: syntax error
To fix this we need to add
define parse.error=verbose
to bison. Unfortunately define parse.error was only added in bison 3.0
and and older versions of bison will break if that is defined in
parser_yacc.y
Instead test for the version of bison available and set define parse.error
as a build flag if supported by the version of bison being called.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/640
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
The parser is not handling variable expansion/rule conditionals
correctly in some cases. Eg. NOT treating unix addr as a path and
filtering slashes after variable expansion. This can lead to errors
where
@{foo}=/a/ unix bind addr=@{foo}/bar,
will always fail because addr is being matched as /a//bar instead of
/a/bar.
Note: this is a first pass and may not fix all cases. Eg. There is a
known issue with profile names not being able to begin with a
variable.
Signed-off-by: John Johansen john.johansen@canonical.com
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Attempt to get clarity on what is valid syntax for mount options and
fstype options.
Note that simple_tests/mount/bad_opt_27.sd is marked TODO, as the
parser accepts it but should not.
Also mark the tests as expecting to fail to raise an exception by the
python utils.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/607
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds support for the new v8 network mediation in the 4.17 and later kernels.
It requires that policy either be tagged with an abi rule that support the v8 network, or the policy be pinned with --policy-features= with a feature abit that supports v8 network.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Acked-by: Steve Beattie <steve.beattie@canonical.com>
af_unix allows for sockets to be bound to a name that is autogenerated.
Currently this type of binding is only supported by a very generic
rule.
unix (bind) type=dgram,
but this allows both sockets with specified names and anonymous
sockets. Extend unix rule syntax to support specifying just an
auto bind socket by specifying addr=auto
eg.
unix (bind) addr=auto,
It is important to note that addr=auto only works for the bind
permission as once the socket is bound to an autogenerated address,
the addr with have a valid unique value that can be matched against
with a regular
addr=@name
expression
Fixes: https://bugs.launchpad.net/apparmor/+bug/1867216
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Signed-off-by: John Johansen <john.johansen@canonical.com>
v8 network permissions extend into the range used by exec mapping
so it is important to not blindly do execmapping on both the
file dfa and policydb dfa any more.
Track what type of dfa and its permissions we are building so
we can properly apply exec mapping only when building the
file dfa.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/521
Signed-off-by: John Johansen <john.johansen@canonical.com>
If an exec gets denied in enforce mode, there are no log events for the target binary/profile. Therefore, trying to set the final_name for the target will crash with a KeyError.
Check for the existence of hashlog\[aamode\]\[target_profile\] in all exec options to prevent this crash.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/634
Acked-by: John Johansen <john.johansen@canonical.com>
Add a basic tool for manipulating the apparmor features abi via
libapparmor. This serves as a basic tool and as an example of using
the library api.
Currently its function is limited to extracting the kernel feature
abi and loading a feature abi from a file and then outputing it.
In the future it will pickup the ability to verify the feature
abi, and merge feature abis.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/613
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
... and allow dovecot to Px to it.
The profile is based on a profile I received in a bugreport, with the
actual script factored out. Note that you'll _need_ to add a rule to the
local/ include to allow executing the actual script, but since the
script to execute is configurable, it's basically impossible to add a
default rule for it.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1166007
If an exec gets denied in enforce mode, there are no log events for the
target binary/profile. Therefore, trying to set the final_name for the
target will crash with a KeyError.
Check for the existence of hashlog[aamode][target_profile] in all exec
options to prevent this crash.
Note that the log doesn't include enough information for EXEC MODE and EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND (comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the libapparmor testsuite.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/631
Acked-by: John Johansen <john.johansen@canonical.com>
When switching to complain or enforce mode (for example with aa-complain and aa-enforce), remove conflicting flags like kill and unconfined.
As ground work, change add_or_remove_flag() to allow to add or remove multiple flags. Multiple flags can be given as string (will be split) or as array.
Also add some tests confirming that everything works as expected.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/633
Acked-by: John Johansen <john.johansen@canonical.com>
Snapd now loads its own policy via its own systemd unit
https://github.com/snapcore/snapd/pull/8467
If A distro is not using snapd systemd unit then dropping snapd policy
from the apparmor unit is a breaking change, distros will either need
to use the snapd systemd unit or revert
0164fd05 init: stop loading snap policy
Signed-off-by: John Johansen <john.johansen@canonical.com>
Note that the log doesn't include enough information for EXEC MODE and
EXEC COND, therefore aa-logprof will always propose ALL as EXEC COND
(comm= might give a hint about EXEC COND, but isn't good enough).
With the added support in aa-logprof, remove the changeprofile tests
from the known-failing list in test-libapparmor-test_multi.py.
Also add another test log (from darix) / expected profile to the
libapparmor testsuite.
... when reading /proc/$pid/attr/{apparmor/,}current
Also add a comment about _not_ adding support for the 'unconfined'
profile mode, because that would give a quite confusing output.
This means moving the code that reads the 'current' file into a new
function read_proc_current()Then call that function for both
/proc/$pid/attr/apparmor/current (preferred) and /proc/$pid/attr/current
(fallback).
New kernels provide an alternative proc attr interface for apparmor
which is needed for LSM stacking.
Update the remaining profiles that use the old interface to
include access to the new interface.
Signed-off-by: John Johansen <john.johansen@canonical.com>
As of [48755ebf], dnsmasq iterates through directory entries in
/proc/self/fd/ to find which file descriptors are open to avoid the
potentially costly operation of closing all possible file descriptors.
[48755ebf]: https://www.openhub.net/p/dnsmasq/commits/1679009518
MR625 fixed hats not emitting the rule to access the proc interface needed for change_hat, but it broke the rule being emitted for the parent (which used to work).
The proc attr access rule should be emitted for any profile that is a hat OR any profile that contains hats.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Acked-by: Steve Beattie <steve.beattie@canonical.com>
This add a test to ensure that the parser is inserting rules to allow
access to the proc interface for change_hat.
Unfortunately the rule the parser inserts is a bare owner write that
we can't replicate in policy as policy write perm maps to create,
append and write.
So to test equality compare profiles using rules granting access to
the proc attr interface except one uses the append permission and
the other uses write. They will differ in permissions unless the
parser inserts the proc attr write rule for change_hat in which
case the permissions will get merged and we have equivalence.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
LSM stacking adds new interfaces
/proc/<pid>/attr/apparmor/*
that should be used in preference of the old interface files in
/proc/<pid>/attr/*
The library has already been updated to use the new path but the
abstraction for change_profile has not so accessing the new interface
in a confined application fails.
Fix this by making the abstraction cover the old and new interfaces.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR625 fixed hats not emitting the rule to access the proc interface
needed for change_hat, but it broke the rule being emitted for the
parent (which used to work).
The proc attr access rule should be emitted for any profile that
is a hat OR any profile that contains hats.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/626
Signed-off-by: John Johansen <john.johansen@canonical.com>
The parser is supposed to add a rule to profiles if they are a hat
or contain hats granting write access to the kernel interfaces
used to perform the change_hat operation.
Unfortunately the check is broken and currently won't add the
rule to hats (it does add it for the parent).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/625
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Acked-by: Steve Beattie <steve.beattie@canonical.com>
serialize_profile() assumes that active_profiles has the /etc/apparmor.d/ filename of a profile initialized.
This patch makes sure this is true even when using an extra profile by initializing it in get_profile().
Ideally serialize_profile() shouldn't always use active_profiles, but that will be part of a bigger change.
Reported by zt1024 including a proposed patch on https://gitlab.com/apparmor/apparmor/-/merge_requests/604 but of course ;-) this patch is better because it selectively does the initialization only in the case that needs it.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/623
Acked-by: John Johansen <john.johansen@canonical.com>
serialize_profile() assumes that active_profiles has the
/etc/apparmor.d/ filename of a profile initialized.
This patch makes sure this is true even when using an extra profile by
initializing it in get_profile().
Ideally serialize_profile() shouldn't always use active_profiles, but
that will be part of a bigger change.
Reported by zt1024 including a proposed patch on
https://gitlab.com/apparmor/apparmor/-/merge_requests/604
but of course ;-) this patch is better because it selectively does the
initialization only in the case that needs it.
php-fpm: fix profile name in php-worker
This is analogous to 9892a032. It allows php-fpm to manage the worker threads by allowing the threads to receive signals from php-fpm (rather than the nonexistent profile `/usr/sbin/php-fpm*`).
See merge request apparmor/apparmor!619
Acked-by: Christian Boltz <apparmor@cboltz.de>
A debug message in reset_parser() gives developers more data about how
the parser is behaving. In addition, it provides much needed context to
the relatively vague debug message in clear_cap_flag().
Another solution might be to pass the profile name into
clear_cap_flag(), however, clear_cap_flag() does not need the profile
name, except potentially for debugging purposes.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
A copy/paste error leads to a build failure due to an undefined variable
"name" in parser_misc.c:clear_cap_flag().
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Add flags to be able to control parser warnings, and convert warnings into errors.
Flags can be enabled by using `--warn` followed by a single flag. If multiple flags need to be set --warn can be specified multiple times.
Eg.
```
--warn=deprecated --warn=rule-downgrade
```
Flags can be disabled no prefix the flag name with `no-`
Eg.
```
--warn=no-config
```
The set of currently enabled flags can be seen by specifying
```
--warn=show
```
and the set of supported flags can be seen by specifying
```
--help=warn
```
The default set of warning flags that are enabled are
```
WARN_CONFIG - config warnings
WARN_CACHE - cache warnings that were not hidden behind debug-cache
WARN_JOBS - warnings around job failures
WARN_UNEXPECTED - warnings about internal consistency checks against what is expected from kernel
WARN_OVERRIDE - warnings about overriding some option, currently limited to forcing an override on a namespace.
```
Warnings can be converted into errors by specifying `-Werror`. It supports all the same options as \`\`\`--warn\`. A warning must be turned on before the warning can be converted into an error message.
```
--warn=deprecated --Werror=all
```
Will create error messages for deprecation warnings but not for rule-downgrades. -Werror is disabled by default.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Acked-by: Steve Beattie <steve.beattie@canonical.com>
--config-file is processed early in a separate argument processing
pass. Adjust --warn and --Werror processing so they are done in
both the early and late arg processing pass.
--warn and --Werror must be run in both argument processing passes
so that
1. They can be used with --config-file as long as they are specified
before --config-file (early pass)
2. They are not overriden by any flags set in the config file, as
command line options take priority over what is in the config
file (hence the need for reprocessing in the second pass)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add basic ability to treat a warning as an error and abort the compile
by specifying the new option --Werror.
--Werror
will turn all warnings into errors. Where if an warning type is
specified only that type of warning will be turned into an error.
--Werror=deprecated.
The full list of supported warning types can be found by using
apparmor_parser --help=warn
or
apparmor_parser --help=Werror
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make all warnings that go through pwarn() controllable by warning
flags. This adds several new warning control flags, documented in
--help=warn
Convert --debug-cache to be unified with warning flags. So it can be
set by either
--debug-cache
or
--warn=debug-cache
Also add an "all" option to be able to turn on all warnings.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/600
Signed-off-by: John Johansen <john.johansen@canonical.com>
Host files accessed by tcp_wrapper can reference other files, from man
5 hosts.allow
```
A string that begins with a '/' character is treated as a file name. A host name or address is matched if it matches any host name or address pattern listed in the named file. The file format is zero or more lines with zero or more host name or address patterns separated by whitespace. A file name pattern can be used anywhere a host name or address pattern can be used.
```
This means adding a file to hosts requires updating multiple profiles
Add a hosts abstraction so users only have to modify a single location.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/605
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1864466
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Fix /usr/lib/postfix binary paths in postfix+dovecot profiles
See merge request apparmor/apparmor!602
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
Provide example rules in parser.conf to pin pre 3.0 policy and
appropriate abi files.
abis for vanilla upstream kernels and outoftree network patched
kernels are provided. With both ABIs dropping v8 support from
advertised by the kernel as 2.x policy/userspace did not support it.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/598
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
prologue.inc:settest() in same cases invokes a sed command that uses
'~' as a pattern separator, on things that can contain filesystem
paths. However, in the debian/ubuntu world, '~' can be used in version
strings, particularly for pre-release versions, and when this happens
and the version is embedded in the path, the sed command breaks
because of the extraneous separator. Fix this by using '#' as a
separator, which has the benefit of being considered a comment if
accidentally interpreted by a shell.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/599
Merge branch 'parser-fix_cap_list_check' into 'master'
The sed command to manipulate the known cap list (`base_cap_names.h`) into
a format to match the generated_cap_names.h was buggy because the
trailing `}` would never match anything, leading to failures when built
against 5.8 kernel headers, due to it not raplcing the base capabilities
correctly.
Fix this by removing the trailing `}` match and instead match the third
comma-delimited field that matches a capability name, and replace that.
See merge request apparmor/apparmor!596
The sed command to manipulate the known cap list (base_cap_names.h) into
a format to match the generated_cap_names.h was buggy because the
trailing '}' would never match anything, leading to failures when built
against 5.8 kernel headers, due to it not replacing the base capabilities
correctly.
Fix this by removing the trailing '}" match and instead match the third
comma-delimited field that matches a capability name, and replace that.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/596
fix bug that some rules will be added to the profile repeatedly
The ask_exec() function may cause some rules to be added to the profile repeatedly.
See merge request apparmor/apparmor!593
Acked-by: Christian Boltz <apparmor@cboltz.de>
In AppArmor 2 distros could pin the feature file being used by setting
the feature-file option in the config file.
With AppArmor 3 policy is now explicitly tagged with an abi rule.
The problem is the interaction on systems that have a mixture of
AppArmor 2 and AppArmor 3 policy and use feature pinning.
The feature pinning is required to make the apparmor 2 policy behave
as expected but it also overrides the abi rules that are explicitly
set as part of the policy. This means we either have the apparmor 2
pinned policy working as desired or the apparmor 3 policy, but not
both.
To fix this make setting the flag in the config file have lower
priority than the feature abi rule. But still allow the config option
specified on the command line to override the config file and the
feature abi rule.
The Priority ordering to determine the policy abi to use is
1. Use abi rules if present
2. if no abi rule use command line option
3. if no abi rule or command line option use config setting
4. if none of the above use the default abi
v2:
add priority ordering to documentation
reorder priority so existing config/command line options behave as expected, make ABI rules highest priority
add new option --override-policy-ABI to allow overriding of abi rules
move arg processing from numbers to defines
v3
update patch description around how the abi pinning problem is being fixed
add MR: tags to patches
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
I screwed up adding the last option, yet again because
EARLY_ARG_CONFIG_FILE was define out of order, and adding the new
option seems to skip a number, ...
Switch to defines to make it easier to update, and keep all these
define numbers together in order.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add an option to allow setting/pinning the feature ABI and overriding
of ABI rules if they exist.
--override-policy-abi
This option is primarily for profile development and testing without
allowing adjusting feature abis temporarily without modifying the
profile.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
In AppArmor 2 distros could pin the feature file being used by setting
the feature-file option in the config file.
With AppArmor 3 policy is now explicitly tagged with an abi rule.
The problem is the interaction on systems that have a mixture of
AppArmor 2 and AppArmor 3 policy and use feature pinning.
The feature pinning is required to make the apparmor 2 policy behave
as expected but it also overrides the abi rules that are explicitly
set as part of the policy. This means we either have the apparmor 2
pinned policy working as desired or the apparmor 3 policy, but not
both.
To fix this make setting the flag on command line or in config file
lower priority than an abi rule specified in policy. The ability
to override abi rules will be added in a separate patch.
The Priority ordering to determine the policy abi to use is
1. Use abi rules if present
2. if no abi rule use command line option
3. if no abi rule or command line option use config setting
4. if none of the above use the default abi
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/579
Signed-off-by: John Johansen <john.johansen@canonical.com>
utils `make check_severity_db` will fail the build if a (probably new)
capability in not listed in severity.db. This also means it should print
out an ERROR, not a warning.
This is a follow-up of lp#1890547 and
https://gitlab.com/apparmor/apparmor/-/merge_requests/589
The warn_once() function is duplicated in 6 different places. A common,
reusable version has been added to parser_common.c.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Nobody told the tools that log events with operation="symlink" exist.
Add this keyword to the list of file or network operations (I don't
expect network symlinks ;-) but keeping everything in that list makes
things easier than special-casing it.)
Also add the log sample and expected result to the libapparmor tests.
Fixes https://gitlab.com/apparmor/apparmor/-/issues/107
The fixes that landed for #98 are incomplete: they only added a check to
ensure that the parser is built, and not libapparmor as well, and also
didn't adjust the invocation of aa-logprof to actually make use of the
built libapparmor or its swig python library.
Furthermore, neither it nor the tests in utils/ add the location of the
swig libapparmor C library to the library search path, meaning that if
the system one does not exist, it will not be found and the test will
fail.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/98
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/586
The commit c8b6d8b393 ("profiles: Update 'make check' to select tools
based on USE_SYSTEM") set a bunch of variables but neglected to apply
them when invoking aa-logprof. This commit addresses this by:
* correcting the PYTHONPATH used with aa-logprof
* setting LD_LIBRARY_PATH when invoking aa-logprof
* adjusting LD_LIBRARY_PATH to include both the directory location
of libapparmor but also the swig libapparmor library needed for
python tools to function.
* adjusts the test for the presence of libapparmor to not use
LD_LIBRARY_PATH but instead a libapparmor specific variable
LIBAPPARMOR_PATH
Bug: https://gitlab.com/apparmor/apparmor/-/issues/98
Fixes: c8b6d8b393
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/586
The authentication, base and nameservice abstraction used /{usr/,}etc/
in several rules. Switch that to the more readable (and tunable)
@{etc_ro} variable.
Add a couple of bitmap renderings of the logo for convenience
128x128 pixel rendering of the logo
700x700 pixel rendering of the logo
Signed-off-by: John Johansen <john.johansen@canonical.com>
The profiles dirs make check is not always using the correct tools.
Update it to be similar to other Makefiles where the var USE_SYSTEM
make check USE_SYSTEM=1
is used to indicated that the system installed tools should be used
and
make check
is used to run the tests against the in tree tools
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/580
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The hashing of the featue set is wrong because it is hashing the
whole feature structure instead of just the feature string.
This results in the refcount and hash field becoming part of the
hash and the feature string not being completely hashed as the
bytes of the refcount and hash field are being counted in the
as part of the string length when the hash is taken.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/583
Reported-by: Samuele Pedroni <samuele.pedroni@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
This adds support for the new PERFMON, and BPF capabilities.
In addition this updates AppArmor's handling in several ways.
* Move to a static capability list that is compared to the automatically generated capability list, and fail the build if new capabilities are detected. This is required to ensure backwards mapping of capabilities is properly handled.
* allow an already built apparmor to dynamically detect any new additions of capabilities so a release patch doesn't need to be made to support new capabilities. This however will be missing support for backwards mapping of capabilities
* support backwards mapping of new capabilities that have been split of existing capabilities for older kernels (eg. CAP_PERFMON was split off of CAP_SYS_ADMIN)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/578
Acked-by: Seth Arnold <seth.arnold@canonical.com>
If a capability is known in policy but not by the kernel, check to see if it has
a backwards mapping to a different capability and use that instead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The kernel and policy abis can be used to detect and support new
capabilities without having to update base_cap_names.h and and
rebuilding the compiler.
This is not perfect however in that the does not provide any backwards
compatibility mappings, so we still need to keep the internal
capability table.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently features doesn't provide a way to query a features
value. So add an api to extract the value string of a feature.
The value string returned is a raw text value and may contain
leading spaces, etc that the caller may need to be aware of.
Signed-off-by: John Johansen <john.johansen@canonical.com>
We need to be able to dynamically add capabilities to the capability
list so switch to using a dynamically allocated table that we can
extend.
Signed-off-by: John Johansen <john.johansen@canonical.com>
We need a set of flags to track where a capability can from so we know
how to processes it for policy compatibility purposes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some capabilities like perfmon and bpf have been split out from
another capability, likely cap sys_admin. Add this backmapping
infomation so that the parser can take advantage of it to support
policy on older kernels that don't support the new capabilities.
Signed-off-by: John Johansen <john.johansen@canonical.com>
There are currently two capability name tables the one that is
autogenerated and an internal hardcoded name table.
Now that the autogenerated table has been converted to a base
static table we can drop the internal static table. This
removes the chance of getting the tables getting out of sync.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The static cap sys_log declaration is no longer needed as
base_cap_names.h contains it and ensures that it will always be
present.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The auto-generated cap_names.h has problems when the parser if the
parser is built against a kernel with a smaller capability list than
the kernel policy is being compiled for.
Moving to a pre-generated list lets us support all capabilities even
when we build against older kernels. However we don't want to only use
the pre-generated list as that would make it too easy to miss when a
new capability has been added.
Keep auto generating the caps list and compare it to the pre-generated
caps list so we can detect when new capabilities are added, and fail
the build so that the pre-generated list can be updated. We screen the
diff for only additions so that the parser can continue to build on
older kernels that don't have the full capability list without errors.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Mikhail Morfikov has discovered [0] that some font packages in Debian
ships font files in /usr/bin/fonts-foo-bar (like
/usr/share/fonts-font-awesome/ for example). This produces denials for
GUI applications.
Update fonts abstraction to allow reading /usr/bin/fonts-* directories.
Also, refactor abstraction to aggregate two old rules into one.
Closes#94
[0] https://gitlab.com/apparmor/apparmor/-/issues/94
This fixes a regression introduced by the abi patches for policy that
is not tagged with an abi rule.
Specifically if the current apparmor (apparmor 3) is used with a
kernel that supports unix rules, and policy has network rules but has
not been updated to use abi rules, without this patch the policy unix
rules will stop working and unix mediation will not be enforced.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/568
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The lexer is broken and passing echo input to stdout that it doesn't match in some states ignoring the error.
First add `%option nodefault` to guarantee the lexer won't ever echo the unknown input to stdout, this will cause the parser to error out with
```
flex scanner jammed
```
and $?=2 if a profile contains unknown/invalid parts. That's not really a helpful error message, but still better than ignoring errors.
Next improve the error message output,
```
AppArmor parser error for tst/simple_tests//vars/vars_simple_assignment_14.sd line 8: Lexer found unexpected character: '
' (0xa) in state: SUB_ID_WS
```
using flex's error output only if there is an mistake made when introducing new states.
Finally fix bugs that are found.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Acked-by: Steve Beattie <steve.beattie@canonical.com>
* allow reading my.cnf in dovecot-dict profile (seen with the mail users in a mysql database)
* allow /proc/\*/attr/current in dovecot imap and lmtp (needed when using the "apparmor" plugin which means dovecot switches to user-specific hats)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/566
Acked-by: John Johansen <john.johansen@canonical.com>
Newlines should generally be treated as whitespace. Expand the list
of states using the newline rule to include almost all rules that
eat WS.
There are two exceptions assign and comment which have special handling
of newlines.
this fixes the failures
not ok 71543 - ./simple_tests//vars/vars_simple_assignment_13.sd: quoted commas should not trigger an error
not ok 71544 - ./simple_tests//vars/vars_simple_assignment_14.sd: quoted commas should not trigger an error
found by introducing nodefault
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
There were several states missing from the default rule which catches
unexpected input in a state.
Update the default rule to catch all input including newlines and
update its error message to include information about which state the
failure occured in. Also update the comment about what to do when
adding new states.
While the lexer now has the "nodefault" option set, it doesn't provide
as much information as the default rule does, so we prefer states
to use our provided default rule.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
... (using `%option nodefault`) instead of echoing the unknown parts to
stdout, and ignoring the error.
This will cause the parser to error out with
flex scanner jammed
and $?=2 if a profile contains unknown/invalid parts. That's not really
a helpful error message, but still better than ignoring errors.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/569
Signed-off-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This fixes a regression due on older policy due to the abi patches.
Specifically if the current apparmor is used with a kernel that
supports v7 networking, and policy has network rules but has not been
updated to use abi rules, without this patch the policy network rules
will stop working and network mediation will be unenforced.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/564
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add profile flags enforce, kill and unconfined.
enforce - is the default mode and is not required but this allows the mode reported by introspection to be used in the profile.
kill - a modified enforce mode that kills tasks instead of returning a denial.
unconfined - allow a named profile to behave as if it is unconfined.
Eg.
profile example flags=(enforce) { ... }
profile example flags=(kill) { ... }
profile example flags=(unconfined) { ... }
Closes#7
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/440
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This is not a real change - since some commits, include_dir_filelist()
gets only called with absolute paths.
Add a check to ensure this, drop the now superfluous get_include_path()
call, and replace usage of include_name_abs with include_name (which are
the same now).
Also drop the superfluous profile_dir parameter, and adjust the only
caller accordingly.
With the check that incfile starts with a '/', incfile and incfile_abs
(as returned by get_include_path()) are always the same.
Drop the get_include_path() call and setting incfile_abs, and replace
usage of incfile_abs with incfile.
This is just a cleanup, no behaviour change.
... so that the include rules proposed by aa-logprof continue to be
relative to the profile directory.
This fixes the behaviour change introduced in the previous commit.
This removes the need to remove profile_dir from include paths at
various places.
A side effect is that aa-logprof / match_includes() now propose more
include rules, for example matching local/ files.
Another side effect is that proposals for include rules
(match_includes() again) now come with the full path.
Both side effects will be fixed in the next commits.
This is needed for running the tests, because test/logprof.conf contains
a relative path, and tests only "manually" set the profile_dir if they
need/have a modified copy of the profiles.
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<unsigned int*, std::vector<unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
chfa.cc: In function ‘void write_flex_table(std::ostream&, int, Iter, Iter) [with Iter = __gnu_cxx::__normal_iterator<short unsigned int*, std::vector<short unsigned int> >]’:
chfa.cc:348:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 16));
^~
chfa.cc:349:3: note: here
case 2:
^~~~
chfa.cc:350:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
os.put((char)(*pos >> 8));
^~
chfa.cc:351:3: note: here
case 1:
^~~~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
private.c: In function ‘_aa_is_blacklisted’:
private.c:140:35: warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
140 | found - name + suffix->len == name_len ) {
| ^~
private.c: In function ‘readdirfd’:
private.c:352:16: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
352 | for (i = 0; i < n; ) {
| ^
private.c:378:17: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘ssize_t’ {aka ‘long int’} [-Wsign-compare]
378 | for (i = 0; i < n; i++)
| ^
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The len parameter returns a value that correlates to a getsockopt
parameter which is typed to socklen_t which is an unsigned int.
This technically changes the fn() api but old code using this is
already broken if the getsockopt parameter is large enough to overflow
the value.
In reality what is returned shouldn't ever be negative and the value
should never be large enough to trip the overflow. This is just
cleaning up a corner case.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
kernel.c: In function ‘aa_getpeercon_raw’:
kernel.c:823:14: warning: comparison of integer expressions of different signedness: ‘socklen_t’ {aka ‘unsigned int’} and ‘int’ [-Wsign-compare]
823 | if (optlen < *len) {
| ^
kernel.c: In function ‘query_label’:
kernel.c:966:10: warning: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
966 | if (ret != size) {
| ^~
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/561 Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/554
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/558
Acked-by: John Johansen <john.johansen@canonical.com>
```
libaalogparse.c: In function 'hex_to_string':
libaalogparse.c:144:16: warning: comparison of integer expressions of different signedness: 'int' and 'size_t' {aka 'long unsigned int'} [-Wsign-compare]
144 | for (i = 0; i < len; i++) {
| ^
```
`i` gets used/changed as counter variable in the for loop and only gets
increased (starting at 0), so making it an (unsigned) size_t should be
safe.
This fixes lots of warnings like
```
*** Checking abstractions from ./apparmor.d/abstractions against apparmor_parser
Warning from stdin (stdin line 1): ../parser/apparmor_parser: File 'stdin' missing feature abi, falling back to default policy feature abi
Warning from stdin ([...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile line 9): ../parser/apparmor_parser: [...]/profiles/./apparmor.d/abstractions/apparmor_api/change_profile features abi 'abi/3.0' differes from policy declared feature abi, using the features abi declared in policy
```
Tag profiles and abstractions with abi information.
Tagging abstractions is not strictly necessary but allows the parser
to detect when their is a mismatch and that policy will need an
update for abi.
We do not currently tag the tunables because variable declarations
are not currently affected by abi.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Add basic support for policy to specify a feature abi. Under the
current implementation the first feature abi specified will be
used as the policy abi for the entire profile.
If no feature abi is defined before rules are processed then the
default policy abi will be used.
If multiple feature abi rules are encountered and the specified
abi is different then a warning will be issued, and the initial abi
will continue to be used. The ability to support multiple policy
feature abis during a compile will be added in a future patch.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
The features abi adds the ability to track the policy abi separate
from the kernel. This allow the compiler to determine whether policy
was developed with a certain feature in mind, eg. unix rules.
This allows the compiler to know whether it should tell the kernel to
enforce the feature if the kernel supports the rule but the policy
doesn't use it.
To find if a feature is supported we take the intersection of what is
supported by the policy and what is supported by the kernel.
Policy encoding features like whether to diff_encode policy are not
influenced by policy so these remain kernel only features.
In addition to adding the above intersection of policy rename
--compile-features to --policy-features as better represents what it
represents. --compile-features is left as a hidden item for backwards
compatibility.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/491
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
Define EXTRA_WARNINGS in the common/Make.rules helper so that adding
additional warnings can be done in one(-ish) location, and replace
locally defined C compiler warning flags with EXTRA_WARNINGS in most
locations in the build tree.
v2: issue a warning for any compiler option that the compiler does not
support
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
... instead of relying on the filesystem(!) ordering, which will look
random to both users and unittests.
Also partially revert the test changes from
c5a7bcd50e /
https://gitlab.com/apparmor/apparmor/-/merge_requests/548 -
sorting the result only in the tests is a bad idea.
This fixes cases when two aliases with the same left side were
configured - instead of "last one wins" in the dict, AliasRuleset now
keeps both.
ProfileList add_alias() changes its parameters and now expects an
AliasRule object. Adjust all callers to that.
Drop the no longer needed write_alias().
Also adjust the tests to use AliasRule and add a dedup test promised in
an earlier patch series.
Add VariableRule and VariableRuleset and use it for variable handling
Besides the usual advantages of switching to classes, we finally get rid of the `filelist` hasher.
While on it, also fix some bugs around variable handling, including https://bugs.launchpad.net/apparmor/+bug/1331856 and some that maybe nobody noticed before.
As usual, see the individual commits for details.
See merge request apparmor/apparmor!544
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Trailing commas in variable values are not allowed (unless they are
quoted). Fix the regex to avoid "eating" the comma, and add a check to
detect invalid commas.
As usual, add some tests, and remove some testcases from the
exception_not_raised list.
... and add a test to ensure that everything works as expected.
Note that broken variable names like '@{foo' match the (quite
permissive) regex, but are invalid nevertheless.
... by calling active_profiles.get_all_merged_variables()
Also remove vars/vars_bad_add_assignment_1.sd from the
exception_not_raised list again - now it raises an exception as
expected.
Add set_variables() to severity.py to set the variables for severity
rating. It typically gets the data from the get_all_merged_variables()
result.
This replaces the slightly broken load_variables() that parsed profile
files for variables. (For example, parsing "@{foo} = /bar" resulted
in a variable name "@{foo} " with trailing space.)
Also adjust aa.py and the severity tests to use set_variables() (with
get_all_merged_variables()) instead of load_variables().
This also re-adds the checks that were removed in the "Store variables
in active_profiles (ProfileList)" commit earlier, while still fixing
lp:1331856.
With this change, unload_variables() becomes useless (the variables get
overwritten in set_variables() anyway), drop it and its calls.
Note that load_variables() silently ignored non-existing files while the
get_all_merged_variables() call only works for existing files that are
known to active_profiles. Since the input of ask_the_questions() and
ask_exec() comes from log_dict (= audit.log or a profile to merge), add
a check if that profile actually exists in the set of active profiles.
Also adjust the severity tests to use set_variables().
Finally, drop the tests that check for handling non-existing include
files, redefining and adding to non-existing variables - all these
things get now handled in include_list_recursive() and
get_all_merged_variables() and their tests.
Fixes: https://bugs.launchpad.net/apparmor/+bug/1331856
This function returns a dict with all variables and their values for a
given profile file. It also checks for redefined variables, and adding
to non-existing variables, and errors out in both cases. Note that it
does not check the include order and is therefore more forgiving than
apparmor_parser.
Also add some tests for get_all_merged_variables().
Note: the tests are based on reading "real" profiles, therefore we need
to initialize apparmor.aa and call some of its functions.
The alternative would be to construct ProfileList objects for some
profiles and includes manually, but doing that in a way that can replace
the tests with "real" profiles would be quite some work, and I'm not
bored enough for that ;-)
Everything handled in 'filelist' gets handled in active_profiles now.
Note: the 'elif' branch in delete_all_duplicates() was probably never
hit because `if include.get(...)` always matched. The only possible
exception might be non-existing include files, but those cause a 'file
not found' error anyway.
... instead of filelist[file]['lvar'], and also write them from there.
Also fix detection of variable definitions inside a profile, which is
not allowed.
Note that ProfileList has a different write order than the old code -
first includes, then variable definitions. This makes more sense because
typical profiles first include tunables/global, and then define
additonal variables (that might use variables from tunables/global) or
extend variables defined in tunables/global.
This change also fixes some problems with the simple_test test profiles.
The "adding to non-existing variable" check currently doesn't exist,
which "fixes" lp:1331856.
OTOH this also means that such cases are not detected, therefore add
vars_bad_add_assignment_1.sd to the exception_not_raised list.
The check will be re-added in a later commit
in get_all_merged_variables().
strip_quotes() assumed its parameter is at least one character long, and errored out on an empty string.
It also converted a string consisting of a single quote to an empty string because that single quote had a quote as first and last char.
This commit fixes these two bugs.
Also rewrite TestStripQuotes to use tests\[\], and add some test for an empty string, a one-char path (just a slash) and a single quote.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/545
Acked-by: John Johansen <john.johansen@canonical.com>
At least on Debian, when DNSSEC validation is enabled, dnsmasq needs to read
/usr/share/dnsmasq-base/trust-anchors.conf
Bug-Debian: https://bugs.debian.org/934869
strip_quotes() assumed its parameter is at least one character long, and
errored out on an empty string.
It also converted a string consisting of a single quote to an empty
string because that single quote had a quote as first and last char.
This commit fixes these two bugs.
Also rewrite TestStripQuotes to use tests[], and add some test for an empty
string, a one-char path (just a slash) and a single quote.
If multiple -p/--profile or -n/--namespace parameters are given to aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of conflicting parameters probably makes more sense.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/540
Acked-by: John Johansen <john.johansen@canonical.com>
If multiple -p/--profile or -n/--namespace parameters are given to
aa-exec, they are handled on a "last one wins" base.
This probably isn't expected behaviour, and erroring out in case of
conflicting parameters probably makes more sense.
This patch series moves include rule handling away from the `filelist` hasher to using IncludeRule and IncludeRuleset. This means that only variable handling is left in `filelist`.
As usual, check the individual commits for details.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/537
Acked-by: John Johansen <john.johansen@canonical.com>
remove_duplicate_rules() composed the 'includes' list from a) the
include rules in the profile and b) the file_includes (preamble
includes), and then checked the profile includes against this
combination of profile and preamble includes.
Checking profile includes against preamble includes is wrong, and the
only reason why this went unnoticed for years is that preamble include
files (like tunables) won't work inside a profile and therefore never
appear there.
In theory this might be a backport candidate, even if this shouldn't
cause a real-world bug.
This function returns a list of all includes in a profile and its
included files.
It's split off from is_known_rule() and get_file_perms() which shared
lots of common code to recursively get the include list. These functions
now use include_list_recursive().
... or calling is_known_rule() on events for non-existing hats.
It's the usual hasher() "fun" again - accessing a non-existing element
will create its parent.
In theory this commit might be worth a backport. In practise, it doesn't cause
any visible problem.
However, starting with the next commit, it will cause lots of test errors.
Also add a missing is_known_rule() call for dbus rules, which might have
caused similar hasher() "fun".
rule_obj.get_full_paths() handles directories and non-existing files
(depending on 'if exists'), so we can simply feed the resulting list
into load_include()
This function returns a list of paths for an include rule. This can be
- a single path if the include file is a file and exists
- a single path if the include file doesn't exist, but doesn't have
'if exists' (this will cause a 'file not found' error when used)
- a list of paths if the include is a directory
- an empty list if the rule has 'if exists' and the file doesn't exist
Also add some tests for get_full_paths()
IncludeRuleset needs to import it (next commit), which is impossible
when it lives in aa.py (would cause recursive import loop because aa.py
imports IncludeRuleset)
... because this is now done via IncludeRule, and keeping the separate
code would mean asking twice.
Note that the user interface changes slightly.
The old workflow was
1 - #include <foo>
2 - #include <bar>
3 - #include <baz>
[select 2 and (A)dd, then get the next question for the remainder]
1 - #include <foo>
2 - #include <baz>
[(A)dd another one, or (I)gnore the remainder]
The new workflow will ask separately for each abstraction, and you can
(A)dd or (I)gnore it. This is probably easier to understand because it's
basically a yes/no question.
... and write them (only) from 'inc_ie' (IncludeRuleset), which can
handle both "include" and "include if exists" rules.
This duplicates storage of include rules because 'include' is still used
and needed at various places that work on/with the include rules.
With this, we get removal of duplicate include lines insinde a profile
in aa-cleanprof "for free" - extend cleanprof_test.in to confirm this.
aa-notify does not need to load the policy includes for its current
features, so drop the unneeded overhead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
alid_include() checks if the given include file exists or is whitelisted in cfg\['settings'\]\['custom_includes'\].
The check if that include file is already part of the profile is unrelated to that. Move it to match_includes() where it fits much better (and drop the now superfluous profile parameter from valid_include())
In theory is_known_rule() should prevent that case from ever happening, but let's restrict this commit to moving the code around and keep this check just to be sure.
While on it, add some documentation to both functions.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/534
Acked-by: John Johansen <john.johansen@canonical.com>
Also rename aa.py delete_duplicates() and make ruletypes a parameter.
See the commit messages for details.
This reduces usage of global variables.
The final change is that aa-genprof now asks about preamble rules that are \*Rule classes (currently `abi` and `include if exists`).
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/532
Acked-by: John Johansen <john.johansen@canonical.com>
valid_include() checks if the given include file exists or is
whitelisted in cfg['settings']['custom_includes'].
The check if that include file is already part of the profile is
unrelated to that. Move it to match_includes() where it fits much
better (and drop the now superfluous profile parameter from
valid_include())
In theory is_known_rule() should prevent that case from ever happening,
but let's restrict this commit to moving the code around and keep this
check just to be sure.
While on it, add some documentation to both functions.
For now, that means aa-mergeprof will ask for `abi` and `include if
exists` rules (currently hardcoded).
This needs storing of `active_profiles` in the Prof object - the
preamble `abi` and `include if exists` rules are stored there.
Since several functions expect an `include` dict, add an empty one to
ProfileList to prevent lots of errors and breakage. It can be removed
again when handling of `include` rules gets moved to IncludeRule.
Rename delete_duplicates() to delete_all_duplicates() to make the
function easier grep-able - the *rule classes have delete_duplicates()
which might be confused with the old name.
Also hand over 'ruletypes' as parameter to delete_all_duplicates()
instead of using the global variable.
ask_the_questions() stays the "main" function, loops over all profiles,
and calls ask_rule_questions() for each profile.
ask_rule_questions() asks the questions for all events in a specific
profile or hat.
This reduces the usage of global variables in ask_rule_questions().
The TODO in write_rules() was long solved - remove the safety net type
check, it's no longer needed.
Without this, the function becomes a simple .get_rules_clean() call
which can easily be inlined into write_piece() at the two places where
it's called.
Replace usage of filelist\[file_name\]\['profiles'\] with the `active_profiles` ProfileList which already has the same data.
Add `profiles_in_file()` to ProfileList as a nice interface to get the list of profiles in a file.
For more details, check the individual commits.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/523
Avked-by: John Johansen <john.johansen>
... but not for abi rules, which (according to the simple_tests
profiles) do not share these bugs)
For unquoted paths, make sure that the path doesn't include whitespace.
Don't commingle stderr and stdout by default, and provide a
function that provides the exit value, stderr, and stdout as a
tuple. Also, expect UTF-8 output by default from the commands (with
universal_newlines), rather than bytes.
(Nifty commands like subprocess.run() weren't added to python until
python 3.5.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/522
... because after the previous three commits, nothing reads/needs this
anymore
Note: file_name in ask_exec() was only used in the (dropped) filelist
usage.
Instead of checking filelist[file]['profiles'] for duplicate hats, check
profile_data[profile][hat].
With this, the duplicate hat check is done in the same way as the check
for duplicate profiles and child profiles.
Also add tests for duplicate child profiles and duplicate hats.
Using a list as parameter for subprocess.call is more secure, and avoids
funny problems if the filename to display contains spaces or other funny
characters.
When aa-genprof proposes a local inactive profile, it had a hardcoded
call to 'less' to display that profile.
Unsurprisingly, this doesn't work in JSON mode and breaks YaST (luckily
it's only a case of "the button doesn't work").
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1171315
... which can mean "too late" in some special cases (if a profile
already exists in /etc/apparmor.d/$non_default_filename).
However, the main reason is that without this change
- the new profile will be added to (otherwise empty) active_profiles
- the first do_logprof_pass() will read all profiles, including the new
one, and add them to active_profiles - which unsurprisingly results in
an error like `ERROR: Profile /usr/sbin/vsftpd exists in
/etc/apparmor.d/usr.sbin.vsftpd and /etc/apparmor.d/usr.sbin.vsftpd`
To fix this,
- change do_logprof_pass to never call read_profiles() (and get rid of
the 'passno' parameter)
- adjust its callers (aa-logprof and aa-genprof) to call read_profiles()
themself
- move printing the 'Updating AppArmor profiles in $directory.' message
to read_profiles(), but only display it if requested (to keep the
current UI behaviour)
Since 4b7108f1e5, aa-genprof and
aa-autodep crashed with
apparmor.common.AppArmorBug: /etc/apparmor.d/usr.sbin.vsftpd not listed in ProfileList files
because the profile was not added to active_profiles.
Add the profile to active_profiles to fix this regression, and to have
all the needed data in active_profiles.
The profile repo is dead since years and most likely won't come back, so
there's no point in keeping and maintaining the code for uploading and
downloading profiles.
The parser tests were missing include tests for include entries in the
preamble section of a profile. This commit adds both #include and
include variants, as well as include if exists variants.
Also added is an exception list for the utils tests -- though it should
be noted that the utils silently drop the "#include if exists" format if
it's in the preamble without raising an exception.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/509
- add_inc_ie() stores include and include if exists rules
- get_clean() and get_raw() return the profile preamble (currently only
the include rules)
Also add tests for the new functions.
This is similar to get_clean(), but keeps the original rule order
instead of sorting them.
This is useful for include rules in the preamble, where the order might
be relevant - for example if the first include defines a variable that
is then used or extended in the second include file.
With the addition of commits to make libapparmor LSM stacking aware, several tests were failing due to:
- aa_getpeercon() having an added aa_enabled() check
- aa_getprocattr() looking in /proc/pid/attr/apparmor/current first
Fix the specific failing testcases with additional permissions, and grant default profiles and hat generated by mkprofile.pl write access to /proc/*/attr/apparmor/current.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/504
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Because of the need to be stacking LSM aware, aa_getpeercon() calls
aa_enable to ensure that apparmor is enabled. Without the permission,
aa_getpeercon() fails, causing test failures.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
While xattr attachments exist without outofband transitions, they are
broken when values contain the null character. The addition of
outofband transitions fixes this problem.
While we could make the test requirement for outofband transitions
specific to a few tests, since the parser currently requires outofband
transitions to generate xattr attachment conditionals, for a first
pass just make the whole test suite require them.
Revisit this when/if the parser will allow xattr attachments without
outofband transitions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <sbeattie@ubuntu.com>
For now, only 'include if exists' rules will be handled by IncludeRule.
Using it also for 'include' rules needs some more code changes so that
included files still get checked etc.
Also remove some testcases from test-parser-simple-tests.py unknown_line
which no longer fail.
These classes are meant to handle 'include' and 'include if exists'
rules.
Due to restrictions in re_match_include_parse(), some cases in
is_covered_localvars() and is_equal_localvars() can't be reached in the
unittests.
Also, IncludeRule isn't used in aa-logprof (yet?), which means
logprof_header_localvars() result format isn't decided yet, and
therefore not tested.
This means test coverage for the new classes isn't 100% this time ;-)
LSM stacking adds the new /proc/<pid>/attr/apparmor directory
dedicated to apparmor so that it won't collide with other LSMs.
The library has already been switched to using this interface by
default, but the parser need to add permissions to access this
location when hats and/or change_profile rules are defined.
Also make sure to update the apache abstraction to support the new
interface as well.
Signed-off-by: John Johansen <john.johansen@canonical.com
Acked-by: Seth Arnold <seth.arnold@canonical.com>
- extend RE_INCLUDE to also match "include if exists"
- rename re_match_include() to re_match_include_parse() and extend it to
also support "include if exists" rules. The return value also includes
"ismagic" now to avoid another future change, but that's not used yet.
- add re_match_include() which is now a wrapper around
re_match_include_parse() and behaves exactly as the old
re_match_include()
AppArmor 3 exposes kernel support for the kill and unconfined profile
modes. Make sure aa-status has basic support for these modes.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Processes that are confined by multiple profiles in a stack can have
more than one profile mode applied. Allow aa-status to report
processes that are in a mixed profile confinement mode.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Switch aa-status over to using the libapparmor proc accessor. This
will ensure that aa-status works correctly under LSM stacking kernels,
and if other new interfaces are added in the future.
Also at the same time switch over the splitting of context's to
the library provided aa_splitcon() fn.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
Make aa-enabled able to distinguish being enabled and enabled without
access to the shared LSM iterfaces.
Signed-off-by: John Johansen <john.johansen@canonical.com>
With LSM stacking apparmor may be available even if it is not
enabled. If apparmor is available but not enabled it does not own
the /proc/<pid>/attr/* interface and should not use it as they
will not provide apparmor context.
Note: this also applies to the SO_PEERSEC interface, currently we
only fail is apparmor does not own the interface. Once the new interface
command is finalized supprt to use that will be added.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
LSM stacking makes the old /proc/<pid>/attr/* interfaces potentially
be shared such that the values returned might be for a different LSM
than apparmor.
Use the new apparmor dedicated interface at /proc/<pid>/attr/apparmor/*
if it is present.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve.beattie@canonical.com>
libapparmor and the pyton tools now require python3 but on systems
where python2 is the default the build will fail unless the PYTHON=
env var is set to the systems python3.
Instead just force autoconf to search for python3 as the python
version to use.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When libintl is available on the system, we need to link against the
libintl library too.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
In order to have all the prerequisite folders before actually installing
os dependent configuration files, we need to guarantee the correct
install sequence.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
If Swig builds the library against python 2 and the utils/tests
run using python3, parts of the library are in accesible with
failures like
Traceback (most recent call last):
File "test-pivot_root_parse.py", line 12, in <module>
import apparmor.aa as aa
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/aa.py", line 28, in <module>
import apparmor.logparser
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/logparser.py", line 19, in <module>
import LibAppArmor
ImportError: No module named LibAppArmor
Signed-off-by: John Johansen <john.johansen@canonical.com>
$ make check
will fail unless PYFLAKES=pyflakes3 is passed as an environment
variable. Fix it so make check will work by default.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The apparmor python tools are written assuming python3. But the tests
are still using python2. This can result in failures of the test suite
on some systems, depending on how libapparmor is built.
Eg. on Ubuntu 18.04
apparmor.git/utils$ PYTHON=/usr/bin/python3 make check PYFLAKES=/usr/bin/pyflakes3
LANG=C podchecker -warning -warning *.pod
aa-audit.pod pod syntax OK.
aa-autodep.pod pod syntax OK.
aa-cleanprof.pod pod syntax OK.
aa-complain.pod pod syntax OK.
aa-decode.pod pod syntax OK.
aa-disable.pod pod syntax OK.
aa-easyprof.pod pod syntax OK.
aa-enforce.pod pod syntax OK.
aa-genprof.pod pod syntax OK.
aa-logprof.pod pod syntax OK.
aa-mergeprof.pod pod syntax OK.
aa-notify.pod pod syntax OK.
aa-remove-unknown.pod pod syntax OK.
aa-sandbox.pod pod syntax OK.
aa-status.pod pod syntax OK.
aa-unconfined.pod pod syntax OK.
logprof.conf.pod pod syntax OK.
aa-notify syntax OK
Checking aa-easyprof
Checking aa-genprof
Checking aa-logprof
Checking aa-cleanprof
Checking aa-mergeprof
Checking aa-autodep
Checking aa-audit
Checking aa-complain
Checking aa-enforce
Checking aa-disable
Checking aa-status
Checking aa-unconfined
Checking apparmor
Checking test/common_test.py
Checking test/minitools_test.py
Checking test/test-aa-decode.py
Checking test/test-aa-easyprof.py
Checking test/test-aa.py
Checking test/test-aamode.py
Checking test/test-aare.py
Checking test/test-baserule.py
Checking test/test-capability.py
Checking test/test-change_profile.py
Checking test/test-common.py
Checking test/test-config.py
Checking test/test-dbus.py
Checking test/test-example.py
Checking test/test-file.py
Checking test/test-libapparmor-test_multi.py
Checking test/test-logparser.py
Checking test/test-mount_parse.py
Checking test/test-network.py
Checking test/test-parser-simple-tests.py
Checking test/test-pivot_root_parse.py
Checking test/test-profile-list.py
Checking test/test-profile-storage.py
Checking test/test-ptrace.py
Checking test/test-regex_matches.py
Checking test/test-rlimit.py
Checking test/test-severity.py
Checking test/test-signal.py
Checking test/test-translations.py
Checking test/test-unix_parse.py
make[1]: Entering directory '/home/jj/apparmor/apparmor-jjohansen.git/utils/test'
=== test-example.py ===
.......
----------------------------------------------------------------------
Ran 7 tests in 0.000s
OK
.......
----------------------------------------------------------------------
Ran 7 tests in 0.000s
OK
=== test-pivot_root_parse.py ===
Traceback (most recent call last):
File "test-pivot_root_parse.py", line 12, in <module>
import apparmor.aa as aa
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/aa.py", line 28, in <module>
import apparmor.logparser
File "/home/jj/apparmor/apparmor-jjohansen.git/utils/apparmor/logparser.py", line 19, in <module>
import LibAppArmor
ImportError: No module named LibAppArmor
Makefile:81: recipe for target 'check' failed
make[1]: *** [check] Error 1
make[1]: Leaving directory '/home/jj/apparmor/apparmor-jjohansen.git/utils/test'
Makefile:95: recipe for target 'check' failed
make: *** [check] Error 2
Signed-off-by: John Johansen <john.johansen@canonical.com>
This function was introduced as a temporary (ahem...) solution in 95404bb2f3 but was never really correct. It checked against other_value.regex (as a string!) and, while this was somewhat generous in the results, could have unintended side effects.
Better error out on the safe side and add/keep a few superfluous rules than having a wrong match in is_covered() and miss to add/keep a rule that would be needed.
The perfect solution would be to really compare one AARE against the other as the parser does. I'm not too keen to implement this in python, and will wait until someone provides this function (which the parser already has) via libapparmor ;-)
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/478
Acked-by: John Johansen <john.johansen@canonical.com>
Only bool and str keys may be overwritten.
Other key types (list, dict, *Ruleset) can have their content modified
(not __setitem__()s job), but replacing the whole collection should
never happen.
This function was introduced as a temporary (ahem...) solution in
95404bb2f3 but was never really correct.
It checked against other_value.regex (as a string!) and, while this was
somewhat generous in the results, could have unintended side effects.
Better error out on the safe side and add/keep a few superfluous rules
than having a wrong match in is_covered() and miss to add/keep a rule
that would be needed.
The perfect solution would be to really compare one AARE against the
other as the parser does. I'm not too keen to implement this in python,
and will wait until someone provides this function (which the parser
already has) via libapparmor ;-)
If create-apparmor.vim.py fails, an empty apparmor.vim gets created. The
next "make" run will assume that apparmor.vim was already created (the
file exists and has a new-enough timestamp) and will therefore skip the
create-apparmor.vim.py run, keeping the broken apparmor.vim forever.
Adjust the Makefile to delete apparmor.vim if the script fails. This
ensures that make tries again in the next run.
In focal users of mdns get denials in apparmor confined applications.
An exampel can be found in the original bug below.
It seems it is a common pattern, see
https://github.com/lathiat/nss-mdns#etcmdnsallow
Therefore I'm asking to add
/etc/mdns.allow r,
to the file
/etc/apparmor.d/abstractions/mdns"
by default.
--- original bug ---
Many repetitions of
audit: type=1400 audit(1585517168.705:63): apparmor="DENIED" operation="open" profile="/usr/sbin/chronyd" name="/etc/mdns.allow" pid=1983815 comm="chronyd" requested_mask="r" denied_mask="r" fsuid=123 ouid=0
in log. I use libnss-mdns for .local name resolution, so /etc/nsswitch.conf contains
hosts: files mdns [NOTFOUND=return] myhostname dns
and /etc/mnds.allow contains the domains to resolve with mDNS (in may case, "local." and "local"; see /usr/share/doc/libnss-mdns/README.html.)
Presumably cronyd calls a gethostbyX() somewhere, thus eventually trickling down through the name service switch and opening /etc/mdns.allow, which the AppArmor profile in the chrony package does not allow.
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1869629
Signed-off-by: John Johansen <john.johansen@canonical.com>
abstractions/mesa: allow checking if the kernel supports the i915 perf interface
See merge request apparmor/apparmor!464
Acked-by: Vincas Dargis <vindrg@gmail.com>
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and 2.13
The error:
type=AVC msg=audit(1585403559.846:34317577): apparmor="DENIED" operation="exec" profile="/usr/sbin/dnsmasq" name="/usr/libexec/libvirt_leaseshelper" pid=7162 comm="sh" requested_mas
k="x" denied_mask="x" fsuid=0 ouid=0
type=AVC msg=audit(1585403559.846:34317578): apparmor="DENIED" operation="open" profile="/usr/sbin/dnsmasq" name="/usr/libexec/libvirt_leaseshelper" pid=7162 comm="sh" requested_mas
k="r" denied_mask="r" fsuid=0 ouid=0
Looks like the path to libvirt_leasehelper is incorrect usr.sbin.dnsmasq, at least in gentoo. Patching the file fixes the problem:
issue: https://gitlab.com/apparmor/apparmor/-/issues/87
Signed-off-by: John Johansen <john.johansen@canonical.com>
This patch fixes a couple of nitpicks that I encountered packaging apparmor for buildroot:
1. In a cross-compiling environment, python executable cannot be trusted for getting the python settings because it is generally compiled for the host. For this reason, we should rely on target python-config.
1. Setup.py for libapparmor swig bindings is always called without taking into account the discovered settings from ac_python_devel.m4
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/462
When crosscompiling, setupy.py should be called passing the settings
discovered by ac_python_devel.m4 and not using the default system
settings.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
In a crosscompiling environment it's common to have a python executable
running for the host system with a python-config reporting the host
configuration and a second python-config reporting the target configuration.
In such cases, relying on the default oython-config is wrong and breaks
the cross compilation.
This patch adds a PYTHON_CONFIG variable that can be pointed to the second
python-config and fixes the rest of the m4 accordingly.
Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
Update usr.sbin.winbindd profile to allow krb5 rcache files locking
See merge request apparmor/apparmor!460
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.11..master
This way we could generate the capabilities in a way that works with
every version of make.
Changes to list_capabilities are intended to exactly replicate the old
behavior.
The library version must be consistent across releases. Since 2.13.4
and master use the same library version it needs to be updated.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Implement set of abstractions to handle opening uris via xdg-open and similar helpers used on different desktop environments.
Abstractions are intended to be included into child profile, together with bundle abstractions such as ubuntu-browsers, ubuntu-email and others, for fine-grained control on what confined application can actually open via xdg-open and similar helpers.
PR: https://gitlab.com/apparmor/apparmor/-/merge_requests/404
Acked-by: John Johansen <john.johansen@canonical.com>
Fixes two resource leaks. https://scan.coverity.com/projects/apparmor
I don't actually know how to link to the individual reports but the
first one comes from an early return. The second comes from an iterator
potentially being empty.
/run/uuidd/request is hardcoded in libuuid from util-linux and uuidd
listens on this socket to provide random and time-based UUIDs in a
secure manner (man 8 uuidd). Some applications (eg, python's uuid)
prefer to use this socket, falling back to getrandom(), /dev/urandom,
etc. Eg:
$ strace -f aa-exec -p test -- \
python3 -c 'import uuid ; print("%s\n" % str(uuid.uuid1()))'
...
socket(AF_INET, SOCK_DGRAM, IPPROTO_IP) = -1 EACCES (Permission denied)
getrandom("\x8e\x89\xa5\xe7\x39\x1b", 6, GRND_NONBLOCK) = 6
...
uuidd itself produces random numbers using getrandom() and
/dev/{,u}random (falling back to time-based if not), which are already
allowed in the base abstraction. The uuidd daemon, when available, runs
unprivileged under a dedicated user, so allowing read-only access to
/run/uuidd/request is reasonable.
Make kde-open5 abstraction more conservative by removing gstreamer
support by default. Update usage example to suggest conditionally including
gstreamer abstraction if required.
Update kde-open5 abstraction to not include accessibility abstraction by
default.
Update documentation to suggest adding it manually in child profile when
using kde-open5 if accessibility access is required by profile author.
Update exo-open abstraction to not include accessibility abstraction by
default.
Update documentation to suggest adding it manually in child profile when
using exo-open if accessibility access is required by profile author.
In case of error (for example failing to open provided uri) message box
is show, and alert sound is played. Currently `exo-open` abstraction
does not allow access to audio subsystems in result producing denials in
system logs.
Update abstraction to include `abstractions/audio`.
Implement set of abstractions to handle opening uris via xdg-open and similar
helpers used on different desktop environments.
Abstractions are intended to be included into child profile, together
with bundle abstractions such as ubuntu-browsers, ubuntu-email and others, for
fine-grained control on what confined application can actually open via xdg-open
and similar helpers.
This helped a lot to debug the CI failure caused by 3db14e8e / !430 (merged), and won't add any noise as long as configure succeeds.
Note that this MR is based on master without !433 (merged) which means it will fail in CI (and display config.log). After !433 (merged) got merged, I'll rebase it to show that it won't add additional noise as long as everything works.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/434
Acked-by: John Johansen <john.johansen@canonical.com>
Allow /usr/etc/ in abstractions/authentication
openSUSE (and hopefully some other distributions) work on moving shipped
config files from /etc/ to /usr/etc/ so that /etc/ only contains files
written by the admin of each system.
See https://en.opensuse.org/openSUSE:Packaging_UsrEtc for details and
the first moved files.
Updating abstractions/authentication is the first step, and also fixes
bugzilla.opensuse.org/show_bug.cgi?id=1153162
See merge request apparmor/apparmor!426
Acked-by: John Johansen <john.johansen@canonical.com> for 2.12..master
Some applications queries network configuration (using QNetworkConfigurationManager class in Qt and similar), and that produces DBus denials under AppArmor confinement when NetworkManager backend is used.
Add abstraction that allows most common read-only DBus queries for getting current network configuration from NetworkManager backend.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/409
Acked-by: John Johansen <john.johansen@canonical.com>
Requires NPAPI which no major browsers support anymore: Mozplugger npviewer / nspluginwrapper
The following have been discontinued Google Talk (and plugin) f-spot conkeror galeon Mozilla Prism rekonq Adobe Reader for Linux
Other gnome-codec-install is not in Ubuntu anymore gstreamer0.10 was replaced by 1.0
PR: https://gitlab.com/apparmor/apparmor/merge_requests/417
Acked-by: John Johansen <john.johansen@canonical.com>
Add support for out of band transitions that be to separate elements that can contain any input character. Out of band transitions can only be triggered by explicit transitions in the code (instruction stream). They function the same as the null transition except they work for all input and when writing expressions there is no need to not match them.
The out of band transitions show up in the dfa as negative offsets from the base. Currently only a single out of band transition of -1 is supported, but it is fairly trivial to add new ones if needed. The negative offset for out of band transitions were chosen instead of positive offsets that are out of input range to leave open the option of extending the input range in the future.
kernel patches: apparmor-kernel!6
v2.
fix spacing error identified by cboltz
update commit messages and signed-off-bys
allow globbing in xattrs to match NULL character
free leaked xattr cond_entry_list
make xattr generation conditional on kernel support
v3.
fix crash in commit: parser: make sure xattr cond_entry_list is not leaked
port eric's patch from !362 "support matching xattr keys but not values" to work with out of band transitions, and not a separate value array
v4.
double free fix crash of xattr.name in parser: make sure xattr cond_entry_list is not leaked
remove xattr/bad_XX tests that are no longer bad because attr names do not have to have a value specified
v5
fix bug in append_rule where the out of band transition was being converted to character 255
v6
fix bug in writing of dfa header flags value
v7
fix bug in out of band transitions where they could in certain cases be matched by an expression and hence not function as a separator.
fix xattr encoding so xattr presence can be determined distinctly from xattr value
fix xattr encoding so xattrs are not treated as optional
fix xattr encoding so that permissions are properly encoded at the end of each segment match
update xattr regression tests to make failures unique and reflect that nul character values are now allowed
v8
update transchar comment
v9
update tranchar comment
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time Out
Update the xattr_profile.sh tests so that
- each test can be uniquely identified
- the embedded nul tests are updated to the now supported/passing state
Signed-off-by: John Johansen <john.johansen@canonical.com>
The current encoding makes every xattr optional and uses this to
propogate the permission from the tail to the individual rule match
points.
This however is wrong. Instead change the encoding so that an xattr
(unless optional) is required to be matched before allowing moving
onto the next xattr match.
The permission is carried on the end on each rule portion file match,
xattr 1, xattr 2, ...
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make sure we can support optional xattrs distinct from optional xattr
values in the encoding.
Currently all xattrs specified are required to be present even
if there value is not specified. However under the old encoding there
was no way to distinguish if the presence of the xattr vs. the
xattr having a null length value.
Fix this so that if we decide to support optional xattrs it is possible
without having to change the abi.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Support profiles that choose to match the presence of an extended
attribute without validating its value. This lets AppArmor target xattrs
with binary data, such as security.ima and security.evm values. For
example, it's now possible to write a profile such as:
profile signed_binaries /** xattrs=(security.ima) {
# ...
}
Both presence and value matches can be used in the same profile. To
match a signed xattr, target both the xattr and the security.ima value:
profile python_script /** xattrs=(
security.evm
security.apparmor="python"
) {
# ...
}
Updated to work using out of band matching instead of separate data
array.
Signed-off-by: Eric Chiang <ericchiang@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs are a byte string that can contain all input characters including
the null character. Allow * ** and ? glob patterns to match the null
character while retaining their apparmor characteristics for '/'.
That is * and ? won't traverse a '/' treating it as a path element.
While ** will match anything.
Signed-off-by: John Johansen <john.johansen@canonical.com>
xattrs can contain NULL characters in their values which means we can
not user regular NULL transitions to separate values. To fix this
use out of band transition instead.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the NULL character is used as an out of band transition
for string/path elements. This works for them as the NULL character
is not valid for this data. However this does not work for binary
data that can contain a NULL character.
So far we have only dealt with fixed length fields of binary data
making the NULL separator either unnecessary.
However binary data like in the xattr match and mount data field are
variable length and can contain NULL characters. To deal with this
add the ability to specify out of band transitions, that can only
be triggered by code not input data.
The out of band transition can be used to separate variable length
data fields just as the NULL transition has been used to separate
variable length strings.
In the compressed hfa out of band transitions are expressed as a
negative offset from the states base. This leaves us room to expand
the character match range in the future if desired and on average
makes the range between the out of band transition and the input
transitions smaller than would be had if the out of band transition
had been stored after the valid input transitions.
Out of band transitions in the dfa will not break old kernels
that don't know about them, but they won't be able to trigger
the out of band transition match. So they should not be used unless
the kernel indicates that it supports them.
It should be noted that this patch only adds support for a single
out of band transition. If multiple out of band transitions are
required. It is trivial to extend.
- Add a tag indicating support in the kernel
- add a oob max range field to the dfa header so the kernel knows
what the max range that needs verifying is.
- extend oob generation fns to generate oob based on value instead
of a fixed -1.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the parser is not correctly setting the dfa flag value
and it hasn't been caught because base policy uses a flag value
of 0.
Signed-off-by: John Johansen <john.johansen@canonical.com>
As a step in preparing for out of band transitions and double walk
transitions rework the backend from using a char index to a class
with an larger range than char.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add nnp tests around the unconfined exception.
An unconfined task is allowed changing to a profile under nnp restrictions. However this has implications when the task has stacked confinement.
profile//&unconfined
profile//&:ns:unconfined
...
will not be caught by the unconfined test. Instead the unconfined exception needs to be tested as part of the subset test. Add tests to ensure we can catch regressions around a stacked unconfined profile.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/424
Signed-off-by: John Johansen <john.johansen@canonical.com>
pyflakes 2.x nowadays complains about
aa-genprof:60: undefined name 'PermissionError'
We already deprecated py2 support in 2.11 (and obviously nobody tests
with py2 anymore), therefore recommending pyflakes3 makes sense.
Maybe we should also change our Makefiles to use py3 and pyflakes3 by
default, but that will be another (master-only) commit.
'localinclude' is/was meant to have 'local/*' includes separate, but
it's write-only and never used, which makes it useless.
Additionally, it causes a crash in the aa-* tools which gets fixed by
removing all the 'localinclude'-related code (what a big word for two
lines ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1848227
openSUSE (and hopefully some other distributions) work on moving shipped
config files from /etc/ to /usr/etc/ so that /etc/ only contains files
written by the admin of each system.
See https://en.opensuse.org/openSUSE:Packaging_UsrEtc for details and
the first moved files.
Updating abstractions/authentication is the first step, and also fixes
bugzilla.opensuse.org/show_bug.cgi?id=1153162
The unconfined exception needs to be applied even when a stack is
being used. When a stack is in use it prevents the unconfined
test from being used and instead it must be done as part of the
subset test.
Signed-off-by: John Johansen <john.johansen@canonical.com>
drop deprecated perl code
* drop repair_obsolete_profiles (updated profiles for abstraction renames pre-2006)
* drop ancient perl aa-* tools and their modules (rewritten to python in 2.9)
* drop rc.aaeventd.{redhat,suse} initscripts (aaeventd was dropped long ago, therefore we don't need to keep its initscripts)
See merge request apparmor/apparmor!423
Acked-by: John Johansen <john.johansen@canonical.com>
Applications running under Xwayland in a GNOME+Wayland session need read access to this file since:
a8984a81c2
… that was first included in mutter 3.33.3.
This rule is presumably only needed for GNOME+Wayland sessions, so one could argue that it should live in abstractions/wayland instead, but Jamie argued that it should be in the X abstraction because Xwayland is a X server.
MR: https://gitlab.com/apparmor/apparmor/merge_requests/419
Bug-Debian: https://bugs.debian.org/935058
Acked-by: John Johansen <john.johansen@canonical.com>
879531b36ec3dfc7f9b72475c68c30e4f4b7b6af changed access for
@{HOME}/.{,cache/}fontconfig/** to include 'w'rite. Fontconfig has been
a source of CVEs. Confined applications should absolutely have read
access, but write access could lead to breaking out of the sandbox if a
confined application can write a malformed font cache file since
unconfined applications could then pick them up and be controlled via
the malformed cache. The breakout is dependent on the fontconfig
vulnerability, but this is the sort of thing AppArmor is meant to help
guard against.
Bug-Debian: https://bugs.debian.org/935058
Applications running under Xwayland in a GNOME+Wayland session need read access
to this file since:
a8984a81c2
… that was first included in mutter 3.33.3.
This rule is presumably only needed for GNOME+Wayland sessions, so one could
argue that it should live in abstractions/wayland instead, but Jamie argued that
it should be in the X abstraction because Xwayland is a X server.
Requires NPAPI which no major browsers support anymore:
Mozplugger
npviewer / nspluginwrapper
The following have been discontinued
Google Talk (and plugin)
f-spot
conkeror
galeon
Mozilla Prism
rekonq
Adobe Reader for Linux
Other
gnome-codec-install is not in Ubuntu anymore
gstreamer0.10 was replaced by 1.0
With the effort to remove perl, it makes no sense to keep the depracted
perl code around for new releases.
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa-status crashed if a profile name contains an opening parenthesis
because the regex enforces (simplified) '^[^(]* \(.*\)' when reading
/sys/kernel/security/apparmor/profiles
This obviously doesn't match if a profile name contains '(' which is
rare and strange, but still allowed, and the match result "None" then
crashes aa-status.
Adjust the regex to allow all chars instead of all except '(' to handle
these corner cases.
Note that '(enforce)' and '(complain)' still get read correctly because
the regex ends with '\((\w+)\)$' and therefore enforces matching
"something inside parenthesis at the end of the line".
This bug exists since aa-status was rewritten into python, and even
existed in the perl version before. However, in the perl version, the
regex matching was protected with an if so profile names with '(' were
skipped and hidden from the aa-status output.
Fixes: https://gitlab.com/apparmor/apparmor/issues/51
When a filesystem is mounted with the option *user*, the file selection
dialogue, e.g. in *Evince*, triggers an access of */run/mount/utab*, which
comes from *libmount* and should be allowed.
Winbind requires access to /var/cache/samba/msg.lock/*.
Move msg.lock/ to abstractions/samba.
On Arch Linux Winbind's pid is set to /run/winbindd.pid.
Signed-off-by: nl6720 <nl6720@gmail.com>
As discussed in https://gitlab.com/apparmor/apparmor/merge_requests/395
RE_LOG_ALL should be more broad so that it doesn't accidently
overlook/ignore valid log events.
Instead of adding more and more known log formats to logparser.py,
simplify the regex to do only a basic check.
If we really hand over a line to libapparmor that isn't an AppArmor log
event, worst thing that can happen is that we waste a few milliseconds
for handing over that line to libapparmor, and get AA_RECORD_INVALID as a
result.
Some applications queries network configuration (using
QNetworkConfigurationManager class in Qt and similar), and that produces
DBus denials under AppArmor confinement when NetworkManager backend is
used.
Add abstraction that allows most common read-only DBus queries for
getting current network configuration from NetworkManager backend.
Improve readability by being explicit about the value to use when the
conditional evaluates to true.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Suggested-by: John Johansen <john.johansen@canonical.com>
Commit ad81ea0e67 ("tests: Add option to dump policy cache dir with
the libapparmor wrapper") modified aa_policy_cache.c to call
aa_policy_cache_dir_path_preview(). That added a hard dependency on
libapparmor >= 2.13, which is the first version to add
policy_cache_dir_path_preview() to libapparmor. The dependency makes it
impossible to build and run the upstream regression tests against an
installed libapparmor older than 2.13 due to aa_policy_cache.c failing
to build.
Add a compatible aa_policy_cache_dir_path_preview() when building
aa_policy_cache.c against a libapparmor older than 2.13 and newer than
or equal to 2.10.
Fixes: ad81ea0e67 ("tests: Add option to dump policy cache dir with the libapparmor wrapper")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Correct the long option used to print the cache directory.
Fixes: e9d9395f91 ("parser: Add option to print the cache directory")
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Test the profile transition limits imposed by NO_NEW_PRIVS to ensure
that behavior doesn't unexpectedly change.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
commit 7297e2f6a8 assumed that exec events
always have a "target=...", but this is only true for events in complain
mode. In enforce mode, the log line doesn't include "target=...".
This commits sets an empty target instead of erroring out on every exec
event in enforce mode.
Also adjust ask_exec() in aa.py to only update
hashlog[aamode][target_profile]['final_name'] if target_profile is set
(hashlog[aamode][''] doesn't exist and trying to write to it would error
out)
- in complain mode, exec events include target=...
- in enforce mode, the log message doesn't include target=...
Note that the utils tests skip exec events (they'd be interactive),
therefore exec*.profile doesn't contain an exec rule.
convert_regexp() needs to escape '(' and ')' in filenames taken from a
logfile to get rid of their special meaning, and to avoid a crash on
unbalanced parenthesis (which makes the regex invalid if they are not
escaped).
Note: The added tests include an example log line, but the tests itsself
don't/didn't trigger the crash because they don't call convert_regexp().
Fixes: https://bugs.launchpad.net/bugs/1835311
v2:
- parse partial log line broken at \n
- add testcase_dbus_10.* for partial log line
- remove quotes from testcasw_dbus_09.profile
The following log format has been seen in the wild, and currently results
in a RECORD_INVALID
[4835959.046111] audit: type=1107 audit(1561053426.749:186): pid=640 uid=103 auid=4294967295 ses=4294967295 msg='apparmor="ALLOWED" operation="dbus_method_call" bus="system" path="/org/freedesktop/systemd1" interface="org.freedesktop.systemd1.Manager" member="LookupDynamicUserByName" mask="send" name="org.freedesktop.systemd1" pid=20596 label="/usr/sbin/sshd" peer_pid=1 peer_label="unconfined"
exe="/usr/bin/dbus-daemon" sauid=103 hostname=? addr=? terminal=?'
Test parsing the above message with and without the \n embedded between
peer_label= and exec=
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add for Certbot on openSUSE Leap
The default path is `/etc/certbot/archive/{some domain}/{file name}.pem`
See merge request apparmor/apparmor!397
Acked-by: Christian Boltz <apparmor@cboltz.de>
The mult_mount test creates a small disk image, formats it, and mounts
it in multiple locations in preparation for the tests. However, the
created raw file (80KB) is too small to make a working file system if
4K blocks are used by mkfs. In Ubuntu 19.10, the default was recently
changed for mkfs to default to always using 4K blocks, causing the
script to fail.
We could force mkfs to use 1K blocks, but instead, in case some future
version of mkfs decides not to support 1K blocks at all, we bump up the
size of the disk image to 512KB; large enough to work with 4K blocks
yet small enough to be workable in small scale test environments.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Bug: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1834192
MR: https://gitlab.com/apparmor/apparmor/merge_requests/396
Instead of setting SFS_MOUNTPOINT in is_apparmor_loaded() (which is
called in most cases) and in is_container_with_internal_policy() (which
covers/fixes the remaining cases), set it globally.
This also fixes a bug in is_container_with_internal_policy() (introduced
in f10e72a14f) where the variable
definition tried to use the no longer existing $MODULE variable and
therefore got a wrong path for $SFS_MOUNTPOINT.
Besides this bug, there's a minor behaviour change / improvement if
securityfs isn't mounted - "file not found" error messages will now
contain the full/correct path ;-)
This change/cleanup is a follow-up of
https://gitlab.com/apparmor/apparmor/merge_requests/363 and some IRC
discussions 2019-04-16.
When dc010bc034 was
backported to the apparmor-2.13 branch (in commit
75236d62e2), it did not take into
account cb8c3377ba, which creates the
common/list_af_names.sh script as used in the test case, was not also
backported to the apparmor-2.13 branch.
Change the test case to get the list of network AF names via the same
make invocation taken by the utils/vim/create-apparmor.vim.py script
before the common/list_af_names.sh existed.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/391
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
update postfix profiles
On openSUSE Leap 15.1, the postfix binaries live in /usr/lib/postfix/bin/ which was not covered in the postfix.\* attachment and mrix rules.
Also add several permissions to the postfix.\* profiles needed on openSUSE Leap 15.1.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/380
Signed-off-by: John Johansen <john.johansen@canonical.com>
is_container_with_internal_policy() is called independently of
apparmor_*() in the systemd unit and potentially other consumers of
rc.apparmor.functions. When the unit and rc.apparmor.functions functions
were rewritten, they were written so that SFS_MOUNTPOINT was only set in
is_apparmor_loaded(), but this is only called in apparmor_start(),
remove_profiles(), apparmor_kill(), apparmor_restart(), apparmor_try_restart()
and apparmor_status() and not is_container_with_internal_policy().
While it is clear that is_container_with_internal_policy() is meant to
be called before apparmor_start(), is is unclear why SFS_MOUNTPOINT is
only defined in is_apparmor_loaded(). There are several ways to fix
this:
1. update is_container_with_internal_policy() to call is_apparmor_loaded()
2. identify the callers of is_container_with_internal_policy() and have
them call is_apparmor_loaded()
3. reorganize the code to remove duplicate calls and assignments
4. define SFS_MOUNTPOINT along with SECURITYFS and MODULE, at the top
level
5. also define SFS_MOUNTPOINT in is_container_with_internal_policy()
'1' would result in redundant calls in many common cases since the
systemd unit would call is_apparmor_loaded() both in
is_container_with_internal_policy() and prior to other calls.
'2' would like break consumers of rc.apparmor.funcions, like
Debian/Ubuntu's profile-load.
'3' is perhaps ok, but requires more effort and is regression-prone.
'4' seems the simplest, most correct fix
'5' is what this patch implements, which is as simple as '4' but tries
to maintain the original author's intent of when to set SFS_MOUNTPOINT.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/363
Signed-off-by: Jamie Strandboge <jamie@strandboge.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add a target to install build dependencies for the apparmor regression
tests. Currently supports Ubuntu and Debian distros.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The translated action character for Deny conflicted with the
untranslated action character for Finish in the Swedish translation.
Remote it, and hope for more action translations.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
The parser currently skips the cache if optimizations are specified
because it can not determine if the cached policy was compiled
with the specified optimization. However this causes cache misses
even if policy is cached with those options, and distros are setting
some optimizations by default.
Instead of skipping reading the cache if optimizations are set, users
can force overwriting the cache if needed, until the parser can
store aditional meta info in the cache.
BugLink: http://bugs.launchpad.net/bugs/1820068
Signed-off-by: John Johansen <john.johansen@canonical.com>
finishing was always false, resulting in always returning 'NORMAL'.
Remove the variable, remove the unused condition - and change the only
place (aa-genprof) that expected a return value from do_logprof_pass()
to not expect it anymore.
found by Coverity, CID 198637
Rewrite log handling
logparser.py puts each log event on a big "stack" in self.pid. Later,
handle_children() in aa.py then converts that (named 'log' in aa.py) to
the prelog hasher.
This commit changes logparser.py to create a prelog-like structure itsself
(named hashlog), which
- removes one level of indirection
- probably saves some memory because the hashlog automatically
de-duplicates events
In aa.py, collapse_log() gets updated to work with hashlog. (There's
also a handle_hashlog() function in this patch series, but it didn't
survive the final patches ;-)
OTOH, the now superfluous code handling capability, network etc.
events gets removed from handle_children(). The remaining parts of
this function get split into ask_exec() and ask_addhat().
logparser.py gets a new function init_hashlog() to initialize hashlog
for each profile. It also gets changed to store capability, network etc.
events into hashlog instead of storing them in self.pid.
hashlog uses the full profile name as key, which is the first baby step
to support nested child profiles. (for now, collapse_log() still
splits the profile name into profile and hat.)
There are many more details, see the individual commits ;-)
See merge request apparmor/apparmor!377
Acked-by: Seth Arnold <seth.arnold@canonical.com>
looping through the first 16 loop devices to find a free device will
fail if those mount devices are taken, and unfortunately there are
now services that use an excessive amount of loop devices causing
the regression test to fail.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
If final_name still includes null-*, that's most likely caused by nested
execs which aren't supported by the tools yet. Ignoring them is better
than creating a useless null-* hat.
Note: The tools always had this restriction, so this is not a regression ;-)
Also note that test-libapparmor-test_multi expects that null-* hats get
created (which makes sense because the one-line log sniplets don't have
any exec indication), therefore add an optional parameter to keep this
behaviour for the tests.
Now that all log events arrive in hashlog, having a separate 'prelog' no
longer makes sense. Changing collapse_log() to accept 'hashlog' directly
removes that level of indirection.
It's only used by two functions:
- handle_hashlog() - writes to prelog, and now returns it
- collapse_log() - reads prelog, and now gets it as parameter
'final_name' by default is the profile name, but ask_exec() will change
it for the target profile (which is a null-* profile at this stage)
based on exec mode choice. ask_addhat() will also change it based on the
chosen hat.
Choosing "deny" or "unconfined" will result in an empty final_name and
ignoring these log events.
All other choices set final_name to the full profile name ("foo" for Px,
"foo//bar" for Cx, current profile for ix).
Also fix the order of handling log events - since ask_exec() changes the
hashlog final_name, it has to run first so that ask_addhat() (which
"only" adjusts the hat name in final_name) and handle_hashlog() can work
with the updated profile name.
Finally, update test-libapparmor-test_multi.py to ignore final_name when
checking if hashlog is empty, and fix the call order of ask_exec() etc.
profile and hat don't get used in the following lines, and later get
overwritten in the next round of the for loop.
The deleted code was last useful for setting the (dropped)
profile_changes array.
... and self.pid which is also unused.
This simple change also means to adjust all the code that uses ReadLog.
We get rid of log_pid in aa.py, and have to change lots of test-*
After all the changes in this branch, parse_event_for_tree always
returns None, which makes the "if event is not None" branch dead code.
This branch was the only place where add_to_tree() was called, therefore
remove this function.
This also makes self.pid and self.log unused. They'll be removed with
separate commits.
The only remaining job of handle_children() was to handle exec events.
(And recursively calling itsself if it hits nested log events, but
logparser.py never created such a log structure.)
Therefore:
- drop the dead code handling nested log (type != str)
- rename the remaining function to ask_exec()
- drop checks for typ = 'exec' (now done as part of the for loop
- drop the "else" branch for unknown event types
- change 'return' to 'continue' because ask_exec handles all exec events
in a loop instead of being called multiple times
- oh, and of course switch over to using hashlog
Finally, change do_logprof_pass() and the tests to call ask_exec()
instead of handle_children().
While on it, update a comment in test-translations.py which held the
last reference to handle_children().
The 'hat' and 'prog' variables are no longer needed, drop them.
Also no longer include denied_mask in the event - operation='exec' means
an exec event, no need to additionally hand over 'x' permissions.
Note: This commit introduces a "brain split", which means exec handling
is temporarily broken. Later commits will fix this.
parse_event_for_tree() always sets prog = 'HINT' and to_name = '' for
exec events.
The only exception is in the 'mandatory profile missing' case where
prog = aamode and to_name = target_profile, but I've never seen that in
practise.
This means the prog != 'HINT' branch in exec handling never gets used
and can be dropped.
Erroring out "if to_name:" also never gets used (to_name is always ''),
therefore drop it as well.
- replace/merge 'uhat' with 'hat'
- adjust ProfileStorage comment to the new function name
- initialize default hat if it gets chosen and doesn't exist yet
- don't exit the function if "Deny" gets chosen. Instead, continue with
the next hat to (possibly) add
Adjust logparser.py to store change_hat events in hashlog.
In aa.py,
- split off ask_addhat() from handle_children()
- change ask_addhat() to use hashlog
- call ask_addhat() from do_logprof_pass()
Also call ask_addhat() in test-libapparmor-test_multi.py to keep it in
sync with do_logprof_pass().
All the variables initialized at the start of the function always get
overwritten in the loop, therefore it's superfluous to initialize them.
Also inline 'entries' to the only place that uses it.
According to John, this isn't supported since 10 years.
In case you wonder why this commit removes the "fork" handling in
handle_children() - logparser.py names clone events "fork" on the event
stack.
In logparser parse_event_for_tree() path event handling, drop mapping
permissions for request_mask because request_mask never gets used.
Also drop the validate_log_mode() call because the function has its own,
more strict check since the last commit.
In aamode.py, drop the now unused validate_log_mode() and
hide_log_mode() functions and the LOG_MODE_RE regex.
Finally, drop the validate_log_mode() tests from test-aamode.py
In logparser.py parse_event_for_tree, convert path handling to hashlog.
While on it, include 'owner' as part of hashlog so that aa.py doesn't
need to guess.
Also switch to a simple for loop instead of using log_str_to_mode() from
aamode.py to convert denied_mask to hasher keys (which would have been
needed to allow merging of several log events for the same path anyway).
Note that the check for 'mrawlk' (intentionally without 'x') is more
strict than the validate_log_mode(), but it should still cover all file
permissions. (validate_log_mode() also allows things like 'Px', which
we'll never hit in a logfile.)
In aa.py collapse_log() update the handling of path events to match the
additional [owner] key in hashlog/prelog. This makes the owner detection
in collapse_log() superfluous.
In aa.py handle_children(), remove 'path' handling from the 'path' or
'exec' section, and add an 'if True:' to avoid lots of whitespace
changes.
In aamode.py, drop the now unused split_mode() function, and
AA_OTHER_REMOVE() that was only used by split_mode().
Finally, remove sample log events with null-* hats from the list of
known failures in test-libapparmor-test_multi.py (we no longer filter
out null-* hats), and fix whitespace in two expected profiles.
According to John, 'inode_permission' wasn't used for 10 years.
This little change also means that we now have a clear separation
between "exec" and "normal" (mrwlk) file events.
Also drop the now superfluous makes peek_at_next_log_entry() and
throw_away_next_log_entry() functions.
logparser.py puts each log event on a big "stack" in self.pid. Later,
handle_children() in aa.py then converts that (named 'log' in aa.py) to
the prelog hasher.
This commit changes logparser.py to create the prelog structure itsself
(named hashlog), which
- removes one level of indirection
- probably saves some memory because the hashlog automatically
de-duplicates events
This commit does this for capability, network and signal events, and
adds the infrastructure needed for all event/rule types.
In aa.py, the new function handle_hashlog() copies the hashlog content
to prelog. OTOH, the now superfluous code handling capability, network
and signal events gets removed from handle_children().
Long-term, hashlog will replace log in aa.py. When this is done,
handle_hashlog() will be replaced by a simple prelog = hashlog.
logparser.py gets a new function init_hashlog() to initialize hashlog
for each profile. It also gets changed to store capability, network and
signal events into hashlog instead of storing them in self.pid.
hashlog uses the full profile name as key, which is the first baby step
to support nested child profiles. (for now, handle_hashlog() still
splits the profile name into profile and hat.)
Known issue: The new implementation doesn't handle exec yet, which means
that events get lost at the exec boundary (= in cases aa-logprof asks
which execute mode to use). This will be fixed in a later commit.
This function was meant to set a process running under a null-* profile
to its "real" profile after deciding about the exec mode/target.
However, this is not supported in the kernel.
"Luckily" the function was also broken and exited early, which
successfully prevented erroring out.
All that means set_process() is useless and we can drop it.
logparser cleanup
- drop old commented out code
- inline two small functions into the places calling them to make the callstack smaller and easier to understand
- rename 'netdomain' log events to 'network' to match rule name
- handle_children: raise exception on unknown event type
As usual, reading the individual commits makes the changes easier to understand.
See merge request apparmor/apparmor!374
Acked-by: Seth Arnold <seth.arnold@canonical.com>
parser/rc.apparmor.functions: fix minor issues detected by shellcheck/shellharden
See merge request apparmor/apparmor!370
Acked-by: Christian Boltz <apparmor@cboltz.de>
The apparmor.d manpage listed 'to' as an alternative for '->' in link
rules.
However, the parser doesn't accept 'to', none of our examples and tests
include it, and nobody ever complained about it. Therefore I'll call
this a documentation bug ;-) and simply adjust the manpage to only list
'->' as valid syntax.
Fix error 'KeyError: 'logfiles'' when no logprof.conf exists
See merge request apparmor/apparmor!365
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
- Code layout based on aa-genprof example
- Extend Python dependencies to cover new need by aa-notify
- Update documentation after aa-notify is no longer in Perl
Related to #22.
Fixes error message:
Traceback (most recent call last):
File "./aa-notify", line 523, in <module>
main()
File "./aa-notify", line 399, in main
aa.set_logfile(args.file)
File "/home/otto/koodia/apparmor/utils/apparmor/aa.py", line 1762,
in set_logfile
print(conf.find_first_file(cfg['settings']['logfiles']))
File "/usr/lib/python3.6/configparser.py", line 1233, in __getitem__
raise KeyError(key)
KeyError: 'logfiles'
Add tests for shared aa library functions used in command line scripts
See merge request apparmor/apparmor!328
Acked-by: Christian Boltz <apparmor@cboltz.de>
Add some tests with the complex profile name (including alternations and
wildcards) to ensure we don't break such cases in the future.
These tests are based on the log from the (invalid) bugreport
https://gitlab.com/apparmor/apparmor/issues/26
Since !345 the set of permissions that are granted (get_file_perms_2)
or suggested (propose_file_rules) has changed. These new sets are
expected due to the changes brought by this MR, so let's adjust
the test suite accordingly.
base abstraction: allow mr on *.so* in common library paths.
For example, VirtualBox guests have /usr/lib/VBoxOGL.so.
Without this changes, in a VirtualBox VM with VBoxVGA graphics,
at least one Qt5 application (OnionShare) won't start and display:
ImportError: libGL.so.1: failed to map segment from shared object
… and the system logs have:
apparmor="DENIED" operation="file_mmap" profile="/usr/bin/onionshare-gui" name="/usr/lib/VBoxOGL.so" pid=11415 comm="onionshare-gui" requested_mask="m" denied_mask="m" fsuid=1000 ouid=0
While this works fine with VBoxSVGA and VMSVGA when 3D acceleration is enabled.
So let's not assume all libraries have a name that starts with "lib".
Bug-Tails: https://redmine.tails.boum.org/code/issues/16414
Candidate for master and 2.13.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/345
Signed-off-by: John Johansen <john.johansen@canonical.com>
For example, VirtualBox guests have /usr/lib/VBoxOGL.so.
Without this changes, in a VirtualBox VM with VBoxVGA graphics,
at least one Qt5 application (OnionShare) won't start and display:
ImportError: libGL.so.1: failed to map segment from shared object
… and the system logs have:
apparmor="DENIED" operation="file_mmap" profile="/usr/bin/onionshare-gui" name="/usr/lib/VBoxOGL.so" pid=11415 comm="onionshare-gui" requested_mask="m" denied_mask="m" fsuid=1000 ouid=0
While this works fine with VBoxSVGA and VMSVGA when 3D acceleration is enabled.
So let's not assume all libraries have a name that starts with "lib".
Add userland support for matching based on extended file attributes. This
leverages DFA based matching already in the kernel:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8e51f908https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=73f488cd
Matching is exposed via flags on the profile:
/usr/bin/* xattrs=(user.foo=bar user.bar=foo) {
# ...
}
xattr values are appended to the existing xmatch via a null transition.
$ echo '/usr/bin/* xattrs=(user.foo=foo user.bar=bar) {}' | \
./parser/apparmor_parser -QT -D expr-tree
DFA: Expression Tree
/usr/bin/[^\0000/]([^\0000/])*(\0000bar)?(\0000foo)?< 0x1>
DFA: Expression Tree
(\a|(\n|(\0002|\t)))< 0x4>
Tested manually on a 4.19 kernel via QEMU+KVM.
TODO:
* ~~Add regression tests~~ (EDIT: done)
* ~~EDIT: add support in the tools~~ (EDIT: done)
Questions for reviewers:
* ~~parser/libapparmor: regex construction probably needs cleaning up~~ (EDIT: done)
* ~~parser/parser_regex.c: confused what xmatch length is for~~ (EDIT: done)
/cc @mjg59
PR: https://gitlab.com/apparmor/apparmor/merge_requests/270
Signed-off-by: John Johansen <john.johansen@canonical.com>
Compiling the parser currently prints a deprecation warning. Remove
throw(int) annotations from function signatures. These aren't required
to catch exceptions. This gets us closer to possibly enabling '-Werror'
in the future.
For example, the following program catches the exception without a
throw(int) annotation:
#include <iostream>
void throw_an_error()
{
throw 3;
return;
}
int main ()
{
try
{
throw_an_error();
}
catch (int e)
{
std::cout << "caught exception " << e << '\n';
}
return 0;
}
This program prints:
$ g++ -o error error.cc
$ ./error
caught exception 3
PR: https://gitlab.com/apparmor/apparmor/merge_requests/356
Signed-off-by: Eric Chiang <ericchiang@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Note that $PARSER_OPTS has to stay without quotes because it can
sometimes be empty, and would (if quoted) be interpreted as empty
filename by apparmor_parser
Compiling the parser currently prints a deprecation warning. Remove
throw(int) annotations from function signatures. These aren't required
to catch exceptions.
For example, the following program catches the exception without a
throw(int) annotation:
#include <iostream>
void throw_an_error()
{
throw 3;
return;
}
int main ()
{
try
{
throw_an_error();
}
catch (int e)
{
std::cout << "caught exception " << e << '\n';
}
return 0;
}
This program prints:
$ g++ -o error error.cc
$ ./error
caught exception 3
Signed-off-by: Eric Chiang <ericchiang@google.com>
The tools also have a list of network keywords, update it:
- add xdp and qipcrtr
- move ib and mpls to match the kernel order
Also add a test to ensure that (at least) the keywords provided by the
running kernel are listed in network_domain_keywords.
Extend the subshell so that the actual (possibly non-zero) value of
$retval gets returned. Before, the changed value was lost at "done"
(= leaving the subshell), and the initial $retval=0 was returned.
(found with shellcheck)
The calling function and the merge() macro both make use of a variable
named "i" but this causes problems when merging. The aa_policy_cache.sh
test script experiences "bad file descriptor" failures due to the merged
list containing invalid fd values (-1).
Fix this by renaming merge()'s index variables from i and j to y and z
to hopefully prevent future overlap. The better fix here would be to
convert merge() to a function but that's a more intrusive change and I
don't have an easy way to test the overlay feature.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
qsort()'s _size_ parameter is used to indicate the size of the elements
in the _base_ array parameter. Adjust the third argument to qsort() to
indicate that we're dealing with an array of struct dirent pointers
rather than an array of struct dirent.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Adjust aa_policy_cache.sh to handle the additional layer in the
directory hierarchy when determining where the policy cache binaries are
stored. This is needed due to the multicache changes that allow multiple
policy caches to exist on a single system.
Differentiate between the cache location (the top level directory
containing all caches) and the cache directory (the directory used to
store the cached policies).
Use the libapparmor wrapper to get the cache directory for the given
cache location and the features of the currently running kernel.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Print the policy cache directory path for the features of the currently
running kernel to stdout so that the aa_policy_cache.sh regression test
script can make use of it when writing out binary policy files.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
parser: determine xmatch priority based on smallest DFA match
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/326
Signed-off-by: John Johansen <john.johansen@canonical.com>
profiles/gnome: add @{HOME}/.cache/gtk-3.0
Found this path is used by gtk_compose_hash_get_cache_path() in
gtkcomposetable.c.
See merge request apparmor/apparmor!342
Acked-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Replace deprecated assertEquals with assertEqual
assertEquals is deprecated since Python 2.7 and 3.2.
See merge request apparmor/apparmor!347
Acked-by: Eric Chiang <ericchiang@google.com>
Update the indetation of work_spawn to correct for the changes made in
cb43e57d27 ("parser: Fix parser failing to handle errors when setting up work")
the indetation was not updated in that patch to make the changes made
easier to review and see in diffs.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Those 3 login daemons should have similiar needs and thus similar
profiles. IMAP is likely the most tested one so let's align the
other 2 with it. Unix and TCP sockets rules were added to pop3-login
after the removal of abstractions/nameservice that included them
implicitly.
Signed-off-by: Simon Deziel <simon@sdeziel.info>
When using passdb/userdb not requiring root (!= /etc/shadow access)
it is recommended to run the auth processes as non root and chroot'ed
Signed-off-by: Simon Deziel <simon@sdeziel.info>
Debian and Ubuntu have releases coming out with usr-merge in place. For these systems, /bin and /sbin are symlinks to their respective /usr directories. This breaks a few tests in the python utils and in the regression tests. This patch series fixes them, mostly by performing realpath() calls when necessary. For the ptrace regression test, it copies the called /bin/true binary into the created temporary directory and executes it from there. (Good for other reasons, too.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
These patches should be safe to backport to the supported releases (but I have not attempted that, yet).
See merge request apparmor/apparmor!331
In a usr-merge ubuntu/debian environment /bin is a symlink to
/usr/bin, which causes invalid apparmor policy to be generated for
/bin/true. Instead, copy /bin/true to the per test temporary directory
and execute it from there.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
When run locally on a development machine or in production, the full test
is likely to run. However inside a CI system container 'last' might fail
to show last login or there might not be access to kern.log and the test
will automatically skip those without failing the whole test suite.
This will help ensure the future rewrite of aa-notify from Perl to Python
is less likely to introduce regressions. Tests run the command line utility
via a subprocess so it does not matter that the tests are in Python but
the aa-notify utility is in Perl (for now).
The length of a xmatch is used to prioritize multiple profiles that
match the same path, with the intent that the more specific match wins.
Currently, the length of a xmatch is computed by the position of the
first regex character.
While trying to work around issues with no_new_privs by combining
profiles, we noticed that the xmatch length computation doesn't work as
expected for multiple regexs. Consider the following two profiles:
profile all /** { }
profile bins /{,usr/,usr/local/}bin/** { }
xmatch_len is currently computed as "1" for both profiles, even though
"bins" is clearly more specific.
When determining the length of a regex, compute the smallest possible
match and use that for xmatch priority instead of the position of the
first regex character.
This makes it possible for e.g. command line tools to have the --debug
option and when invoked print the existing debug messages directly to
stderr so the user running the command can see them.
kde: fix global settings access for Kubuntu and openSUSE
See merge request apparmor/apparmor!322
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.12..master
Defining 'stderr = subprocess.STDOUT' as a default value for function
did not work and the 'stderr' was always empty, thus also 'outerr' was
always empty and not standard error contents was ever considered in any
way.
Best in fact was to remove excess function arguments as they were not even
used and replace it with a simpler and less error prone structure.
Even after reading 'stderr' correctly it did not help much as all tests
used 'assertIn' which ignored excess output. Better replace the normal
output with the error output if there ever was something, since stderr
is most likely a serious thing and tests should stop on it.
- Add documentation stub for what the UI module is and how JSON works
- Make indentation consistent
- Use pythonic 'if' clauses
- Add two spaces after function definitions (Python style)
Exceptions are
- ubuntu-browsers (because we already have ubuntu-browsers.d with
different usage)
- ubuntu-helpers (which includes the sanitized_helper subprofile, so
adding something in the global area wouldn't make much sense)
Also adjust abstractions/postfix-common to use the style all
abstractions use.
Commit b5be596460 added ability to read
/usr/share/drirc.d/ directory to mesa abstraction.
This seems to be a mistake, as it was noted that not all GUI
applications, that need access to drirc.d, also need whole mesa-related
rules (including writing caches).
Move /usr/share/drirc.d/ access to different abstraction. This is not a
breaking change, because any GUI application will have included X
abstraction already, and in the end result - dri-common abstraction too.
Fix typo introduced when resolving a merge conflict for d9ab83281b
("Add support for local additions to abstractions")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Local policy may want to extend or override abstractions, so add support for including local updates to them.
Acked-by: Christian Boltz <apparmor@cboltz.de>
Acked-by: intrigeri <intrigeri@boum.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa-logprof errors out if it hits a log event for a non-existing profile
while a profile file with the default filename for that non-existing
profile exists. This can for example happen after adding a profile name
to a profile if audit.log still contains events for the attachment-based
profile name.
Since we ignore log events for non-existing profiles in general, drop
the code for the special case "but a file matching the default filename
for that non-existing profile exists" and also silently ignore events
for this very special non-existing profile.
Also drop the now unused function get_profile_filename()
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1120472
PR: https://gitlab.com/apparmor/apparmor/merge_requests/296
Signed-off-by: John Johansen <john.johansen@canonical.com>
This updates the mysqld to what I use on my servers nowadays.
Note: my profile also has capability sys_resource,, but I'm not sure why I had to add this and therefore didn't include it in this merge request.
Speaking about "why I had added $whatever" - these changes were collected over the last years and of course ;-) I don't remember any details.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/310
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This change updates parser/Makefile to respect target dependencies and
not rebuild apparmor_parser if nothing's changed. The goal is to allow
cross-compiled tests #17 to run on a target system without the tests
attempting to rebuild the parser.
Two changes were made:
* Generate af_names.h in a script so the script timestamp is compared.
* Use FORCE instead of PHONY for libapparmor_re/libapparmor_re.a
Changes to list_af_names are intended to exactly replicate the old
behavior.
Signed-off-by: Eric Chiang <ericchiang@google.com>
The /usr/sbin/libvirtd profile will get a profile name ("libvirtd").
This patch adjusts the dnsmasq profile to support the named profile in
addition to the "old" path-based profile name.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1118952#c3
*Add anonymous shared memory access rule for QtSettings.conf file
itself.
* Reduce strictness of anonymous shared memory file names - numbers in
them can be smaller or bigger.
* Fix consistency - require anonymous shared memory file names to end
with digit in all rules.
parser/apparmor.systemd: fix minor issues detected by shellcheck
See merge request apparmor/apparmor!293
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and 2.13
abstractions/ssl_{certs,keys}: dehydrated uses /var/lib on Debian
See merge request apparmor/apparmor!299
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
aa-logprof errors out if it hits a log event for a non-existing profile
while a profile file with the default filename for that non-existing
profile exists. This can for example happen after adding a profile name
to a profile if audit.log still contains events for the attachment-based
profile name.
Since we ignore log events for non-existing profiles in general, drop
the code for the special case "but a file matching the default filename
for that non-existing profile exists" and also silently ignore events
for this very special non-existing profile.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1120472
Commit 0d5ab43d59 removed support for loading
modules and introduced a caller, in apparmor_start(), that passes no argument to
is_apparmor_present(), which breaks that function when /bin/sh → /bin/dash.
Passing a module name as argument does not make sense since we dropped support
for the long obsolete "subdomain" module, so let's simplify
is_apparmor_present() and adjust its callers accordingly.
Bug-Debian: https://bugs.debian.org/917874
/var/lib/dhcp/dhcpd6.leases path for lease file on IPv6 has been in
dhcp repository since 2008, now visible in commit 8dea7ba7 ("Add DHCPv6
files in configure").
Signed-off-by: Petr Vorel <pvorel@suse.cz>
/var/lib/dhcp/dhclient6.leases path for lease file on IPv6 has been in
dhcp repository since 2008, now visible in commit 8dea7ba7 ("Add DHCPv6
files in configure").
Adding it via extending dhclient-*.leases pattern.
Not sure whether /var/lib/dhcp6/dhclient.leases was ever used on Linux,
but keep in for backward compatibility.
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Since 04eb2fe3, __parse_profiles_dir can only return 0 or 1, so $STATUS can only
be 0 or 1, so trying to reset this variable to 0 when its value is 2 can only
cause confusion.
Split out RE_FLAGS
... instead of having it duplicated in RE_PROFILE_HAT_DEF and RE_PROFILE_START.
Note that the flags=... handling in RE_PROFILE_HAT_DEF was more/too
strict (for example it didn't allow whitespace around the "="), so this
change also qualifies as a little bugfix.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/287
Acked-by: John Johansen <john.johansen@canonical.com>
dovecot: allow reading /proc/sys/fs/suid_dumpable
This is needed if a dovecot child process segfaults - in this case, dovecot provides a helpful error message like
dovecot[6179]: auth-worker: Fatal: master: service(auth-worker): child 8103 killed with signal 11 (core not dumped - https://dovecot.or /bugreport.html#coredumps - set /proc/sys/fs/suid_dumpable to 2)
which involves reading the current value in suid_dumpable.
I propose this fix for 2.10..master.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/286
Acked-by: John Johansen <john.johansen@canonical.com>
Ignore *.orig and *.rej files when loading profiles
This was "accidently" reported by Ralph on the opensuse-support
mailinglist.
I propose this for 2.10..master (I verified that 2.10 tools and libapparmor have *.orig and *.rej in the ignore list)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/282
Acked-by: John Johansen <john.johansen@canonical.com>
dnsmasq profile updates by Petr Vorel
This merge request includes two dnsmasq profiles Petr Vorel sent to the mailinglist:
dnsmasq: Add pid file used by NetworkManager
dnsmasq: Adjust pattern for log files to comply SELinux
I propose these patches for 2.11..master.
I'm not against also backporting to 2.10, but the profile in 2.10 doesn't allow to write anything in /var/log/, so either we apply/backport the changes manually, or we rely on the fact that we didn't get any bugreports for it ;-) Oh, and since I'm only forwarding these patches, I'll already add
Acked-by: Christian Boltz apparmor@cboltz.de for 2.11..master
Acked-by: John Johansen <john.johansen@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/288
i.e. move '*' from beginning to before suffix.
Commit 025c7dc6 ("dnsmasq: Add permission to open log files") added
pattern, which is not compatible with SELinux. As this pattern has been
in SELinux since 2011 (with recent change to accept '.log' suffix +
logrotate patterns which are not relevant to AppArmor) IMHO it's better
to adjust our profile.
Fixes: 025c7dc6 ("dnsmasq: Add permission to open log files")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
... instead of having it duplicated in RE_PROFILE_HAT_DEF and
RE_PROFILE_START.
Note that the flags=... handling in RE_PROFILE_HAT_DEF was more/too
strict (for example it didn't allow whitespace around the "="), so this
change also qualifies as a little bugfix.
This is needed if a dovecot child process segfaults - in this case,
dovecot provides a helpful error message like
dovecot[6179]: auth-worker: Fatal: master: service(auth-worker): child 8103 killed with signal 11 (core not dumped - https://dovecot.org/bugreport.html#coredumps - set /proc/sys/fs/suid_dumpable to 2)
which involves reading the current value in suid_dumpable.
In fed101920b, the coverity build process
was modified to split out the build logs into separate files, instead of
having one log file that gets overwritten repeatedly, making failures
hard to debug.
However, the coverity service gets upset if there is no file named with
the expected build log name. Therefore, instead, we'll capture the
python bits first, and then capture all the compilation bits in one
cov-build command.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/273
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
`/etc/letsencrypt/live/` contains symlinks to
`/etc/letsencrypt/archive/` which contains the keys. Add the
certs to ssl_certs and the private keys to ssl_keys.
Grant the ability to communicate with the postfix named child profiles
via signals and unix sockets. Include the path-based match names as
a fallback on upgrades.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Convert all the postfix subprocesses to using named profiles instead of
path match profiles, and adjust exec paths for newer debian/ubuntu
releses. Rename profiles to match profile names.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Convert postfix's master profile to use a named profile
(postfix-master) rather than the exec path match pattern. Adjust
postfix-common abstraction to take this into account. Rename profile
name in the profiles/apparmor/profiles/extras/ directory to match
the profile name.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
aa-genprof checks if one of the profiles in the extra profile dir
matches the binary, and proposes to use that profile as a starting
point.
Since 4d722f1839 the "(V)iew profile"
option to display the proposed profile was broken.
The easiest fix is to remember the filename in the extras directory, and
display the file from there.
Sidenote: when choosing to use the extra profile, it gets written to
disk without any problems, so this bug really only affected "(V)iew
profile" to preview the proposed extra profile.
parse_profile_data(): Ensure last line in a profile is valid
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/271
Acked-by: John Johansen <john.johansen@canonical.com>
'lastline' gets merged into 'line' (and reset to None) when reading the
next line. If 'lastline' isn't empty after reading the whole profile,
this means there's something unparseable at the end of the profile,
therefore parse_profile_data() should error out.
Also remove some simple_tests testcases from the 'exception_not_raised'
list - they only didn't raise the exception because the invalid rule was
the last line in the affected profile.
Thanks to Eric Chiang for accidently (and maybe even unnoticedly ;-)
discovering this bug while adding some xattr testcases that surprisingly
didn't fail in the tools.
Expr tree simplification makes multiple passes at simplifying the
expression tree trying to use fatoring rules and heuristics to achieve
the minimum tree, so that dfa construction has fewer nodes to deal
with.
Unfortunately expr tree simplification can slow some policy compiles,
dependent on the type of expressions generated, down, and even worse
is currently subject to never terminating on some expressions as the
left and right passes keep undoing each others work.
Limiting the number of passes that expr tree simplification does can
provide most of its benefits (later passes generally have diminishing
returns), reduces the overhead it has on simple policy where it is of
little benefit, and insures that simplifications can not get stuck in
an infinite loop due to the left and right passes ping-ponging on each
others factoring.
Note: This also results in a performance improvement in evince
compiles, and general policy compiles because it achieves a better
balance between time spent on simplifying the tree to remove nodes and
time the dfa build requires to build with extra nodes and then
eliminate with minimization.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.744s
user 0m2.714s
sys 0m0.028s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/usr.bin.evince
real 0m2.992s
user 0m2.979s
sys 0m0.012s
and
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.568s
user 0m14.529s
sys 0m0.152s
vs.
$ time apparmor_parser -QT /etc/apparmor.d/
real 0m3.741s
user 0m15.400s
sys 0m0.179s
PR: https://gitlab.com/apparmor/apparmor/merge_requests/246
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Remove traces of aa-eventd
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and didn't get any serious updates for several more years, so it's most probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE variables in rc.apparmor.functions anymore.
(In theory I could move these variables to deprecated/rc.aaeventd.* - but in practise that sounds more than superfluous ;-)
PR: https://gitlab.com/apparmor/apparmor/merge_requests/263
Acked-by: John Johansen <john.johansen@canonical.com>
aa-eventd and its initscripts have been moved to deprecated/ in 2014 and
didn't get any serious updates for several more years, so it's most
probably useless and/or broken nowadays.
This also means we don't need to keep the AA_EV_BIN and AA_EV_PIDFILE
variables in rc.apparmor.functions anymore.
The apparmor kernel "module" has not been a loadable module for more
than a decade, it must be built into the kernel and due configuration
requirements it will never go back to being a loadable module.
Remove the long unfunctioning load_module support from the init script.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/257
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: seth.arnold@canonical.com
Use @{sys} tunable in profiles and abstractions
Commit aa065287 made @{sys} tunable available by default.
Update profiles and abstractions to actually use @{sys} tunable for better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
PR: https://gitlab.com/apparmor/apparmor/merge_requests/262
Acked-by: John Johansen <john.johansen@canonical.com>
Commit aa06528790 made @{sys} tunable
available by default.
Update profiles and abstractions to actually use @{sys} tunable for
better confinement in the future (when @{sys} becomes kernel var).
Closes LP#1728551
Elaborate in class comment of firstpos, lastpos, followpos, and nullable
fields beyond just referencing the Dragon book. Also add the section of
the book these are explained in.
If a test is marked as TODO, but matches its EXRESULT, this means the
TODO is superfluous and (probably) a change fixed what the TODO was for.
Instead of more or less ignoring such superfluous TODOs, error out to
make the change visible instantly.
Both result in "superfluous TODO" (for unknown reason), but fail after
removing the TODO.
Disable the tests until we find out why they have this strange
behaviour, to unblock merging the "error out on superfluous TODO" patch.
Imported from the Debian/Ubuntu packaging. We need this function so that
Debian/Ubuntu can switch to using this shell library instead of their own code.
Technical stuff first:
Replace existing_profiles (a dict with the filenames for both active and
inactive profiles) with active_profiles and extra_profiles which are
ProfileList()s and store the active profiles and those in the extra
directory separately. Thanks to ProfileList, now also the relation
between attachments and filenames is easily available.
Also replace all usage of existing_profiles with active_profiles and
extra_profiles, and adjust it to the ProfileList syntax everywhere.
With this change, several bugs in aa-complain and the other minitools
get fixed:
- aa-complain etc. never found profiles that have a profile name
(the attachment wasn't checked)
- even if the profile name was given as parameter to aa-complain, it
first did "which $parameter" so it never matched on named profiles
- profile names with alternations (without attachment specification)
also never matched because the old code didn't use AARE.
References: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882047#92
(search for "As usual" ;-)
Just for completeness - the matching still doesn't honor/expand
variables in the profile name.
ProfileList is meant to store the list of profiles (both name and
attachment) and in which files they live.
Also add unittests to make sure everything works as expected.
parse_profile_data() returns the parsed profiles, but writes to
existing_profiles directly.
read_profiles() calls parse_profile_data() and already handles adding
the parsed profiles to aa, original_aa or extras, which means updating
existing_profiles there is a much better place.
This commit also includes a hidden change: Previously, when parsing
include files, they were also added to existing_profiles. This is
superfluous, only real profiles need to be stored there.
Split get_profile_filename() into
- get_profile_filename_from_profile_name() (parameter: a profile name)
- get_profile_filename_from_attachment() (parameter: an attachment)
Currently both functions call get_profile_filename_orig() (formerly
get_profile_filename()) so the behaviour doesn't change yet.
The most important part of this commit is changing all
get_profile_filename() calls to use one of the new functions to make
clear if they specify a profile or an attachment/executable as
parameter.
As promised, the is_attachment parameter starts to get used in this
patch ;-)
Note: The get_new parameter (which I'll explain in the patch actually
using it) is set to True in all calls to the new functions.
The long term plan is to get rid of it in most cases (hence defaulting
to False), but that will need more testing.
The minitools call write_profile(), write_profile_feedback_ui() and
serialize_profile() with the _attachment_ as parameter.
However, aa-logprof etc. call them with the _profile name_ as parameter.
This patch adds an is_attachment parameter to write_profile() and
write_profile_feedback_ui(). It also passes it through to
serialize_profile() via the options parameter.
If is_attachment is True, the parameter will be handled as attachment,
otherwise it is expected to be a profile name.
tools.py gets changed to set is_attachment to True when calling the
functions listed above to make clear that the parameter is an attachment.
Note: This patch only adds the is_attachment parameter/option, but
doesn't change any behaviour. That will happen in the next patch.
profiles/Makefile: test abstractions against apparmor_parser
See merge request apparmor/apparmor!237
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.13.
Pre-acked for 2.10..2.12 after removing the --config-file option which is not supported in these branches.
aa-notify man page: update user's configuration file path
See merge request apparmor/apparmor!239
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
Also adjust the signal rules in the dovecot-common and apache2-common
abstractions to match the profile names, and to really do that
(peer=...{bin,sbin}... didn't work, the correct syntax would have been
peer=...\{bin,sbin\}...)
This fixes the regression introduced by !149 / commit
4200932d8f
Add most abi/bad_*.sd tests to "exception not raised" list
Interestingly, abi/bad_6.sd is detected as invalid, and therefore not
added to the list.
I propose this for all branches that g[eo]t support for abi rules and the abi testcases.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/238
Acked-by: John Johansen <john.johansen@canonical.com>
AppArmor 3.0 requires policy to use a feature abi rule for access to
new features. However some policy may start using abi rules even if
they don't have rules that require new features. This is especially
true for out of tree policy being shipped in other packages.
Add enough support to older releases that the parser will ignore the
abi rule and warn that it is falling back to the apparmor 2.x
technique of using the system abi.
If the profile contains rules that the older parser does not
understand it will fail policy compilation at the unknown rule instead
of the abi rule.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
We can reduce the INCLUDE/INCLUDE_EXISTS code dup by using a
variable for whether the name was enclosed by '<' and using
processid() to handle the whether the id is quoted or not.
In addition using processid allows include names to contain
escaoe sequences like \n and have them handled correctly.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/196
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Currently if stdin is used the warning
apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
is always displayed but if caching has been disabled there is no need for
this message.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Hasher causes some fun in aa-mergeprof: If the profile in
/etc/apparmor.d/ has a hat or subprofile that doesn't exist in the
to-be-merged profile, aa-mergeprof crashes. This is caused by reading
self.other.aa[program][hat]['include'] which accidently "creates" that
profile inside the aa hasher as empty hasher (instead of ProfileStorage).
Later, the code loops over self.other.aa[profile].keys(), expects
everything to be ProfileStorage, and explodes [1] when for example
trying to run .delete_duplicates on the hasher (which obviously doesn't
provide this method).
This patch adds checks to all self.other.aa accesses in
CleanProf.remove_duplicate_rules() to avoid accidently creating new keys
in the hasher.
Interestingly this bug survived unnoticed for years (at least since
2.11).
[1] last lines of the backtrace:
File ".../utils/apparmor/cleanprofile.py", line 42, in compare_profiles
deleted += self.remove_duplicate_rules(profile)
File ".../utils/apparmor/cleanprofile.py", line 65, in remove_duplicate_rules
deleted += apparmor.delete_duplicates(self.other.aa[program][hat], inc)
File ".../utils/apparmor/aa.py", line 1680, in delete_duplicates
deleted += profile[rule_type].delete_duplicates(include[incname][incname][rule_type])
AttributeError: 'collections.defaultdict' object has no attribute 'delete_duplicates'
.gitignore profiles/apparmor.d/local/* except README
The old patter . doesn't match lsb_release and nvidia_modprobe, and
the only file we ship in local is a README. This patch adjusts the
pattern to ignore everything except README.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/227
Acked-by: John Johansen <john.johansen@canonical.com>
The old patter *.* doesn't match lsb_release and nvidia_modprobe, and
the only file we ship in local is a README. This patch adjusts the
pattern to ignore everything except README.
commit 94dfe15b28 attempted to remove
LD_RUN_PATH unfortunately
But all it actually does is cause the Makefile.perl to embed the rpath
"" instead. Which is still an rpath, only I guess an even worse one.
--
Eli Schwartz
Arch Linux Bug Wrangler and Trusted User
This is because it cleared the setting of the variable LD_RUN_PATH
which was expanded in the command
$(INST_DYNAMIC) : $(OBJECT) $(MYEXTLIB) $(INST_ARCHAUTODIR)$(DFSEP).exists $(EXPORT_LIST) $(PERL_ARCHIVEDEP) $(PERL_ARCHIVE_AFTER) $(INST_DYNAMIC_DEP)
$(RM_F) $@
LD_RUN_PATH="$(LD_RUN_PATH)" $(LD) $(LDDLFLAGS) $(LDFROM) $(OTHERLDFLAGS) -o $@ $(MYEXTLIB) \
$(PERL_ARCHIVE) $(LDLOADLIBS) $(PERL_ARCHIVE_AFTER) $(EXPORT_LIST) \
$(INST_DYNAMIC_FIX)
$(CHMOD) $(PERM_RWX) $@
resulting in LD_RUN_PATH="" being passed to the command.
Finish removing LD_RUN_PATH from Makefile.perl by removing it from
the command invocation if it is present.
Note: we use \x24 instead of $ in the regex as there seems to be a bug
and no level of escaping $ would allow it to be used.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Move @{sys} into tunables/kernelvars to make it readily available for
all profiles by default. In order to avoid duplicate variable
declaration, other tunable includes where modified accordingly.
Additionally, apparmor_api/is_enable has updated comment with regards to
@{sys} availability. tunables/sys is now marked as deprecated and should
not be used as it is noop, left only for backwards compatibility.
This is preparation for fixing bug:
https://bugs.launchpad.net/apparmor/+bug/1728551
Thanks to Christian Boltz for original idea to implement this in
non-breaking manner.
--log-facility option needs to have permission to open files.
Use '*' to allow using more files (for using more dnsmasq instances).
Signed-off-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Jamie Strandboge <jamie@canonical.com>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
We ship newer versions of these profiles in apparmor.d, so there's no
reason to keep the outdated ones (no serious change since > 10 years) in
the extra directory.
parser: fix Makefile hardcoded paths to flex and bison
Closes#4
See merge request apparmor/apparmor!224
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
The hardcoded paths for flex and python can break builds on systems
where those tools are stored in an alternate location. Use which
to lookup where flex and bison are available.
This fixes issue #4
Signed-off-by: John Johansen <john.johansen@canonical.com>
Use nvidia_modprobe named profile inside opencl-nvidia abstraction
Commit 8f9bd5b0 rightfully removed PUx
transition into nvidia-modprobe executable due to security concerns. To
overcome this, commit 327420b1 added
named nvidia_modprobe profile, which allows to use this abstraction
without requiring additional rules to make OpenCL work with NVIDIA
drivers.
Add rule to allow Px transition into nvidia_modprobe profile for
nvidia-modprobe executable.
I propose this for 2.13..master (opencl* are only available in 2.13...). Please check if nvidia_modprobe is already backported.
https://gitlab.com/apparmor/apparmor/merge_requests/219
Acked-by: John Johansen <john.johansen@canonical.com>
aa-notify: Read user's configuration file from XDG_CONFIG_HOME
Legacy path ~/.apparmor/notify.conf is preferred if it exists, otherwise $XDG_CONFIG_HOME/apparmor/notify.conf, with fallback to ~/.config/apparmor/notify.conf, is used.
See merge request https://gitlab.com/apparmor/apparmor/merge_requests/215
Signed-off-by: nl6720 nl6720@gmail.com
Acked-by: John Johansen <john.johansen@canonical.com>
Add missing paths to usr.sbin.nmbd, usr.sbin.smbd and abstractions/samba
See merge request apparmor/apparmor!210
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.10..master
Commit 8f9bd5b0e3 rightfully removed PUx
transition into nvidia-modprobe executable due to security concerns. To
overcome this, commit 327420b151 added
named nvidia_modprobe profile, which allows to use this abstraction
without requiring additional rules to make OpenCL work with NVIDIA
drivers.
Add rule to allow Px transition into nvidia_modprobe profile for
nvidia-modprobe executable.
Legacy path ~/.apparmor/notify.conf is preferred if it exists, otherwise
$XDG_CONFIG_HOME/apparmor/notify.conf, with fallback to
~/.config/apparmor/notify.conf, is used.
Signed-off-by: nl6720 <nl6720@gmail.com>
Add basic support for abi rules to the tools
Add basic support for abi rules to the tools
Add basic "understand and keep" support for abi rules, where "understand" means to not error out when seeing an abi rule, and "keep" simply means to keep the original abi rule when serializing a profile.
On the long term, abi rules should be parsed (similar to include rules), but for now, this patch is the smallest possible changeset and easy to backport.
Note that the only added test is via cleanprof_test.* which is used by minitools_test.py - and does not run if you do a 'make check'. Oh, and of course the simple_tests/abi/ files also get parsed by test-parser-simple-tests.py.
Also note that serialize_profile_from_old_profile() (which no longer exists in master, "only" in <= 2.13) would in theory also need support for abi rules. In practise, making this another case of "serialize_profile_from_old_profile() has known issues" is probably fine, but we should at least test that "(V)iew changes" doesn't break if an abi rule is present.
I propose this patch for 2.10..master.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/202
Acked-by: John Johansen <john.johansen@canonical.com>
nvidia-modprobe is setuid executable is used to create various device
files and load the the NVIDIA kernel module
(https://github.com/NVIDIA/nvidia-modprobe).
Add named profile to be used in application profiles for confining
potentially risky setuid application.
Add a check to logfile_to_profile() that checks the parsed log against a
list of input logs (log_to_profile_known_empty_log) that produce an
empty output.
The function was messing up its use of fds, it could get away with
it because the cb_dirfd passed to fdopendir was still valid until
closedir was called but if code was moved around, or fdopendir
code changed behavior it could easily break.
Also the check for dup failing was wrong fix it.
Reference: coverity #187003
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
Include qt5 into kde abstraction
See merge request apparmor/apparmor!209
Acked-by: Christian Boltz <apparmor@cboltz.de> for master and backporting to 2.10..2.13 (as soon as the qt5 abstraction is backported)
Currently, kde abstraction has rules relevant to Qt 3 and Qt 4
libraries, but are missing rules against latest Qt 5.
Include read-only Qt 5 abstraction to fix styling and similar issues for
software running on KDE 5 desktop.
Fixes https://bugs.launchpad.net/apparmor/+bug/1787201
Add uid and uids kernel var placeholders
Add @{uid} and @{uids} variables to allow migrating profiles in advance while awaiting path mediation implementation, based on current user id, in kernel side.
See merge request apparmor/apparmor!208
Acked-by: John Johansen <john.johansen@canonical.com>
Add @{uid} and @{uids} variables to allow migrating profiles in advance
while awaiting path mediation implementation, based on current user id,
in kernel side.
gio-launch-desktop helper tries to execute /usr/bin/thunderbird wrapper
script, not the /usr/lib/thunderbird... directly.
Add rule allowing to execute /usr/bin/thunderbird.
Harden abstractions
Harden abstractions
remove antiquated abstractions/launchpad-integration
abstractions/opencl-nvidia: don't allow PUx on nvidia-modprobe
abstractions/private-files-strict: disallow access to the dirs of private files
abstractions/private-files: disallow writes to thumbnailer dir (LP: #1788929)
ubuntu-browsers.d/user-files: disallow access to the dirs of private files
Nominating launchpad-integration and opencl-nvidia for 2.13. Nominating private-files-strict, private-files and user-files for 2.10 and higher
See merge request apparmor/apparmor!203
Acked-by: John Johansen <john.johansen@canonical.com>
Add basic "understand and keep" support for abi rules, where
"understand" means to not error out when seeing an abi rule, and "keep"
simply means to keep the original abi rule when serializing a profile.
On the long term, abi rules should be parsed (similar to include rules),
but for now, this patch is the smallest possible changeset and easy to
backport.
Note that the only added test is via cleanprof_test.* which is used by
minitools_test.py - and does _not_ run if you do a 'make check'.
Oh, and of course the simple_tests/abi/ files also get parsed by
test-parser-simple-tests.py.
Also note that serialize_profile_from_old_profile() (which no longer
exists in master, "only" in <= 2.13) would in theory also need support
for abi rules. In practise, making this another case of
"serialize_profile_from_old_profile() has known issues" is probably
fine, but we should at least test that "(V)iew changes" doesn't break if
an abi rule is present.
This reverts commit ab7459ed40.
This commit was meant to go to a branch to setup a merge request in
gitlab. Revert until the commit has gone through the regular review
process.
Signed-off-by: John Johansen <john.johansen@canonical.com>
make simple.pl / parser make check output more useful
Instead of printing the (useless) numbers of no-longer-failing tests
marked as TODO, let prove print out the actual test names.
A side effect is that really unfixed TODOs and disabled tests get
printed (lists about 10 tests), but that's much better than having to
hunt down the no-longer-failing TODOs.
This change will print out lines like (the first one is still a TODO,
the second one got fixed at some time)
not ok 71447 - TODO: ./simple_tests//vars/vars_profile_name_13.sd: reference variables that are the profile name and attachment # TODO Unfixed testcase.
ok 71448 - TODO: ./simple_tests//vars/vars_profile_name_14.sd: reference variables in rules that also have alternations # TODO Unfixed testcase.
before printing the test summary.
I propose this patch for master and (optionally) the maintained branches.
See merge request apparmor/apparmor!194
Acked-by: John Johansen <john.johansen@canonical.com>
Instead of printing the (useless) numbers of no-longer-failing tests
marked as TODO, let `prove` print out the actual test names.
A side effect is that really unfixed TODOs and disabled tests get
printed (lists about 10 tests), but that's much better than having to
hunt down the no-longer-failing TODOs.
This change will print out lines like (the first one is still a TODO,
the second one got fixed at some time)
```
not ok 71447 - TODO: ./simple_tests//vars/vars_profile_name_13.sd: reference variables that are the profile name and attachment # TODO Unfixed testcase.
ok 71448 - TODO: ./simple_tests//vars/vars_profile_name_14.sd: reference variables in rules that also have alternations # TODO Unfixed testcase.
```
before printing the test summary.
usr.sbin.dnsmasq: add paths for NetworkManager connection sharing
dnsmasq needs to access additional paths when used for connection sharing by NetworkManager.
Additionally it needs read permissions to /usr/share/dnsmasq/trust-anchors.conf which contains the DNSSEC trust anchors.
See merge request apparmor/apparmor!193
Acked-by: John Johansen <john.johansen@canonical.com>
Add qt5 writing abstractions
Qt-based applications stores QFileDialog (latest browsed directory) and
other shared user settings inside ~/.config/QtProject.conf. Currently
available qt abstraction only allows to read it (by design), so this
patch introduces abstraction that grants permissions for writing.
Relevant denies discovered with KDE Dragon Player:
/var/log/audit/audit.log.1:type=AVC msg=audit(1533485161.999:981): apparmor="DENIED" operation="mknod" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.lock" pid=29911 comm="dragon" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
type=AVC msg=audit(1533486419.266:1141): apparmor="DENIED" operation="file_lock" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.lock" pid=30406 comm="dragon" requested_mask="k" denied_mask="k" fsuid=1000 ouid=1000
/var/log/audit/audit.log.1:type=AVC msg=audit(1533485206.575:1006): apparmor="DENIED" operation="link" profile="/usr/bin/dragon" name="/home/vincas/.config/QtProject.conf.Gflpds" pid=29946 comm="dragon" requested_mask="l" denied_mask="l" fsuid=1000 ouid=1000 target="/home/vincas/.config/#12982163"
In addition, added new qt-compose-cache-write abstraction as some applications wants to write compose cache. qt5 abstraction is appended with read-only rule (that's enough for LibreOffice using KDE file dialog).
See merge request apparmor/apparmor!159
Acked-by: John Johansen <john.johansen@canonical.com>
use empty parser/tst/parser.conf in all parser and profile tests
See merge request apparmor/apparmor!191
Without this, the system-wide parser.conf gets used, which causes test
failures if for example caching is enabled and the cache dir isn't
writeable for the user running the tests.
I propose this for all branches where the parser understands --config-file.
Acked-by: John Johansen <john.johansen@canonical.com>
Without this, the system-wide parser.conf gets used, which causes test
failures if for example caching is enabled and the cache dir isn't
writeable for the user running the tests.
add python3.7 to logprof.conf
See merge request apparmor/apparmor!190
add python3.7 to logprof.conf
I propose this patch for at least 2.12..master, maybe also 2.10 and 2.11 (even if it's unlikely that someone uses the latest python3 on a distro with old AppArmor)
Acked-by: John Johansen <john.johansen@canonical.com>
The URL redirect ends up at a page in the new wiki that doesn't exist.
We have to link directly to the gitlab URL here since the current URL
redirect doesn't let us use a wiki.apparmor.net URL and still reach the
expected Profiles page.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
The open-coded readdirfd fn used to replace scandirat skipped
checks for memory allocation failures and cleaning on faulures,
fix this.
Acked-by: Tyler Hicks <tyhicks@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The `scandirat` function is a nonstandard GNU extension, which opens a
directory relative to a file descriptor. musl libc does not implement
that function and thus cannot be used to compile libapparmor.
All our uses of `scandirat` directly scan the directory the file
descriptor is referring to, not any directory beneath the FD. Implement
a function `readdirfd()`, which gets as arguments the directory FD, the
location where to put the list of directory entries as well as a
function pointer to a comparing function. `readdirfd` will then scan all
directory entries except "." and ".." and return them via an allocated
array. The array is sorted in case the comparing function is set.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/107
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Using stdin with --write-cache set results in
# apparmor_parser --show-cache --write-cache
Cache: added primary location '/var/cache/apparmor'
Warnung aus stdin (Zeile 1): Cache: added readonly location '/usr/share/apparmor/cache'
Warnung aus stdin (Zeile 1): apparmor_parser: cannot use or update cache, disable, or force-complain via stdin
Cache miss: stdin
Wrote cache: /var/cache/apparmor/9b2cd0d0.0/(null)
The "Wrote cache:" message is referencing a null value and should not
be displayed.
BugLink: http://bugs.launchpad.net/bugs/1787717
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Timeout
abstractions/php: allow ICU (unicode support) data tables
See merge request apparmor/apparmor!184
abstractions/php: allow ICU (unicode support) data tables
Reported by darix on IRC, and also something I noticed in my own usage of PHP.
I propose this addition for 2.10..master.
Acked-by: John Johansen <john.johansen@canonical.com>
profiles: support void-specific binary names for openntp and traceroute
See merge request apparmor/apparmor!183
Acked-by: Christian Boltz <apparmor@cboltz.de>
Fix concatenated-multiple-lines due to lacking Markdown's linebreak markup
by wrapping it with <pre>.
Signed-off-by: 林博仁(Buo-ren, Lin) <Buo.Ren.Lin@gmail.com>
Requiring --config-file to be first in the option list is not user
friendly fix the option parsing so that --config-file can be specified
anywhere in the option list.
This also fixes a bug where even when the --config-file option is
first the option parsing fails because the detection logic is broken
for some option cases.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/175
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The compiler is spitting out the warning
parser_main.c:1291:16: warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]
char *tmp = "/var/cache/apparmor";
fix this by constifying the cacheloc array.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Qt GUI applications that uses "platforminputcontexts"-class of plugins
might need reading and/or writing compose cache. Add read-only rule in
qt5 abstraction and create new writing dedicated for compose cache
writing.
Qt-based applications stores QFileDialog (latest browsed directory) and
other shared user settings inside ~/.config/QtProject.conf. Currently
available qt abstraction only allows to read it (by design), so this
patch introduces abstraction that grants permissions for writing.
The parser config file can affect the parsers behavior during tests.
Allow overriding the default location with the option
--config-file=
the option must be the first option in the commands argument list.
Also provile a
--print-config-file
option to display what the parser is using for a config file.
BugLink: http://bugs.launchpad.net/bugs/1277711
Acked-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
KIconLoader uses ~/.cache/icon-cache.kcache, and it is opened in
read-write mode. Because access to it does not seem to be critical, and
read-only mode is not used, rules for accessing this cache is added to
it's own new "write" abstraction, instead of making kde abstraction more
permissive by default.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently, kde abstraction only allows reading
~/.config/klanguageoverridesrc file (by design). Some KDE applications
has option to change language for it's interface, and this needs write
access. This is fixed by introducing new abstraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently, kde abstraction only allows reading ~/.config/kdeglobals (by
design), though some applications might need to update it's contents
such as KFileDialog settings. This patch fixes it by introducing new
abstraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
prevent that aa-complain etc. overwrites flags in child profiles if they differ from the main profile
See merge request apparmor/apparmor!150
Acked-by: John Johansen <john.johansen@canonical.com>
When running aa-genprof in a lxd instance, printk_ratelimit is readonly
and writing to it fails. Instead of crashing with a backtrace, only
print a warning.
References: https://bugs.launchpad.net/apparmor/+bug/1785391
Let's not store a bunch of automatically generated binary files in /etc.
AppArmor 3.0 will store the cache in /var/cache and most distros
(openSUSE, Debian, and soon Ubuntu) moved it there already.
Bug-Debian: https://bugs.debian.org/904637
This will allow removal of the lsb_release sub-profile from the
chromium, firefox and murmurd profiles, and consolidation of the rules
for /usr/bin/lsb_release in a single file.
The integration changes are taken from the patch at
apparmor-2.12/debian/patches/debian/add-debian-integration-to-lighttpd.patch
and are necessary so that lighttpd doesn't serve everything as
application/octet-stream.
As Simon McVittie wrote, "if a specification or library creates extra caches, or
has .desktop files in a subdirectory, or anything like that, then I don't see
why we wouldn't want to allow reading those too".
As Simon McVittie <smcv@collabora.com> wrote on
https://bugs.debian.org/865206 and on the AppArmor mailing list:
"Anything in /var/lib/flatpak/exports/share or
~/.local/share/flatpak/exports/share is essentially equivalent to
the corresponding path in /usr/{local/,}share, and is something
that has deliberately been "exported" to the rest of the system by a
Flatpak-confined app.
The only reason to prevent reading those directories would be if you do
not want the AppArmor-confined app to be able to enumerate the other
software you have installed on your system, as an anti-fingerprinting
mechanism.".
Bug-Debian: https://bugs.debian.org/865206
... instead of overwriting them with the flags of the main profile.
This fixes a longstanding issue with aa-complain, aa-enforce and
aa-audit which broke the flags of child profiles and hats if they
differed from the main profile.
It also fixes several issues documented in the tests (which obviously
need adjustment to match the fixed behaviour).
Also change the "no profile found" cases to AppArmorException - errors
in a profile are not worth triggering AppArmorBug ;-)
All callers call change_profile_flags(), so it makes sense to test this
function instead of set_profile_flags().
Besides that, set_profile_flags() will be merged into
change_profile_flags() in the next commit ;-)
Note that this commit adds some '# XXX' notes to the tests. These will
be addressed in later commits.
If the old flags are given as str (or None), call split_flags() to
convert them to a list.
This allows to simplify change_profile_flags() which now doesn't need to
call split_flags() on its own.
Also add some tests with a str for the old flags
... instead of set_profile_flags() to keep possibly existing flags like
attach_disconnected.
Note that this function is unused (meant to be used with the
no-longer-existing profile repo), therefore nobody noticed that
set_profile_flags() was called with the wrong number of parameters ;-)
Each coverity command writes its debugging output to
cov-int/build-log.txt, which means that multiple runs of cov-build
overwrite previous logs, resulting in only the last invocation's output
remaining at the end of the build, making debugging why failures to
capture coverity output difficult. Fix this by renaming the build-log to
per-directory log files.
(This would still be an issue even if we had a single build command
for the entire tree, as capturing python and other interpreted
files requires a second invocation of cov-build to scan for those
file types.)
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/145
'case OPTION_OFILE' missed the 'break', which means if did fallthrough
to the default case.
Adding the 'break' means no longer executing another PERROR, and no
longer executing the 'exit(1)' in the default branch.
References: coverity #55994
Add kernel patches that will NEVER be sent upstream. These provide abi
compatibility with the v2.x network and af_unix rules.
The 4.17 network mediation pull request deliberately broke abi
compatibility with the v2.x rules, and these are provided so that
distros who shipped the v2.x compatible patches can provide new
kernels on older releases that require v2.x network support.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When creating a new profile with aa-genprof, get_profile() searches for
an inactive ("extra") profile and, if it finds one, removes the filename
from that profile so that it gets stored in /etc/apparmor.d/ later.
However, it used .pop() to remove the filename, which explodes since
ProfileStorage is a class now.
This patch fixes this (tested manually).
Note that NO_FLAGS was an inverse option, therefore
- NO_FLAGS was changed to FLAGS (also in sync_profile() which is the
only caller that sets FLAGS)
- the default for include_flags (if FLAGS is not set) is True
Add mesa abstraction to allow writing to the Mesa-specific cache
locations and listing devices. Abstraction is needed for applications
utilizing OpenGL API with Mesa implementation available on the system.
The tools can't handle nested child profiles yet. Instead of failing
in funny[tm] ways (parse_profile_start() only returned the first two
segments of the profile name) better error out with a clear message.
The initial apparmor.service assumed the profile cache is in
/var/lib/apparmor/ which was an openSUSE-specific path that was chosen
for historical reasons, but was far from perfect.
In the meantime, the profile cache moved to /var/cache/apparmor/ and the
read-only/packaged version was added in /usr/share/apparmor/cache/
This commit adjusts apparmor.service to use these paths.
These tests verify that
- _is_equal_aare() really raises an exception when it sees an invalid
combination of other_value and other_all
- BaseRuleset.__repr__() works as expected
As a side effect, this commit pushes the test coverage of
apparmor/rule/__init__.py to 100% ;-)
... which is unused since the last commit.
Also delete several functions that were only used by this function:
- write_change_profile()
- write_rlimits()
- write_capabilities()
- write_netdomain()
- write_dbus()
- write_signal()
- write_ptrace()
- write_file()
Finally, no longer import some functions from profile_storage:
- write_links
- write_mount
- write_pivot_root
- write_unix
BTW: Deleting these 460 lines improves test coverage of aa.py from
38% to 44%, and total test coverage from 63% to 66% :-)
- rename prof_data parameter to ref
- drop empty allow, prefix and tail, and their usage in the format
string
- inline sep into the format string
- replace usage of fn with the actual function name (var_transform)
This is needed to get a reproducible output.
Also adjust the tests in test-profile-storage.py and add some example
variable to cleanprof.in and cleanprof.out
The aa-exec tool was indicating, in an error message, that a profile
didn't exist in cases where aa_change_profile() failed with errno set to
ENOENT or EACCES. However, the EACCES error means that changing to the
target profile is not allowed. This patch fixes the error message in the
cause of EACCES:
$ ./aa-exec -p dne -- true
aa-exec: ERROR: profile 'dne' does not exist
$ ./aa-exec -p /usr/sbin/tcpdump -- ./aa-exec -p unconfined -- true
aa-exec: ERROR: insufficient permissions to change to the profile 'unconfined'
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Add the pid of the process to the error, debug, and verbose output. This
is useful for debugging.
For example,
$ aa-exec -v -- whoami
[30389] exec whoami
tyhicks
$ aa-exec -p dne -- true
[30390] aa-exec: ERROR: profile 'dne' does not exist
$ aa-exec -d -p unconfined -- true
[30409] aa-exec: DEBUG: 0 = aa_change_onexec("unconfined")
It can also help when multiple aa-exec invocations are used across a
fork and exec. Here's a contrived example:
$ aa-exec -v -- \
sh -c 'aa-exec -vp /usr/sbin/tcpdump -- aa-exec -p unconfined -- true'
[3424] exec sh -c aa-exec -vp /usr/sbin/tcpdump -- aa-exec -p unconfined -- true
[3425] aa_change_onexec("/usr/sbin/tcpdump")
[3425] exec aa-exec -p unconfined -- true
[3425] aa-exec: ERROR: profile 'unconfined' does not exist
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
This commit adds an initial gitlab-ci.yml file to perform test builds
and run tests on each commit.
v2: add liblocale-gettext-perl dependency for parser simple test
v3:
- set noninteractive prompt to avoid debconf queries when installing
packages
- disable profiles test against aa-logprof; even if library and python
path issues are resolved, aa-logprof early aborts due to being
unable to find /sbin/apparmor_parser
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Tyler Hicks <tyler.hicks@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/101
If a log line contains a denial for a child profile, log_dict will
(obviously) only contain the child profile. However, serialize_profile()
expects that the parent profile is also initialized as ProfileStorage.
This patch makes sure the parent profile gets initialized.
It also removes 26 of the 37 reasons in the TODO note in aa.py :-)
ProfileStorage.get_rules_clean() returns all rules in a profile
(withouth the profile header or the closing '}')
Also change aa.py write_rules() to use get_rules_clean()
ProfileStorage() stores the content of a profile, so it makes sense to
also have the functions to write those rules (including helper functions
used by these functions) in the same file.
Note that I only moved the functions for rule types that are not handled
by *Ruleset classes.
The functions for writing rules stored in a *Ruleset class will
hopefully be superfluous sooner or later (probably later because
serialize_parse_profile_start() depends on them, and rewriting it won't
be easy)
Also move the test for var_transform() to test-profile-storage.py.
The POSIX standard never defines the typedefs `comparison_fn_t` and
`__free_fn_t`, but they are provided by glibc and user in the parsing
code. Provide the typedefs ourselves to fix compiling on musl based
systems.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
The `secure_getenv` function is a non-POSIX compliant extension of
glibc. In contrast to the POSIX `getenv`, `secure_getenv` will return
`NULL` for all environment variables when the program is run with
escalated privileges due to an SUID or SGID bit. Some strictly
POSIX-compliant libc libraries, most notably musl libc, do not have this
function and do not wish to implement it. Thus, AppArmor cannot be
compiled on such systems.
In libapparmor, `secure_getenv` is only used to determine whether the
environment variable DEBUG_ENV_VAR has been set to enable debugging. In
case an unprivileged user runs a SUID/SGID executable linked against
libapparmor, we do not want that user to be able to get additional
information via debug output.
The fix here is to produce an error only in case where debug output is
enabled by defining ENABLE_DEBUG_OUTPUT. Otherwise, we simply define
`secure_getenv` to `NULL` to completely disable the debug output.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
While the parser makes use of the `aa_policy_cache_add_ro_dir` function,
it is not being declared as a global function in the libapparmor.map
file. Due to this, dynamic linking of apparmor_parser with
libapparmor.so is not possible.
[Fixed up to use 2.13.1 symbol section as when the
`aa_policy_cache_add_ro_dir` was introduced -- @smb]
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/107
This is especially handy if your distro doesn't split sbin and bin
and only wants to install into bin (so that the sbin directory doesn't
clash with the sbin -> bin symlink)
[Per feedback, added USR_SBINDIR as a toggle for the install location
of aa-teardown -- @smb]
Signed-off-by: Rasmus Thomsen <cogitri@exherbo.org>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/111/
Instead of calling write_pair() (which is quite complex because it needs
to handle multiple rule types), write the alias rules directly in
write_alias().
This comes with minor code duplication, but makes the code much more
readable (3 instead of 7 %s)
write_pair() ignored the 'tail' parameter, which resulted in writing
invalid alias rules (without the trailing comma).
Also add an alias to test/cleanprof.* to ensure it doesn't break again.
test-libapparmor-test_multi.py converts the libapparmor test_multi log
examples to profiles.
This patch allows to call test-libapparmor-test-multi.py with a logfile
(containing a single log line) as parameter. It will then print the
resulting profile.
Example:
# python3 test-libapparmor-test_multi.py /path/to/libraries/libapparmor/testsuite/test_multi/testcase_dbus_01.in
/tmp/apparmor-2.8.0/tests/regression/apparmor/dbus_service {
dbus send bus=session path=/org/freedesktop/DBus interface=org.freedesktop.DBus member=Hello peer=(label=unconfined),
}
Writing a "link subset" rule missed a space, which resulted in something
like
link subset/foo -> /bar,
Also add a test rule to tests/cleanprof.* to ensure this doesn't break
again.
write_piece() has some funny code that converts the result of
write_header() and write_rules() (which is a list) to... a list.
Needless to say that this is superfluous ;-)
match_includes() and is_known_rule() have safety nets to avoid troube if
include[incname][incname] is not a valid ProfileStorage object.
However, this situation shouldn't happen in practise anymore, so let's
drop these now superfluous safety nets.
I use this patch locally since months without problems.
Commit 63b7cb0660 (libapparmor: convert
multicache from using djb2 hashing to murmur3 hash) mistakenly added
PmurHash.h to the list of files generated by the build process and thus
should be removed when the 'maintainerclean' make target is invoked.
This fixes the issue by removing PmurHash.h from the list of generated
files.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/112
When running the swap regression tests, the swapon command would issue
a warning about insecure permissions on the swapfile being used as
part of the test:
mkswap: /tmp/sdtest.9698-822-2BL034/swapfile: insecure permissions 0644, 0600 suggested.
Fix this by setting the permissions after the swapfile is created.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/108
This excludes the /etc/apparmor.d/cache.d/ directory from aa-logprof
parsing because parsing the binary cache, well, takes a while :-/
Reported on the opensuse-factory mailinglist by Frank Krüger and
confirmed by others.
Split the features file into compile features and kernel features
which is needed for policy versioning and the new caching scheme.
A new flag --kernel-features was added to set the kernel features but
unfortunately -M, --features-file was setup to only specify the
compile features, when it used to effectively specify both the
compile and kernel features.
This broke existing uses of -M.
Fix this by having -M specify both the compile and kernel features,
and a new flag --compile-features that can be used to specify the
compile fature set separate from the kernel feature set.
sbeattie> fixed up error message to refer to compile features when
--compile-features argument fails.
Fixes: 9e48a5da5e ("parser: split kernel features from compile features.")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
PR: https://gitlab.com/apparmor/apparmor/merge_requests/104
The parser currently uses a fork model to do job processing. For
consistency even when the number of jobs is set to 1 a single
work process is forked. However this makes using gdb more difficult
and can be even worse for other debugging tools.
Make -j 0 disable all job spawning so all processing happens in the
main process.
PR: https://gitlab.com/apparmor/apparmor/merge_requests/105
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
The recently added overlay cache directory support added to libapparmor
makes use of reallocarray(3) to resize memory allocations; however,
reallocarray() was only included in glibc 2.26. This commit adds a
configure check for reallocarray() and if it's not available, provides
it as a wrapper around realloc(3).
PR: https://gitlab.com/apparmor/apparmor/merge_requests/100
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The versioning previous patches was inconsistent because the multicache
patch have been in development for a while and the target version has
changed.
Cleanup libapparmor .map file to use the 2.13 release version
Signed-off-by: John Johansen <john.johansen@canonical.com>
Christian Boltz <apparmor@cboltz.de>
There are several small mistakes/typos in the previous patches. Just
fix them all here.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Allow the parser to use cache overlays by extending the --cache-loc
flag to support multiple locations via a comma separated list.
eg.
--cache-loc=/var/cache/apparmor/,/etc/apparmor.d/cache.d/
The overlayed cache directories are searched in the order
specified. So in the above example /var/cache/apparmor is searched
before /etc/apparmor.d/
Time stamps are ignored in the search, the first match found wins
regardless if there exists a matching cache file with a newer timestamp
in a directory is later in the search.
Cache writes will only occur to the first dir in the list. So
/var/cache/apparmor/ in the above example.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
The feature set needs to be split, the kernel features set determines
the cache location and controls features down grades to ensure
policy generates a policy that is usable on a given kernel.
The compile featurs set governs the feature set supported by policy
and primarily determines how policy is parsed and compiled.
Taking the intersection of the two feature sets to determine rule
downgrades for a specific kernel is left to a separate patch.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
For some longer pathname fstat is returning a bad size resulting in
the path being truncated. Fix this by detecting a potential truncation
and re-doing the readlink.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Add the support to have the cache be able to search multiple locations
so that the policy cache can be split into multiple locations and
that there can be a local cache that can override preshipped caches.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
This adds the fn aa_policy_cache_add_ro_dir() to the library allowing
for readonly layers to be added to the policy cache. It does not
make those additional layers functional. Which requires the ability
to create and search an overlay of directories.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Make the internal cache dir tracking use a fixed array and update
all references to the internal dirfd to index the array.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
A policy cache is for a specific set of kernel features so there is no
need to keep these separate.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Adjust the cache directory name from
<cache_loc>/<feature_id>
to
<cache_loc>/<feature_id>.<n>
where <n> is 0 for the first cache created for a given feature_id.
If there is a feature_id collision then <n> will be incremented to
the next number.
The .features file within each cache directory is used to disambiguate
which feature_id cache dir belongs to which feature set.
Cache collisions and missing caches cause a slow path that searches
existing cache dirs that fit the cache_name pattern, to ensure the
proper dir is chosen.
TODO: add regression tests
create cache dir check it
copy different feature set to it
create cache dir again, check it, check that it incremented...
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
It is possible that a given feature set will hash to the same cache
directory as a different feature set. This will be a problem if binary
caches are required, eg. early boot with systemd doing the cache load.
Detect cache collisions and fail. This is a precursor to handling
collision resolution and should not be committed without the follow
up patch to properly handle collisions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Murmur3 hash is a better hash that djb2 and has a lower chance of
hash collisions, so switch over to using it.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Carry the changes made in
libapparmor: Preserve errno across aa_*_unref() functions
into the multicache patcheset
Signed-off-by: John Johansen <john.johansen@canonical.com>
Christian Boltz <apparmor@cboltz.de>
Move the policy cache directory from <cacheloc>/cache/ to
<cacheloc>/cache.d/<features_id>/ where <features_id> is a unique
identifier for a set of aa_features. This allows for multiple AppArmor
policy caches exist on a system. Each policy cache will uniquely
correspond to a specific set of AppArmor kernel features. This means
that a system can reboot into a number of different kernels and the
parser will select the existing policy cache that matches each kernel's
set of AppArmor features.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Add and export aa_features_id() which can be used to get a unique
identifier for an aa_features object. Internally, this is a djb2 hash of
the features string. The hash function used and even the makeup of the
features ID can be easily changed in the future since external consumers
must use this function to fetch the features ID.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Store a hash value that can be used to represent the aa_features
object. This will be useful when storing multiple AppArmor policy cache
directories, each based on a kernel feature set.
The hash algorithm used is currently djb2. It was simple to add for
testing purposes, but may eventually need to be changed to something
that is resilient against collisions since there is no handling of
features file hash collisions.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Use the new --print-cache-dir parser option to construct the policy
cache dir when testing the policy caching functionality.
The majority of the required changes involve fully initializing
self.cmd_prefix prior to calling self.get_cache_dir() since that
function requires self.cmd_prefix to be initialized.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The --print-cache-dir option can be used to have the parser print the
value of the cache directory that is specific to the features used (from
the current kernel, the --match-string option, or the --features-file
option). After printing the path, apparmor_parser will exit. This is
helpful because the final component in the path will become
unpredictable because it will be based on arbitrary hash function
output.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Add and export aa_policy_cache_dir_path_preview() which allows the
parser to know exactly where the policy cache binaries, for the
specified aa_policy_cache and aa_features objects, would be stored. This
function may be useful to preview the policy cache dir without having
sufficient permissions or desires to create a policy cache dir.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Add and export aa_policy_cache_dir_path() which allows the parser to
know exactly where the policy cache binaries, for the current
aa_policy_cache and aa_features objects, will be stored. The parser
previously assumed that it was <cacheloc>/cache/ but it will soon be
<cacheloc>/cache.d/<features_id>/.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Since the latest change, calling {get,set}_profile_flags() with the
profile name failed when attachment was specified ("profile foo /bar").
Catched by the unittests.
Also fix a whitespace issue.
Getting and Setting profile represented by a glob does not work correctly
because they are checked for equality. Use a glob match to check for them.
Also, add a warning stating that the profile being set represents multiple programs.
traceroute is an example whose profile name is represented as
/usr/{sbin/traceroute,bin/traceroute.db} and exhibits the issue:
Setting /usr/sbin/traceroute to enforce mode.
ERROR: /etc/apparmor.d/usr.sbin.traceroute contains no profile
Signed-off-by: Goldwyn <goldwyn@fiona.lan>
The fix for issue #3/merge !86 in commit f0876ea9 contained a syntax
error that prevented libapparmor from building successfully. This
commit addresses the issue.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
... and the apparmor.systemd wrapper.
Also add a new 'install-systemd' target to the Makefile to install these
systemd-related files on (open)SUSE by default. Other distributions can
follow by adding a dependency on 'install-systemd' on their
'install-$DISTRO' target.
Note that apparmor.service has ExecStop=/bin/true to avoid that running
processes get unconfined if someone accidently types
systemctl restart apparmor (instead of using "reload")
Use aa-teardown if you really want to unload all profiles.
The files in this commit are used in openSUSE since a while, and also in
Arch Linux.
BTW: The condition on var-lib.mount is because openSUSE uses
/var/lib/apparmor/cache/ - but with the changed btrfs layout on
openSUSE, maybe I'll change that to /var/cache/apparmor/ which is
a) used by Debian and b) more sane
use_group is only honored if it is defined.
The "real" permission check is reading the logfile - the group check
in aa-notify is just an annoying additional check, and the default
"admin" only works on Ubuntu (other distributions typically use
"wheel").
This commit comments out use_group in the default config, which allows
everybody to use aa-notify. Permissions for reading the log file are of
course still needed.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=1058787
By default, it stays at the "calling directory" instead of the directory
of the Makefile, which breaks "make -C profiles check".
Explicitely set it in the Makefile to get the right directory.
Also adjust the install-suse make target to
- make 'rcapparmor' a symlink to 'service'
- no longer create the 'rcsubdomain' symlink
(open)SUSE does this in apparmor.spec since several releases, so this
commit upstreams the changes the spec did after running make install.
The environ.sh test fails with the following fatal error:
Fatal Error (environ): Unable to run test sub-executable
The reason is due to the tests which use the env_check.sh helper see
unexpected output in the helper's output:
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (C.UTF-8)
I see a number of locale related denials:
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/locale-archive" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/etc/locale.alias" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_IDENTIFICATION" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_CTYPE" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_COLLATE" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_MESSAGES/" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_NUMERIC" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
apparmor="DENIED" operation="open" profile="/aa/tests/regression/apparmor/env_check.sh" name="/usr/lib/locale/C.UTF-8/LC_TIME" pid=738 comm="env_check.sh" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Allowing everything under /usr/lib/locale/ to be read by the helper
results in the environ.sh test passing.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
libaalogparse uses (unsigned long) -1 to indicate that a log entry does
not contain ouid and/or fsuid fields. The utils logparser was
incorrectly using 2^64 - 1 to detect such a condition but that wasn't
sufficient for 32 bit environments.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Coverity now supports scanning python (and other languages). Apply the
fs-capture-search option to the libapparmor and utils directpries to
capture the python source.
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Right now, if you have a named profile with regular expressions to
match binaries, the profile will be shown in aa-status under the
"process list", which doesn't make sense. Instead, show the actual
executable name, and if the profile name differs, report it at the
end (or as a separate field in the json output mode).
Signed-off-by: Kees Cook <keescook@chromium.org>
Update nvidia abstraction to allow creating NVIDIA-specific user directories in
case it is missing (due to fresh $HOME or if manually removed for any reason).
Update base abstraction for ld.so.conf and friends.
See merge request apparmor/apparmor!62
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.9..master
Add new dri-common abstraction to contain basic DRI-specific rules.
This refactoring is based on a decision to have set of dri-* abstractions for
fine grained control on case-by-case basis. While dri-common is included in X
abstraction by default, additional DRI-related abstractions can be introduced
(such as for enumerating graphics devices) while keeping them logically together
with same dri- prefix.
apparmor: fix regression in network mediation when using feature pinning
When the 4.14-rc6 and earlier kernels are used with an upstream 4.13
or earlier pinned feature set, there is a regression in network
mediation where policy is not being correctly enforced, because the
compilation is completely dropping the af mediation table as expected
by pre 4.14 kernels but the 4.14 kernel is not accounting for this.
Resulting in network denials that can not be fixed by policy.
Signed-off-by: John Johansen <john.johansen@canonical.com>
apparmor: fix regression in network mediation when using feature pinning
When the 4.14-rc6 and earlier kernels are used with an upstream 4.13
or earlier pinned feature set, there is a regression in network
mediation where policy is not being correctly enforced, because the
compilation is completely dropping the af mediation table as expected
by pre 4.14 kernels but the 4.14 kernel is not accounting for this.
Resulting in network denials that can not be fixed by policy.
Signed-off-by: John Johansen <john.johansen@canonical.com>
write_include() was the only user of write_single(), and write_single()
had some include-specific code. This patch gets rid of write_single().
write_include() gets a simplified version of the code instead of calling
write_single().
The tools don't support having multiple rules in one line (they expect
\n after each rule), therefore mark some of the bare_include_tests as
known failures.
console_select_and_upload_profiles() and set_profiles_local_only() both
use a local variable named 'profs'. Rename it to 'profiles'.
This is the first baby step for rewriting how aa.py stores the profiles
internally. I plan to use 'profs' as variable name instead of 'aa', and
this commit gets the result for "grep -r profs" down to 0.
Some of the regression tests are missing conditionals or have the
wrong conditionals so that they fail on current upstream kernels.
Fix this by adding and changing conditionals and requires where
appropriate. With the patches the tests report passing on 4.14 and
4.15 kernels.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time out
This is a minimal patch to add conditional includes to the profile
language.
The syntax for conditional includes is similar to regular includes
except with the addition of "if exists" after "include"
include if exists <foo/bar>
include if exists "foo/bar"
include if exists "/foo/bar"
include if exists foo/bar
Note: The patch is designed to be backportable with minimum
effort. Cleanups and code refactoring are planned for follow up
patches that won't be back ported.
Signed-off-by: John Johansen <john.johansen@canonical.com>
On current Debian sid it needs to read
/usr/share/dovecot/protocols.d/imapd.protocol, which is not surprising given it
already needed read access to /usr/share/dovecot/protocols.d/.
I could not find the reason why the upstream Makefile has been installing it
with permissions 555: this predates the migration from SVN.
Regardless, at least on Debian and derivatives, dh_fixperms has been
changing these permissions to 755 forever so it was causing problems,
likely we would know about it by now.
The initial motivation for this change is supporting rootless builds on Debian
and derivatives, also known as "Rules-Requires-Root: no":
- /usr/share/doc/dpkg-dev/rootless-builds.txt* on a Debian system
with a sufficiently recent dpkg-dev installed
- https://nthykier.wordpress.com/2017/10/29/building-packages-without-fakeroot/
- https://lists.debian.org/debian-devel/2017/10/msg00520.html
With this change applied upstream, Debian-based downstreams don't need to adjust
their debian/rules to make this work with "Rules-Requires-Root: no":
chrpath -d $(CURDIR)/debian/tmp/lib/security/pam_apparmor.so
This is needed by new versions of notify-send, as found on openSUSE
Tumbleweed. Without this, desktop notifications don't work anymore, and
notify-send starts to eat up CPU.
If DBUS_SESSION_BUS_ADDRESS is already set, it won't be changed.
This is a minimal patch so that it can be backported to 2.11 and 2.10
which reverts the abort on error failure when the cache can not be
created and write-cache is set.
This is meant as a temporary fix for
https://bugzilla.suse.com/show_bug.cgi?id=1069906https://bugzilla.opensuse.org/show_bug.cgi?id=1074429
where the cache location is being mounted readonly and the cache
creation failure is causing policy to not be loaded. And the
thrown parser error to cause issues for openQA.
Note: A cache failure warning will be reported after the policy load.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Christian Boltz apparmor@cboltz.de
(cherry picked from commit 42b68b65fe1861609ffe31e05be02a007d11ca1c)
This patch supports rolling a tarball for a release, as well as doing
'make tag'. Only stuff that's been committed should get incorporated
into the tarball.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Denying it means netstat -p (actually tested with -tulpen) can't find
out the program name.
sys_ptrace is "only" needed for tracing processes that run under a
different uid.
Also add ptrace (read), for systems that support ptrace rules.
For now we only allow quoted absolute paths without spaces in the name
due to:
- 1738877: include rules don't handle files with spaces in the name
- 1738879: include rules don't handle absolute paths without quotes in
some versions of parser
- 1738880: include rules don't handle relative paths in some versions of
the parser
.gitignore additions for libapparmor tests, binutils, and the vim syntax highlighting files
See merge request apparmor/apparmor!43
Acked-by: Christian Boltz <apparmor@cboltz.de>
- dict needs abstractions/openssl (seen with dovecot 2.2.31 since
using openssl 1.1)
- imap needs to write tempfiles (seen with dovecot 2.2.31)
- managesieve-login needs access to the login-master-notify socket
(seen with dovecot 2.2.33)
- pop3-login needs access to the anvil socket (reported by pfak on
IRC some months ago)
- extend available_buttons() to display an "owner permissions on/off"
button if the rule supports it
- extend ask_the_questions() to handle these buttons
- add some tests to test-translations.py to avoid hotkey conflicts with
the newly added buttons
- move the code of set_options_audit_mode() to a new function
set_options_mode() and make set_options_audit_mode() a wrapper for it.
- add set_options_owner_mode() as another wrapper for set_options_mode()
and add code to switch the owner flag to set_options_mode()
- add tests for set_options_owner_mode()
This flag defines if the "Owner permissions on/off" button gets
displayed in aa-logprof.
False by default for all rule types (most of them don't support the
owner conditional). Also false for non-owner FileRule.
True only for FileRule if owner=True.
Several log examples result in rules where the 'owner' conditional
should be added. With logparser.py fixed to handle owner-only events, we
need to add the owner conditional to several test_multi/*.profile files.
I verified all log files for the changed profiles and made sure that
- the log line contains fsuid= and ouid=
- fsuid == ouid
I also did a quick check on all log events containing ouid= and for
those with fsuid == ouid, I checked that the profile has the owner
conditional.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.11
(see mail from 2017-07-31)
logparser.py failed to notice if file events are owner-only in modern
audit.log (using fsuid=... and ouid=...).
This patch adds a comparison of fsuid and ouid and marks file events
as 'owner' if they match.
Note that log events without fsuid=... or ouid=... will have
18446744073709551615 as fsuid / ouid value (that's 2^64 - 1).
'None' would clearly be better ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1538340
Exit rather than returning from shell snippets in Makefiles. It is
reported that returning causes the following error message with bash:
/bin/sh: line 4: return: can only `return' from a function or sourced script
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Christian Boltz <apparmor@cboltz.de>
The utils have tests that rely on the in-tree parser to be built so it
should be documented that the parser should be built first.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
The test-aa-easyprof.py script relies on the parser to be built so the
check target of the utils/test/Makefile should detect if the parser
exists before running any tests.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Reported-by: Christian Boltz <apparmor@cboltz.de>
Since some kernel versions, inherit (ix) needs mmap permissions. Instead
of annoying the user with an avoidable question after adding an ix rule,
always add m permissions.
Together with the already existing code, this means newly added inherit
rules will now have 'mrix' permissions.
autodep() calls read_inactive_profiles() each time it's called (= for
each binary). The result is a "Conflicting profile" error (showing the
same filename twice) if autodep() runs more than once. This can easily
happen when using "aa-autodep /usr/bin/*".
This patch adds an attribute to read_inactive_profiles() that lets the
function return without doing anything if was called before.
check_po.pl lists lots of false positives saying that
msgstr ""
does not have the (h)otkey translated.
This patch whitelists those untranslated strings.
I also tested (by manually "breaking" a translation) that missing
hotkeys still get noticed.
This bug probably exists since forever, therefore I propose this patch
for 2.9..trunk. (OTOH, nobody noticed it, so maybe trunk is enough ;-)
Note: I still get a few false positives for ru.po (no idea why, similar
texts in the other languages don't cause this) - ideas and fixes welcome.
* Alter paths to allow Java version 8 and up.
* Add file rules to fix IcedTea browser plugin.
* Refactor to keep path consistensy against parent and child profile,
reduce repetitive rules.
Allow to read pulseaudio config subdirectories
See merge request apparmor/apparmor!12
Acked-by: Christian Boltz <apparmor@cboltz.de> for 2.9, 2.10, 2.11 and trunk
After using "view changes", the selection got reset to the first changed
profile. This could mislead the user into saving the wrong profile.
This patch ensures the selection is kept.
I propose this patch for trunk and 2.11.
(2.11 will need different indentation again.)
I'm not sure if we should also apply this in 2.10 and 2.9 - they have
the same behaviour, but OTOH I'm not sure if changing behaviour (even if
it's an improvement) in those old releases is a good idea.
Opinions?
The last change in save_profiles() sorted() the order in which the
changed profiles get displayed. However, it did not honor the sorting
when displaying changes or saving the selected profile, leading to the
wrong profile displayed or saved.
This patch fixes picking the selected profile, and at the same time
replaces the duplicated code for doing this with a single instance.
I propose this patch for trunk and 2.11.
Note that the 2.11 branch needs a slightly different patch (different
indentation).
Also note that this regression made it into 2.11.1, so distributions
shipping 2.11.1 should add this patch.
The RETURN VALUE section contained two typos where "kernel_features" was
used instead of "kernel_interface".
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Callers of aa_features_unref(), aa_kernel_interface_unref(), and
aa_policy_cache_unref() had to store off errno and restore it after
calling those functions in error paths. This patch preserves errno
across those *_unref() functions so that callers don't have to.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Ignoring cscope.* files allows users of cscope to not be bothered by
`git status` reporting that an unknown file is in the source tree.
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
The macro `PATH_MAX` macro is typically defined in the <limits.h>
header by the system's libc implementation. While we do not
include it right now, glibc indirectly includes it via other
headers already and thus compilation of the file succeeds. For
other libc implementations this may not be the case, which would
then lead to a compilation error. This is the case for musl libc.
Explicitly include <limits.h> to fix this.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
The define `RLIMIT_OFILE` is a historic macro originating from
the BSDs, which is nowadays an alias for `RLIMIT_NOFILE`. On some
implementations, it has thus been dropped in favor of the new
define, but we still assume it will always be defined in our
rlimit keywords table. Wrap it in an `ifdef` to fix compilation
on systems where it does not exist.
For the second macro `RLIMIT_RTTIME`, we do check for its
existence in our keywords table, but then forgot to do so in the
YACC rules. Wrap it into an `ifdef`, as well.
Both patches serve the goal to fix compilation on musl libc.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
The macros __BEGIN_DECLS and __END_DECLS are not conforming to
any standard, but are a custom extension of the glibc library. As
such, it may not be available in other libc implementations, with
one example being musl libc. So compiling libapparmor won't work
with a strictly standards-conforming library.
These macros are typically used for header files which might be
included in a C++ project. Depending on whether the header is
seen by a C or C++ compiler, it will hint that functions have C
linkage. The macros themselves are rather simple:
#ifdef __cplusplus
# define __BEGIN_DECLS extern "C" {
# define __END_DECLS }
#else
# define __BEGIN_DECLS
# define __END_DECLS
#endif
To fix compilation with musl libc, simply expand those macros to
explicitly use `extern "C"`. This is already used in other parts
of apparmor and should thus be safe to use.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
[](https://gitlab.com/apparmor/apparmor/pipelines)
[](https://bestpractices.coreinfrastructure.org/projects/1699)
------------
Introduction
------------
@@ -17,9 +23,44 @@ library, available under the LGPL license, which allows change_hat(2)
and change_profile(2) to be used by non-GPL binaries).
For more information, you can read the techdoc.pdf (available after
building the parser) and by visiting the http://apparmor.net/ web
building the parser) and by visiting the https://apparmor.net/ web
site.
----------------
Getting in Touch
----------------
Please send all complaints, feature requests, rants about the software,
Security issues can be filed in GitLab by opening up a new [issue](https://gitlab.com/apparmor/apparmor/-/issues) and selecting the tick box ```This issue is confidential and should only be visible to team members with at least Reporter access.``` or directed to `security@apparmor.net`. Additional details can be found
in the [wiki](https://gitlab.com/apparmor/apparmor/wikis/home#reporting-security-vulnerabilities).
--------------
Privacy Policy
--------------
The AppArmor security project respects users privacy and data and does not collect data from or on its users beyond what is required for a given component to function.
The AppArmor kernel security module will log violations to the audit subsystem, and those will be logged/forwarded/recorded on the user's system(s) according to how the administrator has logging configured. Again this is not forwarded to or collected by the AppArmor project.
The AppArmor userspace tools do not collect information on the system user beyond the logs and information needed to interact with the user. This is not forwarded to, nor collected by the AppArmor project.
Users may submit information as part of an email, bug report or merge request, etc. and that will be recorded as part of the mailing list, bug/issue tracker, or code repository but only as part of a user initiated action.
The AppArmor project does not collect information from contributors beyond their interactions with the AppArmor project, code, and community. However contributors are subject to the terms and conditions and privacy policy of the individual platforms (currently GitLab) should they choose to contribute through those platforms. And those platforms may collect data on the user that the AppArmor project does not.
Currently GitLab requires a user account to submit patches or report bugs and issues. If a contributor does not wish to create an account for these platforms the mailing list is available. Membership in the list is not required. Content from non-list members will be sent to moderation, to ensure that it is on topic, so there may be a delay in choosing to interact in this way.
-------------
Source Layout
@@ -27,6 +68,7 @@ Source Layout
AppArmor consists of several different parts:
```
binutils/ source for basic utilities written in compiled languages
changehat/ source for using changehat with Apache, PAM and Tomcat
common/ common makefile rules
@@ -37,6 +79,7 @@ parser/ source for parser/loader and corresponding documentation
profiles/ configuration files, reference profiles and abstractions
tests/ regression and stress testsuites
utils/ high-level utilities for working with AppArmor
```
--------------------------------------
Important note on AppArmor kernel code
@@ -57,60 +100,86 @@ Building and Installing AppArmor Userspace
------------------------------------------
To build and install AppArmor userspace on your system, build and install in
the following order.
the following order. Some systems may need to export various python-related
environment variables to complete the build. For example, before building
anything on these systems, use something along the lines of:
/* When compiling for windows, we specify a specific calling convention to avoid issues where we are being called from a project with a different default calling convention. For windows you have 3 define options:
CJSON_HIDE_SYMBOLS - Define this in the case where you don't want to ever dllexport symbols
CJSON_EXPORT_SYMBOLS - Define this on library build when you want to dllexport symbols (default)
CJSON_IMPORT_SYMBOLS - Define this if you want to dllimport symbol
For *nix builds that support visibility attribute, you can define similar behavior by
setting default visibility to hidden by adding
-fvisibility=hidden (for gcc)
or
-xldscope=hidden (for sun cc)
to CFLAGS
then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJSON_EXPORT_SYMBOLS does
*/
#define CJSON_CDECL __cdecl
#define CJSON_STDCALL __stdcall
/* export symbols by default, this is necessary for copy pasting the C and header file */
#define CJSON_PUBLIC(type) __declspec(dllexport) type CJSON_STDCALL
#elif defined(CJSON_IMPORT_SYMBOLS)
#define CJSON_PUBLIC(type) __declspec(dllimport) type CJSON_STDCALL
#endif
#else /* !__WINDOWS__ */
#define CJSON_CDECL
#define CJSON_STDCALL
#if (defined(__GNUC__) || defined(__SUNPRO_CC) || defined (__SUNPRO_C)) && defined(CJSON_API_VISIBILITY)
#define CJSON_PUBLIC(type) __attribute__((visibility("default"))) type
#else
#define CJSON_PUBLIC(type) type
#endif
#endif
/* project version */
#define CJSON_VERSION_MAJOR 1
#define CJSON_VERSION_MINOR 7
#define CJSON_VERSION_PATCH 13
#include<stddef.h>
/* cJSON Types: */
#define cJSON_Invalid (0)
#define cJSON_False (1 << 0)
#define cJSON_True (1 << 1)
#define cJSON_NULL (1 << 2)
#define cJSON_Number (1 << 3)
#define cJSON_String (1 << 4)
#define cJSON_Array (1 << 5)
#define cJSON_Object (1 << 6)
#define cJSON_Raw (1 << 7) /* raw json */
#define cJSON_IsReference 256
#define cJSON_StringIsConst 512
/* The cJSON structure: */
typedefstructcJSON
{
/* next/prev allow you to walk array/object chains. Alternatively, use GetArraySize/GetArrayItem/GetObjectItem */
structcJSON*next;
structcJSON*prev;
/* An array or object item will have a child pointer pointing to a chain of the items in the array/object. */
structcJSON*child;
/* The type of the item, as above. */
inttype;
/* The item's string, if type==cJSON_String and type == cJSON_Raw */
char*valuestring;
/* writing to valueint is DEPRECATED, use cJSON_SetNumberValue instead */
intvalueint;
/* The item's number, if type==cJSON_Number */
doublevaluedouble;
/* The item's name string, if this item is the child of, or is in the list of subitems of an object. */
char*string;
}cJSON;
typedefstructcJSON_Hooks
{
/* malloc/free are CDECL on Windows regardless of the default calling convention of the compiler, so ensure the hooks allow passing those functions directly. */
void*(CJSON_CDECL*malloc_fn)(size_tsz);
void(CJSON_CDECL*free_fn)(void*ptr);
}cJSON_Hooks;
typedefintcJSON_bool;
/* Limits how deeply nested arrays/objects can be before cJSON rejects to parse them.
* This is to prevent stack overflows. */
#ifndef CJSON_NESTING_LIMIT
#define CJSON_NESTING_LIMIT 1000
#endif
/* returns the version of cJSON as a string */
CJSON_PUBLIC(constchar*)cJSON_Version(void);
/* Supply malloc, realloc and free functions to cJSON */
/* Memory Management: the caller is always responsible to free the results from all variants of cJSON_Parse (with cJSON_Delete) and cJSON_Print (with stdlib free, cJSON_Hooks.free_fn, or cJSON_free as appropriate). The exception is cJSON_PrintPreallocated, where the caller has full responsibility of the buffer. */
/* Supply a block of JSON, and this returns a cJSON object you can interrogate. */
/* ParseWithOpts allows you to require (and check) that the JSON is null terminated, and to retrieve the pointer to the final byte parsed. */
/* If you supply a ptr in return_parse_end and parsing fails, then return_parse_end will contain a pointer to the error so will match cJSON_GetErrorPtr(). */
/* Render a cJSON entity to text using a buffered strategy. prebuffer is a guess at the final size. guessing well reduces reallocation. fmt=0 gives unformatted, =1 gives formatted */
/* For analysing failed parses. This returns a pointer to the parse error. You'll probably need to look a few chars back to make sense of it. Defined when cJSON_Parse() returns 0. 0 when cJSON_Parse() succeeds. */
/* Append reference to item to the specified array/object. Use this when you want to add an existing cJSON to a new cJSON, but don't want to corrupt your existing cJSON. */
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to audit mode ]"), $0));
UI_Important(sprintf(gettext('Can\'t find AppArmor profiles in %s.'), $profiledir));
exit 1;
}
# what are we profiling?
my @profiling = @ARGV;
unless (@profiling) {
@profiling = (UI_GetString(gettext("Please enter the program to create a profile for: "), ""));
}
for my $profiling (@profiling) {
next unless $profiling;
my $fqdbin;
if (-e $profiling) {
$fqdbin = get_full_path($profiling);
chomp($fqdbin);
} else {
if ($profiling !~ /\//) {
my $which = which($profiling);
if ($which) {
$fqdbin = get_full_path($which);
}
}
}
# make sure that the app they're requesting to profile is not marked as
# not allowed to have it's own profile
if ($qualifiers{$fqdbin}) {
unless ($qualifiers{$fqdbin} =~ /p/) {
UI_Info(sprintf(gettext('%s is currently marked as a program that should not have it\'s own profile. Usually, programs are marked this way if creating a profile for them is likely to break the rest of the system. If you know what you\'re doing and are certain you want to create a profile for this program, edit the corresponding entry in the [qualifiers] section in /etc/apparmor/logprof.conf.'), $fqdbin));
exit 1;
}
}
if (-e $fqdbin) {
if (-e getprofilefilename($fqdbin) && !$force) {
UI_Info(sprintf(gettext('Profile for %s already exists - skipping.'), $fqdbin));
} else {
autodep($fqdbin);
reload($fqdbin) if $sd_mountpoint;
}
} else {
if ($profiling =~ /^[^\/]+$/) {
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to complain mode ]"), $0));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to have profile disabled ]"), $0));
UI_Info(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' as a user with the correct PATH environment set up in order to find the fully-qualified path.'), $profiling, $profiling));
exit 1;
} else {
UI_Info(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
exit 1;
}
}
}
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ program to switch to enforce mode ]"), $0));
fatal_error(gettext("AppArmor does not appear to be started. Please enable AppArmor and try again."));
}
# let's convert it to full path...
$profiledir = get_full_path($profiledir);
unless (-d $profiledir) {
fatal_error "Can't find AppArmor profiles in $profiledir.";
}
# what are we profiling?
my $profiling = shift;
unless ($profiling) {
$profiling = UI_GetString(gettext("Please enter the program to profile: "), "")
|| exit 0;
}
my $fqdbin;
if (-e $profiling) {
$fqdbin = get_full_path($profiling);
chomp($fqdbin);
} else {
if ($profiling !~ /\//) {
my $which = which($profiling);
if ($which) {
$fqdbin = get_full_path($which);
}
}
}
unless ($fqdbin && -e $fqdbin) {
if ($profiling =~ /^[^\/]+$/) {
fatal_error(sprintf(gettext('Can\'t find %s in the system path list. If the name of the application is correct, please run \'which %s\' in the other window in order to find the fully-qualified path.'), $profiling, $profiling));
} else {
fatal_error(sprintf(gettext('%s does not exist, please double-check the path.'), $profiling));
}
}
# make sure that the app they're requesting to profile is not marked as
# not allowed to have it's own profile
check_qualifiers($fqdbin);
# load all the include files
loadincludes();
my $profilefilename = getprofilefilename($fqdbin);
# When reading from syslog, it is possible to hit the default kernel
# printk ratelimit. This will result in audit entries getting skipped,
# making profile generation inaccurate. When using genprof, disable
# the printk ratelimit, and restore it on exit.
my $ratelimit_sysctl = "/proc/sys/kernel/printk_ratelimit";
my $ratelimit_saved = sysctl_read($ratelimit_sysctl);
END { sysctl_write($ratelimit_sysctl, $ratelimit_saved); }
sysctl_write($ratelimit_sysctl, 0);
UI_Info(gettext("\nBefore you begin, you may wish to check if a\nprofile already exists for the application you\nwish to confine. See the following wiki page for\nmore information:\nhttp://wiki.apparmor.net/index.php/Profiles"));
UI_Important(gettext("Please start the application to be profiled in \nanother window and exercise its functionality now.\n\nOnce completed, select the \"Scan\" button below in \norder to scan the system logs for AppArmor events. \n\nFor each AppArmor event, you will be given the \nopportunity to choose whether the access should be \nallowed or denied."));
UI_Info(gettext("Reloaded AppArmor profiles in enforce mode."));
UI_Info(gettext("\nPlease consider contributing your new profile! See\nthe following wiki page for more information:\nhttp://wiki.apparmor.net/index.php/Profiles\n"));
UI_Info(sprintf(gettext('Finished generating profile for %s.'), $fqdbin));
exit 0;
sub usage {
UI_Info(sprintf(gettext("usage: \%s [ -d /path/to/profiles ] [ -f /path/to/logfile ] [ program to profile ]"), $0));
PDEBUG("scandirat of directory '%s' failed: %m\n",name);
return-1;
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.
