On Thu, Mar 18, 2021 at 10:51:29AM +0100, Ondrej Mosnacek wrote:

On Tue, Mar 16, 2021 at 10:09 PM Dave Chinner [off-list ref] wrote:

quoted

On Tue, Mar 16, 2021 at 06:32:26PM +0100, Ondrej Mosnacek wrote:

quoted

diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index bca48b308c02..a99d19c2c11f 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c

@@ -164,7 +164,7 @@ xfs_xattr_put_listent(
               * Only show root namespace entries if we are actually allowed to
               * see them.
               */
-             if (!capable(CAP_SYS_ADMIN))
+             if (!has_capability_noaudit(current, CAP_SYS_ADMIN))
                      return;

              prefix = XATTR_TRUSTED_PREFIX;

This one should absolutely report a denial - someone has tried to
read the trusted xattr namespace without permission to do so. That's
exactly the sort of thing I'd want to see in an audit log - just
because we just elide the xattrs rather than return an error doesn't
mean we should not leave an audit trail from the attempted access
of kernel trusted attributes...

I'm not sure about that... without CAP_SYS_ADMIN the caller would
still get the ACL xattrs, no?

Looks like it, but I have no idea if that's even correct behaviour
or not - access to posix ACL is supposed to be controlled by
the VFS.

IIUC, it's a filter to not show
restricted xattrs to unprivileged users via listxattr(2)**, where the
user is not saying "give me the trusted xattrs", just "give me
whatever I'm allowed to see", so logging the denial wouldn't make much
sense - the user may not even care about trusted xattrs when doing the
syscall (and in 99% of cases a user without CAP_SYS_ADMIN really
won't).

Ok, I keep forgetting that only XFS has attr_list(3) interfaces
(which predate Linux VFS/syscall xattr support, IIRC) and that
interface requires userspace to pass ATTR_ROOT to retrieve trusted
namespace xattrs...

For while it's a silent content filter for one user interface, it's
an explicit request from another user interface. Make of that what
you will....

(**) But I don't understand how exactly that function is used and what
the XFS_ATTR_ROOT flag means, so I may be getting it wrong...

It's the on-disk format flag that says the xattr is in the "root"
namespace (i.e. requires "root" permissions to access). XFS came
from Irix which had different xattr namespaces for system, security,
etc, hence the stuff is stored on XFS is named somewhat differently
compared to native linux filesystems....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com