Bug #1362469 “AppArmor unrequested reply protection generates un...” : Bugs : dbus package : Ubuntu

Revision history for this message

Jamie Strandboge (jdstrand) wrote on 2014-08-28:

#1

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

Revision history for this message

Jamie Strandboge (jdstrand) wrote on 2014-08-28:

#2

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

Jamie Strandboge (jdstrand) on 2014-09-18

tags:

added: application-confinement

Revision history for this message

Ricardo Mendoza (ricmm) wrote on 2014-11-11:

#3

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.

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2014-11-13:

#4

@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

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2015-02-06:

#5

lp1362469.tar.gz Edit (2.4 KiB, application/x-tar)

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

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 the message and generates a denial message.

Outside of the noisy denial message, there is no harm in this. The
original sender of the message stated that a reply was not needed and
the gdbus bindings went ahead and sent an error reply message, which is
fine according to the D-Bus spec. The spec even warns that complex
security policy may result in a denial message for such a reply.

We could patch the gdbus bindings to not send an error reply message in
this situation but I'm not sure upstream would accept such a patch and
there may be other bindings or services that ignore the
NO_REPLY_EXPECTED flag.

In the lp1362469.tar.gz attachment, I have two example services (one
written against the python-dbus bindings and the other against the gdbus
bindgs) that demonstrate that these unrequested reply denials can come
from a service, too. If you gunzip it, untar it, and run 'make', it'll
test both services. It will result in two denials:

apparmor="DENIED" operation="dbus_method_return"  bus="session"
  mask="send" name=":1.154" pid=21732 profile="service" peer_pid=21734
  peer_profile="unconfined"
apparmor="DENIED" operation="dbus_error"  bus="session"
  error_name="org.freedesktop.DBus.Error.UnknownMethod" mask="send"
  name=":1.154" pid=21732 profile="service" peer_pid=21734
  peer_profile="unconfined"

The second denial is the same sort of denial as the pasaffe denial that
I described above. The first denial is an example of a service receiving
a message with the NO_REPLY_EXPECTED flag set and still returning a
reply message. This also results in an unrequested reply denial.

I think this shows that it is unreasonable to try to fix every D-Bus
binding, service, and client that ignores the NO_REPLY_EXPECTED flag in
order to allow or quiet these types of denials.

Our policy language doesn't allow us to easily allow or quiet the
denials, either, since we cannot specify message type (method_call,
signal, method_return, error) and the language has no notion of
unrequested replies. This leaves us with course-grained options such as
"dbus (send) peer=(label=unconfined)," to allow the two denials above. I
don't even know a good way to quiet those denials without disallowing
the ability to send any outgoing messages.

Adjusting the policy language to allow the author to specify
unrequested replies or method_return/error messages in the policy
doesn't feel like the right thing to do. That clutters the already
complex dbus syntax, requires changes from dbus-daemon to the kernel,
and doesn't really add much other than being able to quiet denials
caused by unrequested replies. Additionally, all policy authors are
going to want to do the same thing: deny, without auditing, any
unrequested replies.

I think the most reasonable solution is to simply not audit the denials
for unrequested replies. AppArmor will always block all unrequested
replies. There are potentially lots of innocent unrequested replies that
are allowed by the D-Bus spec they also have the potential to really
litter up the audit logs. The AppArmor mediation code already knows
if the message is a reply type and if the reply was requested. It is
trivial to deny but not audit those messages.

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2015-02-06:

#6

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

Revision history for this message

John Johansen (jjohansen) wrote on 2015-02-06:

#7

thanks for finding this Tyler.

I concur with recommendation for how this should be fixed.

Revision history for this message

Seth Arnold (seth-arnold) wrote on 2015-02-06:

#8

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

Revision history for this message

Tyler Hicks (tyhicks) wrote on 2015-02-19:

#9

dbus_1.8.12-1ubuntu2.debdiff Edit (130.3 KiB, text/plain)

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

Revision history for this message

Launchpad Janitor (janitor) wrote on 2015-02-27:

#10

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

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 <tyhicks@canonical.com>   Thu, 19 Feb 2015 11:06:14 -0600

Changed in dbus (Ubuntu):
status:	Confirmed → Fix Released

Ubuntu
dbus package

AppArmor unrequested reply protection generates unallowable denials

Bug Description

Other bug subscribers

Patches

Bug attachments

Remote bug watches

Ubuntudbus package

AppArmor unrequested reply protection generates unallowable denials

Bug Description

Other bug subscribers

Patches

Bug attachments

Remote bug watches

Ubuntu
dbus package