[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/3] virtiofsd: Add an option to enable/disable posix acls
From: |
Vivek Goyal |
Subject: |
Re: [PATCH v2 1/3] virtiofsd: Add an option to enable/disable posix acls |
Date: |
Thu, 18 Feb 2021 10:04:36 -0500 |
On Wed, Feb 17, 2021 at 06:30:44PM -0500, Vivek Goyal wrote:
> fuse has an option FUSE_POSIX_ACL which needs to be opted in by fuse
> server to enable posix acls.
>
> Add virtiofsd option "-o posix_acl/no_posix_acl" to let users enable/disable
> posix acl support. By default it is disabled as of now.
>
> Currently even if file server has not opted in for FUSE_POSIX_ACL, user can
> still query acl and set acl, and system.posix_acl_access and
> system.posix_acl_default xattrs show up listxattr response.
>
> Miklos said this is confusing. So he said lets block and filter
> system.posix_acl_access and system.posix_acl_default xattrs in
> getxattr/setxattr/listxattr if user has explicitly disabled
> posix acls using -o no_posix_acl.
I forgot to block acl xattrs in lo_removexattr(). Will respin this patch.
Vivek
>
> As of now continuing to keeping the existing behavior if user did not
> specify any option to disable acl support due to concerns about backward
> compatibility.
>
> v2: block system.posix_acl_access and system.posix_acl_default xattrs
> if user explicitly disabled acls. (Miklos)
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 95 +++++++++++++++++++++++++++++++-
> 1 file changed, 94 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c
> b/tools/virtiofsd/passthrough_ll.c
> index 58d24c0010..26cdfbd1f0 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -169,6 +169,7 @@ struct lo_data {
> /* An O_PATH file descriptor to /proc/self/fd/ */
> int proc_self_fd;
> int user_killpriv_v2, killpriv_v2;
> + int user_posix_acl;
> };
>
> static const struct fuse_opt lo_opts[] = {
> @@ -201,6 +202,8 @@ static const struct fuse_opt lo_opts[] = {
> { "announce_submounts", offsetof(struct lo_data, announce_submounts), 1
> },
> { "killpriv_v2", offsetof(struct lo_data, user_killpriv_v2), 1 },
> { "no_killpriv_v2", offsetof(struct lo_data, user_killpriv_v2), 0 },
> + { "posix_acl", offsetof(struct lo_data, user_posix_acl), 1 },
> + { "no_posix_acl", offsetof(struct lo_data, user_posix_acl), 0 },
> FUSE_OPT_END
> };
> static bool use_syslog = false;
> @@ -661,6 +664,23 @@ static void lo_init(void *userdata, struct
> fuse_conn_info *conn)
> conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
> lo->killpriv_v2 = 0;
> }
> +
> + if (lo->user_posix_acl == 1) {
> + /*
> + * User explicitly asked for this option. Enable it unconditionally.
> + * If connection does not have this capability, it should fail
> + * in fuse_lowlevel.c
> + */
> + fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling posix acl\n");
> + conn->want |= FUSE_CAP_POSIX_ACL;
> + } else {
> + /*
> + * Either user specified to disable posix_acl, or did not specify
> + * anything. In both the cases do not enable posix acl.
> + */
> + fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling posix_acl\n");
> + conn->want &= ~FUSE_CAP_POSIX_ACL;
> + }
> }
>
> static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
> @@ -2612,6 +2632,63 @@ static int xattr_map_server(const struct lo_data *lo,
> const char *server_name,
> return -ENODATA;
> }
>
> +static bool block_xattr(struct lo_data *lo, const char *name)
> +{
> + /*
> + * If user explicitly enabled posix_acl or did not provide any option,
> + * do not block acl. Otherwise block system.posix_acl_access and
> + * system.posix_acl_default xattrs.
> + */
> + if (lo->user_posix_acl) {
> + return false;
> + }
> + if (!strcmp(name, "system.posix_acl_access") ||
> + !strcmp(name, "system.posix_acl_default"))
> + return true;
> +
> + return false;
> +}
> +
> +/*
> + * Returns number of bytes in xattr_list after filtering on success. This
> + * could be zero as well if nothing is left after filtering.
> + *
> + * Returns negative error code on failure.
> + * xattr_list is modified in place.
> + */
> +static int remove_blocked_xattrs(struct lo_data *lo, char *xattr_list,
> + unsigned in_size)
> +{
> + size_t out_index, in_index;
> +
> + /*
> + * As of now we only filter out acl xattrs. If acls are enabled or
> + * they have not been explicitly disabled, there is nothing to
> + * filter.
> + */
> + if (lo->user_posix_acl) {
> + return in_size;
> + }
> +
> + out_index = 0;
> + in_index = 0;
> + while (in_index < in_size) {
> + char *in_ptr = xattr_list + in_index;
> +
> + /* Length of current attribute name */
> + size_t in_len = strlen(xattr_list + in_index) + 1;
> +
> + if (!block_xattr(lo, in_ptr)) {
> + if (in_index != out_index) {
> + memmove(xattr_list + out_index, xattr_list + in_index,
> in_len);
> + }
> + out_index += in_len;
> + }
> + in_index += in_len;
> + }
> + return out_index;
> +}
> +
> static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
> size_t size)
> {
> @@ -2625,6 +2702,11 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t
> ino, const char *in_name,
> int saverr;
> int fd = -1;
>
> + if (block_xattr(lo, in_name)) {
> + fuse_reply_err(req, EOPNOTSUPP);
> + return;
> + }
> +
> mapped_name = NULL;
> name = in_name;
> if (lo->xattrmap) {
> @@ -2766,7 +2848,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t
> ino, size_t size)
> if (ret == 0) {
> goto out;
> }
> -
> if (lo->xattr_map_list) {
> /*
> * Map the names back, some attributes might be dropped,
> @@ -2813,6 +2894,12 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t
> ino, size_t size)
> goto out;
> }
> }
> +
> + ret = remove_blocked_xattrs(lo, value, ret);
> + if (ret <= 0) {
> + saverr = -ret;
> + goto out;
> + }
> fuse_reply_buf(req, value, ret);
> } else {
> /*
> @@ -2851,6 +2938,11 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t
> ino, const char *in_name,
> int saverr;
> int fd = -1;
>
> + if (block_xattr(lo, in_name)) {
> + fuse_reply_err(req, EOPNOTSUPP);
> + return;
> + }
> +
> mapped_name = NULL;
> name = in_name;
> if (lo->xattrmap) {
> @@ -3604,6 +3696,7 @@ int main(int argc, char *argv[])
> .allow_direct_io = 0,
> .proc_self_fd = -1,
> .user_killpriv_v2 = -1,
> + .user_posix_acl = -1,
> };
> struct lo_map_elem *root_elem;
> struct lo_map_elem *reserve_elem;
> --
> 2.25.4
>
- [PATCH v2 0/3] virtiofsd: Add options to enable/disable posix acl, Vivek Goyal, 2021/02/17
- [PATCH v2 2/3] virtiofsd: Add umask to seccom allow list, Vivek Goyal, 2021/02/17
- [PATCH v2 1/3] virtiofsd: Add an option to enable/disable posix acls, Vivek Goyal, 2021/02/17
- [PATCH v2 3/3] virtiofsd: Change umask if posix acls are enabled, Vivek Goyal, 2021/02/17
- Re: [PATCH v2 0/3] virtiofsd: Add options to enable/disable posix acl, Luis Henriques, 2021/02/19
- Re: [PATCH v2 0/3] virtiofsd: Add options to enable/disable posix acl, Luis Henriques, 2021/02/23