From: Miklos Szeredi <miklos@szeredi.hu>
To: Mark Salyzyn <salyzyn@android.com>
Cc: linux-kernel@vger.kernel.org,
kernel-team <kernel-team@android.com>,
linux-fsdevel@vger.kernel.org,
overlayfs <linux-unionfs@vger.kernel.org>,
Stephen Smalley <sds@tycho.nsa.gov>,
LSM <linux-security-module@vger.kernel.org>,
Jonathan Corbet <corbet@lwn.net>, Vivek Goyal <vgoyal@redhat.com>,
"Eric W . Biederman" <ebiederm@xmission.com>,
Amir Goldstein <amir73il@gmail.com>,
linux-doc@vger.kernel.org, SElinux list <selinux@vger.kernel.org>
Subject: Re: [RESEND PATCH v18 2/4] overlayfs: handle XATTR_NOSECURITY flag for get xattr method
Date: Fri, 30 Oct 2020 16:07:00 +0100 [thread overview]
Message-ID: <CAJfpegtMoD85j5namV592sJD23QeUMD=+tq4SvFDqjVxsAszYQ@mail.gmail.com> (raw)
In-Reply-To: <20201021151903.652827-3-salyzyn@android.com>
On Wed, Oct 21, 2020 at 5:19 PM Mark Salyzyn <salyzyn@android.com> wrote:
>
> Because of the overlayfs getxattr recursion, the incoming inode fails
> to update the selinux sid resulting in avc denials being reported
> against a target context of u:object_r:unlabeled:s0.
>
> Solution is to respond to the XATTR_NOSECURITY flag in get xattr
> method that calls the __vfs_getxattr handler instead so that the
> context can be read in, rather than being denied with an -EACCES
> when vfs_getxattr handler is called.
>
> For the use case where access is to be blocked by the security layer.
>
> The path then would be security(dentry) ->
> __vfs_getxattr({dentry...XATTR_NOSECURITY}) ->
> handler->get({dentry...XATTR_NOSECURITY}) ->
> __vfs_getxattr({realdentry...XATTR_NOSECURITY}) ->
> lower_handler->get({realdentry...XATTR_NOSECURITY}) which
> would report back through the chain data and success as expected,
> the logging security layer at the top would have the data to
> determine the access permissions and report back to the logs and
> the caller that the target context was blocked.
>
> For selinux this would solve the cosmetic issue of the selinux log
> and allow audit2allow to correctly report the rule needed to address
> the access problem.
>
> Check impure, opaque, origin & meta xattr with no sepolicy audit
> (using __vfs_getxattr) since these operations are internal to
> overlayfs operations and do not disclose any data. This became
> an issue for credential override off since sys_admin would have
> been required by the caller; whereas would have been inherently
> present for the creator since it performed the mount.
>
> This is a change in operations since we do not check in the new
> ovl_do_getxattr function if the credential override is off or not.
> Reasoning is that the sepolicy check is unnecessary overhead,
> especially since the check can be expensive.
>
> Because for override credentials off, this affects _everyone_ that
> underneath performs private xattr calls without the appropriate
> sepolicy permissions and sys_admin capability. Providing blanket
> support for sys_admin would be bad for all possible callers.
>
> For the override credentials on, this will affect only the mounter,
> should it lack sepolicy permissions. Not considered a security
> problem since mounting by definition has sys_admin capabilities,
> but sepolicy contexts would still need to be crafted.
This would be a problem when unprivileged mounting of overlay is
introduced. I'd really like to avoid weakening the current security
model.
The big API churn in the 1/4 patch also seems excessive considering
that this seems to be mostly a cosmetic issue for android. Am I
missing something?
Thanks,
Miklos
next prev parent reply other threads:[~2020-10-30 15:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-21 15:18 [RESEND PATCH v18 0/4] overlayfs override_creds=off & nested get xattr fix Mark Salyzyn
2020-10-21 15:19 ` [RESEND PATCH v18 1/4] Add flags option to get xattr method paired to __vfs_getxattr Mark Salyzyn
2020-10-21 15:19 ` [RESEND PATCH v18 2/4] overlayfs: handle XATTR_NOSECURITY flag for get xattr method Mark Salyzyn
2020-10-30 15:07 ` Miklos Szeredi [this message]
2020-10-30 16:00 ` Mark Salyzyn
2021-02-05 18:01 ` Tyler Hicks
2021-02-12 19:04 ` Tyler Hicks
2020-10-21 15:19 ` [RESEND PATCH v18 3/4] overlayfs: override_creds=off option bypass creator_cred Mark Salyzyn
2020-10-22 5:19 ` [RESEND PATCH v18 0/4] overlayfs override_creds=off & nested get xattr fix Eric Biggers
2020-10-22 13:18 ` Mark Salyzyn
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='CAJfpegtMoD85j5namV592sJD23QeUMD=+tq4SvFDqjVxsAszYQ@mail.gmail.com' \
--to=miklos@szeredi.hu \
--cc=amir73il@gmail.com \
--cc=corbet@lwn.net \
--cc=ebiederm@xmission.com \
--cc=kernel-team@android.com \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=salyzyn@android.com \
--cc=sds@tycho.nsa.gov \
--cc=selinux@vger.kernel.org \
--cc=vgoyal@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 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).