[PATCH] virt-aa-helper: allow common riscv64 loader paths

[Christian Ehrhardt]

On Thu, Sep 29, 2022 at 11:30 PM Jim Fehlig <jfehlig at suse.com> wrote:
>
> On 9/28/22 06:45, christian.ehrhardt at canonical.com wrote:
> > From: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> >
> > Riscv64 usually uses u-boot as external -kernel and a loader from
> > the open implementation of RISC-V SBI. The paths for those binaries
> > as packaged in Debian and Ubuntu are in paths which are usually
> > forbidden to be added by the user under /usr/lib...
>
> Do you know if the path is configurable?

Not on the libvirt stage, here it is
a) whatever users specify as kernel/loader in guest XML
b) (our) qemu 7.0 delivers it in the default paths for firmwares (that
are already allowed in src/security/apparmor/libvirt-qemu, but you'd
not want virt-aa-helper to complain on it still)

> Are distros free to put those binaries where they choose?

Well, in Debian/Ubuntu we'd still obey the FHS rules, but to some
extent yes it could be in other places.

> E.g. /usr/libexec or similar?

The libexec folks are usually fedora/RH which do not care about
apparmor that much, so we have historically not added those in code to
keep it readable (static rules use @libexecdir@ replaced at build
time).

AFAICS you show to copy out the u-boot from the image [1]
Which does not end up in a predictable path-prefix that I could add here.

If you tell me that on Suse the paths for these binaries are different
I'm more than happy to spin a v2 that also has your paths.
Or you provide a follow up with that content once/if you've found a
path that you'll regularly need.

[1]: https://en.opensuse.org/openSUSE:RISC-V

> Regards,
> Jim
>
> >
> > People used to start riscv64 guests only manually via qemu cmdline,
> > but trying to encapsulate that via libvirt now causes failures when
> > starting the guest due to the apparmor isolation not allowing that:
> >     virt-aa-helper: error: skipped restricted file
> >     virt-aa-helper: error: invalid VM definition
> >
> > Explicitly allow the sub-paths used by u-boot-qemu and opensbi
> > under /usr/lib/ as readonly rules.
> >
> > Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
> > ---
> >   src/security/virt-aa-helper.c | 12 +++++++-----
> >   1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> > index f338488da3..ceadaef99b 100644
> > --- a/src/security/virt-aa-helper.c
> > +++ b/src/security/virt-aa-helper.c
> > @@ -476,11 +476,13 @@ valid_path(const char *path, const bool readonly)
> >           "/initrd",
> >           "/initrd.img",
> >           "/usr/share/edk2/",
> > -        "/usr/share/OVMF/",              /* for OVMF images */
> > -        "/usr/share/ovmf/",              /* for OVMF images */
> > -        "/usr/share/AAVMF/",             /* for AAVMF images */
> > -        "/usr/share/qemu-efi/",          /* for AAVMF images */
> > -        "/usr/share/qemu-efi-aarch64/"   /* for AAVMF images */
> > +        "/usr/share/OVMF/",                  /* for OVMF images */
> > +        "/usr/share/ovmf/",                  /* for OVMF images */
> > +        "/usr/share/AAVMF/",                 /* for AAVMF images */
> > +        "/usr/share/qemu-efi/",              /* for AAVMF images */
> > +        "/usr/share/qemu-efi-aarch64/",      /* for AAVMF images */
> > +        "/usr/lib/u-boot/",                  /* u-boot loaders for qemu */
> > +        "/usr/lib/riscv64-linux-gnu/opensbi" /* RISC-V SBI implementation */
> >       };
> >       /* override the above with these */
> >       const char * const override[] = {
>


-- 
Christian Ehrhardt
Senior Staff Engineer, Ubuntu Server
Canonical Ltd