please provide upstart job for apparmor

[Jamie Strandboge]

AppArmor has a complicated multi-stage policy load process that has evolved over time. It consists of:
- /etc/init/network-interface-security.conf to load the policy for dhclient
- /etc/init/click-apparmor.conf to conditionally regenerate click policy then load it into the kernel
- apparmor integration into upstart jobs
- an rcS sysv init script

In addition to being complicated, there are a several problems:
- if a login session occurs before rcS is run, applications may start and run unconfined
- if apparmor-profiles is installed, then daemons with profiles defined may start and run unconfined
- an administer adding apparmor policy for daemons must also adjust the upstart job for the daemon

Historically we did not use an upstart job because it would block boot and affect boot performance. Blocking boot on policy load is actually a feature because it ensures that the policy is in place before anything is started. The boot performance issue was solved long ago when we introduced binary cached profiles. In today's upstart world, rcS is intended to run prior login anyway, so converted the initscript to an upstart job should not affect boot performance. There have also been bugs in the multi-stage policy load that allowed policy load to happen too late with applications starting before policy load.

The security and foundations teams feel there is a better way and that we can achieve everything with a single upstart task (see attached). In essence, the task does 'start on mounted MOUNTPOINT="/"'. Because it is a task, it will block until it completes. The script will do the various checks to make sure apparmor should load policy, conditionally regenerate click policy then load it into the kernel and load all system policy.

If done correctly, this should allow us to remove the network-interface-security.conf job, the click-apparmor.conf job and the rcS initscript and will solve the issues with login sessions starting too soon, apparmor-profiles daemon policy and admin policy. Attached is lightly tested job file to achieve this (it needs a lot of testing-- see the description in the job file). To test:
1. save the job as /etc/init/apparmor.conf
2. disable the click-apparmor job with: sudo sh -c "echo manual > /etc/init/click-apparmor.override"
3. disable the network-interface-security job with: sudo sh -c "echo manual > /etc/init/network-interface-security.override"
4. add 'exit 0' to the top of /etc/init.d/apparmor

This should actually slightly improve boot time since less shell code is being run with the simplified policy load. 14.10 will also support precompiling apparmor policy in kernel postinst and touch image generation to ensure that the cache is available on first boot to further improve (first) boot speeds.

[Jamie Strandboge (jdstrand)]

See bug #1298539 for previous discussion.

[Jamie Strandboge (jdstrand)]

FYI, anecdotally (much more testing needs to be performed) using this upstart job and setting network-interface-security and click-apparmor jobs to 'manual' (see description), I can see:
# the network-interface-security job did not start:
$ sudo initctl list|grep security
network-interface-security stop/waiting
$ ls /run/network-interface-security
ls: cannot access /run/network-interface-security: No such file or directory

# click-apparmor did not start
$ sudo initctl list|grep click-apparmor
click-apparmor stop/waiting

All policy is loaded before confined apps are started:
$ sudo aa-status
apparmor module is loaded.
128 profiles are loaded.
128 profiles are in enforce mode.
...
0 profiles are in complain mode.
32 processes have profiles defined.
32 processes are in enforce mode.
   /sbin/dhclient (2548)
   /usr/lib/firefox/firefox{,*[^s][^h]} (3864)
   /usr/lib/telepathy/mission-control-5 (3172)
   /usr/sbin/avahi-daemon (1305)
   /usr/sbin/avahi-daemon (1310)
   /usr/sbin/cups-browsed (1730)
   /usr/sbin/cupsd (3423)
   ...
   /usr/sbin/dnsmasq (2415)
   /usr/sbin/libvirtd (1742)
   /usr/sbin/nrpe (1678)
   /usr/sbin/ntpd (2983)
   /usr/sbin/rsyslogd (1287)
0 processes are in complain mode.
0 processes are unconfined but have a profile defined.

[Jamie Strandboge (jdstrand)]

