AppArmor no longer mediates access to path-based AF_UNIX socket files

Bug #1208988 reported by Tyler Hicks
264
This bug affects 2 people
Affects Status Importance Assigned to Milestone
AppArmor
Fix Released
High
John Johansen
apparmor (Ubuntu)
Fix Released
Critical
Tyler Hicks
Saucy
Fix Released
Critical
Tyler Hicks
apparmor-easyprof-ubuntu (Ubuntu)
Fix Released
Critical
Jamie Strandboge
Saucy
Fix Released
Critical
Jamie Strandboge
firefox (Ubuntu)
Fix Released
Medium
Jamie Strandboge
Saucy
Fix Released
Medium
Jamie Strandboge
linux (Ubuntu)
Fix Released
High
John Johansen
linux-grouper (Ubuntu)
Fix Released
High
John Johansen
linux-maguro (Ubuntu)
Fix Released
High
John Johansen
linux-mako (Ubuntu)
Fix Released
High
John Johansen
linux-manta (Ubuntu)
Fix Released
High
John Johansen

Bug Description

[Impact]

 * AppArmor removed unix domain socket mediation as part of the 2.4 (karmic) rewrite to the security_path hooks so that it could be upstreamed into the main kernel. The result being apparmor no longer mediates access to AF_UNIX socket files. Or more specifically it does not mediation connections between sockets, creation of a socket within the filesystem is mediated

 * Confined applications can currently read from and write to any AF_UNIX
   socket files

 * Existing AppArmor profiles that contain file rules granting write access to
   AF_UNIX socket files are effectively being ignored

 * The move from the vfs hooks patches (old, out-of-tree) AppArmor and the security_path hooks
   apparmor incorporated into mainline in 2.6.36 were the cause of this regression.

   apparmor 2.4 (version in karmic) also removed other features are part of the rewrite to
   security_path hooks/upstreaming effort.

 * For Ubuntu, Karmic 9.10 and all newer, releases are affected.
   8.04 LTS used the vfs patches and was not affected.

* Mediation of unix domain filesystem based sockets is needed for 13.10 click apps confinement

[Test Case]

 * Confining dbus-send and sending a message to the system bus is an easy
   manual testing method. Load a profile for dbus-send:

$ cat << EOF | sudo apparmor_parser -r
#include <tunables/global>

/usr/bin/dbus-send {
  #include <abstractions/base>
  /usr/bin/dbus-send r,
# /var/run/dbus/system_bus_socket rw,
}
EOF

 * Note that the system_bus_socket rule is commented out. Now, run dbus-send
   under strace and see if the connect() fails. Here's the unexpected output,
   taken from an Ubuntu Saucy system:

$ strace -e connect -- \
 dbus-send --system --dest=org.freedesktop.DBus \
 /org/freedesktop/DBus org.freedesktop.DBus.ListNames
connect(3, {sa_family=AF_LOCAL, sun_path="/var/run/dbus/system_bus_socket"}, 33) = 0
+++ exited with 0 +++

 * Here's the expected output, taken from an 8.04 LTS system:

$ strace -e connect -- \
 dbus-send --system --dest=org.freedesktop.DBus \
 /org/freedesktop/DBus org.freedesktop.DBus.ListNames
connect(3, {sa_family=AF_FILE, path="/var/run/dbus/system_bus_socket"}, 33) = -1 EACCES (Permission denied)
Failed to open connection to system message bus: Failed to connect to socket /var/run/dbus/system_bus_socket: Permission denied

 * Or, you can apply the AppArmor regression test suite patch attached to this
   bug and run the automated tests:

$ cd tests/regression/apparmor
$ make unix_fd_{server,client} unix_socket_file{,_client} >/dev/null
$ sudo bash unix_fd_server.sh
$ sudo bash unix_socket_file.sh

[Regression Potential]

 * Profiles developed with affected kernels aren't likely to have the necessary
   rules because the proper LSM hook was not implemented in those kernels, so
   the policy writer didn't need to grant access to AF_UNIX socket files

 * The profiles shipped with AppArmor can, and will, be updated to grant access
   to AF_UNIX socket files, but local policy modifications cannot be addressed
   by upstream/distros. Once updated kernels begin enforcing mediation of
   AF_UNIX socket files, rules in local profiles may no longer be sufficient,
   resulting in new AppArmor denials for AF_UNIX socket files.

Revision history for this message
Tyler Hicks (tyhicks) wrote :

Back in the days of AppArmor living out-of-tree, it implemented the inode_permission LSM hook. In the upstreamed version of AppArmor, the inode_permission hook is not implemented. That's why mediation occurred back then but not now.

