All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: Vivek Goyal <vgoyal@redhat.com>,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, virtio-fs@redhat.com,
	dwalsh@redhat.com, christian.brauner@ubuntu.com,
	casey.schaufler@intel.com, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, tytso@mit.edu, miklos@szeredi.hu,
	gscrivan@redhat.com, bfields@redhat.com,
	stephen.smalley.work@gmail.com, agruenba@redhat.com,
	david@fromorbit.com
Subject: Re: [PATCH v3 0/1] Relax restrictions on user.* xattr
Date: Mon, 6 Sep 2021 15:55:28 +0100	[thread overview]
Message-ID: <YTYr4MgWnOgf/SWY@work-vm> (raw)
In-Reply-To: <1f33e6ef-e896-09ef-43b1-6c5fac40ba5f@schaufler-ca.com>

* Casey Schaufler (casey@schaufler-ca.com) wrote:
> On 9/2/2021 1:06 PM, Vivek Goyal wrote:

> >  If LSMs are not configured,
> > then hiding the directory is the solution.
> 
> It's not a solution at all. It's wishful thinking that
> some admin is going to do absolutely everything right, will
> never make a mistake and will never, ever, read the mount(2)
> man page.

That is why we run our virtiofsd with a sandbox setup and seccomp; and
frankly anything we can or could turn on we would.

> > So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> > solution. I don't understand that part.
> 
> It comes back to your design, which is fundamentally flawed. You
> can't store system security information in an attribute that can
> be manipulated by untrusted entities. That's why we have system.*
> xattrs. You want to have an attribute on the host that maps to a
> security attribute on the guest. The host has to protect the attribute
> on the guest with mechanisms of comparable strength as the guest's
> mechanisms.

Can you just explain this line to me a bit more: 
> Otherwise you can't trust the guest with host data.

Note we're not trying to trust the guest with the host data here;
we're trying to allow the guest to store the data on the host, while
trusting the host.

> 
> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
> mechanism as implemented today won't scale to the hundreds of individual
> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
> I'm not convinced that there isn't a way to accomplish what you're
> trying to do without privilege, but this isn't it, and I don't know
> what is. Sorry.
> 
> > Also if directory is not hidden, unprivileged users can change file
> > data and other metadata.
> 
> I assumed that you've taken that into account. Are you saying that
> isn't going to be done correctly either?
> 
> >  Why that's not a concern and why there is
> > so much of focus only security xattr.
> 
> As with an NFS mount, the assumption is that UID 567 (or its magically
> mapped equivalent) has the same access rights on both the server/host
> and client/guest. I'm not worried about the mode bits because they are
> presented consistently on both machines. If, on the other hand, an
> attribute used to determine access is security.esprit on the guest and
> user.security.esprit on the host, the unprivileged user on the host
> can defeat the privilege requirements on the guest. That's why.

We're OK with that; remember that the host can do wth it likes to the
guest anyway - it can just go in and poke at the guests RAM if it wants
to do something evil to the guest.
We wouldn't suggest using a scheme like this once you have
encrypted/protected guest RAM for example (SEV/TDX etc)

> >  If you were to block modification
> > of file then you will have rely on LSMs.
> 
> No. We're talking about the semantics of the xattr namespaces.
> LSMs can further constrain access to xattrs, but the basic rules
> of access to the user.* and security.* attributes are different
> in any case. This is by design.

I'm happy if you can suggest somewhere else to store the guests xattr
data other than in one of the hosts xattr's - the challenge is doing
that in a non-racy way, and making sure that the xattr's never get
associated with the wrong file as seen by a guest.

> >  And if LSMs are not configured,
> > then we will rely on shared directory not being visible.
> 
> LSMs are not the problem. LSMs use security.* xattrs, which is why
> they come up in the discussion.
> 
> > Can you please help me understand why hiding shared directory from
> > unprivileged users is not a solution
> 
> Maybe you can describe the mechanism you use to "hide" a shared directory
> on the host. If the filesystem is mounted on the host it seems unlikely
> that you can provide a convincing argument for sufficient protection.