(sorry, I meant to say I saw the results of comment #3 after reboot)

[Marc Deslauriers (mdeslaur)]

A few comments, perhaps xnox can clarify:

"start on mounted MOUNTPOINT="/"":

Does this wait until the upstart job is finished before continuing, or does this simply run it in parallel?
Are we sure this will get completed before networking starts?
Are we sure this will get completed before lightdm starts?
Does this get run after /sys is mounted?
We are calling stuff in /usr, do we need to check for MOUNTPOINT="/usr" also?
What about /var?

[Jamie Strandboge (jdstrand)]

We are going to need this for rtm. I've seen too many times on the phone where a process or two is unconfined but with a profile defined (ie, the 2nd stage sysv initscript is run too late). This seems to happen most often when updating a system profile without updating the cache, which can be worked around by precompiling and shipping the profiles, but that doesn't fix the underlying problem.

[Jamie Strandboge (jdstrand)]

Some thoughts for the discussion:
The attached v1 job does need to load profiles in /var. Looking at the cookbook, seems we might want to have 'start on stopped apparmor' (since it is a task) in at least lightdm. I'm not sure we need anything new for the networking bit since we already have a mechanism for this with symlinks /etc/apparmor/init/network-interface-security/ (ie, like how we do with dhclient). System jobs can use the apparmor stanza or older 'pre-start script /lib/init/apparmor-profile-load endscript' technique to make sure the policy is loaded.

[Jamie Strandboge (jdstrand)]

For the older technique, I meant to say:
pre-start script
    /lib/init/apparmor-profile-load <profile>
end script

[Dimitri John Ledkov (xnox)]

"Does this wait until the upstart job is finished before continuing, or does this simply run it in parallel?"
As per mounted(7) manpage, this does block mountall and no further mounting events are performed until after the task is completed.

"Are we sure this will get completed before networking starts?"
No, as udev triggered upstart jobs brings up networking interfaces.

"Are we sure this will get completed before lightdm starts?"
Yes, as filesystem event is blocked.

"Does this get run after /sys is mounted?"
Yes, as /sys is not mounted by mountall. It is mounted in the initramfs and/or upstart (pid 1 itself) in the initramfsless mode before startup event is emitted.

"We are calling stuff in /usr, do we need to check for MOUNTPOINT="/usr" also?"
No, as the phones will not have /usr on a separate partition. We don't yet have solution for clicks on the converged / desktops. Those can have /usr on a separate partition, and it may even be a networked filesystem. However, aa-clickhook is executed from /usr on boot at the moment because that's first-time after flashing/system-image upgrade. On the desktop, we might not reboot after "apt-get dist-upgrade" nor go for reboot on "system-image upgrade". I would hope postinst & future desktop-upgrader executes aa-clickhook. Another option is to move aa-clickhook to /. At which point it shouldn't be written in python3.

"What about /var?"
I don't believe /var is required, is it?

[Dimitri John Ledkov (xnox)]

I believe the underlying problem would be solved with just init.d script once startpar/dependency based boot will be enabled again. (it was disabled due to various regressions that caused during malta sprint).

[Marc Deslauriers (mdeslaur)]

This script will be used on desktop and server too, so /usr and /var definitely need to be mounted for it to work.

Is there a way to have it be run once "/", "/var" and "/usr" are mounted, but still using "mounted" so it still blocks mountall?

[Dimitri John Ledkov (xnox)]

On 9 June 2014 15:03, Marc Deslauriers <email address hidden> wrote:
> This script will be used on desktop and server too, so /usr and /var
> definitely need to be mounted for it to work.
>
> Is there a way to have it be run once "/", "/var" and "/usr" are
> mounted, but still using "mounted" so it still blocks mountall?

I'll need to think about it, as e.g.:
start on mounted "/" and mounted "/var"

Would dead-lock, as the first mounted event will block mountall and
prevent it from processing further mount-points.

--
Regards,

Dimitri.

[Steve Langasek (vorlon)]

On Mon, Jun 09, 2014 at 02:01:24PM -0000, Dimitri John Ledkov wrote:
> I believe the underlying problem would be solved with just init.d script
> once startpar/dependency based boot will be enabled again. (it was
> disabled due to various regressions that caused during malta sprint).

How does that help? /etc/init/rc.conf is definitely not guaranteed to be
ordered before lightdm.

[Steve Langasek (vorlon)]

On Mon, Jun 09, 2014 at 02:58:22PM -0000, Dimitri John Ledkov wrote:
> I'll need to think about it, as e.g.:
> start on mounted "/" and mounted "/var"

> Would dead-lock, as the first mounted event will block mountall and
> prevent it from processing further mount-points.

Not true; a couple cycles ago, mountall was refactored to let
mounts happen in parallel (needed to fix some issues surrounding nfs
services and deadlocking). However, this *will* deadlock if you have no
/var filesystem, because then you would never see a "mounted" event for it.

[Jamie Strandboge (jdstrand)]

FYI, I just now saw:
2 processes are unconfined but have a profile defined.
   /usr/lib/telepathy/mission-control-5 (2329)

On my phone which is running the code to implement bug #1296415 (only let certain processes talk to ofono), this breaks the dialer. As such, this bug is blocking bug #1296415, and bug #1296415 is tagged rtm14.

[Jamie Strandboge (jdstrand)]

For bug #1296415 to be unblocked, we only need to guarantee that the policy is loaded before the user session starts since we can have the upstart jobs for the system services in bug #1296415 load the policy like we do now with other services. As such, could we not have the apparmor upstart job be a task, happen sometime later in the boot and have the lightdm job do "start on started apparmor"? AIUI "started" for task means that the task has completed.

[Steve Langasek (vorlon)]

On Thu, Jun 12, 2014 at 06:56:45PM -0000, Jamie Strandboge wrote:
> For bug #1296415 to be unblocked, we only need to guarantee that the
> policy is loaded before the user session starts since we can have the
> upstart jobs for the system services in bug #1296415 load the policy
> like we do now with other services. As such, could we not have the
> apparmor upstart job be a task, happen sometime later in the boot and
> have the lightdm job do "start on started apparmor"? AIUI "started" for
> task means that the task has completed.

No, a task job will be in state 'stopped', not 'started'. But with a normal
non-task job, it probably works to add 'and started apparmor' to the lightdm
start condition.

[Marc Deslauriers (mdeslaur)]

We could make the apparmor job "start on filesystem". We would have to modify rc-sysinit to wait for the apparmor job to be done or we may end up having services like apache get started before apparmor is loaded.

[Marc Deslauriers (mdeslaur)]

@Steve:

wouldn't "started apparmor" be racy?
should we emit an "apparmor-loaded" event or something?

[Marc Deslauriers (mdeslaur)]

Meh, disregard comment #19, not sure what I was thinking there.

[Jamie Strandboge (jdstrand)]

I'm not sure why we wouldn't use a task. The definition is "A Task Job is one which runs a short-running process, that is, a program which might still take a long time to run, but which has a definite lifetime and end state." That precisely describes this job. apparmor, click-apparmor and apparmor-easyoprof-ubuntu may all want to restart this job in their postinst too (this job removes the need for the click-apparmor upstart job). Also, the cookbook states that a task will emit started, but if stopped is more idiomatic, that's fine.

Assuming we wanted to use a task, we need to do:
1. adjust lightdm's job to use 'and stopped apparmor' (weird, but 'ok')
2. adjust the attached apparmor job to use 'start on filesystem'?

What happens if a user purges apparmor from the system-- are they expected to adjust the lightdm job (or use an override)?

[Jamie Strandboge (jdstrand)]

Oh and expanding on Marc's comment:
3. modify rc-sysinit like we do with lightdm

[Steve Langasek (vorlon)]

On Thu, Jun 12, 2014 at 07:47:09PM -0000, Marc Deslauriers wrote:
> We could make the apparmor job "start on filesystem". We would have to
> modify rc-sysinit to wait for the apparmor job to be done or we may end
> up having services like apache get started before apparmor is loaded.

Currently, rc-sysinit is:

  start on (filesystem and static-network-up) or failsafe-boot

Presumably we don't want failsafe-boot (which exists to deal with
misconfigured networks) to bypass apparmor. So this would imply changing
rc-sysinit to:

  start on (filesystem and static-network-up and started apparmor) or failsafe-boot

and changing failsafe to:

  start on filesystem and net-device-up IFACE=lo and started apparmor

Regarding the previous comment that we're supposed to pretend didn't happen
;), while there won't be any race conditions, we do need to be aware of
possible risks of deadlock. If we expect the apparmor job to be started and
stopped multiple times over the life of the system, it shouldn't directly be
a dependency of jobs like rc-sysinit.

[Jamie Strandboge (jdstrand)]

@Steve, you said 'and started apparmor', is that because you are stating a preference for this to not be a task or because 'and started apparmor' is also ok with a task?

You stated that rc-sysinit shouldn't directly depend on apparmor if it can be started and stopped if a task over the life of the system. I expected we would 'start' the apparmor task in package upgrades of apparmor, click-apparmor and apparmor-easyprof-ubuntu, however, we can instead just move the script portion of this job out to the new /lib/apparmor/load-policy (or something) and then have the apparmor upstart job use it while having the apparmor, click-apparmor and apparmor-easyprof postinsts call this script directly rather than restarting the upstart job. Is there a better way to handle this that is perhaps more "upstarty" (noting that we are transitioning to systemd soon)?

Thanks!

[Marc Deslauriers (mdeslaur)]

Here is a new version of the upstart job that contains "start on starting rc-sysinit". In theory, this should get run before lightdm, and before the legacy init scripts.

[Marc Deslauriers (mdeslaur)]

So, the upstart job in #25 won't fix the issue on touch, as lightdm on touch doesn't depend on a runlevel.

[Steve Langasek (vorlon)]

On Tue, Jun 17, 2014 at 06:42:44PM -0000, Marc Deslauriers wrote:
> Here is a new version of the upstart job that contains "start on
> starting rc-sysinit". In theory, this should get run before lightdm, and
> before the legacy init scripts.

lightdm is:

start on ((filesystem
           and runlevel [!06]
           and started dbus
           and plymouth-ready)
          or runlevel PREVLEVEL=S)

(which is actually redundant, 'filesystem' is a precondition of 'runlevel')

And 'runlevel' is not emitted until the rc-sysinit job runs.

So yes, blocking rc-sysinit with apparmor sounds to me like the right
approach. This will be strictly ordered before anything that starts in
runlevel 2, which is *almost* everything. Looking at my desktop system, the
exceptions I see here, not counting filesystem daemons (NFS) are:

$ grep -rl 'start on.*filesystem\b' /etc/init | grep -vE 'rc-sysinit|failsafe'
/etc/init/screen-cleanup.conf
/etc/init/binfmt-support.conf
/etc/init/click-system-hooks.conf
/etc/init/cups-browsed.conf
/etc/init/avahi-daemon.conf
/etc/init/passwd.conf
/etc/init/lightdm.conf
/etc/init/rsyslog.conf
/etc/init/cups.conf
/etc/init/flush-early-job-log.conf
/etc/init/upstart-file-bridge.conf
/etc/init/plymouth-log.conf
/etc/init/click-apparmor.conf
$

screen-cleanup, binfmt-support, passwd, flush-early-job-log, plymouth-log
are startup tasks that don't ever need to run confined. I assume this is
also true for click-system-hooks. cups-browsed, avahi-daemon, rsyslog, and
cups include their own direct apparmor handling in the job - maybe that
should be refactored, but it's fine for now. upstart-file-bridge needs to
start as early as possible, and as a component of upstart probably needs to
run unconfined anyway.

click-apparmor may interact with the new apparmor job in some way, I'm not
sure; it's probably worth someone taking a close look.

