change_profile rules need a modifier to allow non-secureexec transitions

Bug #1584069 reported by Tyler Hicks
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Fix Released
High
Tyler Hicks
apparmor (Ubuntu)
Fix Released
High
Tyler Hicks
Xenial
Fix Released
High
Tyler Hicks

Bug Description

[Impact]

Applications which use libapparmor's aa_change_onexec() to set up an AppArmor profile transition across an upcoming exec() cannot pre-initialize the environment. This is caused by AppArmor unconditionally setting the AT_SECURE flag on the process, causing libc to scrub the environment upon exec().

Upstream AppArmor and Yakkety now support policy language that allows the policy author to specify that the environment should not be scrubbed but the changes need to be SRU'ed to Ubuntu 16.04.

[Test Case]

The upstream changes include exhaustive tests for the new policy language keywords. Some of them are run at build time (the apparmor_parser tests) and all of them are run by QRT's test-apparmor.py (the apparmor_parser tests, the Python utility tests, and the kernel regression tests).

If a manual test is desired, see the original report below for steps.

[Regression Potential]

Regression potential is considerable since the fixes add new keywords to the policy language. No kernel changes are required, which mitigates some of the risk. Additionally, as mentioned above, the upstream changes include many new tests to ensure that regressions are not introduced.

[Original Report]

As it stands today, all exec transitions triggered by a change_profile rule cause the AT_SECURE flag in the auxiliary vector to be set due to the kernel function apparmor_bprm_secureexec() returning 1 while setting up the execution environment. This causes libc to always scrub the environment variables during such an exec transition.

There should be a way to indicate, in the policy language, that AT_SECURE should not be triggered. This would be equivalent to the file rule type having the Px permission to trigger AT_SECURE and the px permission to not trigger it. The file rule type even has an 'unsafe' modifier keyword that could be reused as the change_profile modifier keyword.

Steps to show that AT_SECURE is being set:

# Build a test program to dump the AT_SECURE flag
$ cat <<EOF > print_at_secure.c
#include <stdio.h>
#include <sys/auxv.h>

int main(void)
{
 printf("AT_SECURE = %lu\n", getauxval(AT_SECURE));
 return 0;
}
EOF
$ gcc -o print_at_secure print_at_secure.c

# Load the test profile that allows all file accesses and any change_profile operations
$ echo "profile test { file, change_profile, }" | sudo apparmor_parser -qr

# Run bash under the test profile
$ aa-exec -p test -- bash

# Show the AT_SECURE is not set on exec
$ ./print_at_secure
AT_SECURE = 0

# Set up an exec transition (change_profile from the test profile back to the test profile)
$ echo "exec test" > /proc/self/attr/exec

# See that AT_SECURE is now set on exec
$ ./print_at_secure
AT_SECURE = 1

$ exit

Steps to show that the new change_profile syntax allows AT_SECURE to remain
unset:

$ echo "profile test { file, change_profile unsafe /** -> *, }" | \
  sudo apparmor_parser -qr
$ aa-exec -p test -- bash
$ ./print_at_secure
AT_SECURE = 0
$ echo "exec test" > /proc/self/attr/exec
$ ./print_at_secure
AT_SECURE = 0
$ exit

Tyler Hicks (tyhicks)
Changed in apparmor:
assignee: nobody → Tyler Hicks (tyhicks)
importance: Undecided → High
status: New → In Progress
Changed in apparmor (Ubuntu):
status: In Progress → Triaged
Tyler Hicks (tyhicks)
tags: added: aa-parser
Tyler Hicks (tyhicks)
description: updated
Revision history for this message
Christian Boltz (cboltz) wrote :

The tools also need to be updated (as soon as we decided on the exact syntax etc.)

tags: added: aa-tools
Revision history for this message
Tyler Hicks (tyhicks) wrote :
Revision history for this message
Tyler Hicks (tyhicks) wrote :

Fix committed as r3469

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

