apparmor-utils don't work when defining a variable on <tunables/home.d>

Minimal testcase:
Create a directory with the following two files:

# find -not -type d |xargs head -n1000

==> ./tunables/home <==
@{HOMEDIRS}=/home/

#include <tunables/home.d/>

==> ./tunables/home.d/ubuntu <==
@{HOMEDIRS}+=/home2/

Then run aa-logprof -d $minimal_testcase_directory -f emptylog
("emptylog" is just an empty file to avoid side effects from the "real" log)

Some quick testing indicates that aa-logprof (and probably all other aa-* tools) read the profile dir _recursively_. That's not what it should do...

BTW: You'll get a similar (slightly shorter) traceback if you remove the "#include <tunables/home.d/> line.

Here's a proposed fix. Patched against apparmor_2.8.95~2430-0ubuntu5. This modifies the Python parser to propagate information about 'current profile' context and defined variables to included files, in keeping with what I presume are the textual-inclusion semantics of #include. Because included files now modify their "parent" profile_data directly, parse_profile_data no longer caches parsed includes, and will re-parse them in each context where they're included.

This may have substantial performance impacts, which could be mitigated by instead separately storing data on "pending append" operations from includes, and applying those operations in any including context. This would allow an include file's operations to be applied to an including context without re-parsing the file.

@cboltz: I guess this one might have slipped through the cracks. Do you track attached patches, or should the submitter instead file a merge request?

This bug is still present in AppArmor in Xenial. Shouldn't this be an easy fix?

The attachment "Proposed fix (may impact performance)" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

Anyone interested in moving this forward: please send a merge request. We're apparently not very good at tracking patches attached to bug reports, sorry!

I believe I got similar issue while experimenting with Thunderbird profile patch [0] to use `local/tunables/usr.bin.thunderbird` as extension point.

aa-* tools does now like that:

```
vincas@debian-sid:~$ sudo aa-enforce /etc/apparmor.d/*

ERROR: Values added to a non-existing variable @{thunderbird_user_dirs}: /mnt/foo /home/vincas/Archive/ in local/tunables/usr.bin.thunderbird

vincas@debian-sid:~$ sudo aa-logprof

ERROR: Values added to a non-existing variable @{thunderbird_user_dirs}: /mnt/foo /home/vincas/Archive/ in local/tunables/usr.bin.thunderbird
```

Looks like show-stopper for [1] :(.

[0] https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=882218;filename=wip-thunderbird-user-dirs-variable.patch;msg=39
[1] https://lists.ubuntu.com/archives/apparmor/2017-December/011350.html

Vincas, do you want to test the proposed patch?

On 2018-01-07 17:33, intrigeri wrote:
> Vincas, do you want to test the proposed patch?

Yeah, I could try that within next week.

Hm, Ubuntu 18.04 Daily has only apparmor 2.11.0-2ubuntu8 (I was expecting it would already have 2.12 for some reason).

Should I try (somehow) to build apparmor-utils package using 2.12 source? Not sure what's the best way to test latest against latest AppArmor.

Seems that this patch is quite outdated:

vincas@debian-sid:/tmp/apparmor-2.12$ patch -p1 < /home/vincas/Downloads/python-utils-var-append-in-include-support.patch
patching file utils/apparmor/aa.py
Hunk #1 FAILED at 2636.
Hunk #2 FAILED at 2836.
Hunk #3 FAILED at 2979.
Hunk #4 FAILED at 2987.
Hunk #5 FAILED at 3263.
Hunk #6 FAILED at 3274.
Hunk #7 FAILED at 4113.
Hunk #8 succeeded at 3516 with fuzz 2 (offset -993 lines).
Hunk #9 FAILED at 4517.
8 out of 9 hunks FAILED -- saving rejects to file utils/apparmor/aa.py.rej

Well, author mentioned that it was developed on 2.8.95, so no big surprise I guess...

Status changed to 'Confirmed' because the bug affects multiple users.

Uhm I get same error even if variable is introduced in /etc/apparmor.d/tunables/:

```
$ cat /etc/apparmor.d/tunables/usr.bin.qtox | head -n1
@{qtox_prefix} = /usr /usr/local
```

Adding local customisation:

```
$ cat /etc/apparmor.d/tunables/usr.bin.qtox.d/local
@{qtox_prefix} += /tmp/qtox
```

I get:

```
$ sudo aa-logprof

ERROR: Values added to a non-existing variable @{qtox_prefix}: /tmp/qtox in tunables/usr.bin.qtox.d/local
```

Still exists in Ubuntu Bionic Beaver (18.04).
apparmor-utils 2.12-4ubuntu5.1 amd64

# aa-logprof

ERROR: Values added to a non-existing variable @{HOMEDIRS}: /srv/ in tunables/home.d/ubuntu

And still does not work in Eoan Ermine (19.10).
apparmor-utils 2.13.3-5ubuntu1 amd64

# aa-logprof

ERROR: Values added to a non-existing variable @{HOMEDIRS}: /shared/home/ in tunables/home.d/site.local

In fact, this bug was the origin cause of multiple packages failing to upgrade when switching to eoan.
It puzzles me, that this was not detected within release testing and is not getting more attention by the developers.

This bug is finally fixed with https://gitlab.com/apparmor/apparmor/-/merge_requests/544

AppArmor 3.0 will include the fixed tools.

Thank you Christian!