I haven't run this same check on a phone yet to see what might be different
there.

[Jamie Strandboge (jdstrand)]

The click-apparmor job can go away with the new apparmor upstart job (that was one of the secondary goals of the new apparmor job).

[Marc Deslauriers (mdeslaur)]

So, in the touch images, lightdm gets an override which does the following:

start on ((filesystem
           and started dbus
           and android)
          or runlevel PREVLEVEL=S)

This was done for two reasons:
1- starting on a runlevel event was causing the lightdm job to never trigger in the emulator, and nobody has managed to debug it so far
2- starting on a runlevel event was delaying boot for a couple of seconds

[Marc Deslauriers (mdeslaur)]

For now, we could create an apparmor.override with the following:
start on starting lightdm

Since the touch images don't have any important daemons starting with legacy init scripts, that should work.

[Jamie Strandboge (jdstrand)]

Marc has uploaded packages for apparmor, click-apparmor and ubuntu-touch-session for this. I have tested on a mako device and these changes work well with no processes running out of confinement with a profile defined (which once this lands will unblock bug #1296415).

Furthermore, I tested this on a desktop system with lots of profiles (system profiles with profile loading in the upstart job, system profiles without explicit profile loading (ie, processes confined in the user's session and processes started via an initscript) and click profiles. All of them were loaded upon login with no processes running out of confinement with a profile defined. I tested this with and without a valid cache. Without a valid cache boot was paused for profile compilation (which is intended) and in both cases the profiles were loaded correctly.

Testing will proceed over the weekend and we plan on requesting a silo for landing Monday.

(FYI, we have work items to perform policy compilation during kernel upgrades which is planned for this cycle so desktop and server users should never have to feel policy compilation on boot. Furthermore for touch, work has been done to precompile policy during image generation and Marc's packages contain a parser fix to make that work correctly).

[Jamie Strandboge (jdstrand)]

"Marc has uploaded packages for apparmor, click-apparmor and ubuntu-touch-session for this"

I meant to say: Marc has uploaded packages to https://launchpad.net/~ubuntu-security-proposed/+archive/ppa/+packages for apparmor, click-apparmor and ubuntu-touch-session for this.

[Launchpad Janitor (janitor)]

This bug was fixed in the package apparmor - 2.8.96~2541-0ubuntu1

---------------
apparmor (2.8.96~2541-0ubuntu1) utopic; urgency=medium

  * Updated to r2541 snapshot of 2.8.96:
    - removed upstreamed patches: convert-to-rules.patch, list-fns.patch,
      parse-mode.patch, add-decimal-interp.patch, policy_mediates.patch,
      fix-failpath.patch, feature_file.patch, fix-network.patch,
      aare-to-class.patch, add-mediation-unix.patch, parser_version.patch,
      caching.patch, label-class.patch, fix-lexer-debug.patch,
      use-diff-encode.patch, fix-serialize.patch,
      fix-ppc-endian-ftbfs.patch, opt_arg.patch, tests-cond-dbus.patch,
      initialize-mount-flags.patch, fix-typo-in-dbus_write.patch,
      limited-mount-rule-support.patch, bare-capability-rule-support.patch,
      check-config-for-sysctl.patch, increase-swap-size.patch,
      test-v6-policy.patch, test-mount-mediation.patch,
      mediate-signals.patch, change-signal-syntax.patch,
      mediate-ptrace.patch, change-ptrace-syntax.patch,
      test-signal-rules.patch, test-ptrace-rules.patch,
      update-tests-for-new-semantics.patch,
      fix-garbage-in-preprocessor-output.patch,
      fix-double-comma-in-preprocessor-output.patch,
      symtab-tests-and-seenlist-bug.patch, add-profile-name-variable.patch,
      fix-names-treated-as-condlistid.patch, manpage-signal-ptrace.patch,
      python-utils-file-support.patch, python-utils-signal-support.patch,
      python-utils-ptrace-support.patch,
      python-utils-pivot_root-support.patch.
  * Added upstart job (LP: #1305108)
    - debian/apparmor.upstart: new upstart job.
    - debian/apparmor.init: added click handling, move some code to
      unload_obsolete_profiles().
    - debian/lib/apparmor/functions: add unload_obsolete_profiles().
    - debian/apparmor.postinst, debian/apparmor-profiles.postinst: reload
      profiles directly since invoke-rc.d won't allow to do this easily
      with upstart and systemd jobs.
    - debian/rules: pass --no-start to dh_installinit since we're handling
      reloading profiles manually in the postinst scripts.
    - debian/control: add a versioned apparmor Depends to the
      apparmor-profiles package to make sure the required tools are
      installed for the postinst script.
 -- Marc Deslauriers <email address hidden> Fri, 20 Jun 2014 07:20:34 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package apparmor - 2.10.95-0ubuntu2.5~14.04.1

---------------
apparmor (2.10.95-0ubuntu2.5~14.04.1) trusty; urgency=medium

  * Bring apparmor 2.10.95-0ubuntu2.5, from Ubuntu 16.04, to Ubuntu 14.04.
    - This allows for proper snap confinement on Ubuntu 14.04 when using the
      hardware enablement kernel (LP: #1641243)
  * Changes made on top of 2.10.95-0ubuntu2.5:
    - debian/apparmor.upstart: Remove the upstart job and continue using the
      init script in 14.04
    - debian/apparmor.postinst, debian/apparmor-profiles.postinst,
      debian/apparmor-profiles.postrm, debian/rules: Revert to using
      invoke-rc.d to load the profiles, rather than reloading them directly,
      since 14.04 will continue using the init script rather than the upstart
      job.
    - debian/apparmor.init, debian/lib/apparmor/functions,
      debian/apparmor.postinst, debian/apparmor.postrm: Remove functionality
      dealing with AppArmor policy in system image based environments since
      this 14.04 package will not need to handle such environments. This
      removes the handle_system_policy_package_updates(),
      compare_previous_version(), compare_and_save_debsums() functions and
      their callers.
    - debian/apparmor.init: Continue using running-in-container since
      systemd-detect-virt doesn't exist on 14.04
    - debian/lib/apparmor/functions, debian/apparmor.init: Remove the
      is_container_with_internal_policy() function and adjust its call sites
      in apparmor.init so that AppArmor policy is not loaded inside of 14.04
      LXD containers (avoids bug #1641236)
    - debian/lib/apparmor/profile-load, debian/apparmor.install: Remove
      profile-load as upstart's apparmor-profile-load is used in 14.04
    - debian/patches/libapparmor-mention-dbus-method-in-getcon-man.patch:
      Continue applying this patch since the dbus version in 14.04 isn't new
      enough to support fetching the AppArmor context from
      org.freedesktop.DBus.GetConnectionCredentials().
    - debian/patches/libapparmor-force-libtoolize-replacement.patch: Force
      libtoolize to replace existing files to fix a libapparmor FTBFS issue on
      14.04.
    - debian/control: Retain the original 14.04 Breaks and ignore the new
      Breaks from 2.10.95-0ubuntu2.5 since they were put in place as part of
      the enablement of UNIX domain socket mediation. They're not needed in
      this upload since UNIX domain socket mediation is disabled by default so
      updates to the profiles included in those packages are not needed.
    - Preserve the profiles and abstractions from 14.04's
      2.8.95~2430-0ubuntu5.3 apparmor package by recreating them in the
      top-level profiles-14.04/ directory of the source. They'll be installed
      to debian/tmp/etc/apparmor.d/ during the build process and then to
      /etc/apparmor.d/ on package install so that there are no changes to the
      shipped profiles or abstractions. The abstractions from
      2.10.95-0ubuntu2.5 will be installed into
      debian/tmp/snap/etc/apparmor.d/ during the build process and then into
      /etc/apparmor.d/snap/abstractions/ on package install for use wit...