[PATCH] apparmor: avoid denials on libpmem initialization

[Christian Ehrhardt]

On Wed, Apr 8, 2020 at 9:50 PM Jamie Strandboge <jamie at canonical.com> wrote:
>
> On Wed, 08 Apr 2020, Jamie Strandboge wrote:
>
> > On Wed, 08 Apr 2020, Christian Ehrhardt wrote:
> >
> > > With libpmem support compiled into qemu it will trigger the following
> > > denials on every startup.
> > >   apparmor="DENIED" operation="open" name="/"
> > >   apparmor="DENIED" operation="open" name="/sys/bus/nd/devices/"
> > >
> > > This is due to [1] that tries to auto-detect if the platform supports
> > > auto flush for all region.
> > >
> > > Once we know all the paths that are potentially needed if this feature
> > > is really used we can add them conditionally in virt-aa-helper and labelling
> > > calls in case </pmem> is enabled.
> > >
> > > But until then the change here silences the denial warnings seen above.
> > >
> > > [1]: https://github.com/pmem/pmdk/blob/master/src/libpmem2/auto_flush_linux.c#L131
> > >
> > > Bug: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1871354
> > >
> > > +  /sys/bus/nd/devices/* r,
> >
> > Can you list what files libpem init is looking at? I'm a bit
> > uncomfortable with the glob here and would rather not guess that today's
> > and all future files in /sys/bus/nd/devices are safe for all qemu
> > processes to read.
>
> Answering myself after looking at the pmdk source code, fs_new() is
> calling fts_open() without FTS_NOCHDIR (and based on your '/' rule,
> starting in '/'), then calls fs_read() which calls fts_read() on the
> files it finds in /sys/bus/nd/devices (it makes sure to only look at
> symlinks, but that doesn't impact our rules). Writing some test code to
> simulate this and testing on /sys/bus/usb/devices (since I have usb
> devices here, but not nd and this dir is populated with symlinks as the
> libpmem code expects), I think the full rules you want are:
>
> # required by libpmem init to fts_open()/fts_read() the symlinks in
> # /sys/bus/nd/devices
> / r,
> /sys/bus/nd/devices/{,**/} r,

Thanks for your help to make this less of a wildcard.

Since this is qemu startup (without any special config) I tested it and it
works with these improved rules as well, triggering no Denial.

I'll submit a V2 with the adapted rules and comments

If anyone is interested to fake some nvdimms you can take a look at [1],
but as I outlined in the referred bug for the real conditional rules we will
need to check how systems with real devices will look like.

[1]: https://git.launchpad.net/qa-regression-testing/tree/notes_testing/ndctl/README.txt

> Ideally this access would only be needed if using NFIT-ND devices, but
> as you mentioned, that is not possible at the point of the denial. I
> think these rules are fine to apply to the default VM policy since they
> are only a collection of directory reads (note the trailing '/'s in the
> second rule), which should have no impact guest isolation or host
> protections.
>
> --
> Jamie Strandboge             | http://www.canonical.com



-- 
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd