For every item in "cases", a new state is created, but if the creation
of one of them fails, the rest of the items in that list would not be
deleted and would leak. Fix it by continuing to iterate over the items
in the list and delete them, and then re-throw the exception.
$ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all simple_tests/xtrans/x-conflict.sd
==564911== 208 (48 direct, 160 indirect) bytes in 1 blocks are definitely lost in loss record 15 of 19
==564911== at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==564911== by 0x179E74: CharNode::follow(Cases&) (expr-tree.h:447)
==564911== by 0x189F8B: DFA::update_state_transitions(optflags const&, State*) (hfa.cc:376)
==564911== by 0x18A25B: DFA::process_work_queue(char const*, optflags const&) (hfa.cc:442)
==564911== by 0x18CB65: DFA::DFA(Node*, optflags const&, bool) (hfa.cc:486)
==564911== by 0x178263: aare_rules::create_chfa(int*, std::vector<aa_perms, std::allocator<aa_perms> >&, optflags const&, bool, bool) (aare_rules.cc:258)
==564911== by 0x178A4F: aare_rules::create_dfablob(unsigned long*, int*, std::vector<aa_perms, std::allocator<aa_perms> >&, optflags const&, bool, bool) (aare_rules.cc:359)
==564911== by 0x14E4E1: process_profile_regex(Profile*) (parser_regex.c:791)
==564911== by 0x154CDF: process_profile_rules(Profile*) (parser_policy.c:194)
==564911== by 0x154E0F: post_process_profile(Profile*, int) (parser_policy.c:240)
==564911== by 0x154F7A: post_process_policy_list (parser_policy.c:257)
==564911== by 0x154F7A: post_process_policy(int) (parser_policy.c:267)
==564911== by 0x141B17: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1227)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/534
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
When the "conflicting x modifiers" exception was thrown, the DFA
object creation would fail, therefore the destructor would not be
called and the states previously allocated would leak.
Unfortunately there's no way to call the destructor if the object was
not created, so I moved the contents of the destructor into a cleanup
helper function to be called in both instances.
$ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all simple_tests/xtrans/x-conflict.sd
==564911== 592 (112 direct, 480 indirect) bytes in 1 blocks are definitely lost in loss record 16 of 19
==564911== at 0x4846FA3: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==564911== by 0x189C9A: DFA::add_new_state(optflags const&, std::set<ImportantNode*, std::less<ImportantNode*>, std::allocator<ImportantNode*> >*, std::set<ImportantNode*, std::less<ImportantNode*>, std::allocator<ImportantNode*> >*, State*) (hfa.cc:337)
==564911== by 0x18CB22: add_new_state (hfa.cc:357)
==564911== by 0x18CB22: DFA::DFA(Node*, optflags const&, bool) (hfa.cc:473)
==564911== by 0x178263: aare_rules::create_chfa(int*, std::vector<aa_perms, std::allocator<aa_perms> >&, optflags const&, bool, bool) (aare_rules.cc:258)
==564911== by 0x178A4F: aare_rules::create_dfablob(unsigned long*, int*, std::vector<aa_perms, std::allocator<aa_perms> >&, optflags const&, bool, bool) (aare_rules.cc:359)
==564911== by 0x14E4E1: process_profile_regex(Profile*) (parser_regex.c:791)
==564911== by 0x154CDF: process_profile_rules(Profile*) (parser_policy.c:194)
==564911== by 0x154E0F: post_process_profile(Profile*, int) (parser_policy.c:240)
==564911== by 0x154F7A: post_process_policy_list (parser_policy.c:257)
==564911== by 0x154F7A: post_process_policy(int) (parser_policy.c:267)
==564911== by 0x141B17: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1227)
==564911== by 0x135421: main (parser_main.c:1771)
Fixes: https://gitlab.com/apparmor/apparmor/-/issues/534
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Valgrind showed that the disconnected paths variables were leaking
during the merge. That happened because flagvals did not implement a
destructor freeing the variables, so they leaked. flagvals cannot
implement a destructor, because that would make it a non-trivial union
member and parser_yacc.y would not compile. This patch implements a
"clear" function that is supposed to act as the destructor.
$ /usr/bin/valgrind --leak-check=full --error-exitcode=151 ../apparmor_parser -Q -I simple_tests/ -M ./features_files/features.all flags_ok_disconnected_ipc15.sd
...
==3708747== 5 bytes in 1 blocks are definitely lost in loss record 1 of 11
==3708747== at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3708747== by 0x492E35E: strdup (strdup.c:42)
==3708747== by 0x14C74E: set_disconnected_path (profile.h:188)
==3708747== by 0x14C74E: flagvals::init(char const*) (profile.h:223)
==3708747== by 0x14859B: yyparse() (parser_yacc.y:592)
==3708747== by 0x141A99: process_profile(int, aa_kernel_interface*, char const*, aa_policy_cache*) (parser_main.c:1187)
==3708747== by 0x135421: main (parser_main.c:1771)
...
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
GDM 49~beta implements a userdb VarLink service for managing the unix users
running the greeter shell, as well as the gnome-initial-setup users.
```
gdm-launch-environment][1892]: Gdm: GdmSessionWorker: determining if authenticated user (password required:0) is authorized to session
unix_chkpwd[1897]: could not obtain user info (gdm-greeter)
kernel: audit: type=1400 audit(1754399331.488:211): apparmor="DENIED" operation="connect" class="file" profile="unix-chkpwd" name="/run/systemd/userdb/org.gnome.DisplayManager" pid=1897 comm="unix_chkpwd" requested_mask="wr" denied_mask="wr" fsuid=0 ouid=0
gdm-launch-environment][1892]: Gdm: GdmSessionWorker: user is not authorized to log in: Authentication failure
```
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1761
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
GDM 49~beta implements a userdb VarLink service for managing the unix users
running the greeter shell, as well as the gnome-initial-setup users.
gdm-launch-environment][1892]: Gdm: GdmSessionWorker: determining if authenticated user (password required:0) is authorized to session
unix_chkpwd[1897]: could not obtain user info (gdm-greeter)
kernel: audit: type=1400 audit(1754399331.488:211): apparmor="DENIED" operation="connect" class="file" profile="unix-chkpwd" name="/run/systemd/userdb/org.gnome.DisplayManager" pid=1897 comm="unix_chkpwd" requested_mask="wr" denied_mask="wr" fsuid=0 ouid=0
gdm-launch-environment][1892]: Gdm: GdmSessionWorker: user is not authorized to log in: Authentication failure
LP: #2119541
perms = 0, therefore perms & something is always false.
Fixes: coverity#320937 and coverity#320937
Also remove nop code from mnt_rule::post_parse_profile(Profile &prof) as discussed in this MR.
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1759
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When the variable was being expanded, it needed to be reevaluated to
check if there was still unresolved variables. That allowed for a
weird bug to happen: If the string contained a variable preceded by @,
like in "user@@{uid}" and the variable was resolved to a case where {
is used, like in @{uid}={[0-9],[1-9][0-9]}, then on the second pass,
the parser would try to resolve the following variable
@{[0-9],[1-9][0-9]}, which is incorrect behavior. Fix it by not
including part of the string that was already resolved on the
subsequent passes.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1756
Approved-by: Steve Beattie <steve+gitlab@nxnw.org>
Merged-by: Steve Beattie <steve+gitlab@nxnw.org>
When the variable was being expanded, it needed to be reevaluated to
check if there was still unresolved variables. That allowed for a
weird bug to happen: If the string contained a variable preceded by @,
like in "user@@{uid}" and the variable was resolved to a case where {
is used, like in @{uid}={[0-9],[1-9][0-9]}, then on the second pass,
the parser would try to resolve the following variable
@{[0-9],[1-9][0-9]}, which is incorrect behavior. Fix it by not
including part of the string that was already resolved on the
subsequent passes.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The xtable on perms32 capable systems is being padded to the size of
the accept state tables. This was a hack to get around issue in a buggy
perms32 v1. We do not support any system using perms 32 v1 so we can
drop the hack.
Similarly since we don't support perms32v1 we don't support prompt
compat dev or perms32v1, so drop them as well
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1750
Approved-by: Ryan Lee <rlee287@yahoo.com>
Merged-by: John Johansen <john@jjmx.net>
Their is no reason for the parse to stitch 2 dfas together this way.
In the future there will be better ways to do this using unconpressed
dfas.
Dropping this also allows for some simplification, in other parts of
the code.
Drop the dead/unused code
Signed-off-by: John Johansen <john.johansen@canonical.com>
prompt_compat_permsv1 and prompt_compat_dev were used to support
prompt during early dev. We do not support any kernel using these
so drop them.
This also allows us to drop the propogation of prompt as a parameter
through several functions.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The xtable on perms32 capable systems is being padded to the size of
the accept state tables. This was a hack to get around issue in a buggy
perms32 v1. We do not support any system using perms 32 v1 so we can
drop the hack.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Right now coverity is running in two steps, one to collect logs in
case of failures, and a different one to actually send the data to
coverity. The log collection step is failing because when collecting
data for python with the new version of coverity, build-log.txt is not
generated.
The whole way we build with coverity might need changing, but
currently this patch is removing the log collection so the pipeline
passes.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1754
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Right now coverity is running in two steps, one to collect logs in
case of failures, and a different one to actually send the data to
coverity. The log collection step is failing because when collecting
data for python with the new version of coverity, build-log.txt is not
generated.
The whole way we build with coverity might need changing, but
currently this patch is removing the log collection so the pipeline
passes.
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
The unnamed nature of an O_TMPFILE, combined with the delayed linkage of
linkat(2), creates a potential for a filesystem mediation bypass or other
unexpected file mediation behavior. Thus, add a test to verify whether or
not such a bypass occurs.
Signed-off-by: Ryan Lee <ryan.lee@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1743
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Arch Linux qt6-webengine has `/usr/lib/qt6/QtWebEngineProcess` and
qt5-webengine has `/usr/lib/qt/libexec/QtWebEngineProcess`.
Fedora has `/usr/lib64/qt6/libexec/QtWebEngineProcess`.
openSUSE Tumbleweed has `/usr/libexec/qt5/QtWebEngineProcess` and
`/usr/libexec/qt6/QtWebEngineProcess`.
Co-authored-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1726
Approved-by: Maxime Bélair <maxime.belair@canonical.com>
Merged-by: John Johansen <john@jjmx.net>
I'd like to store my wg creds in my TPM module using `systemd-creds`:
```bash
PostUp = systemd-creds --name wg0 decrypt /etc/wireguard/secrets/wg0.cred | wg set wg0 private-key /dev/stdin
```
Currently I use `local/wg-quick` as work-around.
The `Ux` permission is may be a little too open, but 2 problems remain:
- the profile maintainer can't know which creds file need to be accessible
- different TMP module implementations / drivers may require different permissions
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1644
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
Add support for change_onexec logs by converting them to change_profile.
Fix associated test.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1745
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
This MR removes some footguns in aa-notify
- Prevents the modification of special profiles
- Improve the clarity of messages
- Add support for regexes in userns_special_profiles
- Refactor get_event_type.
- Add support for regexes for special profiles
- Optimize aa-notify performances
- Minor bugfixes
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1732
Approved-by: Christian Boltz <apparmor@cboltz.de>
Merged-by: Maxime Bélair <maxime.belair@canonical.com>
import tkinter does not automatically import tkinter.font so calls to
the latter fail if the execution environment does not already contains
it.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Profiles are now updated only at initialization and when aa-notify
itself updates a profile.
A future MR will come to read profiles individually only when an event
for this profile comes to reduce overhead, as more and more profiles are
created.
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Mesa now needs ~/.cache/mesa_shader_cache_db/marker .
Chromium wants uid_map readable, /proc/$PID/smaps_rollup,
/sys/.../report_descriptor, and two XDG utilities used by the "Create
shortcut..." feature. Deny the latter for now, due to additional
permissions that would be needed and a questionable security trade-off
as a result.
Firefox wants a socket for its crash helper, product_{name,sku} from
DMI devices, and .sql files in its cache directory. It also wants
uevent from devices more broadly than currently allowed.
After an upgrade to libfuse 3.17.1-rc0, autopkgtests started to fail
due to a missing x permission for /usr/bin/mount. After looking at the
source code for fusermount, I noticed that it does call /bin/mount and
/bin/umount in certain cases. These uses were already there in
previous versions of libfuse but I'm still not sure why it hasn't
triggered before.
To reproduce it:
```
sudo autopkgtest-buildvm-ubuntu-cloud -v -r questing
autopkgtest archivemount -U --apt-pocket=proposed=src:fuse3 --shell-fail -- qemu autopkgtest-questing-amd64.img
```
After the test fails, enter the vm by
```
ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -p 10022 ubuntu@localhost
```
You can reproduce the test by running
```
cd /tmp/autopkgtest.*/build.*/src/
/tmp/autopkgtest.*/build.*/src/debian/tests/test
```
Note that ix for mount and umount were enough to make the autopkgtest
failures to start passing, but there could be issues in the future
regarding the use of fs specific mount binaries like
/usr/sbin/mount.fuse
Fixes: http://bugs.launchpad.net/bugs/2111845
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1716
Approved-by: John Johansen <john@jjmx.net>
Merged-by: John Johansen <john@jjmx.net>
When variable expansion occurs, the expansion attempts to replace the
memory location of the string containing the variable, and frees the
string it is replacing. However, this occurs before the variable lookup
occurs to determine if there is an appropriate declaration for the
variable. When the failing expansion occurs in a profile name, this
causes a read-after-free (followed by a double free) because the error
handling path attempts to report the profile name in the error message.
This can be reproduced like so, using the
tst/simple_tests/vars/vars_profile_name_23.sd testcase:
```
$ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd
Failed to find declaration for: @{FOO}
ERROR expanding variables for profile #xQV, failed to load
free(): double free detected in tcache 2
```
Fix this by waiting to free the profile name field until after the
variable declaration has successfully been looked up. This results in
the test case reporting the following error:
```
$ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd
Failed to find declaration for: @{FOO}
ERROR expanding variables for profile /does/not/exist@{FOO}, failed to load
```
Signed-off-by: Steve Beattie <steve@nxnw.org>
MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/1747
Approved-by: Georgia Garcia <georgia.garcia@canonical.com>
Merged-by: Georgia Garcia <georgia.garcia@canonical.com>
When variable expansion occurs, the expansion attempts to replace the
memory location of the string containing the variable, and frees the
string it is replacing. However, this occurs before the variable lookup
occurs to determine if there is an appropriate declaration for the
variable. When the failing expansion occurs in a profile name, this
causes a read-after-free (followed by a double free) because the error
handling path attempts to report the profile name in the error message.
This can be reproduced like so, using the
tst/simple_tests/vars/vars_profile_name_23.sd testcase:
```
$ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd
Failed to find declaration for: @{FOO}
ERROR expanding variables for profile #xQV, failed to load
free(): double free detected in tcache 2
```
Fix this by waiting to free the profile name field until after the
variable declaration has successfully been looked up. This results in
the test case reporting the following error:
```
$ ../apparmor_parser --config-file=./parser.conf -M features_files/features.all -S -I /home/sbeattie/git/apparmor/parser/tst/./simple_tests/ ./simple_tests/vars/vars_profile_name_23.sd
Failed to find declaration for: @{FOO}
ERROR expanding variables for profile /does/not/exist@{FOO}, failed to load
```
Fixes: dfbd2dc4b ("parser: refactor variables and symbols table into their own class")
Signed-off-by: Steve Beattie <steve@nxnw.org>
Ref: https://gitlab.com/apparmor/apparmor/-/merge_requests/1747
