Skip to content

Commit

Permalink
main: Isolate security xattrs for STAT_OVERRIDE_CONTAINERS
Browse files Browse the repository at this point in the history 
The major use case of stat override is to enable rootless containers
on network filesystems, and they also lack security xattr support in
non-root user namespaces. Trying to set security xattrs on them result
in ENOTSUP and break things.

It makes little sense to share security xattrs with the underlying
filesystems when overriding stat in the first place. Linux's NFS server
exposes security xattrs only when the user explicitly claims the
security consistencies between the server and clients, and hide them
otherwise. Following this precedent, we should isolate security xattrs
since we know the security policy enforced by fuse-overlayfs is already
distinct from the underlying filesystem when overriding owners and file
mode.

Mark security xattrs inaccessible with STAT_OVERRIDE_CONTAINERS to
prefix all access to them with XATTR_CONTAINERS_OVERRIDE_PREFIX.

Signed-off-by: Akihiko Odaki <[email protected]>
  • Loading branch information
@akihikodaki
akihikodaki committed Jun 17, 2024
1 parent 9810b85 commit 20161f9
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 21 deletions.
46 changes: 25 additions & 21 deletions main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#include <sys/sysmacros.h>
#include <sys/xattr.h>
#include <linux/fs.h>
#include <linux/xattr.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <pthread.h>
Expand Down Expand Up @@ -525,25 +526,27 @@ has_prefix (const char *str, const char *pref)
}

static bool
can_access_xattr (const char *name)
can_access_xattr (const struct ovl_layer *l, const char *name)
{
return ! has_prefix (name, XATTR_PREFIX)
&& ! has_prefix (name, PRIVILEGED_XATTR_PREFIX)
&& ! has_prefix (name, UNPRIVILEGED_XATTR_PREFIX);
return ! (has_prefix (name, XATTR_PREFIX)
|| has_prefix (name, PRIVILEGED_XATTR_PREFIX)
|| has_prefix (name, UNPRIVILEGED_XATTR_PREFIX)
|| (l->stat_override_mode == STAT_OVERRIDE_CONTAINERS &&
has_prefix (name, XATTR_SECURITY_PREFIX)));
}

static bool encoded_xattr_name (const char *name)
static bool encoded_xattr_name (const struct ovl_layer *l, const char *name)
{
return has_prefix (name, XATTR_CONTAINERS_OVERRIDE_PREFIX) &&
! can_access_xattr (name + sizeof(XATTR_CONTAINERS_OVERRIDE_PREFIX) - 1);
! can_access_xattr (l, name + sizeof(XATTR_CONTAINERS_OVERRIDE_PREFIX) - 1);
}

static const char *decode_xattr_name (const char *name)
static const char *decode_xattr_name (const struct ovl_layer *l, const char *name)
{
if (encoded_xattr_name (name))
if (encoded_xattr_name (l, name))
return name + sizeof(XATTR_CONTAINERS_OVERRIDE_PREFIX) - 1;

if (can_access_xattr (name))
if (can_access_xattr (l, name))
return name;

return NULL;
Expand All @@ -552,7 +555,7 @@ static const char *decode_xattr_name (const char *name)
static const char *encode_xattr_name (const struct ovl_layer *l, char *buf,
const char *name)
{
if (can_access_xattr (name))
if (can_access_xattr (l, name))
return name;

if (l->stat_override_mode != STAT_OVERRIDE_CONTAINERS ||
Expand Down Expand Up @@ -2617,7 +2620,7 @@ inherit_acl (struct ovl_data *lo, struct ovl_node *parent, int targetfd, const c

/* in-place filter xattrs that cannot be accessed. */
static ssize_t
filter_xattrs_list (char *buf, ssize_t len)
filter_xattrs_list (struct ovl_layer *l, char *buf, ssize_t len)
{
ssize_t ret = 0;
char *it;
Expand All @@ -2633,7 +2636,7 @@ filter_xattrs_list (char *buf, ssize_t len)

it_len = strlen (it) + 1;

if (can_access_xattr (it))
if (can_access_xattr (l, it))
{
it += it_len;
ret += it_len;
Expand All @@ -2642,7 +2645,7 @@ filter_xattrs_list (char *buf, ssize_t len)
{
char *next = it;

next += encoded_xattr_name (it) ?
next += encoded_xattr_name (l, it) ?
sizeof(XATTR_CONTAINERS_OVERRIDE_PREFIX) - 1 : it_len;

memmove (it, next, buf + len - next);
Expand Down Expand Up @@ -2703,7 +2706,7 @@ ovl_listxattr (fuse_req_t req, fuse_ino_t ino, size_t size)
return;
}

len = filter_xattrs_list (buf, ret);
len = filter_xattrs_list (node->layer, buf, ret);

if (size == 0)
fuse_reply_xattr (req, len);
Expand Down Expand Up @@ -2796,7 +2799,7 @@ ovl_access (fuse_req_t req, fuse_ino_t ino, int mask)
}

static int
copy_xattr (int sfd,
copy_xattr (const struct ovl_layer *sl, int sfd,
const struct ovl_layer *dl, int dfd, char *buf, size_t buf_size)
{
ssize_t xattr_len;
Expand All @@ -2808,7 +2811,7 @@ copy_xattr (int sfd,
for (it = buf; it - buf < xattr_len; it += strlen (it) + 1)
{
cleanup_free char *v = NULL;
const char *decoded_name = decode_xattr_name (it);
const char *decoded_name = decode_xattr_name (sl, it);
const char *encoded_name;
char buf[XATTR_NAME_MAX + 1];
ssize_t s;
Expand Down Expand Up @@ -2904,7 +2907,8 @@ static int create_node_directory (struct ovl_data *lo, struct ovl_node *src);

static int
create_directory (struct ovl_data *lo, int dirfd, const char *name, const struct timespec *times,
struct ovl_node *parent, int xattr_sfd, uid_t uid, gid_t gid, mode_t mode, bool set_opaque, struct stat *st_out)
struct ovl_node *parent, struct ovl_layer *sl, int xattr_sfd,
uid_t uid, gid_t gid, mode_t mode, bool set_opaque, struct stat *st_out)
{
int ret;
int saved_errno;
Expand Down Expand Up @@ -2968,7 +2972,7 @@ create_directory (struct ovl_data *lo, int dirfd, const char *name, const struct
goto out;
}

ret = copy_xattr (xattr_sfd, get_upper_layer (lo), dfd, buf, buf_size);
ret = copy_xattr (sl, xattr_sfd, get_upper_layer (lo), dfd, buf, buf_size);
if (ret < 0)
goto out;
}
Expand Down Expand Up @@ -3061,7 +3065,7 @@ create_node_directory (struct ovl_data *lo, struct ovl_node *src)
if (override_mode (src->layer, sfd, NULL, NULL, &st) < 0 && errno != ENODATA && errno != EOPNOTSUPP)
return -1;

ret = create_directory (lo, get_upper_layer (lo)->fd, src->path, times, src->parent, sfd, st.st_uid, st.st_gid, st.st_mode, false, NULL);
ret = create_directory (lo, get_upper_layer (lo)->fd, src->path, times, src->parent, src->layer, sfd, st.st_uid, st.st_gid, st.st_mode, false, NULL);
if (ret == 0)
{
src->layer = get_upper_layer (lo);
Expand Down Expand Up @@ -3240,7 +3244,7 @@ copyup (struct ovl_data *lo, struct ovl_node *node)
if (ret < 0)
goto exit;

ret = copy_xattr (sfd, get_upper_layer (lo), dfd, buf, buf_size);
ret = copy_xattr (node->layer, sfd, get_upper_layer (lo), dfd, buf, buf_size);
if (ret < 0)
goto exit;

Expand Down Expand Up @@ -5165,7 +5169,7 @@ ovl_mkdir (fuse_req_t req, fuse_ino_t parent, const char *name, mode_t mode)
return;
}

ret = create_directory (lo, get_upper_layer (lo)->fd, path, NULL, pnode, -1,
ret = create_directory (lo, get_upper_layer (lo)->fd, path, NULL, pnode, NULL, -1,
get_uid (lo, ctx->uid), get_gid (lo, ctx->gid), mode & ~ctx->umask,
true, &st);
if (ret < 0)
Expand Down
6 changes: 6 additions & 0 deletions tests/unpriv.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,15 @@ rm -rf lower upper workdir merged
mkdir lower upper workdir merged

touch upper/file
unshare -r setcap cap_net_admin+ep upper/file

fuse-overlayfs -o lowerdir=lower,upperdir=upper,workdir=workdir,xattr_permissions=2 merged

# Ensure the security xattr namespace is isolated.
test "$(unshare -r getcap merged/file)" = ''
unshare -r setcap cap_net_admin+ep merged/file
test "$(unshare -r getcap merged/file)" = 'merged/file cap_net_admin=ep'

# Ensure UID is preserved with chgrp.
podman unshare chgrp 1 merged/file
test $(podman unshare stat -c %u:%g merged/file) = 0:1
Expand Down

0 comments on commit 20161f9

Please sign in to comment.