utils: Simplify logparsing and rule creation from hashlog/event
- Allows to create all rules classes thanks to from_hashlog and hashlog_from_event
- These new functions simplify event/log parsing in logparser.py and aa.py
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1276
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Christian Boltz <apparmor@cboltz.de>
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>
The kernel does not expect a name and it is not used even within the
parser so drop it. Correct the padding calculation.
sizeof(th_version)
includes the trailing \0 in the count so we should not be adding it
explicitly. Doing so made it seem like we were writing an extra byte
and messing things up, because the string write below did not include
the \0 which we had to add explicitly.
Switch to writing the th_version using size_of() bytes as is used in
the pad calculation, to avoid confusion around the header padding.
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>
`execpath` allows to reliably store the path of the binary that triggered a log.
This is useful because comm was not sufficient to reliably identify a binary
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1275
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
`execpath` allows to reliably store the path of the binary that triggered a log.
This is useful because comm was not sufficient to reliably identify a binary
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
This reverts commit 78ae956087.
Commit 78ae956087 causes policy to not
to conform to protocol as determined by the kernel. Technically the
reverted patch is correct and the kernel is wrong but we can not
change 15 years of history.
The reason it breaks the policy in the kernel is because the kernel
does not use the name field, and does not expect it. It just expects
the size with a single trailing 0. This doesn't break because this
section is all padded to 64 bytes so writing the extra 0 doesn't
hurt as it is effectively just manually adding to the padding.
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>
By specifying 0 in the unix type, all rules were allowing only the "none" type, when it wanted to allow all types, so replace it by 0xffffffff. Also, add this testcase to the unix regression tests.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/410
I propose this fix for master and apparmor-4.0
Closes#410
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1273
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Without AA_MAY_MOUNT, mount was not allowed by the allow all
rule. AA_DUMMY_REMOUNT does become AA_MAY_MOUNT, but it fixes the
flags to remount only, so other options are not included. Also, add
allow all rule testcases to the mount regression tests.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/410
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
By specifying 0 in the unix type, all rules were allowing only the
"none" type, when it wanted to allow all types, so replace it by
0xffffffff. Also, add this testcase to the unix regression tests.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/410
Signed-off-by: Georgia Garcia <georgia.garcia@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
Signed-off-by: Georgia Garcia <georgia.garcia@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>
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>
... which so far was not translatable because it was formatted before
being translated.
I propose this fix for master, 4.0 and 3.x
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1271
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
/var/lib/wtmpdb/ contains the Y2038-safe version of wtmpdb.
Proposed by darix.
I propose this patch for master, 4.0 and 3.x.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1267
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Christian Boltz <apparmor@cboltz.de>
... 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>
Update aa-unconfined with several fixes and improvements to make it more useful, and make sure its man page matches its actual behavior.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1269
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
Contrary to what the name would imply aa-unconfined displays info for
both confined and unconfined processes. Add a --short option that only
output processes that are not confined. Eg.
$ ./utils/aa-unconfined
17192 /snap/chromium/2890/usr/lib/chromium-browser/chrome (/snap/chromium/2890/usr/lib/chromium-browser/chrome --password-store=basic --disable-features=TFLiteLanguageDetectionEnabled) confined by 'snap.chromium.chromium (enforce)'
17395 /snap/chromium/2890/usr/lib/chromium-browser/chrome (/snap/chromium/2890/usr/lib/chromium-browser/chrome --type=utility --utility-sub-type=network.mojom.NetworkService --lang=en-US --service-sandbox-type=none --crashpad-handler-pid=17337 --enable-crash-reporter=,snap --change-stack-guard-on-fork=enable --shared-files=v8_context_snapshot_data:100 --field-trial-handle=3,i,16674663885832976354,18417931519279121981,262144 --disable-features=TFLiteLanguageDetectionEnabled --variations-seed-version) confined by 'snap.chromium.chromium (enforce)'
17981 /snap/firefox/4451/usr/lib/firefox/firefox confined by 'snap.firefox.firefox (enforce)'
1353664 /tmp/.mount_OrcaSl7G1va5/bin/orca-slicer not confined
is trimmed to
$ ./utils/aa-unconfined --short
1353664 /tmp/.mount_OrcaSl7G1va5/bin/orca-slicer not confined
Signed-off-by: John Johansen <john.johansen@canonical.com>
The prompt/user upcall mode shows up as a mode of (user). And for
stacked policy with different modes (mixed) is used. Add these to the
list of modes to screen.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the ability to list applications that are unconfined and have
any open network socket open, both listening and non-listening.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add the abiity to list applications that are unconfined and have
open connection ports that are not listening.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The documentation of --paranoid is wrong. It lists all processes and
does not exclude based on whether it has a network port open.
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.
* UnixRule: Fix handling of peers with a ? and peers that are/need to be quoted
`?` is a valid AARE char, add it to the regexes that match the AARE.
Also add some tests to ensure this is really fixed, and make the error
output of the tests more useful/verbose.
* Fix handling of quoted peers in UnixRule (and others)
In UnixRule (and probably also in other rules that use
print_dict_values()` and `initialize_cond_dict()`), the handling of
peers with a value that is quoted and/or needs to be quoted was broken
because
- quotes didn't get stripped in `initialize_cond_dict()`
- `print_dict_values()` didn't use `quote_if_needed()`
Note: print_dict_values also handles integers (like network ports).
Convert them to a string so that `if ' ' in data` in `quote_if_needed()`
doesn't explode.
Also enable the test that uncovered this bug.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/404Closes#404
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1262
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
In UnixRule (and probably also in other rules that use
print_dict_values()` and `initialize_cond_dict()`), the handling of
peers with a value that is quoted and/or needs to be quoted was broken
because
- quotes didn't get stripped in `initialize_cond_dict()`
- `print_dict_values()` didn't use `quote_if_needed()`
Note: print_dict_values also handles integers (like network ports).
Convert them to a string so that `if ' ' in data` in `quote_if_needed()`
doesn't explode.
Also enable the test that uncovered this bug.
`?` is a valid AARE char, add it to the regexes that match the AARE.
Also add some tests to ensure this is really fixed, and make the error
output of the tests more useful/verbose.
Note: One of the added tests (with a space in the peer name) uncovered a
bug in quote handling. This will be fixed in the next commit.
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/404
Update the state machine readme to better reflect how the chfa is
encoded and works. It still needs a lot more but fixes several errors
in the doc and adds some info about state differential encoding, oobs,
and comb compression.
In addition fix an off by own error during chfa encoding. This has
likely never triggered as it gets hidden by being in a section that
is being in a section that is padded to an 8 byte boundary.
Signed-off-by: John Johansen <john.johansen@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1244
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
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>
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
whch gets reduces 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>
Adding mediation classes in unconfined profiles caused nested profiles
to be mediated, inside a container for example.
As a first step, skip the addition of mediation classes into the dfa.
The creation of unprivileged user namespaces is an exception, where we
always want to mediate it.
Fixes: https://bugs.launchpad.net/apparmor/+bug/2067900
Signed-off-by: Georgia Garcia <georgia.garcia@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>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1255
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
The parser writes
sizeof(th)) + th_version + (char)0 + name + (char)0;
but the padding currently is computed as
sizeof(th)) + th_version + name + (char)0;
missing the internal (char)0, add 1 to the pad and fill to ensure
this is correct.
Reported-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Update the state machine readme to better reflect how the chfa is
encoded and works. It still needs a lot more but fixes several errors
in the doc and adds some info about state differential encoding, oobs,
and comb compression.
Signed-off-by: John Johansen <john.johansen@canonical.com>
