pam_apparmor fails to hunt through the hats

Bug #619521 reported by John Gray on 2010-08-17
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Medium
Unassigned
2.5
Medium
Unassigned
apparmor (Ubuntu)
Medium
Unassigned
Lucid
Medium
Unassigned

Bug Description

SRU Justification

1. impact of the bug is medium for stable releases and very much limits the utility of pam_apparmor, but the fix is non-intrusive. It is included here as part of the 2.5.1 update for Lucid (LP: #660077)

2. This has been fixed in natty.

3. Patch simply adjusts changehat/pam_apparmor/pam_apparmor.c to try the next hat on ENOENT rather than failing.

4. TEST CASE: run the AppArmorPAM tests in lp:qa-regression-testing/scripts/test-apparmor.py. Several tests fail with the version in Lucid and all are fixed in the 2.5.1 upload.

5. The regression potential is very low for this patch as it only adds a single ENOENT check, libpam-apparmor is in universe and it is not widely used yet. Getting this fixed would be an important step in getting pam-apparmor more widely used since LTS users are more likely to require the extra security features provided by libpam-apparmor.

Binary package hint: apparmor

I have pam_apparmor set up for sshd as follows.

session optional pam_apparmor.so order=user,group,default debug

It never searches group or default. It thinks it finds a hat the user whether a hat exists for the user or not.

In complain mode, the debug messages are:

Aug 17 16:21:03 zeno sshd[22113]: pam_apparmor(sshd:session): Using username 'gray'
Aug 17 16:21:03 zeno sshd[22113]: pam_apparmor(sshd:session): Successfully changed to hat 'gray'

Note, there is not a hat 'gray' defined. If I put it in enforce mode:

Aug 17 17:02:36 zeno sshd[3955]: pam_apparmor(sshd:session): Using username 'gray'
Aug 17 17:02:36 zeno sshd[3955]: pam_apparmor(sshd:session): Unknown error occurred changing to gray hat: No such file or directory

Maybe we're doing something wrong, but I think its broken.

ProblemType: Bug
DistroRelease: Ubuntu 10.04
Package: libpam-apparmor 2.5-0ubuntu3
ProcVersionSignature: Ubuntu 2.6.32-21.32-generic-pae 2.6.32.11+drm33.2
Uname: Linux 2.6.32-21-generic-pae i686
Architecture: i386
Date: Tue Aug 17 18:30:58 2010
InstallationMedia: Ubuntu-Server 10.04 LTS "Lucid Lynx" - Release i386 (20100427)
ProcEnviron:
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: apparmor

Related branches

John Gray (gray-agora-net) wrote :
Steve Beattie (sbeattie) wrote :

I have reproduced this issue on lucid; it's due to the aa_change_hat(2) call returning -ENOENT when the passed hat doesn't exist, rather than -EPERM or -EACCES that pam_apparmor expects for this situation; instead it treats it as unknown kernel error and aborts. According to John Johanson, this has been the behavior of the kernel portion of apparmor going back to jaunty (and continuing forward in maverick).

It seems to me the right approach to fix this is to teach pam_apparmor the new(er) behavior of the kernel, which ought to be a relatively simple change:

=== modified file 'changehat/pam_apparmor/pam_apparmor.c'
--- changehat/pam_apparmor/pam_apparmor.c 2007-03-13 16:52:28 +0000
+++ changehat/pam_apparmor/pam_apparmor.c 2010-08-19 09:00:23 +0000
@@ -167,6 +167,7 @@
                        break;
                case EPERM: /* Disable when ECHILD patch gets accepted */
                case EACCES:
+ case ENOENT:
                        /* failed to change into attempted hat, so we'll
                         * jump back out and try the next one */
                        break;

though for apparmor trunk the EPERM case ought to go away.

I note that this is not an issue for the apache2 mod_apparmor, as it continues trying hats in the face of any error returned by the kernel.

Changed in apparmor (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Changed in apparmor:
status: New → Triaged
importance: Undecided → Medium
tags: added: jaunty karmic maverick
Steve Beattie (sbeattie) wrote :

I've tested locally the change and sent the proposed patch to the upstream list.

John Gray: for testing the fix on lucid, I've uploaded a version of apparmor with the fix to my personal ppa at https://launchpad.net/~sbeattie/+archive/ppa .

Changed in apparmor:
status: Triaged → In Progress
Steve Beattie (sbeattie) wrote :

Patch was ACKed and applied upstream.

Changed in apparmor:
status: In Progress → Fix Released
John Gray (gray-agora-net) wrote :

Thank you, thank you, thank you.

It works for me. Any idea when this might make it into an actual release?

Changed in apparmor (Ubuntu):
status: Triaged → Fix Released
Steve Beattie (sbeattie) wrote :

AppArmor 2.5.1 has been released: https://launchpad.net/apparmor/2.5/2.5.1

Changed in apparmor (Ubuntu Lucid):
importance: Undecided → Medium
milestone: none → lucid-updates
status: New → In Progress
description: updated

Accepted apparmor into lucid-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in apparmor (Ubuntu Lucid):
status: In Progress → Fix Committed
tags: added: verification-needed
Jamie Strandboge (jdstrand) wrote :

Upgraded to 2.5.1-0ubuntu0.10.04.1 in lucid-proposed and this issue is resolved.

Martin Pitt (pitti) on 2010-12-14
tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :
Download full text (10.1 KiB)

This bug was fixed in the package apparmor - 2.5.1-0ubuntu0.10.04.1

---------------
apparmor (2.5.1-0ubuntu0.10.04.1) lucid-proposed; urgency=low

  * Backport 2.5.1-0ubuntu0.10.10.1 from maverick for userspace tools to work
    with newer kernels (LP: #660077)
    NOTE: user-tmp now uses 'owner' match, so non-default profiles will have
    to be adjusted when 2 separately confined applications that both use the
    user-tmp abstraction depend on being able to cooperatively share files
    with each other in /tmp or /var/tmp.
  * remove the following patches (features not appropriate for SRU):
    - 0002-add-chromium-browser.patch
    - 0003-local-includes.patch
    - 0004-ubuntu-abstractions-updates.patch
  * debian/rules (this makes it the same as what was shipped in 10.04 LTS
    release):
    - don't ship aa-update-browser and its man page (requires
      0004-ubuntu-abstractions-updates.patch)
    - don't ship apparmor.d/local/ (requires 0003-local-includes.patch)
    - don't use dh_apparmor (not in Ubuntu 10.04 LTS)
    - don't ship chromium profile
  * remove debian/profiles/chromium-browser
  * remove debian/aa-update-browser*
  * debian/apparmor-profiles.postinst: revert to that in lucid release
    (requires dh_apparmor and 0002-add-chromium-browser.patch)
  * remove debian/apparmor-profiles.postrm: doesn't make sense without
    0002-add-chromium-browser.patch
  * debian/control:
    - revert Build-Depends on debhelper (>= 5)
    - revert Standards-Version to 3.8.4
    - revert Vcs-Bzr
    - use Conflicts/Replaces version that was in Ubuntu 10.04 LTS
  * debian/patches/0011-lucid-compat-dbus.patch: move /var/lib/dbus/machine-id
    back into dbus, since profiles on 10.04 LTS expect it there
  * debian/patches/0012-lucid-compat-kde.patch: add kde4-config to kde
    abstraction, since the firefox profile on Ubuntu 10.04 LTS expects it to
    be there

apparmor (2.5.1-0ubuntu0.10.10.2) maverick-proposed; urgency=low

  * New upstream release (LP: #660077)
    - The following patches were refreshed:
      + 0001-fix-release.patch
      + 0003-local-includes.patch
      + 0004-ubuntu-abstractions-updates.patch
      + 0008-lp648900.patch: renamed as 0005-lp648900.patch
    - The following patches were dropped (included upstream):
      + 0005-lp601583.patch
      + 0006-network-interface-enumeration.patch
      + 0007-gnome-updates.patch
  * debian/patches/0006-testsuite-fixes.patch: testsuite fixes from head
    of 2.5 branch. These are needed for QRT and SRU testing (LP: #652211)
  * debian/patches/0007-honor-cflags.patch: have the parser makefile honor
    CFLAGS environment variable. Brings back missing symbols for the retracer
  * debian/patches/0008-lp652674.patch: fix warnings for messages without
    denied or requested masks (LP: #652674)
  * debian/apparmor.init: fix path to aa-status (LP: #654841)
  * debian/apport/source_apparmor.py: apport hook should use
    root_command_hook() for running apparmor_status (LP: #655529)
  * debian/apport/source_apparmor.py: use ProcKernelCmdline and don't clobber
    cmdline details (LP: #657091)
  * debian/{rules,control}: move apache2 abstractions into the base package
    so we can put ...

Changed in apparmor (Ubuntu Lucid):
status: Fix Committed → Fix Released
tags: added: testcase
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers