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

Bug #1331856 reported by Sebastian Naury
50
This bug affects 9 people
Affects Status Importance Assigned to Milestone
AppArmor
Medium
Unassigned
apparmor (Ubuntu)
Undecided
Unassigned

Bug Description

When a variable is set in tunables/home.d/ the apparmor-utils programs don't work and return this error messages:

root@ws24:~# aa-logprof
Traceback (most recent call last):
  File "/usr/sbin/aa-logprof", line 50, in <module>
    apparmor.loadincludes()
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 4643, in loadincludes
    load_include(fi)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 4520, in load_include
    incdata = parse_profile_data(data, incfile, True)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 2839, in parse_profile_data
    store_list_var(profile_data[profile]['lvar'], list_var, value, var_operation)
  File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 3279, in store_list_var
    raise AppArmorException(_('Values added to a non-existing variable: %s') % list_var)
apparmor.common.AppArmorException: 'Values added to a non-existing variable: @{HOMEDIRS}'
root@ws24:~#

Revision history for this message
Christian Boltz (cboltz) wrote :

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.

tags: added: aa-tools
Changed in apparmor:
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Walter Mundt (waltermundt) wrote :

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.

Revision history for this message
intrigeri (intrigeri) wrote :

@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?

Revision history for this message
Red Ink (redink) wrote :

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

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

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.]

tags: added: patch
Revision history for this message
intrigeri (intrigeri) wrote :

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!

Revision history for this message
Vincas Dargis (talkless) wrote :

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

Revision history for this message
intrigeri (intrigeri) wrote :

Vincas, do you want to test the proposed patch?

Revision history for this message
Vincas Dargis (talkless) wrote : Re: [Bug 1331856] Re: apparmor-utils don't work when defining a variable on <tunables/home.d>

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.

Revision history for this message
Vincas Dargis (talkless) wrote :

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.

Revision history for this message
Vincas Dargis (talkless) wrote :

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...

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in apparmor (Ubuntu):
status: New → Confirmed
Revision history for this message
Vincas Dargis (talkless) wrote :

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
```

Revision history for this message
Marco Schmidt (kunzol) wrote :

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

Revision history for this message
Joerg Delker (ubuntu-delker) wrote :

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.

Revision history for this message
Christian Boltz (cboltz) wrote :

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

AppArmor 3.0 will include the fixed tools.

Changed in apparmor:
status: Triaged → Fix Committed
Revision history for this message
Vincas Dargis (talkless) wrote :

Thank you Christian!

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers