[apparmor] Policy cache

[Christian Boltz]

Hello,

Am Mittwoch, 9. November 2011 schrieb John Johansen:
> On 11/09/2011 04:54 PM, Kees Cook wrote:
> > On Wed, Nov 09, 2011 at 04:43:44PM -0800, Seth Arnold wrote:
> >> On Wed, Nov 9, 2011 at 3:52 PM, Kees Cook <kees at ubuntu.com> wrote:
> >>> On Wed, Nov 09, 2011 at 03:38:36PM -0800, John Johansen wrote:
> >>>> With that said we should add an option to the
> >>>> apparmor_parser to allow setting the cache location.  If an
> >>>> alternate cache location is desired a new default could be
> >>>> set in the /etc/apparmor.d/parser.conf file 
> >>> I would recommend /lib/apparmor/cache as the new upstream
> >>> default location. It should be an option, though, yes.

/lib is meant to be read-only from the FHS view, so we would replace a 
FHS violation (cache files in /etc) with another one (dynamic files in 
/lib) which is even worse IMHO. /etc is at least ment to be writeable.

> >> I know /lib is usually available early, but I don't care for the
> >> idea of considering it a "writable location". That's what /var
> >> is there for -- per-service dynamic storage. 

Exactly - /var/cache is _the_ place for cache files.

> > Hrm. So /var by default? And people with /var not in / can
> > relocated it as needed? 

Yes, /var/cache/apparmor would be the correct default.

If someone wants another location, he can create a symlink (similar to 
the symlink /etc/apparmor.d/cache -> /var/cache/apparmor I'm using in 
the openSUSE package ;-)

If you really want, make it a config option - but please try to avoid 
that AppArmor gets renamed to KAppArmor one day *eg*

> > I'd almost rather keep it in /etc. More people more /var than /lib. 
                                                   ^^^^
                                                  s/r/v/ ?

Maybe nearly nobody puts /etc on a separate partition.
OTOH: Do you have some numbers about how many people have a separate 
/var partition? Would using /var/cache/ be a real problem for them?

> > For example, udev uses /etc for cached rules.

Sounds like a bug in udev ;-)

That said: What's the exact cache location? 
/etc/udev on openSUSE looks quite clean - it has some files in rules.d 
and an udev.conf. Well, some of the files in rules.d are placed there by 
a package (which would probably better install them in 
/lib/udev/rules.d/), but I don't see anything that looks like a cache.

And besides that: see signature ;-)

> This is exactly how the discussion have gone, /etc isn't right, should
> be /var, but it might not be available, how about /lib, no /lib
> shouldn't be writable that is what /var is for, and around and around
> ...

;-)

> And we keep coming back to /etc/ is the best generic default because
> no where matches, but /etc/ works and its what we are using already
> 
> I am not opposed to moving but we can't seem to agree where, hence
> lets just make it configurable and be done with it.

Yes, I'm afraid it needs to be a config or compile time option, even if 
I really don't like the idea to use anything except /var/cache/.

The problem with making it a config or a compile-time option is that it 
will make the documentation funny. "Profiles are cached in 
/etc/apparmor.d/cache, except if you use openSUSE which uses 
/var/cache/apparmor, except if you use $foo_distro which uses 
/lib/apparmor/cache, except if you use $bar_distro which uses 
/var/cache/apparmor-profiles, except if you use $other_distro ..." ;-)


If /var is not (yet) available, starting AppArmor will take some 
additional seconds because of the non-available cache. That's not nice, 
but totally different from "it will burn down your system" ;-)

BTW: The openSUSE initscript (rc.apparmor.suse) contains "Should-Start: 
$local_fs", which means AppArmor will be started after all partitions 
are mounted.

The other rc.apparmor.* scripts don't contain something like that - 
maybe that's something to fix?

> >> The hash need not be this complicated: it could just be:
> >> 
> >> /lib/apparmor/cache/3.2.0-5-generic/f3.1-c2.0-n1.0-ch1.5-cp1.1-n
> >> s1.1-r1.1/ reduced to:
> >> /lib/apparmor/cache/3.2.0-5-generic/31201015111111

Both variants mean you'll need a program to generate the hash for the 
cleanup script. What about just using the same name that is used in 
/lib/modules/?

> > But that means we can't ever go from .9 to .10. I think a hash
> > would be fine -- we're just looking for whatever matches.
> 
> yes I think the hash will be adequate, especially if we continue with
> the practice of putting .features file in the directory, so that if
> there ever is a hash collision we can catch it.

Before we discuss about the method to name the directory - do we really 
need cache directories for different kernels? I somehow doubt ;-)

Let me start with a question: Does apparmor_parser notice a kernel 
change and update the cache (if the "write-cache" option is enabled)? 
[1]

My thoughts about having subdirectories for each kernel version:

+ faster boot time for people who switch forth and back between 
  different kernels (but seriously - who does that?)
- something[tm] needs to cleanup/delete the cache for old kernel 
  versions
- needs to be implemented in apparmor_parser

IMHO the + argument is too small and a corner-case when compared to the 
other arguments - having the cache only for one kernel would be fine 
IMHO.


Regards,

Christian Boltz

[1] If yes: fine. 
    If no: fix it ;-)

-- 
There is nothing wrong with making mistakes, but... make *new* ones.
[D.Sim]