Re: [PATCH] virt-aa-helper: allow common riscv64 loader paths
On Fri, Sep 30, 2022 at 6:37 PM Jim Fehlig wrote: > > On 9/29/22 23:43, Christian Ehrhardt wrote: > > On Thu, Sep 29, 2022 at 11:30 PM Jim Fehlig wrote: > >> > >> On 9/28/22 06:45, christian.ehrha...@canonical.com wrote: > >>> From: Christian Ehrhardt > >>> > >>> 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. > > Thanks for the info. Let's go with the latter option. I'll send a follow up > patch if/when needed. Thanks for the review Michal, thanks for the discussion Jim. Since v8.8.0 is finalized we are open again and this LGTM so I pushed it to the main branch. > Regards, > Jim -- Christian Ehrhardt Senior Staff Engineer, Ubuntu Server Canonical Ltd
Re: [PATCH] virt-aa-helper: allow common riscv64 loader paths
On 9/29/22 23:43, Christian Ehrhardt wrote: On Thu, Sep 29, 2022 at 11:30 PM Jim Fehlig wrote: On 9/28/22 06:45, christian.ehrha...@canonical.com wrote: From: Christian Ehrhardt 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. Thanks for the info. Let's go with the latter option. I'll send a follow up patch if/when needed. Regards, Jim
Re: [PATCH] virt-aa-helper: allow common riscv64 loader paths
On Thu, Sep 29, 2022 at 11:30 PM Jim Fehlig wrote:
>
> On 9/28/22 06:45, christian.ehrha...@canonical.com wrote:
> > From: Christian Ehrhardt
> >
> > 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
> > ---
> > 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
Re: [PATCH] virt-aa-helper: allow common riscv64 loader paths
On 9/28/22 06:45, christian.ehrha...@canonical.com wrote:
From: Christian Ehrhardt
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? Are distros free to put those binaries
where they choose? E.g. /usr/libexec or similar?
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
---
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[] = {
Re: [PATCH] virt-aa-helper: allow common riscv64 loader paths
On 9/28/22 14:45, christian.ehrha...@canonical.com wrote: > From: Christian Ehrhardt > > 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... > > 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 > --- > src/security/virt-aa-helper.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) Reviewed-by: Michal Privoznik Michal
[PATCH] virt-aa-helper: allow common riscv64 loader paths
From: Christian Ehrhardt
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...
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
---
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[] = {
--
2.37.3
