All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: virtio-fs-list <virtio-fs@redhat.com>, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [PATCH 0/2] virtiofsd: drop Linux capabilities(7)
Date: Fri, 19 Jun 2020 11:57:37 -0400	[thread overview]
Message-ID: <20200619155737.GA12225@redhat.com> (raw)
In-Reply-To: <CAJfpeguhS3w-AZTpyzO2QqcX_7F1qDm__5C8r3pBnCgPoxTKmQ@mail.gmail.com>

On Fri, Jun 19, 2020 at 05:26:37PM +0200, Miklos Szeredi wrote:
> On Fri, Jun 19, 2020 at 4:25 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Fri, Jun 19, 2020 at 04:16:30PM +0200, Miklos Szeredi wrote:
> > > On Thu, Jun 18, 2020 at 9:08 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > >
> > > > On Thu, Apr 16, 2020 at 05:49:05PM +0100, Stefan Hajnoczi wrote:
> > > > > virtiofsd doesn't need of all Linux capabilities(7) available to root.  Keep a
> > > > > whitelisted set of capabilities that we require.  This improves security in
> > > > > case virtiofsd is compromised by making it hard for an attacker to gain further
> > > > > access to the system.
> > > >
> > > > Hi Stefan,
> > > >
> > > > I just noticed that this patch set breaks overlayfs on top of virtiofs.
> > >
> > > How so?  Virtiofs isn't mounting overlayfs, is it?  Only the mounter
> > > requires CAP_SYS_ADMIN, not the accessor.
> >
> > virtiofsd needs CAP_SYS_ADMIN, otherwise fsetxattr(trusted.overlay.opaque)
> > fails in lo_setxattr().
> >
> > This is triggered when we mount overlayfs on top of virtiofs and overlayfs
> > tries to set OVL_XATTR_OPAQUE on upper to check if trusted xattrs are
> > supported or not.
> 
> Ah, right.
> 
> Plan is to use "user.*" xattr for unprivileged overlay.  This would be
> a good way to eliminate this attack surface in the overlay on virtiofs
> case as well.

So unpriviliged overlay is one which is mounted from inside a user
namespace. But in this case we might be mounting it from init_user_ns
of guest. So from overlayfs perspective this will still be treated
as priviliged overlay instance and it will use trusted xattrs, IIUC?

Thanks
Vivek

> 
> Other ways to minimize risk is to separate operations requiring
> CAP_SYS_ADMIN into a separate process, preferably a separate
> executable, that communicates with virtiofsd using a pipe and contains
> the minimum amount of code.

> 
> Thanks,
> Miklos
> 


WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: virtio-fs-list <virtio-fs@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Virtio-fs] [PATCH 0/2] virtiofsd: drop Linux capabilities(7)
Date: Fri, 19 Jun 2020 11:57:37 -0400	[thread overview]
Message-ID: <20200619155737.GA12225@redhat.com> (raw)
In-Reply-To: <CAJfpeguhS3w-AZTpyzO2QqcX_7F1qDm__5C8r3pBnCgPoxTKmQ@mail.gmail.com>

On Fri, Jun 19, 2020 at 05:26:37PM +0200, Miklos Szeredi wrote:
> On Fri, Jun 19, 2020 at 4:25 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Fri, Jun 19, 2020 at 04:16:30PM +0200, Miklos Szeredi wrote:
> > > On Thu, Jun 18, 2020 at 9:08 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> > > >
> > > > On Thu, Apr 16, 2020 at 05:49:05PM +0100, Stefan Hajnoczi wrote:
> > > > > virtiofsd doesn't need of all Linux capabilities(7) available to root.  Keep a
> > > > > whitelisted set of capabilities that we require.  This improves security in
> > > > > case virtiofsd is compromised by making it hard for an attacker to gain further
> > > > > access to the system.
> > > >
> > > > Hi Stefan,
> > > >
> > > > I just noticed that this patch set breaks overlayfs on top of virtiofs.
> > >
> > > How so?  Virtiofs isn't mounting overlayfs, is it?  Only the mounter
> > > requires CAP_SYS_ADMIN, not the accessor.
> >
> > virtiofsd needs CAP_SYS_ADMIN, otherwise fsetxattr(trusted.overlay.opaque)
> > fails in lo_setxattr().
> >
> > This is triggered when we mount overlayfs on top of virtiofs and overlayfs
> > tries to set OVL_XATTR_OPAQUE on upper to check if trusted xattrs are
> > supported or not.
> 
> Ah, right.
> 
> Plan is to use "user.*" xattr for unprivileged overlay.  This would be
> a good way to eliminate this attack surface in the overlay on virtiofs
> case as well.

So unpriviliged overlay is one which is mounted from inside a user
namespace. But in this case we might be mounting it from init_user_ns
of guest. So from overlayfs perspective this will still be treated
as priviliged overlay instance and it will use trusted xattrs, IIUC?

Thanks
Vivek

> 
> Other ways to minimize risk is to separate operations requiring
> CAP_SYS_ADMIN into a separate process, preferably a separate
> executable, that communicates with virtiofsd using a pipe and contains
> the minimum amount of code.

> 
> Thanks,
> Miklos
> 



  reply	other threads:[~2020-06-19 15:57 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-16 16:49 [Virtio-fs] [PATCH 0/2] virtiofsd: drop Linux capabilities(7) Stefan Hajnoczi
2020-04-16 16:49 ` Stefan Hajnoczi
2020-04-16 16:49 ` [Virtio-fs] [PATCH 1/2] virtiofsd: only retain file system capabilities Stefan Hajnoczi
2020-04-16 16:49   ` Stefan Hajnoczi
2020-04-28 11:48   ` [Virtio-fs] " Dr. David Alan Gilbert
2020-04-28 11:48     ` Dr. David Alan Gilbert
2020-04-16 16:49 ` [Virtio-fs] [PATCH 2/2] virtiofsd: drop all capabilities in the wait parent process Stefan Hajnoczi
2020-04-16 16:49   ` Stefan Hajnoczi
2020-04-16 17:50   ` [Virtio-fs] " Philippe Mathieu-Daudé
2020-04-16 17:50     ` Philippe Mathieu-Daudé
2020-04-16 20:10 ` [Virtio-fs] [PATCH 0/2] virtiofsd: drop Linux capabilities(7) Vivek Goyal
2020-04-16 20:10   ` Vivek Goyal
2020-04-17  9:42   ` [Virtio-fs] " Stefan Hajnoczi
2020-04-17  9:42     ` Stefan Hajnoczi
2020-05-01 18:28 ` [Virtio-fs] " Dr. David Alan Gilbert
2020-05-01 18:28   ` Dr. David Alan Gilbert
2020-06-18 19:08 ` [Virtio-fs] " Vivek Goyal
2020-06-18 19:16   ` Dr. David Alan Gilbert
2020-06-18 19:16     ` Dr. David Alan Gilbert
2020-06-18 19:27     ` Vivek Goyal
2020-06-18 19:27       ` Vivek Goyal
2020-06-19  4:46       ` Chirantan Ekbote
2020-06-19  4:46         ` Chirantan Ekbote
2020-06-19  8:39         ` Dr. David Alan Gilbert
2020-06-19  9:17           ` Chirantan Ekbote
2020-06-19 11:12             ` Dr. David Alan Gilbert
2020-06-19 19:15         ` Vivek Goyal
2020-06-19 19:15           ` Vivek Goyal
2020-06-25  3:19           ` Chirantan Ekbote
2020-06-25  3:19             ` Chirantan Ekbote
2020-06-25 12:55             ` Vivek Goyal
2020-06-25 12:55               ` Vivek Goyal
2020-07-13  8:54               ` Chirantan Ekbote
2020-07-13  8:54                 ` Chirantan Ekbote
2020-07-13 13:39                 ` Vivek Goyal
2020-07-13 13:39                   ` Vivek Goyal
2020-07-13 21:39                 ` Daniel Walsh
2020-07-14 12:33                   ` Vivek Goyal
2020-07-14 17:16                     ` Daniel Walsh
2020-06-19  8:27       ` Dr. David Alan Gilbert
2020-06-19  8:27         ` Dr. David Alan Gilbert
2020-06-19 11:39         ` Daniel P. Berrangé
2020-06-19 11:39           ` Daniel P. Berrangé
2020-06-19 11:49           ` Dr. David Alan Gilbert
2020-06-19 11:49             ` Dr. David Alan Gilbert
2020-06-19 12:05             ` Daniel P. Berrangé
2020-06-19 12:05               ` Daniel P. Berrangé
2020-06-19 17:41               ` Vivek Goyal
2020-06-19 17:41                 ` Vivek Goyal
2020-06-19 19:12           ` Vivek Goyal
2020-06-19 19:12             ` Vivek Goyal
2020-06-26 11:26             ` Dr. David Alan Gilbert
2020-06-26 11:26               ` Dr. David Alan Gilbert
2020-06-19 16:09         ` Vivek Goyal
2020-06-19 16:09           ` Vivek Goyal
2020-06-19 16:16           ` Dr. David Alan Gilbert
2020-06-19 16:16             ` Dr. David Alan Gilbert
2020-06-19 17:11             ` Vivek Goyal
2020-06-19 17:11               ` Vivek Goyal
2020-06-19 17:16               ` Dr. David Alan Gilbert
2020-06-19 17:16                 ` Dr. David Alan Gilbert
2020-06-19 14:16   ` Miklos Szeredi
2020-06-19 14:16     ` Miklos Szeredi
2020-06-19 14:25     ` Vivek Goyal
2020-06-19 14:25       ` Vivek Goyal
2020-06-19 15:26       ` Miklos Szeredi
2020-06-19 15:26         ` Miklos Szeredi
2020-06-19 15:57         ` Vivek Goyal [this message]
2020-06-19 15:57           ` Vivek Goyal
2020-06-19 14:29     ` Vivek Goyal
2020-06-19 14:29       ` Vivek Goyal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200619155737.GA12225@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=miklos@szeredi.hu \
    --cc=qemu-devel@nongnu.org \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.