CVE-2017-6507: apparmor service restarts and package upgrades unload privately managed profiles

Subscribed the LXC security team, mostly for awareness if we get more such reports from users.

The LXD profiles are still available on disk in /var/lib/lxd/security/apparmor but I'm not sure we'll be able to easily fix all those processes that now got relabeled...

It may be simpler for the advisory to recommend all LXD users restart all their containers which will get LXD to regenerate and load the apparmor profile.

Something like "sudo lxd shutdown && sudo lxd activateifneeded" should do the trick, though note that having a maintainer script do so isn't a good idea as it'd be equivalent to having a maintainer script issuing random "reboot" commands to remote hosts.

The way that I plan to address this is by removing the calls to unload_obsolete_profiles() in the apparmor postinst and init script/upstart job/systemd unit.

I've prepared untested security updates for all releases and will test them early next week.

This is CVE-2017-6507

This patch is the actual CVE fix. It simply removes the functionality. A followup patch will be posted that adds a small utility that contains the old functionality for administrators that depend on it.

This patch adds the helper utility.

aa-remove-unknown:
...
+ else
+ echo "Removing '${profile}'"
+ echo -n "$profile" > "${REMOVE}"
+ fi
+ done
+
+# will not catch all errors, but still better than nothing
+exit $?

I slightly disagree with the "better than nothing" part ;-)

If I didn't miss something, the only thing that could go wrong is
+ echo -n "$profile" > "${REMOVE}"

It would be much better to use something like
+ echo -n "$profile" > "${REMOVE}" || failure=1
...
exit $failure

(obviously, also initialize failure=0 before the loop)

On 03/12/2017 09:22 AM, Christian Boltz wrote:
> aa-remove-unknown:
> ...
> + else
> + echo "Removing '${profile}'"
> + echo -n "$profile" > "${REMOVE}"
> + fi
> + done
> +
> +# will not catch all errors, but still better than nothing
> +exit $?
>
> I slightly disagree with the "better than nothing" part ;-)
>
> If I didn't miss something, the only thing that could go wrong is
> + echo -n "$profile" > "${REMOVE}"

There's much more that could go wrong. To name a few:

1) Things could go wrong in the profiles_names_list() function
2) Things could go wrong while executing the awk and/or sort binaries
3) Things could go wrong in the awk script itself

Since all of that goop is piped together, none of those errors would be
caught. IMO, It is some pretty bad code that needs to be improved
outside of this security bug.

The reason I don't feel like it needs to be fixed at the same time as
this security bug is because it should fail safely. In other words,
failing would mean that processes are left confined rather than unconfined.

> It would be much better to use something like
> + echo -n "$profile" > "${REMOVE}" || failure=1
> ...
> exit $failure
>
> (obviously, also initialize failure=0 before the loop)

I agree that it would be better. I'll include an updated patch. Thanks
for the review!

> Since all of that goop is piped together,

... you should "set -o pipefail" ;-)

On 03/14/2017 01:14 PM, Christian Boltz wrote:
>> Since all of that goop is piped together,
>
> ... you should "set -o pipefail" ;-)

Not possible with POSIX shell. We can save all of these enhancements for
after these CVE fixes land and before the next upstream release.

I'd argue that requiring /bin/bash shouldn't be a real problem, but I'm aware that you try to use /bin/sh whenever possible.

Completely different question: does this bug only affect Ubuntu, or is a similar fix needed in rc.apparmor.functions? (__apparmor_restart() has code to unload unknown profiles)

On 03/15/2017 01:26 PM, Christian Boltz wrote:
> I'd argue that requiring /bin/bash shouldn't be a real problem, but I'm
> aware that you try to use /bin/sh whenever possible.

It shouldn't but I don't feel comfortable switching between the two
shells when this is supposed to be a minimal change.

> Completely different question: does this bug only affect Ubuntu, or is a
> similar fix needed in rc.apparmor.functions? (__apparmor_restart() has
> code to unload unknown profiles)

Upstream is affected. The only patches that I've attached so far are for
the upstream project.

The Ubuntu/Debian-specific init code is also affected and I'm working on
patches for that now.

Can you tell me if openSUSE is affected by this bug? A quick test is to do

$ echo "profile test {}" | sudo apparmor_parser -qr
$ sudo grep "test (enforce)" /sys/kernel/security/apparmor/profiles
test (enforce)
$ # do whateveris required to restart the apparmor init script/upstart
$ # job/systemd unit that your distro uses
$ sudo grep "test (enforce)" /sys/kernel/security/apparmor/profiles
test (enforce)

If the test profile is still loaded, you're not affected. If it was
unloaded, you're affected.

Testing confirmed what I expected from reading rc.apparmor.functions __apparmor_restart() - the test profile gets unloaded on "rcapparmor reload".

I'm making this bug public since this behavior is not attacker-controlled. It is controlled by the system administrator and the distro package maintainer(s) for AppArmor.

The fixes have been committed to the master branch:

http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3647
http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3648

This bug was fixed in the package apparmor - 2.11.0-2ubuntu3

---------------
apparmor (2.11.0-2ubuntu3) zesty; urgency=medium

  * SECURITY UPDATE: Don't unload unknown profiles during package
    configuration or when restarting the apparmor init script, upstart job, or
    systemd unit as this could leave processes unconfined (LP: #1668892)
    - debian/apparmor.postinst, debian/apparmor.init, debian/apparmor.upstart:
      Remove calls to unload_obsolete_profiles()
    - debian/patches/utils-add-aa-remove-unknown.patch,
      debian/apparmor.install debian/apparmor.manpages: Include a new utility,
      aa-remove-unknown, which can be used to unload unknown profiles. Based
      on an upstream patch but adjusted to source the /lib/apparmor/functions
      shipped in Debian/Ubuntu.
    - CVE-2017-6507
  * debian/patches/r3645-profiles-update-nvidia-abstraction.patch: Update
    nvidia abstraction for newer nvidia drivers (LP: #1590561)

 -- Tyler Hicks <email address hidden> Fri, 24 Mar 2017 05:26:28 +0000