- update version file
- update library version. addition of saddr/... parsing extends the
record struct which is an interface addition.
Signed-off-by: John Johansen <john.johansen@canonical.com>
technically a # leading a value in an assignment expression is allowed,
however people are also using it to a comment at the end of a line.
ie.
```
@{var1}=value1 # comment about this value or for a given system
```
this unsurprisingly leads to odd/unexpected behavior when the variable
is used.
```
allow rw /@{var1},
```
expands into
```
allow rw /{value1,#,comment,about,this,value,or,for,a,given,system},
```
change a leading # of a value in an assignment expression to a comment.
If the # is really supposed to lead the value, require it to be escaped
or in quotes.
ie.
```
@{var1}=value1 \#not_a_comment
```
Note: this could potentially break som policy if the # was used as the
leading character for a value in an assignment expression, but
is worth it to avoid the confusion.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/407
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1255
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 642db8a37c)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The abi is not being respected by mqueue rules in many cases. If policy
does ot specify an mqueue rule the abi is correctly applied but if
an mqueue rule is specified explicitly or implicitly (eg. allow all).
without setting the mqueue type OR setting the mqueue type to sysv.
The abi will be ignored and mqueue will be enforced for policy regadless.
Known good mqueue rule that respects abi
mqueue type=posix,
# and all variations that keep type=posix
Known bad mqueue rules that do not respect abi
mqueue,
# and all variants that do not specify the type= option
mqueue type=sysv,
# and all variants that specify the type=sysv option
Issue: https://gitlab.com/apparmor/apparmor/-/issues/412
Fixes: d98c5c4cf ("parser: add parser support for message queue mediation")
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1277
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit d0d75abd02)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Some applications using the bwrap profile don't function properly due to "Failed name lookup - deleted entry". The following denials trying to start flatpak KeePassXC is an example showing that it happens for both bwrap and unpriv_bwrap profiles:
Jul 12 09:44:37 ubuntu2404 kernel: audit: type=1400 audit(1720741477.106:310): apparmor="DENIED" operation="link" class="file" info="Failed name lookup - deleted entry" error=-2 profile="bwrap" name="/home/\*\*\*\*/.var/app/org.keepassxc.KeePassXC/config/keepassxc/#317211" pid=4021 comm="keepassxc" requested_mask="l" denied_mask="l" fsuid=1000 ouid=1000
Jul 12 09:44:37 ubuntu2404 kernel: audit: type=1400 audit(1720741477.341:317): apparmor="DENIED" operation="link" class="file" profile="unpriv_bwrap" name="/home/**/.var/app/org.keepassxc.KeePassXC/config/keepassxc/keepassxc.ini" pid=4021 comm="keepassxc" requested_mask="l" denied_mask="l" fsuid=1000 ouid=1000 target="/home/**/.var/app/org.keepassxc.KeePassXC/config/keepassxc/#317214"
Fixes: https://launchpad.net/bugs/2072811
I propose this fix for master and apparmor-4.0
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1272
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 7f35adef41)
Signed-off-by: John Johansen <john.johansen@canonical.com>
Saw these couple of accesses fail recently on my Ubuntu 22.04 system:
`Jun 3 15:29:24 darkstar kernel: [5401883.070129] audit: type=1107 audit(1717442964.884:9223): pid=729 uid=102 auid=4294967295 ses=4294967295 subj=unconfined msg='apparmor="DENIED" operation="dbus_method_call" bus="system" path="/org/freedesktop/UPower" interface="org.freedesktop.DBus.Properties" member="GetAll" mask="send" name=":1.28" pid=2164500 label="firefox" peer_pid=2502 peer_label="unconfined"`
`Jun 3 15:29:24 darkstar kernel: [5401883.070588] audit: type=1107 audit(1717442964.884:9224): pid=729 uid=102 auid=4294967295 ses=4294967295 subj=unconfined msg='apparmor="DENIED" operation="dbus_method_call" bus="system" path="/org/freedesktop/UPower" interface="org.freedesktop.UPower" member="EnumerateDevices" mask="send" name=":1.28" pid=2164500 label="firefox" peer_pid=2502 peer_label="unconfined"`
Also, I noticed that the `firefox` profile in the Ubuntu 24.04 package has a rule for `/etc/writable/timezone` that is not present in Git. Figured that should be in here.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/409Closes#409
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1253
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 09d8f886ca)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is needed to avoid a "Conflicting profiles" error if there are two
profiles for an application, with one of them disabled.
This is not a theoretical usecase - for example, apparmor.d ships some
profiles that replace our "userns+unconfined" profiles. These profiles
use a different filename, and apparmor.d also creates a disable symlink
for the "userns+unconfined" profile it replaces.
I propose this patch for 4.0 and master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1264
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 834c28b5fe)
Signed-off-by: John Johansen <john.johansen@canonical.com>
... and not only for events in missing hats.
This fixes a crash if the log contains exec events for a hat where not
even the parent profile exists.
I propose this patch for master, 4.0 and 3.1.
(In 3.0, `aa` is still a `hasher` which avoids the crash, therefore it doesn't really need this patch.)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1265
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit 3ea19d5f32)
7928c416 ask_exec: ignore events for missing profiles
Co-authored-by: Christian Boltz <apparmor@cboltz.de>
Expression simplification can get into an infinite loop due to eps
pairs hiding behind and alternation that can't be caught by
normalize_eps() (which exists in the first place to stop a similar
loop).
The loop in question happens in AltNode::normalize when a subtree has
the following structure.
1. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
2. if (normalize_eps(dir)) results in
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
alt alt
/\ /\
/ \ / \
/ \ / \
eps eps eps eps
4. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
5. if (normalize_eps(dir)) results in
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
6. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
back to beginning of cycle
Fix this by detecting the creation of an eps_pair in rotate_node(),
that pair can be immediately eliminated by simplifying the tree in that
step.
In the above cycle the pair creation is caught at step 3 resulting
in
3. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
alt eps
/\
/ \
/ \
eps eps
4. elseif (child[dir]->is_type(ALT_NODE)) rotate_node too
alt
/\
/ \
/ \
eps alt
/\
/ \
/ \
eps eps
which gets reduced to
alt
/\
/ \
/ \
eps eps
breaking the normalization loop. The degenerate alt node will be caught
in turn when its parent is dealt with.
This needs to be backported to all releases
Closes: https://gitlab.com/apparmor/apparmor/-/issues/398
Fixes: 846cee506 ("Split out parsing and expression trees from regexp.y")
Reported-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Closes#398
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1252
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit a6691ca53e)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This needs adding of an empty_ok flag in _aare_or_all().
Also add a few tests from boo#1226031 to utils and parser tests.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1226031
(cherry picked from commit 1f33fc9b29,
test-mount.py changes adjusted for 4.0 branch)
While on it, make the output for failing tests more verbose for easier
debugging.
(cherry picked from commit 900f233101,
test-mount.py adjusted for 4.0 branch)
aa-remove-unknown doesn't deal properly with profiles that contain
spaces in their names.
Using profile "MongoDB Compass" as an example, awk's sub returns the
number of matches - either 1 or 0 and replaces the actual string ($0)
with the substitution. By accessing the return of sub with $, awk
would be accessing $1 which would return only "MongoDB".
Fix this by using $0 instead of $str.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/395
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Closes#395
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1243
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit 920120ffea)
7ef7704f utils: fix removing unknown profiles that contain spaces
Co-authored-by: John Johansen <john@jjmx.net>
This file contains the same kind of information as @{PROC}/@{pid}/net/route
and both files are world readable:
```
$ ls -l /proc/self/net/*route
-r--r--r-- 1 root root 0 Jun 3 15:33 /proc/self/net/ipv6_route
-r--r--r-- 1 root root 0 Jun 3 15:33 /proc/self/net/route
```
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1246
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit 5adeb86bbe)
7770cd71 abstractions/nameservice: allow reading @{PROC}/@{pid}/net/ipv6_route
Co-authored-by: Christian Boltz <apparmor@cboltz.de>
Without this patch, aa-remove-unknown uses 'profile_name (unconfined)'
when trying to unload unconfined profiles, which fails for obvious
reasons with (picking a random example)
Removing 'busybox (unconfined)'
/sbin/aa-remove-unknown: line 112: echo: write error: No such file or directory
I propose this patch for 4.0 and master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1240
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit cf5be7d356)
cabd88a9 Fix aa-remove-unknown for 'unconfined' profiles
Co-authored-by: Georgia Garcia <georgia.garcia@canonical.com>
Mount Rules with options in { remount, [make-] { [r]unbindable, [r]shared, [r]private, and [r]slave }} do not support specifying a source. This commit aligns utils implementation to apparmor_parser's, which prohibits having a both source and a destination simultaneously, instad of just prohibiting source.
Therefore, both `mount options=(unbindable) /a,` and `mount options=(unbindable) -> /a,` are now supported (and equivalent for apparmor_parser). However, `mount options=(unbindable) /a -> /b,` is invalid.
For the same reason, specifying a fstype in these cases is also prohibited.
Similarly, we prohibit to specify a fstype for bind mount rules.
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2065685
(cherry picked from commit 60acc4a405)
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1237
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
Mount Rules with options in { remount, [make-] { [r]unbindable, [r]shared, [r]private, and [r]slave }} do not support specifying a source. This commit aligns utils implementation to apparmor_parser's, which prohibits having a both source and a destination simultaneously, instad of just prohibiting source.
Therefore, both `mount options=(unbindable) /a,` and `mount options=(unbindable) -> /a,` are now supported (and equivalent for apparmor_parser). However, `mount options=(unbindable) /a -> /b,` is invalid.
For the same reason, specifying a fstype in these cases is also prohibited.
Similarly, we prohibit to specify a fstype for bind mount rules.
Fixes: https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/2065685
(cherry picked from commit 60acc4a405)
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
audit.log entries for mount events don't always include `class=mount`,
but can still be the base for mount rules.
Change logparser.py to also consider `operation=mount` as a mount event.
Actually we already had such a log and profile in our collection
(testcase_mount_01), but since it existed years before MountRule was
implemented, it was excluded in test-libapparmor-test_multi.py.
Therefore we didn't notice that it failed to produce a profile rule when
MountRule was introduced.
Remove testcase_mount_01 from the list of known failures so that it gets
tested - and fix the syntax error in the hand-written
testcase_mount_01.profile.
Also add testcase_mount_02 which is a mount event without fstype,
srcname and class.
I propose this fix for 4.0 and master.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1229
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 48a936e985)
b475ed0d Handle mount events/log entries without class
Co-authored-by: Georgia Garcia <georgia.garcia@canonical.com>
- Before this commit, fstype had to match a known fs. However, having and maintaining the exhaustive list of fstypes proved challenging (see !1195 and !1176). Therefore, we add support for any filesystem name.
- Completing AARE support for fstype (brace expressions like ext{3,4} are now supported).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1198
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
(cherry picked from commit baa8b67248)
dad5ee28 MountRule: Relaxing constraints on fstype and completing AARE support
Co-authored-by: Christian Boltz <apparmor@cboltz.de>
AppArmor 4.0.1 will be the official release superseding the 4.0.0 tag
the only change being it adds a regression test for CVE-2016-1585
- update version file
Signed-off-by: John Johansen <john.johansen@canonical.com>
This updates the man page for the recent inet mediation patch.
This is an extension of MR 1202, it adds a patch that changes the anonymous ip address anon to be ip address none which is a better fit.
This patch adds documentation of the recent network changes which extended all network rules to support access permissions, and added address and port matching for inet and inet6 families.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1213
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit ab9e6311f3)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds a bwrap profile to allow it to function on a system with
user namespace restrictions enabled.
The child task of bwrap will enter into a profile without capabilities
thus preventing bwrap from being able to be used to arbitrarily
by-pass user namespace restrictions.
This profile does prevent applications launch with privilege (eg.
sudo bwrap ...) from functioning so it may break some use cases.
Note: The unpriv_bwrap profile is deliberately stacked against the
bwrap profile due to bwraps uses of no-new-privileges.
Fixes: https://bugs.launchpad.net/ubuntu/+source/pageedit/+bug/2046844
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1205
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
(cherry picked from commit b6f2a3d9d2)
Signed-off-by: John Johansen <john.johansen@canonical.com>
The network cmp function was missing the new attributes added, causing
rules to be dropped.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
When the ip is not specified, then we should generate rules for ip
types: anonymous, ipv4 and ipv6. And that's the case for both local
and peer when considering recv and send permissions.
std::ostringstream does not have a copy constructor, that's why in
several places one can see streaming the string of one stream into
another.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
We want to be able to determine label in the future and build the
policy dfa based on its presence or not.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
According to the protocol expected by the kernel, the field
representing the ip size should be an enum instead of the actual ip
size. This is more future-proof.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Before the inet patches, protocol was not handled, so the information
was ignored. This patch introduces the ability to start mediating
protocol.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
We're not trying to execute a command in EXTRA_WARNINGS, so don't try to spawn
a subshell for it which gives:
```
./configure: 14770: EXTRA_WARNINGS: not found
checking whether C compiler accepts -flto-partition=none... yes
```
We can either use ${} or just $ (style). Use $ to be consistent with other
uses in the file.
Signed-off-by: Sam James <sam@gentoo.org>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1184
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
We're not trying to execute a command in EXTRA_WARNINGS, so don't try to spawn
a subshell for it which gives:
```
./configure: 14770: EXTRA_WARNINGS: not found
checking whether C compiler accepts -flto-partition=none... yes
```
We can either use ${} or just $ (style). Use $ to be consistent with other
uses in the file.
Signed-off-by: Sam James <sam@gentoo.org>
Latest pam_unix always runs /usr/sbin/unix_chkpwd instead of reading
/etc/shadow itsself. Add exec permissions to abstraction/authentication.
It also needs to read /proc/@{pid}/loginuid
Also cleanup the now-superfluous rules from the smbd profile.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1219139
With abstractions/openssl now being included from abstraction/base
(via the indirection of abstractions/crypto) anything already
including abstraction/base can stop including abstractions/openssl
directly.
This is a follow up to 3d1dedfa as suggested by @cboltz
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1179
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
With abstractions/openssl now being included from abstraction/base
(via the indirection of abstractions/crypto) anything already
including abstraction/base can stop including abstractions/openssl
directly.
Administrators might want to define global limits (e.g. disabling
a particular feature) via configuration files, but to make that work
all confined software needs to be allowed to read those files or
otherwise the risk is to silently fall back to internal defaults.
This adds the abstraction already defined for openssl to
abstraction/crypto as it is about cryptography, but also because
abstraction/base includes abstraction/crypto and therefore it will
be allowed in general.
Administrators might want to define global limits (e.g. disabling
a particular feature) via configuration files, but to make that work
all confined software needs to be allowed to read those files or
otherwise the risk is to silently fall back to internal defaults.
This adds the paths usually used by gnutls to abstraction/crypto
as it is about cryptography, but also because abstraction/base
includes abstraction/crypto and therefore it will be allowed
in general.
The error message when autoconf-archive is not installed is not very
intuitive:
```
./configure: line 14422: EXTRA_WARNINGS: command not found
./configure: line 14423: syntax error near unexpected token `-flto-partition=none,'
./configure: line 14423: `AX_CHECK_COMPILE_FLAG(-flto-partition=none, , , -Werror)'
```
So, check if AX_CHECK_COMPILE_FLAG is defined and if not, complain
that autoconf-archive is missing.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1174
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The permission for network rules when the inet mediation was not
available, or for when the family was not af_inet or af_inet6 was
being generated as one that would allow anything. Make them specific
using perms.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The layout for AF_INET and AF_INET6 rules were being applied to all
families, which causes failures in their mediation.
Fixes: ddefe11a ("parser: add fine grained conditionals to network rule")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
- replace example calls of /etc/init.d/apparmor with apparmor.service
- drop /etc/init.d/apparmor in filelist
- replace /var/lib/apparmor/ with /var/cache/apparmor/
The error message when autoconf-archive is not installed is not very
intuitive:
./configure: line 14422: EXTRA_WARNINGS: command not found
./configure: line 14423: syntax error near unexpected token `-flto-partition=none,'
./configure: line 14423: `AX_CHECK_COMPILE_FLAG(-flto-partition=none, , , -Werror)'
So, check if AX_CHECK_COMPILE_FLAG is defined and if not, complain
that autoconf-archive is missing.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
* **MountRule: sync flags_keywords with parser code**
... based on /mount.cc mnt_opts_table
Several keywords and aliases were missing in flags_keywords:
- B
- M
- make-private
- make-rprivate
- make-rshared
- make-rslave
- make-runbindable
- make-shared
- make-slave
- make-unbindable
- r
- R
- read-only
- w
Also sort the keywords in the same order as in mount.cc.
Note: AARE handling is still a TODO.
After that, update the list of known parsing failures:
- several valid profiles are now correctly parsed
- some `"make-*" mount opt and an invalid src` bad profiles are no
longer detected as being invalid
* **test-mount.py: fix MountRule instance creation**
If fstype or options is a str, it has to be exactly one keyword, because
\__init__() / check_and_split_list() won't parse a str.
Our "normal" code already honors this, and only hands over fstype and
options as sets or a single-keyword str.
However, a few tests (wrongly) handed over a str that would need further
parsing. Adjust the tests to no longer do this.
* **MountRule: check for unknown fstype and options**
... now that the previous commits fixed issues that ended up as unknown
keywords.
Also add mount/ok_12.sd as known-failing test. It uses fstype=AARE which
MountRule doesn't support (yet?).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1169
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
* Fix writing 'mount {options,fstype} in ...' rules
We need spaces around the 'in' keyword.
Also add some tests for this.
* Make error check more readable
* MountRule: make get_clean() more readable
... by getting rid of two mostly-identical, big return statements.
Also add tests for bare umound and remount rules to ensure full test
coverage.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1168
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
... now that the previous commits fixed issues that ended up as unknown
keywords.
Also add mount/ok_12.sd as known-failing test. It uses fstype=AARE which
MountRule doesn't support (yet?).
If fstype or options is a str, it has to be exactly one keyword, because
__init__() / check_and_split_list() won't parse a str.
Our "normal" code already honors this, and only hands over fstype and
options as sets or a single-keyword str.
However, a few tests (wrongly) handed over a str that would need further
parsing. Adjust the tests to no longer do this.
... based on /mount.cc mnt_opts_table
Several keywords and aliases were missing in flags_keywords:
- B
- M
- make-private
- make-rprivate
- make-rshared
- make-rslave
- make-runbindable
- make-shared
- make-slave
- make-unbindable
- r
- R
- read-only
- w
Also sort the keywords in the same order as in mount.cc.
Note: AARE handling is still a TODO.
After that, update the list of known parsing failures:
- several valid profiles are now correctly parsed
- some `"make-*" mount opt and an invalid src` bad profiles are no
longer detected as being invalid
Now that we have MountRule and MountRuleset, drop the old "just store
the whole rule" code for mount rules.
Also drop some old tests that used that "store the whole mount rule"
code, and adjust the regex_matches tests to import the regex directly
from apparmor.regex.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1165
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
If /proc/filesystems contains a filesystem that is not listed in
MountRule valid_fs, print a useful error message that says what exactly
is going on, instead of only saying "False is not True".
Now that we have MountRule and MountRuleset, drop the old "just store
the whole rule" code for mount rules.
Also drop some old tests that used that "store the whole mount rule"
code, and adjust the regex_matches tests to import the regex directly
from apparmor.regex.
A simple rule without conditionals need to be generated for when the
kernel does not support fine grained inet network mediation.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
By not having quotes in $@, the string splits by the whitespace.
That prevents us from checking if the parser supports rules
that have spaces in them.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Options available are ip= and port= inside the peer group or outside,
representing local addresses and ports:
network peer=(ip=127.0.0.1 port=8080),
network ip=::1 port=8080 peer=(ip=::2 port=8081),
The 'ip' option supports both IPv4 and IPv6. Examples would be
ip=192.168.0.4, or ip=::578d
The 'port' option accepts a 16-bit unsigned integer. An example would
be port=1234
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Nautilus uses user namespaces to load thumbnails, hence it needs an
unconfined profile when user namespaces are restricted from unconfined
like other applications in MR #1123
Although nautilus has extensions that would allow opening a terminal
from the nautilus interface, they do not inherit nautilus' AppArmor
label, therefore the use of unconfined does not allow arbitrary use of
unprivileged user namespaces using the nautilus label.
https://gitlab.com/apparmor/apparmor/-/merge_requests/1123
In addition this serves as a handle to uniquely identify them instead
of unconfined to peers in policy.
Note that unconfined mode should be changed for default_allow when
https://gitlab.com/apparmor/apparmor/-/merge_requests/1109 is merged.
Fixes: https://bugs.launchpad.net/bugs/2047256
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
This commit add fixes for issues found in coverity's snapshot 70858.
- CID 323127: Performance inefficiencies (COPY_INSTEAD_OF_MOVE)
- CID 323125: Uninitialized members (UNINIT_CTOR)
I'm also removing Novell, Inc. from the copyright notice added by a
copy-paste error, and an unused variable left over from debugging.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Precompile each filter regex with re.compile so they don't need to be
recompiled for each log message when using re.match directly.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
aa-unconfined can fault if it looses the race between checkking if
proc/*/attr/{apparmor/,}current exists, and actually opening the file.
Catch open/file errors and ignore them like the file doesn't exist.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/355
Signed-off-by: John Johansen <john.johansen@canonical.com>
Allow notification filtering of the fields profile, operation, name,
denied_mask, net_family and net_socket using regex. Both command line
and config options in notify.conf are available.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The tests for aa-notify that were related to the last login were
assuming that the machine had been logged in at least once in the last
30 days, but that might not be the case.
Update the test to check for the last login date and update the test
logs considering that value.
Fixes: https://bugs.launchpad.net/bugs/1939022
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Failures on regex and exec are related to /usr/bin and /bin merge.
The userns failure is related to the addition of the unprivileged_userns
profile and the kernel ability to transition to it when an unconfined
unprivileged user namespace is created.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1146
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The short form of the help parameter was not accepting an optional
argument as described in the help message. Update the help message to
remove the extra whitespace in between -h and the options since
getopt(1) says that
"If the option has an optional argument, it must be written directly
after the option character if present."
even though that's not described in getopt(3).
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The help message from aa-status referenced -h filter when it should
have been -h filters. Also added some whitespaces for consistency.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
element-desktop needs to use user namespaces, hence it needs an unconfined profile when user namespaces are restricted from unconfined
like other applications in MR #1123!1123
In addition this serves as a handle to uniquely identify them instead
of unconfined to peers in policy.
Note that unconfined mode should be changed for default_allow when
!1109
is merged.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1150
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
There is a kernel feature, available under
namespaces/userns_create/pciu&, that enables the transition of
unconfined tasks to a special profile called unprivileged_userns when
they try to create an unprivileged user namespace with
clone/unshare. This transition allows the creation of the unprivileged
user namespace but hinders its privileges by not allowing
capabilities. Refer to the unprivileged_userns profile to check what
rules are allowed.
If either the feature is not present in the kernel, or the
unprivileged_userns profile is not loaded, then the defined behavior
is to deny the creation of the unprivileged user namespace
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Some of the tests are failing because of /usr/bin/true vs /bin/true.
Similarly to what was done in 8c09b328, to make the tests more
reliable, copy the true binary to $tmpdir and use this path on the
tests instead.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Unprivileged user namespace creation is allowed an will result in a
transition into the unprivileged_userns profile. The
unprivileged_userns profile with then deny all capabilities within the
profile. Execution of applications is allowed within the
unprivileged_userns profile but, they will result in a stack with the
unprivileged_userns profile, that is to say the unprivileged_userns
profile can not be dropped (capabilities can not be gained).
If the unprivileged_userns profile does not exist, unprivileged user
namespace creation is denied as before.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
/proc/$pid/cmdline can be changed by an application, therefore escape it
before printing.
The program name in /proc/$pid/exe can also contain any characters
(except \0 and shashes) and needs escaping.
Note: repr() wraps the string into single quotes, which we have to
remove to avoid changing the output format.
The test program from issue 364 now gets displayed as
28443 /path/to/issue364 (/\x1b]0;X\x07) not confined
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/364
I propose this patch for 2.13..master
Closes#364
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1142
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The call aa_getpeercon() can return ENOPROTOOPT error in some cases, specifically when the kernel lacks 'fine grained unix mediation'. Currently, this capability isn't available in upstream kernels, but only in patched ones (for example, the regular Ubuntu kernels). Unfortunately, the manpage lacks this info. This patch fixes this.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/366
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1143
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
/proc/$pid/cmdline can be changed by an application, therefore escape it
before printing.
The program name in /proc/$pid/exe can also contain any characters
(except \0 and shashes) and needs escaping.
Note: repr() wraps the string into single quotes, which we have to
remove to avoid changing the output format.
The test program from issue 364 now gets displayed as
28443 /path/to/issue364 (/\x1b]0;X\x07) not confined
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/364
There are some tests like attach_disconnected and posix_mq that can
have a program that calls another. For example, posix_mq_rcv calls
posix_mq_snd. Both of them write to the same output file, but the code
that checks the result expects only one line. This change enables
checking multiple lines in the output file and passing or failing
accordingly.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1140
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
There are some tests like attach_disconnected and posix_mq that can
have a program that calls another. For example, posix_mq_rcv calls
posix_mq_snd. Both of them write to the same output file, but the code
that checks the result expects only one line. This change enables
checking multiple lines in the output file and passing or failing
accordingly.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The move_mount tests were returning -1 in case of failure causing it
to become 255 in some systems, but checktestbg in the testsuite
considers any return value greater than 128 to be a signal error.
That would cause tests that should fail to display the following test
error:
... was expected to 'fail'. Reason for failure 'killed by
signal 127'
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1139
Approved-by: Steve Beattie <steve+gitlab@nxnw.org>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
The move_mount tests were returning -1 in case of failure causing it
to become 255 in some systems, but checktestbg in the testsuite
considers any return value greater than 128 to be a signal error.
That would cause tests that should fail to display the following test
error:
... was expected to 'fail'. Reason for failure 'killed by
signal 127'
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Instead of ignoring all exec events that happen in a hat/child profile,
only disallow child exec. ix and px are valid options inside a hat and
are now offered to the user.
(When the tools support nested child profiles one day, we can even allow
child exec again.)
[This MR is for master only. I opened separate MRs for 3.1 and 3.0]
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1133
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
the stacking tests need to be able to read and write the new apparmor
dir in proc, if that interface has been selected. Update the tests to
make sure they have the permissions needed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of ignoring all exec events that happen in a hat/child profile,
only disallow child exec. ix and px are valid options inside a hat and
are now offered to the user.
(When the tools support nested child profiles one day, we can even allow
child exec again.)
Add support for a default_allow mode that facillitates writing profiles
in that allow everything by default. This is not normally recomended
but fascilitates creating basic profiles while working to transition
policy away from unconfined.
This mode is being added specifically to replace the use of the
unconfined flag in these transitional profiles as the use of unconfined
in policy is confusing and does not reflect the semantics of what is
being done.
Generally the goal for policy should be to remove all default_allow
profiles once the policy is fully developed.
Note: this patch only adds parsing of default_allow mode. Currently
it sets the unconfined flag to achieve default allow but this
prevents deny rules from being applied. Once dominance is fixed a
subsequent patch will transition default_allow away from using
the unconfined flag.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1109
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Happy holidays, been meaning to push these updates out for some time:
* Add `owner` keyword to several rules to tighten them up. I've tested these for several months in normal usage and encountered no denials;
* Add new DBus access rules for the following:
```
Oct 18 06:26:06 darkstar kernel: [4369444.223230] audit: type=1107 audit(1697624766.349:2448): pid=745 uid=102 auid=4294967295 ses=4294967295 subj=unconfined msg='apparmor="DENIED" operation="dbus_signal" bus="system" path="/org/freedesktop/login1" interface="org.freedesktop.DBus.Properties" member="PropertiesChanged" name=":1.1" mask="receive" pid=1484746 label="firefox" peer_pid=773 peer_label="unconfined"
Oct 19 19:18:20 darkstar kernel: [4502177.573224] audit: type=1107 audit(1697757500.040:2456): pid=745 uid=102 auid=4294967295 ses=4294967295 subj=unconfined msg='apparmor="DENIED" operation="dbus_method_call" bus="system" path="/org/freedesktop/login1" interface="org.freedesktop.DBus.Properties" member="GetAll" mask="send" name=":1.1" pid=1677547 label="firefox" peer_pid=773 peer_label="unconfined"
Oct 19 19:18:20 darkstar kernel: [4502177.700071] audit: type=1107 audit(1697757500.168:2457): pid=745 uid=102 auid=4294967295 ses=4294967295 subj=unconfined msg='apparmor="DENIED" operation="dbus_method_call" bus="system" path="/org/freedesktop/timedate1" interface="org.freedesktop.DBus.Properties" member="GetAll" mask="send" name=":1.5878" pid=1484746 label="firefox" peer_pid=1677582 peer_label="unconfined"
```
* Deny write access to `/etc/**`, due to this odd bit:
```
Jul 27 15:23:21 darkstar kernel: [6530015.183715] audit: type=1400 audit(1690485801.308:128963): apparmor="DENIED" operation="mknod" profile="firefox" name="/etc/igfx_user_feature_next.txt" pid=2618266 comm="vaapitest" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
```
* Allow read access to a cgroup `cpu.max` variable. I lost the relevant log bit here, I'm afraid;
* Relocate the ptrace rule, as it relates to the crash reporter, not (as far as I've found) the Widevine plugin.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1131
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
man apparmor_parser gives examples for the --warn command line option as
apparmor_parser --warn=rules-not-enforced ...
and
apparmor_parser --warn=no-rules-not-enforced ...
but the actual --warn options are rule-not-enforced / no-rule-not-enforced
(without s)
Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1057453
Given the following profile:
profile foo {
profile bar {
profile baz {
}
}
}
The parser would correctly serialize the "foo" profile and the
"foo//bar" profile, but it would incorrectly name "bar//baz" when it
should be "foo//bar//baz". This would cause issues loading the profile
in certain kernels causing a "parent does not exist" error.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1127
Merged-by: John Johansen <john@jjmx.net>
Given the following profile:
profile foo {
profile bar {
profile baz {
}
}
}
The parser would correctly serialize the "foo" profile and the
"foo//bar" profile, but it would incorrectly name "bar//baz" when it
should be "foo//bar//baz". This would cause issues loading the profile
in certain kernels causing a "parent does not exist" error.
Partially addresses #346.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The recently added unconfined profiles use the binary name for the
local include instead of the profile name. Switch to using the
profile name for the local include.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for a default_allow mode that facillitates writing profiles
in that allow everything by default. This is not normally recomended
but fascilitates creating basic profiles while working to transition
policy away from unconfined.
This mode is being added specifically to replace the use of the
unconfined flag in these transitional profiles as the use of unconfined
in policy is confusing and does not reflect the semantics of what is
being done.
Generally the goal for policy should be to remove all default_allow
profiles once the policy is fully developed.
Note: this patch only adds parsing of default_allow mode. Currently
it sets the unconfined flag to achieve default allow but this
prevents deny rules from being applied. Once dominance is fixed a
subsequent patch will transition default_allow away from using
the unconfined flag.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Adding profiles for applications even if they allow all operations
will allow them to be referenced as peer by other policies. This is
a step towards a more comprehensive system policy, adding names,
instead of just unconfined, to peers of existing policy and to
applications that are known to use unprivileged user namespaces.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1123
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When the regex parser failed, the Chars objects created/used in rules
charset and cset_chars would not be cleaned up properly and would
leak.
Closes#361
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Adding profiles for applications even if they allow all operations
will allow them to be referenced as peer by other policies. This is a
step towards a more comprehensive system policy, adding names, instead
of just unconfined, to peers of existing policy and to applications
that are known to use unprivileged user namespaces.
Note that unconfined mode should be changed for default_allow
when https://gitlab.com/apparmor/apparmor/-/merge_requests/1109 is
merged.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
upstream kernels only have network_v8 unfortunately the tcp tests were
only being run against kernels that had network (which is v7). Kernels
that support both (Ubuntu) would be tested against v8, so v8 has been
tested but pure upstream kernels were failing to be tested correctly.
This patch will only make sure one of the supported verserions are
tested. This is determined by the parser which prefers v8. In the
future the tests need to be extended to run the tests against all
kernel supported versions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1120
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
upstream kernels only have network_v8 unfortunately the tcp tests were
only being run against kernels that had network (which is v7). Kernels
that support both (Ubuntu) would be tested against v8, so v8 has been
tested but pure upstream kernels were failing to be tested correctly.
This patch will only make sure one of the supported versions are
tested. This is determined by the parser which prefers v8. In the
future the tests need to be extended to run the tests against all
kernel supported versions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Teach CapabilityRule about the list of known capabilities, and ensure that only valid capabilities are allowed in profiles.
This comes with several test additions (and removals from the `exception_not_raised` list for the parser simple_tests), see the individual commits for details.
Reviewing each commit on its own is probably easier than reading the merged diff.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1117
Merged-by: John Johansen <john@jjmx.net>
Even if this is very unlikely to happen (because of the previously added
test, and because CapabilityRule only allows to specify known severity
keywords), ensure proper behaviour if an unknown severity gets rated.
- convert a string parameter to a list to avoid duplication of the
validation logic
- add separate check for empty cap_list
- remove check for empty strings - the previous commit already added
such a check to the for loop. Also, move the comment to that check.
... and error out if an unknown capability is given.
This also means recognizing bad capabilities in the parser simple_tests
now works (so remove these from the exception_not_raised list), and that
we can no longer hand over an unknown capability in test-capability.py
to test their severity.
... instead of non-existing ones.
This is a search-and-replace commit:
ptrace -> sys_ptrace
chgrp -> fowner (because fowner wasn't used in the test before)
Out of the box the KDE plasma-browser-integration package does not work
after a user installed the corresponding Firefox extension: The browser
can't start the native host binary. The same is probably true for
Chromium.
This was originally reported to KDE at https://bugs.kde.org/show_bug.cgi?id=397399
tools.py contained quite some things that need a big cleanup.
See the individual commits for details and more readable diffs.
Note: This MR "only" does cleanups and some refactoring. It does not change the (user-visible) behaviour of the code.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1114
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This also means we can get rid of most cleanprof-specific conditions
without changing the behaviour (because the other functions don't use
'profile' yet).
Also hand over prof_filename to clean_profile() so that it doesn't need
to find it out itsself.
Before, the 'profile' return value was either a profile name or a
profile filename, depending on the active module (cleanprof vs.
everything else).
Separate the return values so that it's clear what we get.
Notes:
- This commit doesn't change functionality, only the number of return
values and some variable names.
- There's no guarantee that all return values are set. They can also be
None. (This might change in the future.)
Also adjust the callers of get_next_to_profile(), and rename 'profile'
to 'prof_filename' in calling functions that actually use the profile
filename.
This needed replacement of "program" with "profile" at various places in
tools.py (of course this description is over-simplified).
The changes in get_next_to_profile() (which is used by several aa-*
minitools) are restricted to cleanprof to avoid side effects in the
other aa-* minitools.
However, the other aa-* minitools possibly also suffer from problems
with named profiles, but checking and fixing that is left for another
commit ;-)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/351
.
The fix needs an additional function in ProfileList (`profile_from_attachment()`) to get the profile name for a given attachment.
Since this is not very different from filename_from_attachment(), move
most of the code into a thing_from_attachment() function, and make
{profile,filename}_from_attachment wrappers for it.
Also adjust the tests to the changed internal data structure, and add
tests for profile_from_attachment().
I propose this patch for 3.0..master. (3.0 will probably need a slightly different patch - I'll submit a separate MR once this MR is merged.)
Closes#351
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1108
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This needed replacement of "program" with "profile" at various places in
tools.py (of course this description is over-simplified).
The changes in get_next_to_profile() (which is used by several aa-*
minitools) are restricted to cleanprof to avoid side effects in the
other aa-* minitools.
However, the other aa-* minitools possibly also suffer from problems
with named profiles, but checking and fixing that is left for another
commit ;-)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/351
... to get the profile name for a given attachment.
Since this is not very different from filename_from_attachment(), move
most of the code into a thing_from_attachment() function, and make
{profile,filename}_from_attachment wrappers for it.
Also adjust the tests to the changed internal data structure, and add
tests for profile_from_attachment().
Extend the policy syntax to have a rule that allows specifying all
permissions for all rule types.
allow all,
This is useful for making blacklist based policy, but can also be
useful when combined with other rule prefixes, eg. to add audit
to all rules.
audit access all,
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1103
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Extend the policy syntax to have a rule that allows specifying all
permissions for all rule types.
allow all,
This is useful for making blacklist based policy, but can also be
useful when combined with other rule prefixes, eg. to add audit
to all rules.
audit access all,
Signed-off-by: John Johansen <john.johansen@canonical.com>
This lays the ground work for fine grained network mediation. The switch to using the rules class brings the advantages of shared infrastructure and is needed for tracking all the different combinations possible with finer control.
In addition range generation and support for large numbers (needed for ipv6) are added. This patchset does not make policy visible changes to network rules.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1104
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Since network rules don't use the "perms" attribute, it is using the
dedup class in which duplicate rules are removed.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
There is one significant difference in the encoding of the network
rules. Before this change, when the parser was encoding a "network,"
rule, it would generate an entry for every family and every
type/protocol. After this patch the parser should generate an entry
for every family, but the type/protocol is changed to .. in the pcre
syntax. There should be no difference in behavior.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
using dfprintf for error messages subjects them to the other
output controls and can cause them to be surpressed when they
shouldn't.
Instead use a dedicated error function and add a quiet flag to
allow silencing errors.
Signed-off-by: John Johansen <john.johansen@canonical.com>
By default aa-status outputs with --verbose enabled - if --json is also
specified then aa-status would currently output in its first line "apparmor
module is loaded.":
aa-status --json | head -n1
apparmor module is loaded.
And only after this the actual json output would follow. This then results in
failures to parse this JSON output:
aa-status --json | jq .
parse error: Invalid numeric literal at line 1, column 9
This in turn then breaks tools / tests which expect the output of aa-status
--json to be purely json - e.g:
https://salsa.debian.org/apparmor-team/apparmor-profiles-extra/-/blob/debian/unstable/debian/tests/policy-is-loaded#L12
So ensure dprintf() etc do not output when --json is specified to restrict the
output of aa-status to pure JSON.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1097
Approved-by: Seth Arnold <seth.arnold@gmail.com>
Merged-by: John Johansen <john@jjmx.net>
coverity is reporting an overrun of the profile_mode_table
217 if (merge_profile_mode(mode, rhs.mode) == MODE_CONFLICT)
>>> CID 322989: (OVERRUN)
>>> Overrunning array "profile_mode_table" of 6 8-byte elements at element index 6 (byte offset 55) using index "this->mode" (which evaluates to 6).
this is because it is being indexed by the profile_mode enum which can
go up to a 6th entry. The code tests for MODE_CONFLICT before using
the table so it shouldn't trigger a bug today, but play it safe for
the future and also get rid of the coverity scan error by adding a
"conflict" entry to the mode_table.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1098
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
SIGNAL makes more sense because it's about a single signal.
Besides that, a9494f5523 introduced a (at
that point broken) usage of SIGNAL which becomes valid with this commit.
coverity is reporting an overrun of the profile_mode_table
217 if (merge_profile_mode(mode, rhs.mode) == MODE_CONFLICT)
>>> CID 322989: (OVERRUN)
>>> Overrunning array "profile_mode_table" of 6 8-byte elements at element index 6 (byte offset 55) using index "this->mode" (which evaluates to 6).
this is because it is being indexed by the profile_mode enum which can
go up to a 6th entry. The code tests for MODE_CONFLICT before using
the table so it shouldn't trigger a bug today, but play it safe for
the future and also get rid of the coverity scan error by adding a
"conflict" entry to the mode_table.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds two new profile flags
* `interruptible` which can be used with prompt
* `kill.signal=XXX` which can be used to set the signal used by kill mode or the kill rule prefix
In addition it adds a few cleanups and fixes around profile flag handling
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1096
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Add a flag that allows setting the signal used to kill the process.
This should not be normally used but can be very useful when
debugging applications, interaction with apparmor.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When merging flags with a disconnected_path specified, there is no
check for a conflict and we can just throw away (and leak) one of
the paths.
Signed-off-by: John Johansen <john.johansen@canonical.com>
We have a basic flagvals class it makes sense to move the parser
code into it, to help make management easier going forward.
Signed-off-by: John Johansen <john.johansen@canonical.com>
By default aa-status outputs with --verbose enabled - if --json is also
specified then aa-status would currently output in its first line "apparmor
module is loaded.":
aa-status --json | head -n1
apparmor module is loaded.
And only after this the actual json output would follow. This then results in
failures to parse this JSON output:
aa-status --json | jq .
parse error: Invalid numeric literal at line 1, column 9
This in turn then breaks tools / tests which expect the output of aa-status
--json to be purely json - e.g:
https://salsa.debian.org/apparmor-team/apparmor-profiles-extra/-/blob/debian/unstable/debian/tests/policy-is-loaded#L12
So ensure dprintf() etc do not output when --json is specified to restrict the
output of aa-status to pure JSON.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
ask_exec() and ask_addhat() set
hashlog[aamode][full_profile]['final_name'].
While this was used to get profile and hat split, it was not used as key
for log_dict. This resulted in entries like
log_dict['PERMITTING']['foo//null-/usr/bin/cat']
which are obviously wrong.
Use final_name as log_dict key so that we end up with (assuming child
exec was selected)
log_dict['PERMITTING']['foo///usr/bin/cat']
This fixes a regression introduced in 3.1. Due to other changes in collapse_log() done in master, picking this into 3.1 isn't that easy. I'll submit a separate patch for 3.1.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1091
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Ideally we'd update them to the chosen exec target - but until this is
implemented, it doesn't make sense to ask about adding a //null-* peer
to a profile.
I propose this patch for all branches.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1090
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
setns tests part of the userns could fail if the parent process opened
the child pipe to write it was done before the child opened the pipe
with read permissions.
From the fifo(7) man page:
A process can open a FIFO in nonblocking mode. In this case, opening
for read‐only succeeds even if no one has opened on the write side yet
and opening for write‐only fails with ENXIO (no such device or
address) unless the other end has already been opened.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1093
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
setns tests part of the userns could fail if the parent process opened
the child pipe to write it was done before the child opened the pipe
with read permissions.
From the fifo(7) man page:
A process can open a FIFO in nonblocking mode. In this case, opening
for read‐only succeeds even if no one has opened on the write side yet
and opening for write‐only fails with ENXIO (no such device or
address) unless the other end has already been opened.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Now that we have `final_name` as shortcut for
`hashlog[aamode][full_profile]['final_name']`, update the code that used
the long version to make it more readable.
ask_exec() and ask_addhat() set
hashlog[aamode][full_profile]['final_name'].
While this was used to get profile and hat split, it was not used as key
for log_dict. This resulted in entries like
log_dict['PERMITTING']['foo//null-/usr/bin/cat']
which are obviously wrong.
Use final_name as log_dict key so that we end up with (assuming child
exec was selected)
log_dict['PERMITTING']['foo///usr/bin/cat']
Ideally we'd update them to the chosen exec target - but until this is
implemented, it doesn't make sense to ask about adding a //null-* peer
to a profile.
On (terribly, but real-world) slow buid hosts, running test-logprof.py
fails with a timeout. Increase the timeout so that even those build
hosts get enough time to finish the aa-logprof tests.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1087
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
On (terribly, but real-world) slow buid hosts, running test-logprof.py
fails with a timeout. Increase the timeout so that even those build
hosts get enough time to finish the aa-logprof tests.
Currently you need to use attach_disconnected with
attach_disconnected.path=XXX to be able to attach to a different
location than / whic is ugly and redundant.
Make it so attach_disconnected.path implies attach_disconnected.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for specifying the path prefix used when attach disconnected
is specified. The kernel supports prepending a different value than
/ when a path is disconnected. Expose through a profile flag.
Signed-off-by: John Johansen <john.johansen@canonical.com>
... and a simple test for a single (fake) event for ping.
Notes:
- to let aa-logprof work in the CI environment, we need to skip checking
for the AppArmor mountpoint. Introduce --no-check-mountpoint for this.
- PYTHONPATH and LD_LIBRARY_PATH need to be explicitely forwarded when
starting aa-logprof via subprocess.Popen()
- if the test runs with coverage enabled, it will also start aa-logprof
with coverage (parameters copied from Makefile).
Speaking about coverage - this test adds 4% overall coverage, and 10%
more coverage for apparmor/aa.py.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1082
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
... and a simple test for a single (fake) event for ping.
Notes:
- to let aa-logprof work in the CI environment, we need to skip checking
for the AppArmor mountpoint. Introduce --no-check-mountpoint for this.
- PYTHONPATH and LD_LIBRARY_PATH need to be explicitely forwarded when
starting aa-logprof via subprocess.Popen()
- if the test runs with coverage enabled, it will also start aa-logprof
with coverage (parameters copied from Makefile).
Speaking about coverage - this test adds 4% overall coverage, and 10%
more coverage for apparmor/aa.py.
(except the deprecated tunables/sys)
This allows users to extend variables without editing the main tunables
files.
It also allows to cleanly introduce new tunable files (via
tunables/global.d) and new aliases (via tunables/alias.d).
Note: some files already had `include <tunables/$FILE.d>`. These get
changed to `include if exists`, and the comments for these includes get
unified.
Also extend make check to ensure that all tunables include tunables/$FILE.d
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/347
I propose this patch for 3.0..master.
Closes#347
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1077
Approved-by: timeout
Merged-by: Christian Boltz <apparmor@cboltz.de>
Allow for the default libexec subdir, /usr/libexec/dovecot, as well
as the more common /usr/lib/dovecot.
Signed-off-by: Peter Levine <plevine457@gmail.com>
The permissions for AA_NET_OPT need to be bounded by mask so we can
make sure it matches when a policy specified only setopt or only
getopt. This was causing failures on the regression tests
unix_socket_pathname, unix_socket_abstract, unix_socket_unnamed and
unix_socket_autobind
Fixes: 44f3be091 ("parser: convert the stored audit from a bit mask to a bool")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1079
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The permissions for AA_NET_OPT need to be bounded by mask so we can
make sure it matches when a policy specified only setopt or only
getopt. This was causing failures on the regression tests
unix_socket_pathname, unix_socket_abstract, unix_socket_unnamed and
unix_socket_autobind
Fixes: 44f3be091 ("parser: convert the stored audit from a bit mask to a bool")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Add a json_log option (default: disabled) to logprof.conf that enables
logging of all aa-logprof and aa-genprof input and output to a
/tmp/aa-jsonlog-* file.
This can be useful for debugging, and maybe also to create tests that do
a full aa-logprof run.
This patch introduces a minor behaviour change if aa-logprof errors out
on startup (for example if the config file is broken or the parser can't
be found):
Before:
```
$ aa-logprof --json
{"dialog": "apparmor-json-version","data": "2.12"}
ERROR: Can't find apparmor_parser at /sbin/apparmor_parser
```
After:
```
$ aa-logprof --json
ERROR: Can't find apparmor_parser at /sbin/apparmor_parser
```
Note that the json version line will not be printed if aa-logprof or
aa-genprof error out that early.
If there are no startup errors, the behaviour will not change.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1078
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Minor fixes for the firefox profile:
1. The attachment spec was failing to match `/usr/lib/firefox-esr/firefox-esr` on Debian;
2. The `Mounted` method of `org.gtk.vfs.MountTracker` is received, not sent.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1076
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Add a json_log option (default: disabled) to logprof.conf that enables
logging of all aa-logprof and aa-genprof input and output to a
/tmp/aa-jsonlog-* file.
This can be useful for debugging, and maybe also to create tests that do
a full aa-logprof run.
This patch introduces a minor behaviour change if aa-logprof errors out
on startup (for example if the config file is broken or the parser can't
be found):
Before:
```
$ aa-logprof --json
{"dialog": "apparmor-json-version","data": "2.12"}
ERROR: Can't find apparmor_parser at /sbin/apparmor_parser
```
After:
```
$ aa-logprof --json
ERROR: Can't find apparmor_parser at /sbin/apparmor_parser
```
Note that the json version line will not be printed if aa-logprof or
aa-genprof error out that early.
If there are no startup errors, the behaviour will not change.
(except the deprecated tunables/sys)
This allows users to extend variables without editing the main tunables
files.
It also allows to cleanly introduce new tunable files (via
tunables/global.d) and new aliases (via tunables/alias.d).
Note: some files already had `include <tunables/$FILE.d>`. These get
changed to `include if exists`, and the comments for these includes get
unified.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/347
It appears that lto1 does not support -dynamic, and that ld no longer
documents -dynamic. Which results in the following build failure
when usine lto1 for link time optimizations
```
[ 45s] libtool: link: gcc -shared -fPIC -DPIC .libs/grammar.o .libs/libaalogparse.o .libs/kernel.o .libs/scanner.o .libs/private.o .libs/features.o .libs/kernel_interface.o .libs/policy_cache.o .libs/PMurHash.o -flto-partition=none -O2 -fstack-protector-strong -flto=auto -dynamic -Wl,--version-script=../src/libapparmor.map -flto=auto -pthread -Wl,-soname -Wl,libapparmor.so.1 -o .libs/libapparmor.so.1.17.0
[ 45s] lto1: warning: unrecognized gcc debugging option: y
[ 45s] lto1: warning: unrecognized gcc debugging option: n
[ 45s] lto1: warning: unrecognized gcc debugging option: m
[ 45s] lto1: warning: unrecognized gcc debugging option: i
[ 45s] lto1: warning: unrecognized gcc debugging option: c
```
Switch to -Bdynamic which is documented.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1071
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
The test to check if the kernel supports a feature covers two cases:
1. The file/directory indicates a feature is supported.
2. The feature is supported if it's in the contents of the file.
When the intended check is for case 1, and the file does not exist,
then the code checks if it's case 2, but since it was not supposed to
be, we end up grepping a directory, causing an error message. Fix this
by checking if we're grepping a file.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1074
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The test to check if the kernel supports a feature covers two cases:
1. The file/directory indicates a feature is supported.
2. The feature is supported if it's in the contents of the file.
When the intended check is for case 1, and the file does not exist,
then the code checks if it's case 2, but since it was not supposed to
be, we end up grepping a directory, causing an error message. Fix this
by checking if we're grepping a file.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
This reverts commit 460c3d5b59.
The 4.0 ABI was pinned for the development cycle but it can cause
in-tree uses of parser/parser.conf to fail if it cannot find the 4.0
file in /etc/apparmor.d/abi/.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
It appears that lto1 does not support -dynamic, and that ld no longer
documents -dynamic. Which results in the following build failure
when usine lto1 for link time optimizations
[ 45s] libtool: link: gcc -shared -fPIC -DPIC .libs/grammar.o .libs/libaalogparse.o .libs/kernel.o .libs/scanner.o .libs/private.o .libs/features.o .libs/kernel_interface.o .libs/policy_cache.o .libs/PMurHash.o -flto-partition=none -O2 -fstack-protector-strong -flto=auto -dynamic -Wl,--version-script=../src/libapparmor.map -flto=auto -pthread -Wl,-soname -Wl,libapparmor.so.1 -o .libs/libapparmor.so.1.17.0
[ 45s] lto1: warning: unrecognized gcc debugging option: y
[ 45s] lto1: warning: unrecognized gcc debugging option: n
[ 45s] lto1: warning: unrecognized gcc debugging option: m
[ 45s] lto1: warning: unrecognized gcc debugging option: i
[ 45s] lto1: warning: unrecognized gcc debugging option: c
Switch to -Bdynamic which is documented.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add tests for passing the parser a file that doesn't exist, a symlink
to a file that doesn't exist, and a directory that contains that
latter. Also include tests for different levels of -j passed as an
argument. These tests are based on the fixing commit 1259319508
("parser: Fix parser failing to handle errors when setting up work")
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1070
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Instead of pushing the cmp logic for rule merging into each rule
class make it the default behavior for the perms_rule_t parent class.
Also save off the original perms for the merged rule.
For classes that don't want perms merging add an alternate
dedup_perms_rule_t clase.
Signed-off-by: John Johansen <john.johansen@canonical.com>
speedup and reduce memory usage of dfa generation
A variety of changes to improve dfa generation
- By switching to Nodevec instead of Node sets we can reduce memory usage slightly and reduce code
- By using charsets for chars we reduce code and increase chances of node merging/reduction which reduces memory usage slightly
- By merging charsets we reduce the number of nodes
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1066
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
By changing the compare function from each rule to use class_rule_t,
instead of perms_rule_t, we temporarily ignore if permissions are
different. If every rule attribute is the same, then the permissions
can be merged. This is done at the perms_rule_t's level.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1068
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
By changing the compare function from each rule to use class_rule_t,
instead of perms_rule_t, we temporarily ignore if permissions are
different. If every rule attribute is the same, then the permissions
can be merged. This is done at the perms_rule_t's level.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Currently File rules are the only rules that have rule dedup/merging performed. Extend support for rule merging to all other rule types.
This can result in a small performance regression when rules can not be merged/deduped but can result in a large performance increase when lots of rules can be eliminated.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1065
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Instead of having multiple tables, since we have room post split
of optimization and dump flags just move all the optimization and
dump flags into a common table.
We can if needed switch the flag entry size to a long in the future.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the ability to control whether rule merging is done.
TODO: in the furture cleanup display of flags split accross two tables
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for more flags (not all of the backend dfa based),
rework the optimization and dump flag handling which has been exclusively
around the dfa up to this point.
- split dfa control and dump flags into separate fields. This gives more
room for new flags in the existing DFA set
- rename DFA_DUMP, and DFA_CONTROL to CONTROL_DFA and DUMP_DFA as
this will provide more uniform naming for none dfa flags
- group dump and control flags into a structure so they can be passed
together.
Signed-off-by: John Johansen <john.johansen@canonical.com>
this is reuired because af_rule merging does not take into account
the potential af_unix addresses and could incorrectly merge af_unix
rules.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently only file rules get merged. Finish adding basic support
for rule merging and make the default the behavior to dedup
merge rules that are exact matches.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Compiling of io_uring tests fail if liburing-dev is not installed.
Also, the tests were not running as part of the test suite.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
In MR #1063 the tests/regression/apparmor/syscall.sh script was updated to
account for kernel lockdown, but the capabilities.sh script also exercises these
system calls so this also needs to be updated as well.
Also required to fix issue #226.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
When kernel lockdown is enabled the ioperm and iopl tests will fail regardless
since lockdown prevents these syscalls before AppArmor has a chance to mediate
them. So workaround this by detecting when lockdown is enabled and expect the
tests to fail in that case.
Fixes issue #226.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Closes#226
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1063
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When kernel lockdown is enabled the ioperm and iopl tests will fail regardless
since lockdown prevents these syscalls before AppArmor has a chance to mediate
them. So workaround this by detecting when lockdown is enabled and expect the
tests to fail in that case.
Fixes issue #226.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
character sets are just a way of enumerating to exact match rules
more succinctly, so loosen the exact match check to allow them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Allowing access to a debug flag can greatly improve policy debugging.
This is different than the debug mode of old, that was removed. It only
will trigger additional messages to the kernel ring buffer, not
the audit log, and it does not change mediation.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Begin preparing policy for the 4.0 release. This may result in new
denials. This is expected and needed to make sure policy is ready
for the 4.0 release.
Signed-off-by: John Johansen <john.johansen@canonical.com>
I have a number of updates for the Firefox profile, based on monitoring AppArmor logs in the course of my own usage.
I'm going to try annotating the diff with the appropriate log messages, to see if that is a useful way of documenting the changes.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1055
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Dir "d" is not closed when asprintf fails.
Found by coverity: CID 321416: Resource leaks (RESOURCE_LEAK)
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The check isn't correct, it should be checking for capability
MAC_ADMIN, but in the future that won't be correct either. Instead
rely on the kernel to check permission to load policy, which it alread
does as it is possible to by-pass the parser to load policy.
Also improve the error message when the kernel does deny
loading policy due to failed permission checks.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1044
Approved-by: Seth Arnold <seth.arnold@gmail.com>
Merged-by: John Johansen <john@jjmx.net>
The check isn't correct, it should be checking for capability
MAC_ADMIN, but in the future that won't be correct either. Instead
rely on the kernel to check permission to load policy, which it alread
does as it is possible to by-pass the parser to load policy.
Also improve the error message when the kernel does deny
loading policy due to failed permission checks.
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa-load is a tool that loads cached (compiled) policies into
the kernel. It can receive as argument a file, a cache directory
containing the hash subtree, and a directory containing cached
files directly underneath - no hash.
This tool can be used in the as a guide for other init
systems to load the cached policies directly.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/770
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This Fixes coverity CID 254463: Memory - illegal accesses (USE_AFTER_FREE)
which was introduced in 35e58273 / https://gitlab.com/apparmor/apparmor/-/merge_requests/713 (merged)
The coverity report it self is wrong because while the pointer use is
after free. It is deliberately a pointer comparison to see if the
value was pointing to proc_attr_base_old. This is not actually
problematic as far as it goes. However it does surface a bug in the
logic, and regardless if it is allowed doing a post-free pointer
comparison like this will confuse static checkers and humans alike.
procattr_path() will never return proc_attr_base_old, it will return a
copy of it (if it did return it, we couldn't free it). So the test tmp
!= proc_attr_base_old is bad, in that it will always be true even if
tmp was a copy of proc_attr_base_old.
Fix this by makint tmp auto free, and switching to a string comparison.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1042
Aked-by: time-out
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
made it so rules like
mount slave /snap/bin/** -> /**,
mount /snap/bin/** -> /**,
would get passed into change_mount_type rule generation when they
shouldn't have been. This would result in two different errors.
1. If kernel mount flags were present on the rule. The error would
be caught causing an error to be returned, causing profile compilation
to fail.
2. If the rule did not contain explicit flags then rule would generate
change_mount_type permissions based on souly the mount point. And
the implied set of flags. However this is incorrect as it should
not generate change_mount permissions for this type of rule. Not
only does it ignore the source/device type condition but it
generates permissions that were never intended.
When used in combination with a deny prefix this overly broad
rule can result in almost all mount rules being denied, as the
denial takes priority over the allow mount rules.
Fixes: https://bugs.launchpad.net/apparmor/+bug/2023814
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1211989
Fixes: 9d3f8c6cc ("parser: fix parsing of source as mount point for propagation type flags")
Fixes: MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1054
Signed-off-by: John Johansen <john.johansen@canonical.com>
(cherry picked from commit 86d193e183)
Signed-off-by: John Johansen <john.johansen@canonical.com>
This Fixes coverity CID 254463: Memory - illegal accesses (USE_AFTER_FREE)
which was introduced in 35e58273 / https://gitlab.com/apparmor/apparmor/-/merge_requests/713 (merged)
The coverity report it self is wrong because while the pointer use is
after free. It is deliberately a pointer comparison to see if the
value was pointing to proc_attr_base_old. This is not actually
problematic as far as it goes. However it does surface a bug in the
logic, and regardless if it is allowed doing a post-free pointer
comparison like this will confuse static checkers and humans alike.
procattr_path() will never return proc_attr_base_old, it will return a
copy of it (if it did return it, we couldn't free it). So the test tmp
!= proc_attr_base_old is bad, in that it will always be true even if
tmp was a copy of proc_attr_base_old.
Fix this by makint tmp auto free, and switching to a string comparison.
Signed-off-by: John Johansen <john.johansen@canonical.com>
... so that each change in master gets scanned (until we reach the scan
limit).
This will give us more timely results than only scanning the coverity
branch whenever someone manually updates it.
I have been testing the `usr.sbin.sshd` profile with the [three "for internal-sftp" rules](ad3750058d/profiles/apparmor/profiles/extras/usr.sbin.sshd (L138-140)) commented out, as they are quite broad. This has turned up a few oversights, a couple of which have been fixed in abstractions. This MR brings in the missing rules that don't currently seem to have a better place to go, and addresses a couple of other minor issues in the profile.
Here are the associated syslog messages:
```
Jun 8 21:09:11 testvm kernel: [ 28.211637] audit: type=1400 audit(1686272951.369:66): apparmor="DENIED" operation="sendmsg" profile="/usr/sbin/sshd" name="/run/systemd/notify" pid=830 comm="sshd" requested_mask="w" denied_mask="w" fsuid=0 ouid=0
Jun 9 00:27:24 testvm kernel: [10846.886139] audit: type=1400 audit(1686284844.516:1268): apparmor="DENIED" operation="mkdir" profile="/usr/sbin/sshd" name="/home/username/.cache/" pid=5815 comm="sshd" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
Jun 9 00:28:57 testvm kernel: [10940.152625] audit: type=1400 audit(1686284937.780:1337): apparmor="DENIED" operation="mknod" profile="/usr/sbin/sshd" name="/home/username/.cache/motd.legal-displayed" pid=5979 comm="sshd" requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1051
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Found in testing a slimmed-down `usr.sbin.sshd` profile:
```
Jun 8 21:09:38 testvm kernel: [ 54.847014] audit: type=1400 audit(1686272978.009:68): apparmor="DENIED" operation="open" profile="/usr/sbin/sshd" name="/sys/kernel/mm/transparent_hugepage/hpage_pmd_size" pid=1035 comm="sshd" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
```
Not sure what glibc/system call uses this, but it seems pretty broadly applicable, and read access is presumably harmless. [THP reference](https://www.kernel.org/doc/html/latest/admin-guide/mm/transhuge.html)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1050
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
/etc/ld-musl-*.path is required to perform dynamic linking on musl libc.
The wildcard is to match all CPU architectures, like x86_64.
type=AVC msg=audit(1686087677.497:67): apparmor="DENIED" operation="open" class="file" profile="syslog-ng" name="/etc/ld-musl-x86_64.path" pid=25866 comm="syslog-ng" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Closes#333
Signed-off-by: Nikita Romaniuk <kelvium@yahoo.com>
Closes#333
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1047
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Before 300889c3a, mount rules would compile policy when using source
as mount point for rules that contain propagation type flags, such as
unbindable, runbindable, private, rprivate, slave, rslave, shared, and
rshared. Even though it compiled, the rule generated would not work as
expected.
This commit fixes both issues. It allows the usage of source as mount
point for the specified flags, albeit with a deprecation warning, and
it correctly generates the mount rule.
The policy fails to load when both source and mount point are
specified, keeping the original behavior (reference
parser/tst/simple_tests/mount/bad_opt_10.sd for example).
Fixes: https://bugs.launchpad.net/bugs/1648245
Fixes: https://bugs.launchpad.net/bugs/2023025
It should be backported to versions 2.13, 3.0, 3.1.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1048
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Before 300889c3a, mount rules would compile policy when using source
as mount point for rules that contain propagation type flags, such as
unbindable, runbindable, private, rprivate, slave, rslave, shared, and
rshared. Even though it compiled, the rule generated would not work as
expected.
This commit fixes both issues. It allows the usage of source as mount
point for the specified flags, albeit with a deprecation warning, and
it correctly generates the mount rule.
The policy fails to load when both source and mount point are
specified, keeping the original behavior (reference
parser/tst/simple_tests/mount/bad_opt_10.sd for example).
Fixes: https://bugs.launchpad.net/bugs/1648245
Fixes: https://bugs.launchpad.net/bugs/2023025
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
/etc/ld-musl-*.path is required to perform dynamic linking on musl libc.
The wildcard is to match all CPU architectures, like x86_64.
type=AVC msg=audit(1686087677.497:67): apparmor="DENIED" operation="open" class="file" profile="syslog-ng" name="/etc/ld-musl-x86_64.path" pid=25866 comm="syslog-ng" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Closes#333
Signed-off-by: Nikita Romaniuk <kelvium@yahoo.com>
When opening snap browsers with evince using the snap_browsers
abstraction, we get the following AppArmor denials which prevent the
browsers from opening
audit: type=1400 audit(1685996894.479:225): apparmor="DENIED" operation="open" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=13282 comm="snap" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
audit: type=1400 audit(1685997517.142:259): apparmor="DENIED" operation="file_lock" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=14200 comm="snap" requested_mask="k" denied_mask="k" fsuid=1000 ouid=0
This MR should be cherry-picked into 2.13, 3.0, 3.1
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1045
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
When opening snap browsers with evince using the snap_browsers
abstraction, we get the following AppArmor denials which prevent the
browsers from opening
audit: type=1400 audit(1685996894.479:225): apparmor="DENIED" operation="open" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=13282 comm="snap" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
audit: type=1400 audit(1685997517.142:259): apparmor="DENIED" operation="file_lock" class="file" profile="/usr/bin/evince//snap_browsers" name="/var/lib/snapd/inhibit/firefox.lock" pid=14200 comm="snap" requested_mask="k" denied_mask="k" fsuid=1000 ouid=0
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
If `else if (preprocess_only)` is true, the more strict condition
`else if (!include_file && preprocess_only)` won't be reached if it gets
checked after the shorter condition.
Exchange the two sections so that both code paths can be reached.
Fixes coverity CID 312499: Control flow issues (DEADCODE)
In some cases (if profiles in complain and enforce mode are loaded), the
`i` loop runs more than once, which also means `j == 0` is true in the
middle of the json. This causes invalid json.
This patch fixes this.
This is a regression related to 22aa9b6161
/ https://gitlab.com/apparmor/apparmor/-/merge_requests/964 /
https://gitlab.com/apparmor/apparmor/-/issues/295
which fixed another case of invalid json if a process was unconfined
while having a profile defined.
Note: I also tested this patch for the "unconfined, but has a profile
defined" case to ensure it doesn't break what
22aa9b6161 fixed.
Enhance aa-status by adding posix regex filter support and support for new modes
this MR adds the following command line options
```
--show={profiles,processes,all}
--count
--filter.mode=filter
--filter.processs=filter
--filter.profiles=filter
--filter.pid=filter
```
Where currently filters use the basic posix regexs, do we want to go with extended regex, or apparmor globbing
An example of using the new filters
```
$ sudo ./aa-status --filter.exe="firefox" --show=processes
apparmor module is loaded.
65 processes have profiles defined.
25 processes are in enforce mode.
/snap/firefox/2605/usr/lib/firefox/firefox (4406) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (4686) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (4721) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (4826) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (19204) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (19206) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (23159) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (23195) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (23292) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (30929) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (31487) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (31528) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (71708) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2271631) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2271969) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2272030) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2272347) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2654995) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2655042) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2655082) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2655152) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2655237) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2656215) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2656334) snap.firefox.firefox
/snap/firefox/2605/usr/lib/firefox/firefox (2657837) snap.firefox.firefox
0 processes are in complain mode.
0 processes are in prompt mode.
0 processes are in kill mode.
0 processes are unconfined but have a profile defined.
0 processes are in mixed mode.
```
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1016
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Bring in updates and improvements that have been made to the Chromium profile at `apparmor-profiles/ubuntu/20.04/usr.bin.chromium-browser`.
The changes are largely as follows:
* Use `@{pid}` or `@{tid}` instead of `[0-9]*`
* Use the discrete `lsb_release` profile instead of a subprofile
* Add a few additional libraries to the `sandbox` subprofile
Once this is in, the Chromium profiles in `apparmor-profiles` can be considered deprecated, and I would advise slating them for removal altogether.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1032
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: John Johansen <john@jjmx.net>
`unscd` is a drop-in replacement for `nscd` that uses the same binary location (`/usr/sbin/nscd`) and config file (`/etc/nscd.conf`). The `usr.sbin.nscd` profile only needs one additional permission to support it.
```
May 9 18:07:42 darkstar kernel: [ 2706.138823] audit: type=1400
audit(1683670062.580:839): apparmor="DENIED" operation="sendmsg"
profile="nscd" name="/run/systemd/notify" pid=4343 comm="nscd"
requested_mask="w" denied_mask="w" fsuid=125 ouid=0
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1031
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
The mount options MS_LAZYTIME and MS_NOSYMFOLLOW were added in
kernels 4.0 and 5.10, respectively. Update the mount test script
and helper to skip testing those options if they are not available.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
To emphasize that the
--mode
--processs
--profiles
--pid
options are filters rename them to
--filter.mode
--filter.processs
--filter.profiles
--filter.pid
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the basic infrastructure for adding regex based filters and
allow filtering process and profiles by mode based on a user supplied
filter.
Signed-off-by: John Johansen <john.johansen@canonical.com>
So that the user can control the set of data being shown add a --show
flag. Currently it allows showing
all
profiles
processes
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for new options and controls, split detailed_output()
into detailed_profiles() and detailed_processes() and This switch the
output processing to one larger pass that uses option flags.
This doesn't change current behavior and is in not as clean as the
current code for the current option set. However it will allow us to
easily add new options in one place and have them apply correctly
every where.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The unix network tests are not being run on a v8 network capable kernel. Under v8 there needs to be some adjustments to the tests because unix rules get downgraded to the socket rule ```network unix,``` which does not have the same set of conditionals or fine grained permissions, meaning some tests that would fail under af_unix (like missing permission tests) will pass under v8 network rules.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/893
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
This patch adds the following mount options: 'nostrictatime',
'lazytime', and 'nolazytime'.
The MS_STRICTATIME mount flag already existed, and 'nostrictatime' was
listed along with 'strictatime' in the comments of parser/mount.cc, so
this patch adds a mapping for 'nostrictatime' to clear MS_STRICTATIME.
Additionally, the Linux kernel includes the 'lazytime' option with
MS_LAZYTIME mapping to (1<<25), so this patch adds MS_LAZYTIME to
parser/mount.h and the corresponding mappings in parser/mount.cc for
'lazytime' and 'nolazytime'.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1005
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The conflicting flags value message was hard to read
conflicting flag value = lazytimenolazytime
change it to
conflicting flag values = lazytime, nolazytime
Signed-off-by: John Johansen <john.johansen@canonical.com>
Adds the corresponding `MS_NOSYMFOLLOW` flag to parser/mount.h as well,
defined as (1 << 8) just as in the util-linux and the kernel.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This patch adds the following mount options: 'nostrictatime',
'lazytime', and 'nolazytime'.
The MS_STRICTATIME mount flag already existed, and 'nostrictatime' was
listed along with 'strictatime' in the comments of parser/mount.cc, so
this patch adds a mapping for 'nostrictatime' to clear MS_STRICTATIME.
Additionally, the Linux kernel includes the 'lazytime' option with
MS_LAZYTIME mapping to (1<<25), so this patch adds MS_LAZYTIME to
parser/mount.h and the corresponding mappings in parser/mount.cc for
'lazytime' and 'nolazytime'.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make it so the output options that count profiles and processes can
be output to a file.
This does not add a user option to do so
Signed-off-by: John Johansen <john.johansen@canonical.com>
For detailed output split up where output is going from whether json
is being produced.
This also allows none json output to go to a file.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When the replacement regex for multiple qualifiers matches, the
capture group variables ($1, $2, etc) are overwritten to match the
most recent regex. Since there are no capture groups in the 's/,/ /g'
regex, then $2 was empty, causing an error on policy generation.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The chfa equivalence class shouldn't be a reference. Its needs to
actually exist and be part of the class during later method calls.
As a reference it leads to bad references when used later.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Historically, if you create a `profile foo /bin/foo` with aa-logprof (by
choosing "named exec"), it will be saved as `/etc/apparmor.d/profile_foo`
This patch drops the `profile_` filename prefix so that the filename
will be `/etc/apparmor.d/foo`
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1014
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The parser front end is inconsistent about how it handles permissions and rules. This makes extending the permission set and fixing inconsistencies between say file and af_unix rules difficult and prone to failures.
This is a set of work to cleanup the frontend parse handling, and middle semantic checks, dedup, and processing of rules and their permissions. It also is starting to lay the ground work for the parser retaining more information to output better error, and debug messages.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/802
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Historically, if you create a `profile foo /bin/foo` with aa-logprof (by
choosing "named exec"), it will be saved as `/etc/apparmor.d/profile_foo`
This patch drops the `profile_` filename prefix so that the filename
will be `/etc/apparmor.d/foo`
When a user choooses to execute to a named profile (not: named child),
make sure to get the profile filename in the correct way to avoid a
crash.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/314
This is used by various applications including libreoffice etc so it may as well
be added to the base abstraction along with the existing zoneinfo DB access.
AVC apparmor="DENIED" operation="open" class="file" profile="snap.libreoffice.calc" name="/usr/share/zoneinfo-icu/44/le/zoneinfo64.res" pid=44742 comm="soffice.bin" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
Signed-off-by: Alex Murray <alex.murray@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1007
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This is used by various applications including libreoffice etc so it may as well
be added to the base abstraction along with the existing zoneinfo DB access.
AVC apparmor="DENIED" operation="open" class="file" profile="snap.libreoffice.calc" name="/usr/share/zoneinfo-icu/44/le/zoneinfo64.res" pid=44742 comm="soffice.bin" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0
Signed-off-by: Alex Murray <alex.murray@canonical.com>
The description of @{etc_ro} and @{etc_rw} were not good enough in
explaining which directories they should contain, and when to use which
of the variables in a profile.
I propose this patch for all branches.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1000
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Probably thanks to O_MAYEXEC, denials for file access can now contain a
mix of x (exec) and other file permissions.
The actual exec should appear in a separate "exec" log event, therefore
ignore 'x' in file events for now if it's mixed with other permissions.
Note that file events ("open", "link" etc.) that contain denied_mask="x"
without another permission will still cause an error. (So far, this
hasn't been seen in the wild.)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/303
Also add the log line from the bugreport and the (for now) expected
result as test_multi testcase.
I propose this patch for all branches.
Closes#303
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1001
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The code for add_local_entry is actually currently unused and will
have to change anyways by the time it is. Some drop it and the
associated variables.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Another step towards having a block rule and retaining parsed rule
structure. Setup the parse to use a common block pattern, that when
we are ready will become an actual rule.
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for file rules and rule duplication removal add
flags to rule_t with the first flag indicating if the rule is
deleted.
We do this instead of actually deleting the rule so we can hold
on to the rule for debug and printing output in the future.
Signed-off-by: John Johansen <john.johansen@canonical.com>
in preparation for file rules switching to rule_t add a method to
indicate whether a particular rule is mergeable/dedupable.
Whether a rule merges or dedups will be up to the rules comparison
and merge methods.
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for rule comparison and elemination have each rule
carry a type that can be used as the base of comparison. The
rule class is folded into this type.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Instead of having each rule individually handle the class info
introduce a class_rule_t into the hierarchy and consolidate.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The post_process() method is misnamed, it fires when the profile is
finished parsing but fires before variable expansion. Rename it
to better reflect what it does and move the trigger code into
profile as a start of cleaning this stage up.
Also document the order the hooks fire in
Signed-off-by: John Johansen <john.johansen@canonical.com>
The previous patch enable the prefix based rules all to use the
same code pattern. Group them together
Signed-off-by: John Johansen <john.johansen@canonical.com>
Cleanup the parse code by making shared prefix and perms classes for
rules and convert rules to use them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This removes the struct wrapper used in the previous patch to ensure
that all uses are properly converted.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Audit control support is going to be extended to support allowing
policy to which rules should quiet auditing. Update the frontend
internals to prepare for this.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This removes the struct wrapper used in the previous patch to ensure
that all uses are properly converted.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This delays the convertion of the audit flag until passing to the
backend. This is a step towards fix the parser front end so that it
doesn't use encoded permission mappings.
Note: the patch embedds the bool conversion into a struct to ensure
the compiler will fail to build unless every use is fixed. The
struct is removed in the following patch.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Update (most of the) code and inline comments/docstrings to follow
https://peps.python.org/pep-0008/ so that future maintenance is slightly
easier.
Continue to keep long lines as splitting them does not always improve
the code readability.
Probably thanks to O_MAYEXEC, denials for file access can now contain a
mix of x (exec) and other file permissions.
The actual exec should appear in a separate "exec" log event, therefore
ignore 'x' in file events for now if it's mixed with other permissions.
Note that file events ("open", "link" etc.) that contain denied_mask="x"
without another permission will still cause an error. (So far, this
hasn't been seen in the wild.)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/303
Also add the log line from the bugreport and the (for now) expected
result as test_multi testcase.
Make mount permission set consistent with the other rule types. This
is a step towards refactoring.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is a step towards restructuring how "audit" is handled so we
can add quiet support and push mapping of audit bits later.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Move from using and int for permissions bit mask to a perms_t type.
Also move any perms mask that uses the name mode to perms to avoid
confusing it with other uses of mode.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Recent work on the parser has surfaced missing header dependencies
and other issues. Cleanup up and simplify the dependencies so it
is harder to break them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This is a partial fix for CVE-2016-1585, it address the frontend rule encoding problems particularly
- Permissions being given that shouldn't happen
- Multiple option conditionals in a single rule resulting in wider permission instead of multiple rules
- optional flags not being handled correctly
- multiple backend rules being created out of one frontend rule when they shouldn't be
it does not address the backend issue of short cut permissions not being correctly updated when deny rules carve out permissions on an allow rule that has a short cut permission in the encoding.
Thanks to the additional work by Alexander Mikhalitsyn for beating this MR into shape so we can land it
Alexander Changelog:
- rebased to an actual tree
- addressed review comments from @wbumiller and @setharnold
- fixed compiler warnings about class_mount_hdr is uninitialized
- infinite loop fix
- MS_MAKE_CMDS bitmask value fixed
- fixed condition in `gen_flag_rules` to cover cases like `mount options in (bind) /d -> /4,` when flags are empty and only opt_flags are present
- marked some tests as a FAIL case behavior was changed after `parser: add conflicting flags check for options= conditionals` commit
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/333
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The description of @{etc_ro} and @{etc_rw} were not good enough in
explaining which directories they should contain, and when to use which
of the variables in a profile.
These tests contains incompatible mount options and broken
after ("parser: add conflicting flags check for options= conditionals")
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
gen_flag_rules has a boolean vs bit and case where parenthesis are
helpful to express the intended order of operations.
It also doesn't handle the case where there are no matches. Fix this
by causing that case to fail.
also improve the debug of option extraction.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Now that flag processing for mount rules with single option
conditionals are fixed e-enable multiple mount conditionals on a
single mount rule. The mount conditionals are equivalent to specifying
multiple rules.
mount options=(a,b,c) options=(c,d),
is the same as
mount options=(a,b,c),
mount options=(c,d),
and
mount options in (a,b,c) options in (c,d),
is the same as
mount options in (a,b,c),
mount options in (c,d),
when multiple options= and options in are combined in a single rule
it is the same as the cross product of the options.
where
mount options=(a,b,c) options in (d,e),
is a single rule.
mount options=(a,b,c) options=(d,e) options in (f),
is equivalent to
mount options=(a,b,c) options in (f),
mount options=(d,e) options in (f),
and while it is not recommended that multiple options= and options in
conditions be used in a single rule.
mount options=(a,b,c) options=(d,e) options in (f) options in (g),
is equivalent to
mount options=(a,b,c) options in (f),
mount options=(a,b,c) options in (g),
mount options=(d,e) options in (f),
mount options=(d,e) options in (g),
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- fixed infinite loop in mnt_rule::gen_policy_re
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
The combined optional flag and exact match flag processing is problematic
separate out the optional flag processing so it is only combined during
match string generation.
While doing so we fix the flag output so that multiple rules are
not output when they shouldn't be.
In addition we temporarily break multiple options= and 'options in'
conditionals in a single rule, which we will fix in a separate patch.
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- made tests happy by changing condition in gen_policy_re()
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Do a minimal code refactoring (ie. no functional changes, just moving
code,adding boiler plate and glue) in preparation to fix
bug https://bugs.launchpad.net/apparmor/+bug/1597017
Bug Link: https://bugs.launchpad.net/apparmor/+bug/1597017
Signed-off-by: John Johansen <john.johansen@canonical.com>
- rebased to bba1a023bf
- fixed compiler warnings:
<built-in>: In member function ‘int mnt_rule::gen_policy_new_mount(Profile&, int&, unsigned int, unsigned int)’:
<built-in>: note: by argument 1 of type ‘const char*’ to ‘long unsigned int __builtin_strlen(const char*)’ declared here
mount.cc:880:14: note: ‘class_mount_hdr’ declared here
880 | char class_mount_hdr[64];
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
Acked-by: John Johansen <john.johansen@canonical.com>
dbus 1.14.4 changed the behavior of unix:tmpdir to be equivalent to
unix:dir, which cases dbus-daemon to generate path based sockets,
instead of the previous abstract sockets. [1]
In this change we force dbus-daemon to generate an abstract socket by
specifying the abstract socket address in the command.
[1] https://gitlab.freedesktop.org/dbus/dbus/-/blob/dbus-1.14/NEWS#L64
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
For example winbindd when configured on a samba system using
sssd can trigger
apparmor="DENIED" operation="file_mmap" profile="/usr/sbin/winbindd" name="/usr/lib64/krb5/plugins/authdata /sssd_pac_plugin.so" pid=2798 comm="winbindd" requested_mask="m" denied_mask="m" fsuid=52311 ouid=0
Signed-off-by: Noel Power <noel.power@suse.com>
# profiles/apparmor.d/samba*: allow access to pid files directly in /run/
On Arch Linux, `samba-dcerpcd.pid` is in `/run/`, not `/run/samba/`.
apparmor="DENIED" operation="mknod" profile="samba-dcerpcd" name="/run/samba-dcerpcd.pid" pid=80920 comm="samba-dcerpcd" requested_mask="c" denied_mask="c" fsuid=0 ouid=0
The same is true for `nmbd.pid`, `smbd.pid` and probably others too.
# samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/987
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
On Arch Linux, `samba-dcerpcd.pid` is in `/run/`, not `/run/samba/`.
apparmor="DENIED" operation="mknod" profile="samba-dcerpcd" name="/run/samba-dcerpcd.pid" pid=80920 comm="samba-dcerpcd" requested_mask="c" denied_mask="c" fsuid=0 ouid=0
The same is true for `nmbd.pid`, `smbd.pid` and probably others too.
samba-dcerpcd requires access to `/var/cache/samba/names.tdb`.
audit: type=1400 audit(1676835286.187:62): apparmor="DENIED" operation="open" profile="samba-dcerpcd" name="/var/cache/samba/names.tdb" pid=6948 comm="samba-dcerpcd" requested_mask="wrc" denied_mask="wrc" fsuid=0 ouid=0
See also https://bbs.archlinux.org/viewtopic.php?id=281411
Since `usr.sbin.winbindd` already has a rule for it, and `usr.sbin.nmbd`
has similar ones, simply add `/var/cache/samba/*.tdb rwk` to
`abstractions/samba`.
This MR continues #280 work by changing string formatting in Python tests. `str.format()` is now used in places where bracket escapes are not necessary. In places where escapes would be necessary, I've ensured that format arguments are tuple-guarded.
I also fixed some potential future bugs that I noticed in the process:
- mutable default arguments that were luckily not being mutated
- a tuple missing a comma that conveniently functioned properly because `iter('_')` and `iter(('_',))` are effectively the same
Additionally, I fixed one current bug in the process:
- `dict.keys()` no longer returns a list in Python 3, so you can no longer call `.sort()` on its return value
I figured that the bug fixes are small enough that they don't deserve separate MRs, but if you disagree then I will happily split this up.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/985
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
... to just the basename and profile name.
This also means to make the firefox and firefox.sh profiles named profiles.
Note: in the `firefox.sh` profile, the local include was added just today, therefore I think changing it (instead of adding) should be fine.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/975
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
contributed by Werner Fink via
https://bugzilla.opensuse.org/show_bug.cgi?id=1065388 comment 25
Note that - compared to the file in bugzilla - I removed the `rix` rules
for /usr/bin/groff and /usr/bin/nroff so that people can choose to ix,
Px or Cx groff/nroff as they wish, and then include the abstraction
inside the target profile to allow executing all the helpers.
I also added `include if exists <abstractions/groff.d>`
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/973
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This MR addresses step 1 of 6 in #280, i.e. merging Launchpad translations into `master`. Tests are currently failing in this branch due to newly-introduced hotkey conflicts. I therefore prefixed this MR's title with "Draft: " to prevent premature merging. How would you like these hotkey conflicts resolved?
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/966
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
contributed by Werner Fink via
https://bugzilla.opensuse.org/show_bug.cgi?id=1065388 comment 25
Note that - compared to the file in bugzilla - I removed the `rix` rules
for /usr/bin/groff and /usr/bin/nroff so that people can choose to ix,
Px or Cx groff/nroff as they wish, and then include the abstraction
inside the target profile to allow executing all the helpers.
I also added `include if exists <abstractions/groff.d>`
Resolves#292.
This fix is the same as !830 but for Brave.
Opening links in Brave now works as intended.
Note that now a separate denial is caused, related to WidevineCDM, is produced:
```
[ERROR:content_main_runner_impl.cc(415)] Unable to load CDM /home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so (error: /home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so: failed to map segment from shared object)
```
In the syslog:
```
audit: type=1400 audit(1671108748.090:117): apparmor="DENIED" operation="file_mmap" profile="/usr/bin/evince//sanitized_helper" name="/home/username/.config/BraveSoftware/Brave-Browser/WidevineCdm/4.10.2557.0/_platform_specific/linux_x64/libwidevinecdm.so" pid=65765 comm="brave" requested_mask="m" denied_mask="m" fsuid=1000 ouid=100
```
I'm not sure if granting permission(s) for this is desirable. In either case, the potential relevant changes are out of the scope of this MR.
If I disable WidevineCDM in Brave, I get the following denial on cap sys_admin:
```
audit: type=1400 audit(1671112807.666:174): apparmor="DENIED" operation="capable" profile="/usr/bin/evince//sanitized_helper" pid=112098 comm="brave" capability=21 capname="sys_admin"
```
which is fine, as mentioned by @jjohansen [here](https://gitlab.com/apparmor/apparmor/-/merge_requests/830#note_831915024).
Closes#292
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/957
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
- ssl_certs: /{etc,usr/share}/pki/trust/ has more than the 'anchors' subdirectory
- crypoto: allow reading /etc/gcrypt/hwf.deny
I propose this patch for 3.0..master (2.13 doesn't have abstractions/crypto).
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/961
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The key of SYSV message queues is an integer and the kernel uses an
integer to store the key. In order to improve performance when
travelling the DFA in the kernel, we should use an integer instead of
the string.
This [patch](5501f45f40) contains a rough implementation of what that would look like on the kernel side
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/968
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
The key of SYSV message queues is an integer and the kernel uses an
integer to store the key. In order to improve performance when
travelling the DFA in the kernel, we should use an integer instead of
the string.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The mqueue tests were using the previous format which was specific for
capabilities. The qual= prefix should be used instead.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
dbus-broker requires some modification of the test suite. In summary:
* refactor to support starting and stopping both dbus and dbus-broker.
* Make it so we can run the tests on each, where appropriate
* skip unrequested reply and eavesdrop tests for dbus broker because they are not supported.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/965
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
DBus Broker was enabled for the dbus_message and dbus_service
regression tests.
The dbus_eavesdropping test does not run with dbus-broker because
eavesdropping was deprecated in favor or monitoring, so new tests for
the "BecomeMonitor" method need to be added.
The dbus_unrequested_reply test is also not supported by dbus-broker,
therefore the tests are skipped.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The test "eavesdrop (confined w/o dbus perms)" was failing for the
wrong reason. While it should fail because it is missing dbus rules, it
was actually failing because it didn't have the required unix rule.
The error message was:
"FAIL: Failed to open connection to "session" message bus: Failed to open socket: Permission denied"
Corresponding audit log:
[28306.743863] audit: type=1400 audit(1671048091.505:297): apparmor="DENIED" operation="create" class="net" profile="/home/georgia/apparmor/tests/regression/apparmor/dbus_eavesdrop" pid=6787 comm="dbus_eavesdrop" family="unix" sock_type="stream" protocol=0 requested_mask="create" denied_mask="create" addr=none
After the change, the error message is:
FAIL: Failed to open connection to "session" message bus: An AppArmor policy prevents this sender from sending this message to this recipient; type="method_call", sender="(null)" (inactive) interface="org.freedesktop.DBus" member="Hello" error name="(unset)" requested_reply="0" destination="org.freedesktop.DBus" (bus)
Corresponding audit log:
[28444.248268] audit: type=1107 audit(1671048229.009:300): pid=6826 uid=0 auid=1000 ses=5 subj=unconfined msg='apparmor="DENIED" operation="dbus_method_call" bus="session" path="/org/freedesktop/DBus" interface="org.freedesktop.DBus" member="Hello" mask="send" name="org.freedesktop.DBus" pid=6854 label="/home/georgia/apparmor/tests/regression/apparmor/dbus_eavesdrop" peer_label="unconfined" exe="/usr/bin/dbus-daemon" sauid=0 hostname=? addr=? terminal=?'
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The profile generated by dbus did not include this rule
which caused the following DENIED audit logs:
[26937.013475] audit: type=1400 audit(1671046721.776:246): apparmor="DENIED" operation="getattr" class="file" profile="/home/georgia/apparmor/tests/regression/apparmor/dbus_message" name="/tmp/sdtest.5720-14413-VQMPsH/output.dbus_message" pid=5866 comm="dbus_message" requested_mask="r" denied_mask="r" fsuid=0 ouid=0
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
As reported in issue #295, the json output from aa-status would be invalid if
there were profiles defined for processes that were unconfined. Fix this by
ensuring the json for the processes array is closed properly.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Commit 8cf3534a5 ("tests regression: fix failure on older versions of
Make") from https://gitlab.com/apparmor/apparmor/-/merge_requests/639
was incorrectly applied, including the `+` prefixes from the proposed
patch. This causes the sysctl syscall() checks to not correctly be
applied and results in a mismatch of expectations in the
syscall_sysctl.sh test script, causing it and the testsuite to fail.
Thus, remove the bogon `+` characters from the Makefile, to make
USE_SYSCTL be set correctly.
Fixes: 8cf3534a5 ("tests regression: fix failure on older versions of Make")
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
getopt returns an int, not a char.
Error caused by this issue could only be observed on non-x86 systems.
Closes LP#2000359
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
If the "operation" field is not present, we get the following error:
AttributeError: 'NoneType' object has no attribute 'startswith'
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
States are not guaranteed to have transitions, but when inserting
a state into the chfa table there is an unconditional dereference
to the states first transition.
This will result in a bad reference and could result in an OOB
flag being set on the state when it shouldn't be.
Fixes: 16b67ddbd ("add ability to use out of band transitions"
Closes: https://gitlab.com/apparmor/apparmor/-/issues/290
Reported-by: Nobel Barakat <nobelbarakat@google.com>
Reported-by: Oleksandr Tymoshenko <ovt@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This reverts commit 1aa474b25c.
This is a debug patch that unconditionally sets a profile debug flag
if mqueue rules are present. It should not be present in merged
code, and instead the user should set a debug flag on the profile if
that is desired.
This was merged with the patch to expedite the MR with the understanding
that this patch would be reverted.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This MR expands the user namespace regression tests to use unshare and setns.
Note that setns associates itself to an existing user namespace, so it is not creating it.
This is relevant because the mediation only happens on the namespace creation for now.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/952
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
On Debian Sid, NVIDIA driver spams log with:
```
type=AVC msg=audit(1669542108.552:11855): apparmor="DENIED"
operation="open" profile="qtox" name="/proc/21222/comm" pid=21222
comm="qtox" requested_mask="r" denied_mask="r" fsuid=1000
ouid=1000FSUID="vincas" OUID="vincas"
```
```
type=AVC msg=audit(1669541506.703:11329): apparmor="DENIED"
operation="open" profile="skypeforlinux" name="/proc/19851/comm"
pid=19851 comm="skypeforlinux" requested_mask="r" denied_mask="r"
fsuid=1000 ouid=1000FSUID="vincas" OUID="vincas"
```
Read is initiated within libnvidia-glcore.so:
```
Thread 1 "qtox" hit Catchpoint 1 (call to syscall openat), 0x00007fb797b16ed0 in __libc_open64 (file=file@entry=0x7fb742adbb50 "/proc/self/comm", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:41
41 in ../sysdeps/unix/sysv/linux/open64.c
$27 = 0x7fb742adbb50 "/proc/self/comm"
0 0x00007fb797b16ed0 in __libc_open64 (file=file@entry=0x7fb742adbb50 "/proc/self/comm", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:41
1 0x00007fb797aa0862 in __GI__IO_file_open (fp=fp@entry=0x55795176e600, filename=filename@entry=0x7fb742adbb50 "/proc/self/comm", posix_mode=<optimized out>, prot=prot@entry=438, read_write=8, is32not64=<optimized out>) at ./libio/fileops.c:188
2 0x00007fb797aa0a1b in _IO_new_file_fopen (fp=fp@entry=0x55795176e600, filename=filename@entry=0x7fb742adbb50 "/proc/self/comm", mode=<optimized out>, mode@entry=0x7fb7428effe2 "r", is32not64=is32not64@entry=1) at ./libio/fileops.c:280
3 0x00007fb797a950f9 in __fopen_internal (filename=0x7fb742adbb50 "/proc/self/comm", mode=0x7fb7428effe2 "r", is32=1) at ./libio/iofopen.c:75
4 0x00007fb7423d791f in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
5 0x00007fb7423d4515 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
6 0x00007fb7423d0226 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
7 0x00007fb7423e1961 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
8 0x00007fb74824bc79 in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
9 0x00007fb7482b1c56 in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
10 0x000000000000001d in ?? ()
11 0x00005579518975f0 in ?? ()
12 0x0000000000000001 in ?? ()
13 0x00007fb74824b1eb in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
14 0x00005579518975f0 in ?? ()
15 0x00007fb79b2dda79 in call_init (env=0x7ffd92d7aac8, argv=0x7ffd92d7aab8, argc=-1831363648, l=0x7fb748516f70) at ./elf/dl-init.c:56
16 call_init (l=0x7fb748516f70, argc=-1831363648, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-init.c:26
17 0x00007fb79b2ddba4 in _dl_init (main_map=0x5579518975f0, argc=1, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-init.c:117
18 0x00007fb797b6def4 in __GI__dl_catch_exception (exception=<optimized out>, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:182
19 0x00007fb79b2e430e in dl_open_worker (a=a@entry=0x7ffd92d79f20) at ./elf/dl-open.c:808
20 0x00007fb797b6de9a in __GI__dl_catch_exception (exception=<optimized out>, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:208
21 0x00007fb79b2e46a8 in _dl_open (file=0x557951888020 "libGLX_nvidia.so.0", mode=<optimized out>, caller_dlopen=0x7fb78d7d4d27, nsid=<optimized out>, argc=1, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-open.c:884
22 0x00007fb797aa42d8 in dlopen_doit (a=a@entry=0x7ffd92d7a190) at ./dlfcn/dlopen.c:56
23 0x00007fb797b6de9a in __GI__dl_catch_exception (exception=exception@entry=0x7ffd92d7a0f0, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:208
24 0x00007fb797b6df4f in __GI__dl_catch_error (objname=0x7ffd92d7a148, errstring=0x7ffd92d7a150, mallocedp=0x7ffd92d7a147, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:227
25 0x00007fb797aa3dc7 in _dlerror_run (operate=operate@entry=0x7fb797aa4280 <dlopen_doit>, args=args@entry=0x7ffd92d7a190) at ./dlfcn/dlerror.c:138
26 0x00007fb797aa4389 in dlopen_implementation (dl_caller=<optimized out>, mode=<optimized out>, file=<optimized out>) at ./dlfcn/dlopen.c:71
27 ___dlopen (file=<optimized out>, mode=<optimized out>) at ./dlfcn/dlopen.c:81
28 0x00007fb78d7d4d27 in ?? () from /lib/x86_64-linux-gnu/libGLX.so.0
29 0x00007fb78d7d6335 in ?? () from /lib/x86_64-linux-gnu/libGLX.so.0
30 0x00007fb78d7cf9f8 in glXChooseFBConfig () from /lib/x86_64-linux-gnu/libGLX.so.0
31 0x00007fb748646f6a in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
32 0x00007fb748644450 in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
33 0x00007fb7486421b7 in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
34 0x00007fb79838262d in QOpenGLContext::create() () from /lib/x86_64-linux-gnu/libQt5Gui.so.5
35 0x00007fb74bb4303c in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/platformthemes/KDEPlasmaPlatformTheme.so
36 0x00007fb797eb7aaf in qt_call_pre_routines () at kernel/qcoreapplication.cpp:317
37 QCoreApplicationPrivate::init (this=<optimized out>) at kernel/qcoreapplication.cpp:849
38 0x00007fb7983379dc in QGuiApplicationPrivate::init() () from /lib/x86_64-linux-gnu/libQt5Gui.so.5
39 0x00007fb798b684c9 in QApplicationPrivate::init() () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
40 0x0000557950f1d597 in main ()
```
Add read rule to allow reading @{pid}/comm.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/954
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
On Debian Sid, NVIDIA driver spams log with:
```
type=AVC msg=audit(1669542108.552:11855): apparmor="DENIED"
operation="open" profile="qtox" name="/proc/21222/comm" pid=21222
comm="qtox" requested_mask="r" denied_mask="r" fsuid=1000
ouid=1000FSUID="vincas" OUID="vincas"
```
```
type=AVC msg=audit(1669541506.703:11329): apparmor="DENIED"
operation="open" profile="skypeforlinux" name="/proc/19851/comm"
pid=19851 comm="skypeforlinux" requested_mask="r" denied_mask="r"
fsuid=1000 ouid=1000FSUID="vincas" OUID="vincas"
```
Read is initiated within libnvidia-glcore.so:
```
Thread 1 "qtox" hit Catchpoint 1 (call to syscall openat), 0x00007fb797b16ed0 in __libc_open64 (file=file@entry=0x7fb742adbb50 "/proc/self/comm", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:41
41 in ../sysdeps/unix/sysv/linux/open64.c
$27 = 0x7fb742adbb50 "/proc/self/comm"
0 0x00007fb797b16ed0 in __libc_open64 (file=file@entry=0x7fb742adbb50 "/proc/self/comm", oflag=0) at ../sysdeps/unix/sysv/linux/open64.c:41
1 0x00007fb797aa0862 in __GI__IO_file_open (fp=fp@entry=0x55795176e600, filename=filename@entry=0x7fb742adbb50 "/proc/self/comm", posix_mode=<optimized out>, prot=prot@entry=438, read_write=8, is32not64=<optimized out>) at ./libio/fileops.c:188
2 0x00007fb797aa0a1b in _IO_new_file_fopen (fp=fp@entry=0x55795176e600, filename=filename@entry=0x7fb742adbb50 "/proc/self/comm", mode=<optimized out>, mode@entry=0x7fb7428effe2 "r", is32not64=is32not64@entry=1) at ./libio/fileops.c:280
3 0x00007fb797a950f9 in __fopen_internal (filename=0x7fb742adbb50 "/proc/self/comm", mode=0x7fb7428effe2 "r", is32=1) at ./libio/iofopen.c:75
4 0x00007fb7423d791f in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
5 0x00007fb7423d4515 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
6 0x00007fb7423d0226 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
7 0x00007fb7423e1961 in ?? () from /lib/x86_64-linux-gnu/libnvidia-glcore.so.470.141.03
8 0x00007fb74824bc79 in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
9 0x00007fb7482b1c56 in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
10 0x000000000000001d in ?? ()
11 0x00005579518975f0 in ?? ()
12 0x0000000000000001 in ?? ()
13 0x00007fb74824b1eb in ?? () from /lib/x86_64-linux-gnu/libGLX_nvidia.so.0
14 0x00005579518975f0 in ?? ()
15 0x00007fb79b2dda79 in call_init (env=0x7ffd92d7aac8, argv=0x7ffd92d7aab8, argc=-1831363648, l=0x7fb748516f70) at ./elf/dl-init.c:56
16 call_init (l=0x7fb748516f70, argc=-1831363648, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-init.c:26
17 0x00007fb79b2ddba4 in _dl_init (main_map=0x5579518975f0, argc=1, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-init.c:117
18 0x00007fb797b6def4 in __GI__dl_catch_exception (exception=<optimized out>, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:182
19 0x00007fb79b2e430e in dl_open_worker (a=a@entry=0x7ffd92d79f20) at ./elf/dl-open.c:808
20 0x00007fb797b6de9a in __GI__dl_catch_exception (exception=<optimized out>, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:208
21 0x00007fb79b2e46a8 in _dl_open (file=0x557951888020 "libGLX_nvidia.so.0", mode=<optimized out>, caller_dlopen=0x7fb78d7d4d27, nsid=<optimized out>, argc=1, argv=0x7ffd92d7aab8, env=0x7ffd92d7aac8) at ./elf/dl-open.c:884
22 0x00007fb797aa42d8 in dlopen_doit (a=a@entry=0x7ffd92d7a190) at ./dlfcn/dlopen.c:56
23 0x00007fb797b6de9a in __GI__dl_catch_exception (exception=exception@entry=0x7ffd92d7a0f0, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:208
24 0x00007fb797b6df4f in __GI__dl_catch_error (objname=0x7ffd92d7a148, errstring=0x7ffd92d7a150, mallocedp=0x7ffd92d7a147, operate=<optimized out>, args=<optimized out>) at ./elf/dl-error-skeleton.c:227
25 0x00007fb797aa3dc7 in _dlerror_run (operate=operate@entry=0x7fb797aa4280 <dlopen_doit>, args=args@entry=0x7ffd92d7a190) at ./dlfcn/dlerror.c:138
26 0x00007fb797aa4389 in dlopen_implementation (dl_caller=<optimized out>, mode=<optimized out>, file=<optimized out>) at ./dlfcn/dlopen.c:71
27 ___dlopen (file=<optimized out>, mode=<optimized out>) at ./dlfcn/dlopen.c:81
28 0x00007fb78d7d4d27 in ?? () from /lib/x86_64-linux-gnu/libGLX.so.0
29 0x00007fb78d7d6335 in ?? () from /lib/x86_64-linux-gnu/libGLX.so.0
30 0x00007fb78d7cf9f8 in glXChooseFBConfig () from /lib/x86_64-linux-gnu/libGLX.so.0
31 0x00007fb748646f6a in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
32 0x00007fb748644450 in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
33 0x00007fb7486421b7 in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
34 0x00007fb79838262d in QOpenGLContext::create() () from /lib/x86_64-linux-gnu/libQt5Gui.so.5
35 0x00007fb74bb4303c in ?? () from /usr/lib/x86_64-linux-gnu/qt5/plugins/platformthemes/KDEPlasmaPlatformTheme.so
36 0x00007fb797eb7aaf in qt_call_pre_routines () at kernel/qcoreapplication.cpp:317
37 QCoreApplicationPrivate::init (this=<optimized out>) at kernel/qcoreapplication.cpp:849
38 0x00007fb7983379dc in QGuiApplicationPrivate::init() () from /lib/x86_64-linux-gnu/libQt5Gui.so.5
39 0x00007fb798b684c9 in QApplicationPrivate::init() () from /lib/x86_64-linux-gnu/libQt5Widgets.so.5
40 0x0000557950f1d597 in main ()
```
Add read rule to allow reading @{pid}/comm.
The kernel will allow for a couple of debug flags on a profile that
can be used to trigger debug messages for only profiles/labels that
have the flag set. Add basic support for these to the parser.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Based on what was done in the parser, replicate the logic
so it can be used in the python tools.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Setns is used to associate to an existing user namespace, so the
kernel security hook for user namespace creation is not called.
The restriction for setns is that it should have the capability
sys_admin.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
This MR closes#286.
Strings with only invalid escape sequences were prefixed with an `r`. Strings containing both valid and invalid escape sequences were handled on a case-by-case basis, as simply changing to a raw string breaks valid escape sequences.
Closes#286
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/951
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This MR closes#276.
In b150c40ed1, I removed the big comment at the top of `BaseRule` stating what methods have to be implemented by subclasses. This was out-of-date, had redundant method descriptions, and was no longer necessary now that `BaseRule` has an `ABCMeta` metaclass and auto-enforces the implementation of abstract methods.
`rule._logprof_header_localvars()` implementations weren't consistent, so I took the opportunity to clean them up in db99138f56. Return types differed, the presence of empty `header` lists differed, how size-two collections were returned differed, etc.
Otherwise, this MR closely follows the discussion in #276.
Closes#276
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/950
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This closes#277 by introducing the following changes:
1. `BaseRule` was made a "proper" abstract base class in that it now enforces that subclasses implement all abstract methods.
2. As a consequence of this change, `BaseRule` can no longer be directly instantiated; tests were changed accordingly.
3. These test changes led to test failures, the reason being that many rule methods hardcoded class names, breaking inheritance. I made a commit to address this.
4. Once tests were passing, code coverage regressed due to the fact that there was now effectively-unreachable code in abstract instance methods. This code, which was only raising `NotImplementedError`s, was removed. I say "effectively-unreachable" here because it could be reached by calling `super()` via a subclass.
5. There are three "bonus" commits as well; one bug fix, one typo fix, and one cleanup.
Closes#277
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/930
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
Glibc in 2.36 and later will [1] access sysfs at
/sys/devices/system/cpu/possible when usig sysconf
for _SC_NPROCESSORS_CONF.
That will make a lot of different code, for example
anything linked against libnuma, trigger this apparmor
denial.
apparmor="DENIED" operation="open" class="file" ...
name="/sys/devices/system/cpu/possible" ...
requested_mask="r" denied_mask="r" fsuid=0 ouid=0

This entry seems rather safe, and it follows others
that are already in place. Instead of fixing each
software individually this should go into the base
profile as well.
Initially reported via
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1989073
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/267
MR: none - ML
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The 3.1 release bumped its library version, and for consistency
needs to be bumped here as well.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This allows regression tests to generate profiles that use rule qualifiers,
such as allow, deny, and audit. Qualifiers can be specified for a rule by
prepending 'qual=', followed by a comma-separated list of rule qualifiers,
then a ':', then the rule itself.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/925
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
This allows regression tests to generate profiles that use rule qualifiers,
such as allow, deny, and audit. Qualifiers can be specified for a rule by
prepending 'qual=', followed by a comma-separated list of rule qualifiers,
then a ':', then the rule itself.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
The test kernel_features only checked if the feature file exists, but
there are cases when we need to check if the feature is part of the
file's content. For example, we might want to check if a mode is present
in the feature "mask" file.
This is already supported by libapparmor's aa_features_supports.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Print out a summary of the tests that passed and failed at the end of the
regression tests. Merge the tests and alltests make targets since they
only differ in which tests are run.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
This update to the regression test framework allows a test script to
execute a test binary as a different user by passing '-u <username>'
to the settest function.
Signed-off-by: Jon Tourville <jon.tourville@canonical.com>
When WSL automatically generates a resolv.conf for an instance, the /etc/resolv.conf file is a symlink to /mnt/wsl/resolv.conf. This patch adds an entry for this to the other policies to handle such management.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/935
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
audit.log lines on Arch have an additional FSUID="username" OUID="username",
separated from the previous part of the log line with 0x1d.
Extend the log parsing to accept 0x1d as whitespace, and to recognize
(and ignore) FSUID and OUID.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/271
In order to decrease the number of characters in the audit logs
from the kernel, we will drop the "_mask" from the fields
"requested_mask" and "denied_mask".
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
GUI applications such as KDE dragon player, qTox, LibreOffice tries to
access .config/kwinrc, .config/kdedefaults/kwinrc and
.config/kdedefaults/kdeglobals.
Update abstractions/kde to fix denials for applications running under
KDE.
This adds a single e2e test to check that raw_data of a loaded
policy matches the generated policy (cached).
Signed-off-by: Spyros Seimenis <spyros.seimenis@canonical.com>
A single '$()' results in variable expansion, which makes
"$(rpm --eval ..)" always an empty string.
Signed-off-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
As discussed in !920, this MR renames BaseRule's `parse()` and `_parse()` methods to `create_instance()` and `_create_instance()`, respectively. It then removes the `selection_to_rule_obj` function from `apparmor.aa`, which is now an unnecessary alias for the renamed public method.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/923
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
This MR addresses #269.
Notes:
- In all `is_equal_localvars()` Rule methods, `type(self)` is now used instead of the explicit class. This approach works better with inheritance, and was already used in `IncludeRule` for this reason.
- Remaining direct type comparisons were changed to use `is`/`is not` instead of `==`/`!=`.
- I fixed two typos in `alias.py`; the plural of alias is aliases, not aliass.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/920
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
If audit.log contains entries for a profile that doesn't exist (for
example when working with a log file from another system), skip these
log entries instead of crashing.
Reproducer (crashes without this patch):
aa-logprof -f <(echo 'type=AVC msg=audit(1661739121.578:77893): apparmor="DENIED" operation="open" profile="no_such_profile" name="/run/" pid=33099 comm="no" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0')
I propose this patch for 3.1 and master. (3.0 and older are not affected and do not need this fix.)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/919
Approved-by: Jon Tourville <jon.tourville@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
If audit.log contains entries for a profile that doesn't exist (for
example when working with a log file from another system), skip these
log entries instead of crashing.
Reproducer (crashes without this patch):
aa-logprof -f <(echo 'type=AVC msg=audit(1661739121.578:77893): apparmor="DENIED" operation="open" profile="no_such_profile" name="/run/" pid=33099 comm="no" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0')
As discussed in #241 and !906, there are some overly broad `except` statements that should be fixed. This MR does so. Some notes:
- `profile_dir` in `aa.py` may be `None`, hence the `TypeError` catches. The other globals are not caught in the code, however. E.g. there are possibilities of TypeErrors due to `extra_profile_dir` being `None`.
- I added a TODO in `common.py` that I'd like some eyes on. I do not think that `os.path.normpath` can raise an exception (the [Python docs](https://docs.python.org/3/library/os.path.html?#os.path.normpath) don't mention exceptions, and none are raised in the [CPython source code](https://github.com/python/cpython/blob/main/Lib/posixpath.py#L345)). [After discussing this in the MR, the `os.path.normpath` check was removed.]
- The `except Exception: raise` occurrences throughout `test-aa-easyprof.py` do nothing, so I removed them.
- In `valgrind_simple.py`, I fixed a possible `NameError` in the `finally` clause.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/912
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
The upstream php-fpm.conf file carries the following pid file example
path:
[global]
; Pid file
; Note: the default prefix is @EXPANDED_LOCALSTATEDIR@
; Default Value: none
;pid = run/php-fpm.pid
Add this path to profiles/apparmor.d/php-fpm, alongside the current
nested "@{run}/php{,-fpm}/php*-fpm.pid" wildcard.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/267
Suggested-by: Ali Abdallah <ali.abdallah@suse.com>
Signed-off-by: David Disseldorp <ddiss@suse.de>
In commit 7c7224004 ("Prepare for AppArmor 3.1 release"), as preperation
for the AppArmor 3.1.0 release, the SO versioning information was
adjusted, using a more significant bump to give prior AppArmor releases
room to address bugs in libapparmor without ending up with conflicting
SO versions. Unfortunately, that process was untested and because
AA_LIB_AGE was not incremented by the same amount as AA_LIB_CURRENT,
this resulted in an accidental major SO versions bump with the library
SO version being:
libapparmor.so.4.9.0
This commit increments AA_LIB_AGE by the same amount, resulting in a
library versioned as:
libapparmor.so.1.12.0
and adds a note to mention that AA_LIB_AGE needs to be incremented
in the same way as AA_LIB_CURRENT. This fix is intended to address
this for the 3.1 branch; I'd like to find a better approach for
the development branch that can be used in future AppArmor primary
releases.
In general, thanks to symbol versioning (see
`libraries/libapparmor/src/libapparmor.map`) we should not need to
ever bump the SO version except in an extreme case.
Fixes: 7c7224004 ("Prepare for AppArmor 3.1 release")
Signed-off-by: Steve Beattie <steve.beattie@canonical.com>
Bug: https://gitlab.com/apparmor/apparmor/-/issues/266
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/913
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
(cherry picked from commit 075c69a4eb)
f9dbaa38 [3.1] libapparmor: fix mistaken SO version bump
by adding a warning flag that is disabled by default. This will enable
devs to find when and where #include is in use by adding the compile
flag
--warn=pound-include
and can even abort policy compiles by using
--warn=pound-include --Werror=pound-include
The resulting messages look like
Warning from /etc/apparmor.d/usr.sbin.cupsd (/etc/apparmor.d/usr.sbin.cupsd line 5): deprecated use of '#include'
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
This change tries to give us some room to deal with versioning issues
like we had in
145136f6 Fix 2.13 libapparmor so version
which revered the library version bump in
5c47e448 libapparmor: Bump revision in preparation for release.
which was done because of the symbol added by
351014c3 libapparmor: add _aa_asprintf to private symbols
unfortunately this collided with the 3.0 library symbol versioning
which had a different set of symbols with the same version numbering.
Leaving us no way to increase the version number to when fixing an
exported symbol version in a released library.
Ideally this would have been fixed with library version synchs but
in this case that option wasn't open as we were dealing with two
released versions of the library.
Bumping the current version by 5 on the larger releases gives us room
to deal with these kind of bugs in the future.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Time-out
Note: This was reported for /usr/libexec/libvirt_leaseshelper, but since
this is probably unrelated to the path or a path change, this commit
also adds r permissions for the previous path.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1202161
I propose this patch for 3.0 and master (optionally also for 2.12 and 2.13 - please tell me if you want that after reviewing the patch, or just merge ;-)
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/905
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Note: This was reported for /usr/libexec/libvirt_leaseshelper, but since
this is probably unrelated to the path or a path change, this commit
also adds r permissions for the previous path.
Fixes: https://bugzilla.opensuse.org/show_bug.cgi?id=1202161
profiles/Makefile:
Set PYTHON_DIST_BUILD_PATH using
libraries/libapparmor/swig/python/test/buildpath.py as it solves the
problem of setting the build directory generically.
libraries/libapparmor/swig/python/test/buildpath.py:
The changes introduced in
cc7f549665
targetted a wrong setuptools version (61.2).
The change in build directory naming has been introduced with 62.1
(1c23f5e1e4).
aa-load is a tool that loads cached (compiled) policies into
the kernel. It can receive as argument a file, a cache directory
containing the hash subtree, and a directory containing cached
files directly underneath - no hash.
This tool can be used in the as a guide for other init
systems to load the cached policies directly.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
This commit adds a helper function that splits a string
of overlay directories by either , (comma) or : (colon)
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
test -f ./.libs/$(EXPECTED_SO_NAME)||{echo'*** unexpected .so name/number for libapparmor (expected $(EXPECTED_SO_NAME), the actual filename is shown below) ***'; ls -l ./.libs/libapparmor.so.*.* ;exit 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.