/sbin/dhclient is unconfined after switch to systemd (aka, equivalent of upstart's network-interface-security.conf not implemented)

Bug #1438249 reported by Jamie Strandboge
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
systemd (Ubuntu)
Fix Released
High
Martin Pitt

Bug Description

dhclient is starting before the apparmor profile for it is loaded which results in the following output from aa-status:

$ sudo aa-status
...
4 profiles are in enforce mode.
   /sbin/dhclient
...
1 processes are unconfined but have a profile defined.
   /sbin/dhclient (634)

Upstart had the network-interface-security.conf job to make sure this didn't happen. We wanted the cache loading library to be implemented in time (bug #1385414), but it still hasn't landed. Having the cache loading library in place would mean that this bug would also be fixed, but now we need to fix this bug differently for 15.04 and it must be fixed by release.

Revision history for this message
Martin Pitt (pitti) wrote :

/etc/init/network-interface-security.conf is in ifupdown, so let's put the corresponding system unit there, too.

affects: systemd (Ubuntu) → ifupdown (Ubuntu)
tags: added: systemd-boot
Revision history for this message
Martin Pitt (pitti) wrote :

So at the moment, apparmor starts After=local-fs.target and Before=sysinit.target.

network-interface-security.conf does:

    start on (starting network-interface or starting network-manager or starting networking)

network-interface corresponds to ifup@.service, networking is just the ifupdown init.d script; these two need an After=apparmor.service. NetworkManager.service has DefaultDependencies=yes (the default), thus the ordering there is fine already.

It seems to me that adding these two ordering constraints is simpler and potentially also more efficient than running /sbin/apparmor_parser manually?

Revision history for this message
Martin Pitt (pitti) wrote :

ifup@.service is shipped by systemd, let's just add the After= there.

Changed in systemd (Ubuntu):
assignee: nobody → Martin Pitt (pitti)
importance: Undecided → High
status: New → Triaged
Revision history for this message
Martin Pitt (pitti) wrote :

To clarify: even if this introduces a stronger boot ordering than necessary, it's just a temporary solution until bug 1385414 lands. So IMHO we should keep this simple.

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

Hi Martin - What you've described sounds good to me but I should note that I'm missing a lot of historical context around the details of the AppArmor init script. I'd like for Jamie to chime in when he has a chance.

Also, in regards to comment #4, whatever solution we come up with to fix this bug is likely what will ship in 15.04 since the fixes for bug 1385414 are landing a bit too late for 15.04.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

As far as historical context for network-interface-security.conf, it is all about loading the profiles that the symlinks in /etc/apparmor/init/network-interface-security/* point to in time. Looking at a 14.10 system, I see that there are two things there: sbin.dhclient and usr.sbin.ntpd. This suggests to me that Martin's approach of changing the dependencies is best. That said, I'm not yet incredibly familiar with systemd boot ordering-- it sounds like you are saying that ifup@.service will always run before networking comes up or NetworkManager. Therefore if we change ifup@.service to use After=apparmor.service, then this sounds sufficient. In terms of user experience when the cache is invalidated, it only shifts the policy recompilation earlier (ie, the boot speed to login remains the same).

Revision history for this message
Martin Pitt (pitti) wrote :

> it sounds like you are saying that ifup@.service will always run before networking comes up or NetworkManager.

Not necessarily. We need to make sure that all three run After=apparmor.service. This is already the case for NetworkManager, but not the other two.

Changed in systemd (Ubuntu):
status: Triaged → In Progress
milestone: none → ubuntu-15.04
Revision history for this message
Martin Pitt (pitti) wrote :
no longer affects: ifupdown (Ubuntu)
Changed in systemd (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
John Johansen (jjohansen) wrote :

So just a little more context around this whole split in policy loading. AppArmor does a check that the cached policy is current and matches to the kernel before loading and then if not falls back to recompiling policy. The policy load was split into an early stage (/etc/apparmor/init) and full policy (/etc/apparmor.d/), so that early boot would only load the minimum amount of policy needed during early boot to minimize potential delays (specifically fallback to compile) during early boot. The recompile is not such an issue in later boot as it can be run in parallel with more services.

TLDR: The split is all about boot speed. It would be easier and safer if all policy was loaded as early as possible.

More context:
* apparmor init scripts all sys V and not upstart based (for various reasons)
* apparmor_parser at the time would only process one file and had to be called multiple times (which slowed down loading) - (FIXED)
* apparmor_parser is single threaded, so shell scripts were used to get parallel loading/compiles - (NOT fixed)
* apparmor_parser was a LOT (at least a couple orders of magnitude) slower at compiles when this was done
* apparmor_parser couldn't properly build policy for anything but the current kernel, so policy compile was always left to first boot of a given kernel
  - specifying the feature set was broken (FIXED)
  - the abi matching had issues (FIXED)
  - new kernels didn't ship the needed feature information to compile policy for the kernel during kernel installed so policy compiles needed to wait until booting into the new kernel (NOT fixed)
* apparmor only supported a single cache ( NOT fixed - soon)
  - needed to support policy for multiple kernels, having policy compiled before boot.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (4.5 KiB)

This bug was fixed in the package systemd - 219-6ubuntu1

---------------
systemd (219-6ubuntu1) vivid; urgency=medium

  * Merge with Debian experimental branch. Remaining Ubuntu changes:
    - Hack to support system-image read-only /etc, and modify files in
      /etc/writable/ instead.
    - Keep our much simpler udev maintainer scripts (all platforms must
      support udev, no debconf).
    - initramfs init-top: Drop $ROOTDELAY, we do that in a more sensible way
      with wait-for-root. Will get applicable to Debian once Debian gets
      wait-for-root in initramfs-tools.
    - initramfs init-bottom: If LVM is installed, settle udev,
      otherwise we get missing LV symlinks. Workaround for LP #1185394.
    - Add debian/udev.lvm2.init: Dummy SysV init script to satisfy insserv
      dependencies to "lvm2" which is handled with udev rules in Ubuntu.
    - Provide shutdown fallback for upstart. (LP: #1370329)
    - debian/extra/ifup@.service: Additionally run for "auto" class. We don't
      really support "allow-hotplug" in Ubuntu at the moment, so we need to
      deal with "auto" devices appearing after "/etc/init.d/networking start"
      already ran. (LP: #1374521) Also, check if devices are actually defined
      in /etc/network/interfaces as we don't use Debian's net.agent.
    - ifup@.service: Drop dependency on networking.service (i. e.
      /etc/init.d/networking), and merely ensure that /run/network exists.
      This avoids unnecessary dependencies/waiting during boot and dependency
      cycles if hooks wait for other interfaces to come up (like ifenslave
      with bonding interfaces). (LP: #1414544)
    - Add Get-RTC-is-in-local-time-setting-from-etc-default-rc.patch: In
      Ubuntu we currently keep the setting whether the RTC is in local or UTC
      time in /etc/default/rcS "UTC=yes|no", instead of /etc/adjtime.
      (LP: #1377258)
    - Put session scopes into all cgroup controllers. This makes unprivileged
      user LXC containers work under systemd. (LP: #1346734)
    - systemctl: Don't forward telinit u to upstart. This works around
      upstart's Restart() always reexec'ing /sbin/init on Restart(), even if
      that changes to point to systemd during the upgrade. This avoids running
      systemd during a dist-upgrade. (LP: #1430479)
    - Lower Breaks: to plymouth version which has the udev inotify fix in
      Ubuntu.
    - Lower libappamor1 dep to the Ubuntu version where it moved to /lib.
    - Change systemd-sysv's conflicts to upstart-sysv. (LP: #1422681)
    - Make failure of boot-and-services NSpawn.test_boot non-fatal for now.
      This currently fails when being triggered by Jenkins, but is totally
      unreproducible when running this manually on the exact same machine.

    Upgrade fixes, keep until 16.04 LTS release:
    - systemd Conflicts/Replaces/Provides systemd-services.
    - Remove obsolete systemd-logind upstart job.
    - Clean up obsolete /etc/udev/rules.d/README.

  * Add debian/udev.lvm2.service to avoid running the dummy lvm2 init script.
    (LP: #1431107)

systemd (219-6) experimental; urgency=medium

  [ Martin Pitt ]
  * Import patches from v219-stable branch (up to 85a6fab).
 ...

Read more...

Changed in systemd (Ubuntu):
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.