From: Christian Brauner <brauner@kernel.org>
To: Paul Moore <paul@paul-moore.com>
Cc: linux-fsdevel@vger.kernel.org, Seth Forshee <sforshee@kernel.org>,
Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
linux-integrity@vger.kernel.org,
Stephen Smalley <stephen.smalley.work@gmail.com>,
Eric Paris <eparis@parisplace.org>,
selinux@vger.kernel.org, linux-security-module@vger.kernel.org
Subject: Re: [PATCH v4 10/30] selinux: implement get, set and remove acl hook
Date: Fri, 30 Sep 2022 10:38:04 +0200 [thread overview]
Message-ID: <20220930083804.eiar274qhclpo5uw@wittgenstein> (raw)
In-Reply-To: <CAHC9VhSHSk9MNK+FmydGTZDzDOuwF0b1A3SqYhG+X0NSCwoUEg@mail.gmail.com>
On Thu, Sep 29, 2022 at 03:15:17PM -0400, Paul Moore wrote:
> On Thu, Sep 29, 2022 at 11:31 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > The current way of setting and getting posix acls through the generic
> > xattr interface is error prone and type unsafe. The vfs needs to
> > interpret and fixup posix acls before storing or reporting it to
> > userspace. Various hacks exist to make this work. The code is hard to
> > understand and difficult to maintain in it's current form. Instead of
> > making this work by hacking posix acls through xattr handlers we are
> > building a dedicated posix acl api around the get and set inode
> > operations. This removes a lot of hackiness and makes the codepaths
> > easier to maintain. A lot of background can be found in [1].
> >
> > So far posix acls were passed as a void blob to the security and
> > integrity modules. Some of them like evm then proceed to interpret the
> > void pointer and convert it into the kernel internal struct posix acl
> > representation to perform their integrity checking magic. This is
> > obviously pretty problematic as that requires knowledge that only the
> > vfs is guaranteed to have and has lead to various bugs. Add a proper
> > security hook for setting posix acls and pass down the posix acls in
> > their appropriate vfs format instead of hacking it through a void
> > pointer stored in the uapi format.
> >
> > I spent considerate time in the security module infrastructure and
> > audited all codepaths. SELinux has no restrictions based on the posix
> > acl values passed through it. The capability hook doesn't need to be
> > called either because it only has restrictions on security.* xattrs. So
> > these are all fairly simply hooks for SELinux.
> >
> > Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org [1]
> > Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
> > ---
> >
> > Notes:
> > /* v2 */
> > unchanged
> >
> > /* v3 */
> > Paul Moore <paul@paul-moore.com>:
> > - Add get, and remove acl hook
> >
> > /* v4 */
> > unchanged
> >
> > security/selinux/hooks.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
>
> One small nitpick below, but looks good regardless.
>
> Acked-by: Paul Moore <paul@paul-moore.com>
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 79573504783b..0e3cd67e5e92 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3239,6 +3239,27 @@ static int selinux_inode_setxattr(struct user_namespace *mnt_userns,
> > &ad);
> > }
> >
> > +static int selinux_inode_set_acl(struct user_namespace *mnt_userns,
> > + struct dentry *dentry, const char *acl_name,
> > + struct posix_acl *kacl)
> > +{
> > + return dentry_has_perm(current_cred(), dentry, FILE__SETATTR);
> > +}
> > +
> > +static int selinux_inode_get_acl(struct user_namespace *mnt_userns,
> > + struct dentry *dentry, const char *acl_name)
> > +{
> > + const struct cred *cred = current_cred();
> > +
> > + return dentry_has_perm(cred, dentry, FILE__GETATTR);
> > +}
>
> Both the set and remove hooks use current_cred() directly in the call
> to dentry_has_perm(), you might as well do the same in the get hook.
Done.
next prev parent reply other threads:[~2022-09-30 8:38 UTC|newest]
Thread overview: 17+ 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 10/30] selinux: implement get, set and remove acl hook Christian Brauner
2022-09-29 19:15 ` Paul Moore
2022-09-30 8:38 ` Christian Brauner [this message]
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 15/30] acl: add vfs_set_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 25/30] evm: remove evm_xattr_acl_change() 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=20220930083804.eiar274qhclpo5uw@wittgenstein \
--to=brauner@kernel.org \
--cc=eparis@parisplace.org \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=selinux@vger.kernel.org \
--cc=sforshee@kernel.org \
--cc=stephen.smalley.work@gmail.com \
--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).