when aa-logprof processed file access rules with mask of "c" the resulting profile doesn't work

Bug #1324608 reported by timdaman on 2014-05-29
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Medium
Unassigned
apparmor (Ubuntu)
Medium
Unassigned

Bug Description

To set the scene

++++++++++++/usr/local/bin/test2.sh++++++++++++++++++
#!/bin/bash

echo "hello world"
cat /etc/passwd
echo 'echo "I am a pink bunny!"' >> ~/.profile
++++++++++++++++++++++++++++++

++++++++++++/etc/apparmor.d/usr.local.bin.test2.sh++++++++++++++++++
# Last Modified: Thu May 29 11:34:54 2014
#include <tunables/global>

/usr/local/bin/test2.sh {
  #include <abstractions/base>
  #include <abstractions/bash>

  /bin/bash ix,
  /bin/cat rix,
  /dev/tty rw,
  /usr/local/bin/test2.sh r,

}
++++++++++++++++++++++++++++++

So I run test2.sh and the
  >> ~/.profile
is denied as expected. When I go to run aa-logprof it picks up on the logline

   apparmor="DENIED" operation="open" profile="/usr/local/bin/test2.sh" name="/root/.profile" pid=30693 comm="test2.sh" requested_mask="c" denied_mask="c" fsuid=0 ouid=0

It as me about adding an append mode which makes sense so I add and allow. Now my profile looks like so

++++++++++++/etc/apparmor.d/usr.local.bin.test2.sh++++++++++++++++++
# Last Modified: Thu May 29 12:32:31 2014
#include <tunables/global>

/usr/local/bin/test2.sh {
  #include <abstractions/base>
  #include <abstractions/bash>

  /bin/bash ix,
  /bin/cat rix,
  /dev/tty rw,
  /root/.profile a,
  /usr/local/bin/test2.sh r,

}
++++++++++++++++++++++++++++++

The problem is when I run test2.sh again the append rule is still denied. I think this likely because although the request_mask of "c" is "converted to "a" by logparser.py in the runtime code (kernel?, apparmor_parse?) "c" is not converted to "a" nor is a subset of "a". It is in fact a subset of "w" which works. Here is the workaround in diff format.

--- /usr/lib/python3/dist-packages/apparmor/logparser.py 2014-05-29 12:39:23.844284290 -0400
+++ /usr/lib/python3/dist-packages/apparmor/logparser.py.new 2014-05-29 12:39:11.444283996 -0400
@@ -126,12 +126,12 @@
         LibAppArmor.free_record(event)
         # Map c (create) to a and d (delete) to w, logprof doesn't support c and d
         if rmask:
- rmask = rmask.replace('c', 'a')
+ rmask = rmask.replace('c', 'w')
             rmask = rmask.replace('d', 'w')
             if not validate_log_mode(hide_log_mode(rmask)):
                 raise AppArmorException(_('Log contains unknown mode %s') % rmask)
         if dmask:
- dmask = dmask.replace('c', 'a')
+ dmask = dmask.replace('c', 'w')
             dmask = dmask.replace('d', 'w')
             if not validate_log_mode(hide_log_mode(dmask)):
                 raise AppArmorException(_('Log contains unknown mode %s') % dmask)

Right now I don't have time to identify where this bug is exactly but if I do I will update this ticket.

timdaman (timdaman-gmail) wrote :

Additionally problem. When there is an already existing deny rule with a "w" mask
  deny /home/*/.profile w,
the "a" mask is not recognized as being matched by it and thus aa-logprof prompts to create a new rule when the permission is already affirmatively denied.

timdaman (timdaman-gmail) wrote :

Bug https://bugs.launchpad.net/apparmor/+bug/539433 might be related though it is listed as fixed

Christian Boltz (cboltz) wrote :

denied_mask="c" means "create file", and "a" permissions cover it ("w" would be too much).

Can you please paste the log line you get in the second run of the script? I wouldn't be surprised if you get something different on the next run... (In other words: now that /root/.profile exists.)

The problem you describe in comment #1 sounds like a valid bug.

Bug 539433 (mentioned in comment #2) is really fixed (since a long time, even if it was closed today).

Changed in apparmor (Ubuntu):
status: New → Incomplete
tags: added: aa-tools
Changed in apparmor (Ubuntu):
importance: Undecided → Medium
Changed in apparmor:
importance: Undecided → Medium
status: New → Incomplete
Christian Boltz (cboltz) wrote :

See also bug 1339099 - I wouldn't be surprised if the main part of this bug is on the parser or kernel side.

Christian Boltz (cboltz) wrote :

For comment #1, I just opened bug 1385474 to make sure it doesn't get lost.

Launchpad Janitor (janitor) wrote :

[Expired for AppArmor because there has been no activity for 60 days.]

Changed in apparmor:
status: Incomplete → Expired
Launchpad Janitor (janitor) wrote :

[Expired for apparmor (Ubuntu) because there has been no activity for 60 days.]

Changed in apparmor (Ubuntu):
status: Incomplete → Expired
Christian Boltz (cboltz) wrote :

trunk r3279 (and similar commits to the version branches, which will be in 2.10.1 and 2.9.3) changed the behaviour - aa-logprof and aa-genprof will now propose 'w' for creating a file. The commit message explains why:

  Map c (create) log events to w instead of a

  Creating a file is in theory covered by the 'a' permission, however
  discussion on IRC brought up that depending on the open flags it might
  not be enough (real-world example: creating the apache pid file).

  Therefore change the mapping to 'w' permissions. That might allow more
  than needed in some cases, but makes sure the profile always works.

Changed in apparmor:
status: Expired → Fix Committed
milestone: none → 2.10.1
Changed in apparmor:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers