[apparmor] [PATCH] enhance aa-status to deal with lack of interface patch

[Kees Cook]

On Wed, Apr 25, 2012 at 04:37:15PM -0700, Steve Beattie wrote:
> On Wed, Apr 25, 2012 at 12:32:48PM -0700, Kees Cook wrote:
> > Yeah. I've kind of always felt that this was a mis-feature. The things
> > the init system wants to know are:
> > 
> > - is AppArmor capable of functioning? (in kernel and enabled)
> > - is it time to start loading profiles? (... in Ubuntu we cheated by
> >     looking at if profiles are already loaded -- since all systems with
> >     AppArmor will have preloaded the network-early-start profiles. This
> >     is, however, a catch-22 and takes advantage of a specific side-effect
> >     of how Ubuntu loads profiles.)
> > 
> > So, if we can eliminate the need for the second thing above, then the
> > problem goes away, and --enabled can stop lying. :)
> > 
> > If I remember correctly, doing the profile count was to avoid loading
> > profiles in certain conditions. Unfortunately, I can't remember for sure
> > what those situations were. I think it was: LiveCD and release upgrades,
> > but I'm pretty sure both are covered now.
> 
> Actually, there are more conditions than that, at least for operating
> within an LSB compliant init script. In particular, the 'try-restart'
> option needs to "restart" apparmor only if it's already running. Given
> that apparmor is not a daemon, but a kernel element that needs to
> be configured, we (upstream) made the decision to assume that even
> if apparmor is enabled in the kernel but has 0 profiles loaded,
> this was considered "not running", under the assumption that the
> initscript had either been disabled or an admin had done a 'stop'
> action, which would unload all profiles (and yes, I'm aware that the
> ubuntu/debian initscript differs in behavior here). In other words,
> we map the LSB running state to "apparmor is enabled *and* enforcing
> at least one policy".
> 
> Identifying the situation where apparmor was enabled but not enforcing
> anything *in a way that can be detected programmatically without
> worrying about translations/locales/strings that change, etc.* was
> deemed important and why it was done as a return code.
> 
> Now, that said, the initscripts we provide don't actually make use
> of aa-status to determine the state of apparmor, other than for
> the 'status' target (I think because we worked with the assumption
> that the apparmor utils weren't guaranteed to be installed; also
> that people wouldn't necessarily want to pay the costs of starting a
> perl/python interpreter during boot). However, the non-verbose options
> were added to aa-status to support being invoked by other programs
> that could potentially make decisions/take actions like restarting
> services based on its output; things like CIM providers and other
> monitoring/controlling services.

Hrm, okay. Well, aa-status is used by dh_apparmor to decide if a profile
should be loaded. I guess it should check for rc == 0 || rc == 2. E.g.:

    # Reload AppArmor profile
    APP_PROFILE="/etc/apparmor.d/usr.sbin.named"
    if [ -f "$APP_PROFILE" ] && aa-status --enabled 2>/dev/null; then
        apparmor_parser -r "$APP_PROFILE" || true
    fi

What do you think this should be doing here?

> > I'd like to have --enabled not lie, and work out the bugs this triggers
> > as separate issues. If this isn't cool for now, how about having it only
> > lie if the legacy "profiles" interface exists?
> > 
> > > This should possibly be its own error code state, I think, to
> > > distinguish from "apparmor enabled, no policy loaded" to "apparmor
> > > loaded, but the version doesn't let me determine how many profiles
> > > are loaded". (It's up to the calling program to treat the return codes
> > > as fatal events -- there's no real other way for aa-status to return
> > > complex results.)
> > 
> > Well... as I mention in the comment, I don't think it's actually
> > an error condition. I think aa-status should attempt to handle it,
> > but emit a warning (which this does). It just means it can't tell the
> > user about processes that are running that are unconfined by a loaded
> > profile. It can still report all the other states sanely (since it reads
> > /proc/$pid/attr/current).
> 
> The thing about aa-status is that it serves two purposes: 1) as a
> user facing tool to get a snapshot of the state of apparmor policy and
> enforcement on the system and 2) a tool intended to be used by other
> programs to capture the state of apparmor. Emitting warning messages is
> fine in the former situation, but less so in the latter.  Knowing *why*
> 'aa-status --profiled' returned 0 is useful (are there actually zero
> profiles loaded or is it that this is using an upstream kernel?).

My goal is to have aa-status able to run, even in the face of a missing
interface file. Since my other goal is to solving the missing interface
file, I also don't want to over-engineer an aa-status solution. I am
comfortable with this warning about not being able to read the list of
in-kernel profiles.

> That said, I think we can construct things in such a way to satisfy
> people's needs. Arguably, in hindsight, I can see the utility
> of having a separate '--running' option and make the '--enabled'
> strictly determine if apparmor is enabled on the host. However, I
> don't know how to answer the case of whether apparmor is "running"
> when the introspection interface isn't present; do you walk the
> /proc/PID trees looking for the sign of any profile being applied
> and return false if not?

It already walks all the pid trees looking for profiles. What it can't
know is if a profile is in the kernel but not applied to a process. There
is no sensible way around this without the introspection interface
finished.

> I'll look more at coming up with a solution here.
> 
> (In an ideal world, the state detection logic would get shoved into a
> python class that could be imported into any number of python utilities
> and raised exceptions based on the situation it encountered, and the
> caller could take and report as appropriate; the sys.exit()s would
> need to go.)

Well, I personally really like the current use of the upstart helper
/lib/init/apparmor-profile-load. I'd love to be able to use that in sysv
init scripts and maintainer scripts. It's much cleaner than doing a Perl
load for "aa-status --enabled". Perhaps that helper should get moved to
the AppArmor package, and change current users from:

    /lib/init/apparmor-profile-load usr.sbin.avahi-daemon

in to:

    if [ -x /lib/init/apparmor-profile-load ]; then
        /lib/init/apparmor-profile-load usr.sbin.cupsd
    fi

Half the users already check for /lib/init/apparmor-profile-load.

-Kees

-- 
Kees Cook