Back then, nameidata was passed to inode_permission, which allowed AppArmor to get to the dentry and vfsmount. Today, nameidata is gone and only the inode is available in inode_permission, making it difficult to simply reintroduce apparmor_inode_permission().

The unix_stream_connect LSM hook may be viable. Using the sock, we could do something similar to unix_getname() to get a buffer containing the path. However, John says that the path may not be valid for the current namespace.

Revision history for this message
Tyler Hicks (tyhicks) wrote :
Download full text (3.2 KiB)

Output from the patched regression tests:

* Expected output (from Hardy)

$ make unix_fd_{server,client} unix_socket_file{,_client} >/dev/null
$ sudo VERBOSE=1 bash unix_fd_server.sh
ok: fd passing; unconfined client
ok: fd passing; confined client w/ rw
ok: fd passing; confined client w/ w only
ok: fd passing; confined client w/o socket access
$ sudo VERBOSE=1 bash unix_socket_file.sh
ok: socket file (stream); unconfined
ok: socket file (stream); confined server w/ access
ok: socket file (stream); confined server w/o access
ok: socket file (stream); confined server w/ bad access
ok: socket file (stream); confined client w/ access
ok: socket file (stream); confined client w/o access
ok: socket file (stream); confined client w/ bad access
ok: socket file (dgram); unconfined
ok: socket file (dgram); confined server w/ access
ok: socket file (dgram); confined server w/o access
ok: socket file (dgram); confined server w/ bad access
ok: socket file (dgram); confined client w/ access
ok: socket file (dgram); confined client w/o access
ok: socket file (dgram); confined client w/ bad access
ok: socket file (seqpacket); unconfined
ok: socket file (seqpacket); confined server w/ access
ok: socket file (seqpacket); confined server w/o access
ok: socket file (seqpacket); confined server w/ bad access
ok: socket file (seqpacket); confined client w/ access
ok: socket file (seqpacket); confined client w/o access
ok: socket file (seqpacket); confined client w/ bad access

* Unexpected output (from Saucy)

$ make unix_fd_{server,client} unix_socket_file{,_client} >/dev/null
$ sudo VERBOSE=1 bash unix_fd_server.sh
ok: fd passing; unconfined client
ok: fd passing; confined client w/ rw
ok: fd passing; confined client w/ w only
Error: unix_fd_server passed. Test 'fd passing; confined client w/o socket access' was expected to 'fail'
$ sudo VERBOSE=1 bash unix_socket_file.sh
ok: socket file (stream); unconfined
ok: socket file (stream); confined server w/ access
ok: socket file (stream); confined server w/o access
ok: socket file (stream); confined server w/ bad access
ok: socket file (stream); confined client w/ access
Error: unix_socket_file passed. Test 'socket file (stream); confined client w/o access' was expected to 'fail'
Error: unix_socket_file passed. Test 'socket file (stream); confined client w/ bad access' was expected to 'fail'
ok: socket file (dgram); unconfined
ok: socket file (dgram); confined server w/ access
ok: socket file (dgram); confined server w/o access
ok: socket file (dgram); confined server w/ bad access
ok: socket file (dgram); confined client w/ access
Error: unix_socket_file passed. Test 'socket file (dgram); confined client w/o access' was expected to 'fail'
Error: unix_socket_file passed. Test 'socket file (dgram); confined client w/ bad access' was expected to 'fail'
ok: socket file (seqpacket); unconfined
ok: socket file (seqpacket); confined server w/ access
ok: socket file (seqpacket); confined server w/o access
ok: socket file (seqpacket); confined server w/ bad access
ok: socket file (seqpacket); confined client w/ access
Error: unix_socket_file passed. Test 'socket file (seqpacket); confined client w/o access' was expected t...

Read more...

description: updated
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Updated test case to fix minor merge issue with current apparmor trunk and to more accurately test the required file permissions.

AF_UNIX socket creation requires w perms. Calling connect() on an AF_UNIX socket requires rw perms.

Changed in apparmor (Ubuntu Saucy):
importance: Undecided → Critical
status: New → In Progress
Changed in apparmor-easyprof-ubuntu (Ubuntu Saucy):
importance: Undecided → Critical
Changed in apparmor (Ubuntu Saucy):
assignee: nobody → Tyler Hicks (tyhicks)
Changed in apparmor-easyprof-ubuntu (Ubuntu Saucy):
assignee: nobody → Jamie Strandboge (jdstrand)
status: New → Triaged
status: Triaged → In Progress
description: updated
description: updated
Tyler Hicks (tyhicks)
description: updated
description: updated
information type: Private Security → Public Security
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