This bug was fixed in the package apparmor - 2.10.95-4ubuntu2

---------------
apparmor (2.10.95-4ubuntu2) yakkety; urgency=medium

  * Drop the following change now that click-apparmor has been updated:
    - Continue installing aa-exec into /usr/sbin/ for now since
      click-apparmor's aa-exec-click autopkgtest expects it to be there
  * debian/patches/allow-stacking-tests-to-use-system.patch,
    debian/patches/r3430-allow-stacking-tests-to-use-system.patch: Replace
    patch with the final version that landed upstream and annotate the patch
    headers accordingly
  * debian/patches/r3460-ignore-file-events-with-send-or-receive-request.patch:
    Prevent an aa-logprof crash by ignoring file events that contains
    send or receive in the request mask. (LP: #1577051, LP: #1582374)
  * debian/patches/r3463-r3475-change-profile-exec-modes.patch: Allow policy
    authors to specify if the environment should scrubbed during exec
    transitions allowed by a change_profile rule. (LP: #1584069)
  * debian/patches/r3478-make-overlapping-safe-and-unsafe-rules-conflict.patch:
    Make sure that multiple change_profile rules with overlapping safe and
    unsafe exec modes conflict when they share the same exec conditional
    (LP: #1588069)
  * debian/patches/r3479-create-fcitx-abstractions.patch: Include fcitx and
    fcitx-strict abstractions that fcitx client profiles can reuse.
  * debian/control: Do a conffile move of /etc/apparmor.d/abstractions/fcitx
    from the fcitx-data to apparmor by setting up the correct Breaks and
    Replaces.
  * debian/patches/r3480-create-mozc-abstraction.patch: Include a mozc
    abstraction that mozc client profiles can reuse.
  * debian/patches/r3488-r3489-fix-racy-onexec-test.patch: Fix racy regression
    test so that the kernel SRU process is not interrupted by the onexec.sh
    periodically failing
  * debian/patches/r3490-utils-handle-change-profile-exec-modes.patch: Update
    the Python utilities to handle the new exec mode keywords in
    change_profile rules. (LP: #1584069)
  * debian/patches/r3492-allow-dbus-user-session-path.patch: Allow read/write
    access to the dbus-user-session socket file. (LP: #1604872)

 -- Tyler Hicks <email address hidden> Tue, 26 Jul 2016 23:03:05 -0500

Changed in apparmor (Ubuntu):
status: In Progress → Fix Released
Tyler Hicks (tyhicks)
Changed in apparmor (Ubuntu Xenial):
assignee: nobody → Tyler Hicks (tyhicks)
importance: Undecided → High
status: New → In Progress
Tyler Hicks (tyhicks)
description: updated
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Hello Tyler, or anyone else affected,

Accepted apparmor into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/apparmor/2.10.95-0ubuntu2.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in apparmor (Ubuntu Xenial):
status: In Progress → Fix Committed
tags: added: verification-needed
Tyler Hicks (tyhicks)
description: updated
description: updated
Revision history for this message
Tyler Hicks (tyhicks) wrote :

I've thoroughly tested apparmor 2.10.95-0ubuntu2.2 in xenial-proposed. I've verified that this bug is fixed (via the test in the Original Report section of the description and the newly added upstream automated tests) and I've also went through the AppArmor Test Plan (excluding the Ubuntu Touch specific tests):

  https://wiki.ubuntu.com/Process/Merges/TestPlans/AppArmor

The Test Plan includes running the entire set of upstream AppArmor tests, a number of integration tests, libvirt tests, LXC/LXD tests, docker.io tests, Snappy confinement tests, etc.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Tyler Hicks (tyhicks) wrote :

This bug was fixed in Ubuntu 16.04 with apparmor 2.10.95-0ubuntu2.2

Changed in apparmor (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Christian Boltz (cboltz) wrote :

Fixed in AppArmor 2.11.

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

Other bug subscribers

Remote bug watches

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