Why? What can a guests fs mounted on the host, under one of the
directories that's already typically used for container fs's do - it's
already what fileservers, and existing container systems do.

Dave



> >  (With both LSMs configured or
> > not configured on host). That's a requirement for virtiofs anyway. 
> > And if we agree on that, then I don't see why using "user.*" xattrs
> > for storing guest sercurity attributes is a problem.
> >
> > Thanks
> > Vivek
> >
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: gscrivan@redhat.com, tytso@mit.edu, agruenba@redhat.com,
	miklos@szeredi.hu, selinux@vger.kernel.org,
	stephen.smalley.work@gmail.com, david@fromorbit.com,
	linux-kernel@vger.kernel.org, virtio-fs@redhat.com,
	casey.schaufler@intel.com, linux-security-module@vger.kernel.org,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
	bfields@redhat.com, christian.brauner@ubuntu.com,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [Virtio-fs] [PATCH v3 0/1] Relax restrictions on user.* xattr
Date: Mon, 6 Sep 2021 15:55:28 +0100	[thread overview]
Message-ID: <YTYr4MgWnOgf/SWY@work-vm> (raw)
In-Reply-To: <1f33e6ef-e896-09ef-43b1-6c5fac40ba5f@schaufler-ca.com>

* Casey Schaufler (casey@schaufler-ca.com) wrote:
> On 9/2/2021 1:06 PM, Vivek Goyal wrote:

> >  If LSMs are not configured,
> > then hiding the directory is the solution.
> 
> It's not a solution at all. It's wishful thinking that
> some admin is going to do absolutely everything right, will
> never make a mistake and will never, ever, read the mount(2)
> man page.

That is why we run our virtiofsd with a sandbox setup and seccomp; and
frankly anything we can or could turn on we would.

> > So why that's not a solution and only relying on CAP_SYS_ADMIN is the
> > solution. I don't understand that part.
> 
> It comes back to your design, which is fundamentally flawed. You
> can't store system security information in an attribute that can
> be manipulated by untrusted entities. That's why we have system.*
> xattrs. You want to have an attribute on the host that maps to a
> security attribute on the guest. The host has to protect the attribute
> on the guest with mechanisms of comparable strength as the guest's
> mechanisms.

Can you just explain this line to me a bit more: 
> Otherwise you can't trust the guest with host data.

Note we're not trying to trust the guest with the host data here;
we're trying to allow the guest to store the data on the host, while
trusting the host.

> 
> It's a real shame that CAP_SYS_ADMIN is so scary. The capability
> mechanism as implemented today won't scale to the hundreds of individual
> capabilities it would need to break CAP_SYS_ADMIN up. Maybe someday.
> I'm not convinced that there isn't a way to accomplish what you're
> trying to do without privilege, but this isn't it, and I don't know
> what is. Sorry.
> 
> > Also if directory is not hidden, unprivileged users can change file
> > data and other metadata.
> 
> I assumed that you've taken that into account. Are you saying that
> isn't going to be done correctly either?
> 
> >  Why that's not a concern and why there is
> > so much of focus only security xattr.
> 
> As with an NFS mount, the assumption is that UID 567 (or its magically
> mapped equivalent) has the same access rights on both the server/host
> and client/guest. I'm not worried about the mode bits because they are
> presented consistently on both machines. If, on the other hand, an
> attribute used to determine access is security.esprit on the guest and
> user.security.esprit on the host, the unprivileged user on the host
> can defeat the privilege requirements on the guest. That's why.

We're OK with that; remember that the host can do wth it likes to the
guest anyway - it can just go in and poke at the guests RAM if it wants
to do something evil to the guest.
We wouldn't suggest using a scheme like this once you have
encrypted/protected guest RAM for example (SEV/TDX etc)