In addition to the above, we need to:
* add to apparmor's abstractions/private-files-strict
  # don't allow access to any gnome-keyring modules
  audit deny /{,var/}run/user/[0-9]*/keyring** mrwkl,

* add to apparmor's abstractions/p11-kit:
  # gnome-keyring pkcs11 module
  owner /{,var/}run/user/[0-9]*/keyring*/pkcs11 rw,

Adding this will allow telepathy and evince to work with the kernel change.

no longer affects: evince (Ubuntu)
no longer affects: evince (Ubuntu Saucy)
Changed in firefox (Ubuntu Saucy):
status: New → Triaged
importance: Undecided → Medium
tags: added: application-confinement
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "[PATCH] tests: Verify mediation of path-based UNIX domain sockets" 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
Tyler Hicks (tyhicks) wrote :

Updated debdiff containing an audio abstraction change for bug #1211380

Revision history for this message
Brad Figg (brad-figg) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 1208988

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
Andy Whitcroft (apw)
Changed in linux-grouper (Ubuntu):
assignee: nobody → John Johansen (jjohansen)
status: New → Fix Committed
Changed in linux-maguro (Ubuntu):
assignee: nobody → John Johansen (jjohansen)
status: New → Fix Committed
Changed in linux-mako (Ubuntu):
assignee: nobody → John Johansen (jjohansen)
status: New → Fix Committed
Changed in linux-manta (Ubuntu):
assignee: nobody → John Johansen (jjohansen)
status: New → Fix Committed
Changed in linux (Ubuntu):
assignee: nobody → John Johansen (jjohansen)
status: Incomplete → Fix Committed
Andy Whitcroft (apw)
Changed in linux (Ubuntu):
importance: Undecided → High
Changed in linux-grouper (Ubuntu):
importance: Undecided → High
Changed in linux-maguro (Ubuntu):
importance: Undecided → High
Changed in linux-mako (Ubuntu):
importance: Undecided → High
Changed in linux-manta (Ubuntu):
importance: Undecided → High
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apparmor-easyprof-ubuntu - 1.0.35

