Thread (21 messages) 21 messages, 7 authors, 2025-02-25

Re: [PATCH v19 0/4] overlayfs override_creds=off & nested get xattr fix

From: Amir Goldstein <amir73il@gmail.com>
Date: 2021-11-17 07:36:57
Also in: linux-doc, linux-fsdevel, linux-unionfs, lkml, selinux

On Wed, Nov 17, 2021 at 3:58 AM David Anderson [off-list ref] wrote:
Mark Salyzyn (3):
  Add flags option to get xattr method paired to __vfs_getxattr
  overlayfs: handle XATTR_NOSECURITY flag for get xattr method
  overlayfs: override_creds=off option bypass creator_cred

Mark Salyzyn + John Stultz (1):
  overlayfs: inode_owner_or_capable called during execv

The first three patches address fundamental security issues that should
be solved regardless of the override_creds=off feature.

The fourth adds the feature depends on these other fixes.

By default, all access to the upper, lower and work directories is the
recorded mounter's MAC and DAC credentials.  The incoming accesses are
checked against the caller's credentials.

If the principles of least privilege are applied for sepolicy, the
mounter's credentials might not overlap the credentials of the caller's
when accessing the overlayfs filesystem.  For example, a file that a
lower DAC privileged caller can execute, is MAC denied to the
generally higher DAC privileged mounter, to prevent an attack vector.

We add the option to turn off override_creds in the mount options; all
subsequent operations after mount on the filesystem will be only the
caller's credentials.  The module boolean parameter and mount option
override_creds is also added as a presence check for this "feature",
existence of /sys/module/overlay/parameters/overlay_creds

Signed-off-by: Mark Salyzyn <redacted>
Signed-off-by: David Anderson <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Eric W. Biederman <redacted>
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Randy Dunlap <redacted>
Cc: Stephen Smalley <redacted>
Cc: John Stultz <redacted>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-unionfs@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: kernel-team@android.com
Cc: selinux@vger.kernel.org
Cc: paulmoore@microsoft.com
Cc: Luca.Boccassi@microsoft.com

---

v19
- rebase.
Hi David,

I see that the patch set has changed hands (presumably to Android upstreaming
team), but you just rebased v18 without addressing the maintainers concerns [1].

Specifically, the patch 2/4 is very wrong for unprivileged mount and
I think that the very noisy patch 1/4 could be completely avoided:
Can't you use -o userxattr mount option for Android use case and limit
the manipulation of user.ovrelay.* xattr based on sepolicy for actors
that are allowed
to make changes in overlayfs mount? or not limit at all?
The access to those xattr is forbidden via "incoming" xattr ops on
overlay inodes.

Also, IMO, the Documentation section about "Non overlapping credentials" does
not hold the same standards as the section about "Permission model" -
it does not
state the requirements clear enough for my non-security-oriented brain to be
able to understand if the "Ignore mounter's credentials" model can be exploited.

Can an unprivileged user create an overlay over a directory that they have
access to and redirect an innocent looking file name to an underlying file that
said the mounting user has no access to and by doing that, tricking a privileged
user to modify the innocent looking file on the  mounter's behalf?
Of course this could be avoided by forbidding unprivileged mount with
override_creds=off, but there could be other scenarios, so a clear model
would help to understand the risks.

For example:
If user 1 was able to read in lower dir A, now the content of overlay dir A
is cached and user 2, that has permissions to read upper dir A and does
not have read permissions on lower dir A will see the content of lower dir A.

I think that the core problem with the approach is using Non-uniform
credentials to access underlying layers. I don't see a simple way around
a big audit that checks all those cases, but maybe I'm missing some quick
shortcut or maybe your use case can add some restrictions about the
users that could access this overlay that would simplify the generic problem.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-unionfs/CAJfpegtMoD85j5namV592sJD23QeUMD=+tq4SvFDqjVxsAszYQ@mail.gmail.com/ (local)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help