> >  If you were to block modification
> > of file then you will have rely on LSMs.
> 
> No. We're talking about the semantics of the xattr namespaces.
> LSMs can further constrain access to xattrs, but the basic rules
> of access to the user.* and security.* attributes are different
> in any case. This is by design.

I'm happy if you can suggest somewhere else to store the guests xattr
data other than in one of the hosts xattr's - the challenge is doing
that in a non-racy way, and making sure that the xattr's never get
associated with the wrong file as seen by a guest.

> >  And if LSMs are not configured,
> > then we will rely on shared directory not being visible.
> 
> LSMs are not the problem. LSMs use security.* xattrs, which is why
> they come up in the discussion.
> 
> > Can you please help me understand why hiding shared directory from
> > unprivileged users is not a solution
> 
> Maybe you can describe the mechanism you use to "hide" a shared directory
> on the host. If the filesystem is mounted on the host it seems unlikely
> that you can provide a convincing argument for sufficient protection.

Why? What can a guests fs mounted on the host, under one of the
directories that's already typically used for container fs's do - it's
already what fileservers, and existing container systems do.

Dave



> >  (With both LSMs configured or
> > not configured on host). That's a requirement for virtiofs anyway. 
> > And if we agree on that, then I don't see why using "user.*" xattrs
> > for storing guest sercurity attributes is a problem.
> >
> > Thanks
> > Vivek
> >
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


  parent reply	other threads:[~2021-09-06 14:55 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 15:22 [PATCH v3 0/1] Relax restrictions on user.* xattr Vivek Goyal
2021-09-02 15:22 ` [Virtio-fs] " Vivek Goyal
2021-09-02 15:22 ` [PATCH v3 1/1] xattr: Allow user.* xattr on symlink and special files Vivek Goyal
2021-09-02 15:22   ` [Virtio-fs] " Vivek Goyal
2021-09-02 15:38 ` [PATCH 2/1] man-pages: xattr.7: Update text for user extended xattr behavior change Vivek Goyal
2021-09-02 15:38   ` [Virtio-fs] " Vivek Goyal
2021-09-02 15:43 ` [PATCH v3 0/1] Relax restrictions on user.* xattr Casey Schaufler
2021-09-02 15:43   ` [Virtio-fs] " Casey Schaufler
2021-09-02 17:05   ` Vivek Goyal
2021-09-02 17:05     ` [Virtio-fs] " Vivek Goyal
2021-09-02 17:42     ` Vivek Goyal
2021-09-02 17:42       ` [Virtio-fs] " Vivek Goyal
2021-09-02 18:55       ` Casey Schaufler
2021-09-02 18:55         ` [Virtio-fs] " Casey Schaufler
2021-09-02 20:06         ` Vivek Goyal
2021-09-02 20:06           ` [Virtio-fs] " Vivek Goyal
2021-09-02 22:34           ` Casey Schaufler
2021-09-02 22:34             ` [Virtio-fs] " Casey Schaufler
2021-09-03 15:26             ` Vivek Goyal
2021-09-03 15:26               ` [Virtio-fs] " Vivek Goyal
2021-09-03 18:49               ` Casey Schaufler
2021-09-03 18:49                 ` [Virtio-fs] " Casey Schaufler
2021-09-06  7:45             ` Sergio Lopez
2021-09-06  7:45               ` Sergio Lopez
2021-09-06 14:55             ` Dr. David Alan Gilbert [this message]
2021-09-06 14:55               ` Dr. David Alan Gilbert
2021-09-13 19:05               ` Casey Schaufler
2021-09-13 19:05                 ` [Virtio-fs] " Casey Schaufler
2021-09-14 12:51                 ` Vivek Goyal
2021-09-14 12:51                   ` [Virtio-fs] " Vivek Goyal
2021-09-14 13:56                   ` Casey Schaufler
2021-09-14 13:56                     ` [Virtio-fs] " Casey Schaufler
2021-09-14 13:59                   ` Bruce Fields
2021-09-14 13:59                     ` [Virtio-fs] " Bruce Fields
2021-09-14 14:32                     ` Vivek Goyal
2021-09-14 14:32                       ` [Virtio-fs] " Vivek Goyal
2021-09-14 15:01                       ` Bruce Fields
2021-09-14 15:01                         ` [Virtio-fs] " Bruce Fields
2021-09-15 16:33                       ` Dr. Greg
2021-09-15 16:33                         ` [Virtio-fs] " Dr. Greg
2021-09-02 15:47 ` [PATCH 3/1] xfstests: generic/062: Do not run on newer kernels Vivek Goyal
2021-09-02 15:47   ` [Virtio-fs] " Vivek Goyal
2021-09-03  4:55   ` Dave Chinner
2021-09-03  4:55     ` [Virtio-fs] " Dave Chinner
2021-09-03  6:31   ` Andreas Gruenbacher
2021-09-03  6:31     ` [Virtio-fs] " Andreas Gruenbacher
2021-09-03  6:56     ` Andreas Gruenbacher
2021-09-03  6:56       ` [Virtio-fs] " Andreas Gruenbacher
2021-09-03 14:42       ` Bruce Fields
2021-09-03 14:42         ` [Virtio-fs] " Bruce Fields
2021-09-03 15:43         ` Vivek Goyal
2021-09-03 15:43           ` [Virtio-fs] " Vivek Goyal
2021-09-03 15:50           ` Bruce Fields
2021-09-03 15:50             ` [Virtio-fs] " Bruce Fields
2021-09-03 16:01             ` Casey Schaufler
2021-09-03 16:01               ` [Virtio-fs] " Casey Schaufler
2021-09-03 16:03             ` Vivek Goyal
2021-09-03 16:03               ` [Virtio-fs] " Vivek Goyal
2021-09-03  6:31   ` Zorro Lang
2021-09-03  6:31     ` [Virtio-fs] " Zorro Lang
2021-09-02 15:50 ` [PATCH 4/1] xfstest: Add a new test to test xattr operations Vivek Goyal
2021-09-02 15:50   ` [Virtio-fs] " Vivek Goyal
2021-09-02 17:52 ` [PATCH v3 0/1] Relax restrictions on user.* xattr Andreas Gruenbacher
2021-09-02 17:52   ` [Virtio-fs] " Andreas Gruenbacher
2021-09-02 18:48   ` Vivek Goyal
2021-09-02 18:48     ` [Virtio-fs] " Vivek Goyal
2021-09-02 19:19     ` Casey Schaufler
2021-09-02 19:19       ` [Virtio-fs] " Casey Schaufler
2021-09-06 14:39   ` Dr. David Alan Gilbert
2021-09-06 14:39     ` [Virtio-fs] " Dr. David Alan Gilbert
2021-09-06 14:56     ` Miklos Szeredi
2021-09-06 14:56       ` [Virtio-fs] " Miklos Szeredi
2021-09-07 21:40       ` Vivek Goyal
2021-09-07 21:40         ` [Virtio-fs] " Vivek Goyal
2021-09-08  7:37         ` Miklos Szeredi
2021-09-08  7:37           ` [Virtio-fs] " Miklos Szeredi
2021-09-08 14:20           ` Eric W. Biederman
2021-09-08 14:20             ` [Virtio-fs] " Eric W. Biederman

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=YTYr4MgWnOgf/SWY@work-vm \
    --to=dgilbert@redhat.com \
    --cc=agruenba@redhat.com \
    --cc=bfields@redhat.com \
    --cc=casey.schaufler@intel.com \
    --cc=casey@schaufler-ca.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=david@fromorbit.com \
    --cc=dwalsh@redhat.com \
    --cc=gscrivan@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=selinux@vger.kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tytso@mit.edu \
    --cc=vgoyal@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --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.