---------------
apparmor-easyprof-ubuntu (1.0.35) saucy; urgency=low

  * apparmor-easyprof-ubuntu.install: install data/hardware/*, thus allowing
    porters, OEMs, etc to ship their own policy without having to modify this
    package (LP: #1197133)
  * add data/hardware/graphics.d/* and data/hardware/audio.d/*, namespaced to
    this package. We will move these out to lxc-android-config later
  * tests/test-data.py: adjust to test data/hardware/*
  * accounts: move to reserved status until LP: 1230091 is fixed
  * calendar: remove workaround rule for gio DBus path (LP: #1227295)
  * add usermetrics policy group so apps can update the infographic
  * ubuntu-* templates:
    - allow StartServiceByName on the system bus too. This is needed by the
      new usermetrics policy group and we will presumably have more going
      forward (eg location)
    - account for /org/freedesktop/dbus object path. This seems to be used by
      the python DBus bindings (eg, friends)
    - move hardware specific accesses out of the templates into
      hardware/graphics.d/ in preparation of the move to shipping these in
      lxc-android-config (note, this doesn't change apparmor policy in any
      way)
    - add 'r' to dbus system bus socket (LP: #1208988)
    - add ixr access to thumbnailer helper (LP: #1234543)
    - finetune HUD access
    - don't use ibus abstraction but instead use 'r' access for
      owner @{HOME}/.config/ibus/**
    - don't use freedesktop.org abstraction but instead add read accesses
      for /usr/share/icons and various mime files
    - updates for new gstreamer
      - move in gstreamer accesses from audio policy groupd due to hybris
  * ubuntu-sdk template:
    - remove workaround paths now that ubuntu-ui-toolkit is using
      QCoreApplication::applicationName based on MainView's applicationName
      (LP: #1197056, #1197051, #1224126, LP: #1231863)
  * ubuntu-webapp template:
    - allow read access to /usr/share/unity-webapps/userscripts/**
    - allow rix to gst-plugin-scanner
  * add reserved friends policy group (reserved because it needs integration
    with trust-store to be used by untrusted apps)
  * remove peer from receive DBus rules in the ubuntu-* templates and the
    contacts, history, and location policy groups (LP: #1233895)
  * audio:
    - move gstreamer stuff out to templates since hybris pulls it in for all
      apps
    - include hardware/audio.d for hardware specific accesses
 -- Jamie Strandboge <email address hidden> Mon, 07 Oct 2013 13:18:27 -0500

Changed in apparmor-easyprof-ubuntu (Ubuntu Saucy):
status: In Progress → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

apparmor-easyprof-ubuntu needs to allow /dev/socket/property_service and use attach_disconnected on touch images.

Changed in apparmor-easyprof-ubuntu (Ubuntu Saucy):
status: Fix Released → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apparmor-easyprof-ubuntu - 1.0.36

---------------
apparmor-easyprof-ubuntu (1.0.36) saucy; urgency=low

  * ubuntu-* templates:
    - due to AF_UNIX use attach_disconnected and allow rw on
      /dev/socket/property_service (LP: #1208988)
    - add temporary workaround to use /tmp/mir_socket (LP: 1236912)
 -- Jamie Strandboge <email address hidden> Tue, 08 Oct 2013 13:11:46 -0500

Changed in apparmor-easyprof-ubuntu (Ubuntu Saucy):
status: In Progress → Fix Released
Changed in apparmor (Ubuntu Saucy):
status: In Progress → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package apparmor - 2.8.0-0ubuntu30

---------------
apparmor (2.8.0-0ubuntu30) saucy; urgency=low

  [ Tyler Hicks ]
  * debian/patches/0059-dbus-rules-for-dbus-abstractions.patch: Add an
    abstraction for the accessibility bus. It is currently very permissive,
    like the dbus and dbus-session abstractions, and grants all permissions on
    the accessibility bus. (LP: #1226141)
  * debian/patches/0071-lp1226356.patch: Fix issues in parsing D-Bus and mount
    rules. Both rule classes suffered from unexpected auditing behavior when
    using the 'deny' and 'audit deny' rule modifiers. The 'deny' modifier
    resulting in accesses being audited and the 'audit deny' modifier
    resulting in accesses not being audited. (LP: #1226356)
  * debian/patches/0072-lp1229393.patch: Fix cache location for .features
    file, which was not being written to the proper location if the parameter
    --cache-loc= is passed to apparmor_parser. This bug resulted in using the
    .features file from /etc/apparmor.d/cache or always recompiling policy.
    Patch thanks to John Johansen. (LP: #1229393)
  * debian/patches/0073-lp1208988.patch: Update AppArmor file rules of UNIX
    domain sockets to include read and write permissions. Both permissions are
    required when a process connects to a UNIX domain socket. Also include new
    tests for mediation of UNIX domain sockets. Thanks to Jamie Strandboge for
    helping with the policy updates and testing. (LP: #1208988)
  * debian/patches/0075-lp1211380.patch: Adjust the audio abstraction to only
    grant access to specific pulseaudio files in the pulse runtime directory
    to remove access to potentially dangerous files (LP: #1211380)

  [ Jamie Strandboge ]
  * debian/patches/0074-lp1228882.patch: typo in ubuntu-browsers.d/multimedia
    (LP: #1228882)
  * 0076_sanitized_helper_dbus_access.patch: allow applications run under
    sanitized_helper to connect to DBus
 -- Tyler Hicks <email address hidden> Fri, 04 Oct 2013 17:29:52 -0700

Changed in apparmor (Ubuntu Saucy):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 3.11.0-12.18

---------------
linux (3.11.0-12.18) saucy; urgency=low

  [ Andy Whitcroft ]

  * [Packing] tools -- when tools are off they are off
  * [config] tools -- linux-tools-common really is common
  * [Packaging] tools -- make cpupower optional
  * [Packaging] tools -- fix crosscompilation
  * [config] tools -- enable cpupower
  * SAUCE: storvsc -- host takes MAINTENANCE_IN commands badly elide them
    - LP: #1234417

  [ John Johansen ]

  * SAUCE: apparmor: fix unix domain sockets to be mediated on connection
    - LP: #1208988
  * SAUCE: apparmor: allocate path lookup buffers during init
    - LP: #1208988
  * SAUCE: apparmor: fix memleak of the profile hash
    - LP: #1235523
  * SAUCE: apparmor: fix memleak of replacedby struct
    - LP: #1235973
  * SAUCE: apparmor: fix bad lock balance when introspecting policy
    - LP: #1235977

  [ Paolo Pisati ]

  * [Config] arm: VIRTIO_[BLK|NET|MMIO]=y

  [ Rob Herring ]

  * SAUCE: (no-up) net: calxedaxgmac: fix clearing of old filter addresses
    - LP: #1235272
  * SAUCE: (no-up) net: calxedaxgmac: add uc and mc filter addresses in
    promiscuous mode
    - LP: #1235272
  * SAUCE: (no-up) net: calxedaxgmac: determine number of address filters
    at runtime
    - LP: #1235272

  [ Tim Gardner ]

  * [Config] CONFIG_ANDROID=n
    - LP: #1235161
  * [Config] CONFIG_L2TP_V3=y
    - LP: #1235914
  * Release tracker
    - LP: #1236999

  [ Upstream Kernel Changes ]

  * Revert "HID: core: fix reporting of raw events"
    - LP: #1218004
 -- Andy Whitcroft <email address hidden> Fri, 04 Oct 2013 13:08:59 +0100

Changed in linux (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux-grouper - 3.1.10-6.25

---------------
linux-grouper (3.1.10-6.25) saucy; urgency=low

  [ John Johansen ]

  * SAUCE: apparmor: fix unix domain sockets to be mediated on connection
    - LP: #1208988
  * SAUCE: apparmor: allocate path lookup buffers during init
    - LP: #1208988
  * SAUCE: apparmor: fix memleak of the profile hash
    - LP: #1235523
  * SAUCE: apparmor: fix memleak of replacedby struct
    - LP: #1235973
  * SAUCE: apparmor: fix bad lock balance when introspecting policy
    - LP: #1235977
 -- Andy Whitcroft <email address hidden> Mon, 07 Oct 2013 16:50:39 +0100

Changed in linux-grouper (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux-maguro - 3.0.0-3.18

---------------
linux-maguro (3.0.0-3.18) saucy; urgency=low

  [ John Johansen ]

  * SAUCE: apparmor: fix unix domain sockets to be mediated on connection
    - LP: #1208988
  * SAUCE: apparmor: allocate path lookup buffers during init
    - LP: #1208988
  * SAUCE: apparmor: fix memleak of the profile hash
    - LP: #1235523
  * SAUCE: apparmor: fix memleak of replacedby struct
    - LP: #1235973
  * SAUCE: apparmor: fix bad lock balance when introspecting policy
    - LP: #1235977
 -- Andy Whitcroft <email address hidden> Mon, 07 Oct 2013 17:16:14 +0100

Changed in linux-maguro (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux-mako - 3.4.0-3.21

---------------
linux-mako (3.4.0-3.21) saucy; urgency=low

  [ John Johansen ]

  * SAUCE: apparmor: fix unix domain sockets to be mediated on connection
    - LP: #1208988
  * SAUCE: apparmor: allocate path lookup buffers during init
    - LP: #1208988
  * SAUCE: apparmor: fix memleak of the profile hash
    - LP: #1235523
  * SAUCE: apparmor: fix memleak of replacedby struct
    - LP: #1235973
  * SAUCE: apparmor: fix bad lock balance when introspecting policy
    - LP: #1235977

  [ Scott James Remnant ]

  * SAUCE: (no-up) trace: add trace events for open(), exec() and uselib()
    - LP: #1194127
 -- Andy Whitcroft <email address hidden> Mon, 07 Oct 2013 18:17:50 +0100

Changed in linux-mako (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux-manta - 3.4.0-4.19

---------------
linux-manta (3.4.0-4.19) saucy; urgency=low

  [ John Johansen ]

  * SAUCE: apparmor: fix unix domain sockets to be mediated on connection
    - LP: #1208988
  * SAUCE: apparmor: allocate path lookup buffers during init
    - LP: #1208988
  * SAUCE: apparmor: fix memleak of the profile hash
    - LP: #1235523
  * SAUCE: apparmor: fix memleak of replacedby struct
    - LP: #1235973
  * SAUCE: apparmor: fix bad lock balance when introspecting policy
    - LP: #1235977

  [ Scott James Remnant ]

  * SAUCE: (no-up) trace: add trace events for open(), exec() and uselib()
    - LP: #1194127
 -- Andy Whitcroft <email address hidden> Mon, 07 Oct 2013 18:23:03 +0100

Changed in linux-manta (Ubuntu):
status: Fix Committed → Fix Released
Changed in firefox (Ubuntu Saucy):
milestone: none → saucy-updates
status: Triaged → Fix Committed
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This was fixed in at least 25.0.1+build1-0ubuntu0.13.10.1

Changed in firefox (Ubuntu Saucy):
status: Fix Committed → Fix Released
Changed in apparmor:
status: Triaged → Fix Committed
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Trusty firefox has the fix as well.

Changed in firefox (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Marking the apparmor task as 'fixed' since this is available in the upstream beta tarballs.

Changed in apparmor:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.