[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 6/8] apparmor: allow virt-aa-helper to read from tmp





On Mon, Aug 3, 2020 at 5:11 PM Jamie Strandboge <jamie canonical com> wrote:
On Mon, 03 Aug 2020, Christian Ehrhardt wrote:

> From: Stefan Bader <stefan bader canonical com>
>
> temporary directories are a common place images are placed by users
> for any sort of quick evaluation. Allow virt-aa-helper access to tmp
> via the existing user-tmp apparmor abstraction.
>
> That way if a guest definition has paths in temporary directories
> virt-aa-helper can properly probe them e.g. for further backing files in
> the case of qcow2.
>
> Signed-off-by: Christian Ehrhardt <christian ehrhardt canonical com>
> ---
>  src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
> index dfc61e8de4..3f204799a6 100644
> --- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
> +++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
> @@ -3,6 +3,7 @@
>  profile virt-aa-helper @libexecdir@/virt-aa-helper {
>    #include <abstractions/base>
>    #include <abstractions/nameservice>
> +  #include <abstractions/user-tmp>

user-tmp allows write and all other accesses for disks are read. We have
these rules:

  /**.img r,
  /**.raw r,
  /**.qcow{,2} r,
  /**.qed r,
  /**.vmdk r,
  /**.vhd r,
  /**.[iI][sS][oO] r,
  /**/disk{,.*} r,

Why are these not sufficient? What was the denial that triggered the
issue?

Great question to ask - this is one of the Deltas that was "just carried" for quite some time.
But you made me analyze the background and it isn't reasonable IMHO.
It was added quite some time ago without outlining  a particular reason.

The list you refer to above wasn't as long back then (~5 years ago), maybe extending the list would have been all that was needed and instead the user-tmp abstraction was added.

I'll drop the commit from this series as well as from Ubuntu on next merge and check for any fallout - it is easy to be added back and most likely not needed.

--
Jamie Strandboge             | http://www.canonical.com


--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]