AppArmor unrequested reply protection generates unallowable denials

Bug #1362469 reported by Tyler Hicks on 2014-08-28
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
dbus (Ubuntu)
Medium
Unassigned

Bug Description

Starting with utopic's dbus 1.8.6-1ubuntu1 package, the new AppArmor unrequested reply protections can generate some denials that can't easily be allowed in policy. For example, when running a confined pasaffe, you see these denials when starting and closing pasaffe:

apparmor="DENIED" operation="dbus_error" bus="session" error_name="org.freedesktop.DBus.Error.UnknownMethod" mask="send" name=":1.22" pid=4993 profile="/usr/bin/pasaffe" peer_pid=3624 peer_profile="unconfined"

It isn't obvious how to construct an AppArmor D-Bus rule to allow that operation. A bare "dbus," rule allows it but that's not acceptable for profiles implementing tight D-Bus confinement.

The code that implements unrequested reply protections should be reviewed for issues and, if everything looks good there, investigations into how to allow the operation that triggers the above denial should occur.

Jamie Strandboge (jdstrand) wrote :

FYI, we shouldn't try to land 1.8.6 in ubuntu-rtm until this bug is fixed.

Jamie Strandboge (jdstrand) wrote :

Note: the security team does not require 1.8.6 in ubuntu-rtm at this time.

tags: added: application-confinement
Ricardo Mendoza (ricmm) wrote :

Whats the status of this? We might have a serious issue that needs forwarding dbus to 1.8, however we haven't tracked it down to a single commit that we could cherry pick yet.

Tyler Hicks (tyhicks) wrote :

@ricmm: I've spent a couple days, in the past, trying to chase down this bug but didn't have any luck. It is on par, in terms of difficulty to debug and fix, with the CPU pegging bug that you're trying to solve.

If it has been shown that 1.8 solves the CPU pegging bug, I think someone should be able to track down the commits that fix that issue before we'll have a fix for this bug.

Changed in dbus (Ubuntu):
status: Triaged → In Progress
Tyler Hicks (tyhicks) wrote :
Download full text (6.2 KiB)

I now have a good understanding of the problem as well as a standalone
reproducer. The issue stems from the NO_REPLY_EXPECTED flag in the
message header. When the sender of a message sets the NO_REPLY_EXPECTED
flag, it means tells the recipient that a reply, either in the form of a
method_return or an error message, is not needed even if the method
normally provides a reply message. The D-Bus spec
(http://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-messages)
does not require that the receiver of the message honor the
NO_REPLY_EXPECTED flag. Here's the text:

  "... the reply can be omitted as an optimization. It is compliant with
   this specification to return the reply despite this flag, although
   doing so on a bus with a non-trivial security policy (such as the
   well-known system bus) may result in access denial messages being
   logged for the reply."

The dbus-daemon code has a mechanism for tracking whether or not a reply
message (method_return and error messages) is a reply that the sender of
the original message actually asked for. If the reply message was not
requested, the dbus-daemon code treats it as an "unrequested reply".
This is important to the AppArmor mediation code in dbus-daemon because
it lets reply messages that *were* requested through without requiring
any sort of AppArmor policy query. The thought is that the AppArmor
security policy allowed the original message to go through so surely the
security policy author expects any corresponding reply message to go
through. That all works great except for when the original message has
the NO_REPLY_EXPECTED flag. The dbus-daemon code treats any replies to
that message as an unrequested reply. That means that it is subject to
the AppArmor policy and may be rejected if the process sending the reply
does not have "send" permissions.

The bug description for this bug mentions an AppArmor denial of pasaffe
sending an UnknownMethod error message to an unconfined process:

apparmor="DENIED" operation="dbus_error" bus="session"
  error_name="org.freedesktop.DBus.Error.UnknownMethod" mask="send"
  name=":1.22" pid=4993 profile="/usr/bin/pasaffe" peer_pid=3624
  peer_profile="unconfined"

What is happening there is that some unconfined UI component (global
menus backend?) is communicating with the UI components of the confined
pasaffe process over D-Bus. The confined UI components are tearing
themselves down after the user enters their master password into a
pasaffe modal dialog box and presses "Ok". This results in the confined
UI components unregistering the D-Bus object path corresponding to the
modal dialog box. After this happens, the unconfined UI components are
attempting to send one last message, with the NO_REPLY_EXPECTED flag
set, to the confined UI components (the "End" method call). This is an
error condition since the D-Bus object has already been unregistered.
The gdbus (glib) bindings notice that this is an error and send an
UnknownMethod error message reply, despite the fact that the
NO_REPLY_EXPECTED flag was set on the original message. This causes
dbus-daemon to treat the reply as an unrequested reply and the AppArmor
mediation code blocks th...

Read more...

Tyler Hicks (tyhicks) wrote :

Also important to note is that this bug is no longer considered to be a blocker when syncing the RTM archive with the Vivid packages. Outside of the noisy denial message, the fact that AppArmor is blocking unrequested replies should not result in any breakage of software relying on D-Bus communications.

Tyler Hicks (tyhicks) on 2015-02-06
Changed in dbus (Ubuntu):
status: In Progress → Triaged
John Johansen (jjohansen) wrote :

thanks for finding this Tyler.

I concur with recommendation for how this should be fixed.

Seth Arnold (seth-arnold) wrote :

Good work. I think you're right, most policy authors would rather just drop the unrequested replies, so dropping them on the floor silently feels like the path of least surprise.

Maybe it would be useful to provide some debug logging option for these but I doubt it'd ever repay the time it would take to add one. If it's easier than I suspect, though, maybe..

Thanks

Tyler Hicks (tyhicks) wrote :

The AppArmor D-Bus mediation patches have been merged in upstream D-Bus. The final patch set that was merged includes a fix for this bug, among others that do not have Launchpad bugs but were discovered during the upstream patch review process.

This is a debdiff to refresh the patches in our Vivid dbus package with the versions that landed upstream.

I've tested it in an amd64 Vivid VM, using the test-dbus.py script from lp:qa-regression-testing as well as the D-Bus regression tests in lp:apparmor. I manually verified that the org.freedesktop.DBus.GetConnectionCredentials method was working correctly, as well as the legacy org.freedesktop.DBus.GetAppArmorSecurityContext method. I also ran the test case in comment #5 of this bug in addition to confining pasaffe and verifying that this bug was fixed. Finally, I did manual exploratory testing in the VM.

I also tested it on a Mako device using a vivid-proposed image (build 105). The testing was manual but I verified that I could use the browser, use a webapp, install and use a new app, adjust system settings, etc.

A PPA build of this package exists in ppa:ubuntu-security-proposed/ppa

Changed in dbus (Ubuntu):
assignee: Tyler Hicks (tyhicks) → nobody
status: Triaged → Confirmed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package dbus - 1.8.12-1ubuntu2

---------------
dbus (1.8.12-1ubuntu2) vivid; urgency=medium

  * Refresh the patches related to AppArmor D-Bus mediation to reflect what
    landed upstream in 1.9.12.
    - 0001-New-a-sv-helper-for-using-byte-arrays-as-the-variant.patch,
      0002-Add-LSM-agnostic-support-for-LinuxSecurityLabel-cred.patch,
      0003-Add-regression-test-for-LinuxSecurityLabel-credentia.patch,
      0004-Add-LinuxSecurityLabel-to-specification.patch: Add patches that
      report the AppArmor confinement context in the bus driver's
      GetConnectionCredentials method. A "LinuxSecurityLabel" key will be
      present in the dictionary returned by the GetConnectionCredentials
      method. The corresponding value will be the AppArmor confinement context
      of the connection.
    - 0001-Document-AppArmor-enforcement-in-the-dbus-daemon-man.patch,
      0002-Add-apparmor-element-and-attributes-to-the-bus-confi.patch,
      0003-Update-autoconf-file-to-build-against-libapparmor.patch,
      0004-Add-apparmor-element-support-to-bus-config-parsing.patch,
      0005-Initialize-AppArmor-mediation.patch,
      0006-Store-AppArmor-label-of-bus-during-initialization.patch,
      0007-Store-AppArmor-label-of-connecting-processes.patch,
      0008-Mediation-of-processes-that-acquire-well-known-names.patch,
      0009-Do-LSM-checks-after-determining-if-the-message-is-a-.patch,
      0010-Mediation-of-processes-sending-and-receiving-message.patch,
      0011-Mediation-of-processes-eavesdropping.patch: Replace the patches
      with the version that were merged upstream. The upstream review process
      revealed a number of bugs and useful cleanups that are addressed in the
      new patches.
      + No longer audit denials of unrequested reply messages (LP: #1362469)
    - aa-get-connection-apparmor-security-context.patch: Update patch to
      include a bug fix, from Simon McVittie, for AppArmor labels that contain
      non UTF-8 characters.
    - 0012-apparmor-tighten-up-terminology-for-context-vs.-labe.patch,
      0013-apparmor-Fix-build-failure-with-disable-apparmor.patch: New patches
      that were merged upstream to clean up the AA mediation code and fix a
      build failure
    - 0012-New-a-sv-helper-for-using-byte-arrays-as-the-variant.patch: Drop
      this patch. It became part of the "LinuxSecurityLabel" patch set and is
      added back with a new file name.
      0013-Add-AppArmor-support-to-GetConnectionCredentials.patch: Drop this
      patch in favor of the "LinuxSecurityLabel" patch set. This means that
      the AppArmorContext and AppArmorMode keys will not be present in the
      dictionary returned by GetConnectionCredentials. Ubuntu shipped this
      patch in 14.10 but, as far as I know, those keys were not used by any
      applications in 14.10. Since this patch was not accepted upstream,
      Ubuntu should drop it and new applications should begin using
      "LinuxSecurityLabel".
 -- Tyler Hicks <email address hidden> Thu, 19 Feb 2015 11:06:14 -0600

Changed in dbus (Ubuntu):
status: Confirmed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers