linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve French <smfrench@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Christian Brauner <brauner@kernel.org>,
	linux-fsdevel@vger.kernel.org, Seth Forshee <sforshee@kernel.org>,
	Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	linux-security-module@vger.kernel.org,
	Steve French <sfrench@samba.org>
Subject: Re: [PATCH v4 04/30] fs: add new get acl method
Date: Tue, 4 Oct 2022 14:53:41 -0500	[thread overview]
Message-ID: <CAH2r5muRDdy1s4xS7bHePEF3t84qGaX3rDXUgGLY1k_XG4vuAg@mail.gmail.com> (raw)
In-Reply-To: <CAJfpegsu9r84J-3wN=z8OOzHd+7YRBn9CNFMDWSbftCEm0e27A@mail.gmail.com>

On Fri, Sep 30, 2022 at 5:06 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, 30 Sept 2022 at 11:09, Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Sep 30, 2022 at 10:53:05AM +0200, Miklos Szeredi wrote:
> > > On Thu, 29 Sept 2022 at 17:31, Christian Brauner <brauner@kernel.org> wrote:
> > >
> > > > This adds a new ->get_acl() inode operations which takes a dentry
> > > > argument which filesystems such as 9p, cifs, and overlayfs can implement
> > > > to get posix acls.
...
> > > So what's the difference and why do we need both?  If one can retrive
> > > the acl without dentry, then why do we need the one with the dentry?
> >
> > The ->get_inode_acl() method is called during generic_permission() and
> > inode_permission() both of which are called from various filesystems in
> > their ->permission inode operations. There's no dentry available during
> > the permission inode operation and there are filesystems like 9p and
> > cifs that need a dentry.
>
> This doesn't answer the question about why we need two for overlayfs
> and what's the difference between them.
> >
> > > If a filesystem cannot implement a get_acl() without a dentry, then
> > > what will happen to caller's that don't have a dentry?
> >
> > This happens today for cifs where posix acls can be created and read but
> > they cannot be used for permission checking where no inode is available.
> > New filesystems shouldn't have this issue.

Can you give an example of this?   How can you read an ACL without an
inode or open file struct?  ACL wouldn't fit in a dentry right?  By
the way there is an option that we can use on open to return the
"maximal access" that that user/group has for the file (a 32 bit mask
showing whether the effective user has read, write, append, read
attributes, write acl etc. permissions).  Would this be helpful for
you to have us do when you revalidate dentries?

> That's weird, how does it make sense to set acl on a filesystem that
> cannot use it for permission checking?   Maybe the permission checking
> is done by the server?
>
> Steve?

It doesn't do much good to check if user1 on client1 can access the
file on server if any user on client2 can access the file - unless the
server is checking ACLs, so the server checks are the more important
ones.

The permission checking on the client doesn't really matter in many
scenarios (the security checks that matter are usually only on the
server).    The ACLs are stored on the server and evaluated by the
server so duplicating ACL evaluation on BOTH client and server only
helps in cases where the server doesn't know who the local Linux user
is (e.g. single user mounts - where all local users use the same
authenticated session).  It is common e.g. to mount with "noperm"
mount option - in which case the client checks are turned off (since
the server ones are the checks that matter the most).

Note that the SMB3 protocol (and also NFSv4.1/4.2) support a richer
ACL model on the server that is more secure (or at least more
granular) in some scenarios than the simpler POSIX ACL model.

Are there examples of how this would work for the richacl examples
(e.g. NFSv4.1/4.2 or cifs.ko or NTFS or ...)?


-- 
Thanks,

Steve

  parent reply	other threads:[~2022-10-04 19:53 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 15:30 [PATCH v4 00/30] acl: add vfs posix acl api Christian Brauner
2022-09-29 15:30 ` [PATCH v4 01/30] orangefs: rework posix acl handling when creating new filesystem objects Christian Brauner
2022-09-29 15:30 ` [PATCH v4 02/30] fs: pass dentry to set acl method Christian Brauner
2022-09-29 15:30 ` [PATCH v4 03/30] fs: rename current get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 04/30] fs: add new " Christian Brauner
2022-09-30  8:53   ` Miklos Szeredi
2022-09-30  9:09     ` Christian Brauner
2022-09-30  9:43       ` Miklos Szeredi
2022-09-30 10:05         ` Christian Brauner
2022-09-30 12:24           ` Miklos Szeredi
2022-09-30 12:49             ` Christian Brauner
2022-09-30 13:01               ` Miklos Szeredi
2022-09-30 13:51                 ` Christian Brauner
2022-10-04 19:53         ` Steve French [this message]
2022-10-05  7:15           ` Christian Brauner
2022-10-06  6:31             ` Miklos Szeredi
2022-10-06  7:40               ` Christian Brauner
2022-10-06  9:07                 ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 05/30] cifs: implement " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 06/30] cifs: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 07/30] 9p: implement get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 08/30] 9p: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 09/30] security: add get, remove and set acl hook Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-29 15:30 ` [PATCH v4 10/30] selinux: implement get, set and remove " Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-30  8:38     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 11/30] smack: " Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-30  8:40     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 12/30] integrity: implement get and set " Christian Brauner
2022-09-29 19:14   ` Paul Moore
2022-09-30  3:19     ` Mimi Zohar
2022-09-30 14:11       ` Paul Moore
2022-09-30  8:11     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 13/30] evm: add post " Christian Brauner
2022-09-30  1:44   ` Mimi Zohar
2022-09-30  2:51     ` Mimi Zohar
2022-09-30  8:44     ` Christian Brauner
2022-09-30 11:48       ` Mimi Zohar
2022-10-04  7:04         ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 14/30] internal: add may_write_xattr() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 15/30] acl: add vfs_set_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 16/30] acl: add vfs_get_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 17/30] acl: add vfs_remove_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 18/30] ksmbd: use vfs_remove_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 19/30] ecryptfs: implement get acl method Christian Brauner
2022-09-29 15:30 ` [PATCH v4 20/30] ecryptfs: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 21/30] ovl: implement get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 22/30] ovl: implement set " Christian Brauner
2022-10-06 12:39   ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 23/30] ovl: use posix acl api Christian Brauner
2022-10-06 12:50   ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 24/30] xattr: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 25/30] evm: remove evm_xattr_acl_change() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 26/30] ecryptfs: use stub posix acl handlers Christian Brauner
2022-09-29 15:30 ` [PATCH v4 27/30] ovl: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 28/30] cifs: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 29/30] 9p: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 30/30] acl: remove a slew of now unused helpers Christian Brauner

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=CAH2r5muRDdy1s4xS7bHePEF3t84qGaX3rDXUgGLY1k_XG4vuAg@mail.gmail.com \
    --to=smfrench@gmail.com \
    --cc=brauner@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=sforshee@kernel.org \
    --cc=sfrench@samba.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).