All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: virtio-fs@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH] virtiofsd: xattr mapping add a new type "unsupported"
Date: Mon, 25 Oct 2021 18:48:44 +0100	[thread overview]
Message-ID: <YXbt/JnPITP780GO@work-vm> (raw)
In-Reply-To: <YUt9qbmgAfCFfg5t@redhat.com>

* Vivek Goyal (vgoyal@redhat.com) wrote:
> Right now for xattr remapping, we support types of "prefix", "ok" or "bad".
> Type "bad" returns -EPERM on setxattr and hides xattr in listxattr. For
> getxattr, mapping code returns -EPERM but getxattr code converts it to -ENODATA.
> 
> I need a new semantics where if an xattr is unsupported, then
> getxattr()/setxattr() return -ENOTSUP and listxattr() should hide the xattr.
> This is needed to simulate that security.selinux is not supported by
> virtiofs filesystem and in that case client falls back to some default
> label specified by policy.
> 
> So add a new type "unsupported" which returns -ENOTSUP on getxattr() and
> setxattr() and hides xattrs in listxattr().
> 
> For example, one can use following mapping rule to not support
> security.selinux xattr and allow others.
> 
> "-o xattrmap=/unsupported/all/security.selinux/security.selinux//ok/all///"
> 
> Suggested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Queued

> ---
>  docs/tools/virtiofsd.rst         |    6 ++++++
>  tools/virtiofsd/passthrough_ll.c |   17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===================================================================
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c	2021-09-22 08:37:16.070377732 -0400
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c	2021-09-22 14:17:09.543016250 -0400
> @@ -2465,6 +2465,11 @@ static void lo_flock(fuse_req_t req, fus
>   * Automatically reversed on read
>   */
>  #define XATTR_MAP_FLAG_PREFIX  (1 <<  2)
> +/*
> + * The attribute is unsupported;
> + * ENOTSUP on write, hidden on read.
> + */
> +#define XATTR_MAP_FLAG_UNSUPPORTED     (1 <<  3)
>  
>  /* scopes */
>  /* Apply rule to get/set/remove */
> @@ -2636,6 +2641,8 @@ static void parse_xattrmap(struct lo_dat
>              tmp_entry.flags |= XATTR_MAP_FLAG_OK;
>          } else if (strstart(map, "bad", &map)) {
>              tmp_entry.flags |= XATTR_MAP_FLAG_BAD;
> +        } else if (strstart(map, "unsupported", &map)) {
> +            tmp_entry.flags |= XATTR_MAP_FLAG_UNSUPPORTED;
>          } else if (strstart(map, "map", &map)) {
>              /*
>               * map is sugar that adds a number of rules, and must be
> @@ -2646,8 +2653,8 @@ static void parse_xattrmap(struct lo_dat
>          } else {
>              fuse_log(FUSE_LOG_ERR,
>                       "%s: Unexpected type;"
> -                     "Expecting 'prefix', 'ok', 'bad' or 'map' in rule %zu\n",
> -                     __func__, lo->xattr_map_nentries);
> +                     "Expecting 'prefix', 'ok', 'bad', 'unsupported' or 'map'"
> +                     " in rule %zu\n", __func__, lo->xattr_map_nentries);
>              exit(1);
>          }
>  
> @@ -2749,6 +2756,9 @@ static int xattr_map_client(const struct
>              if (cur_entry->flags & XATTR_MAP_FLAG_BAD) {
>                  return -EPERM;
>              }
> +            if (cur_entry->flags & XATTR_MAP_FLAG_UNSUPPORTED) {
> +                return -ENOTSUP;
> +            }
>              if (cur_entry->flags & XATTR_MAP_FLAG_OK) {
>                  /* Unmodified name */
>                  return 0;
> @@ -2788,7 +2798,8 @@ static int xattr_map_server(const struct
>  
>          if ((cur_entry->flags & XATTR_MAP_FLAG_SERVER) &&
>              (strstart(server_name, cur_entry->prepend, &end))) {
> -            if (cur_entry->flags & XATTR_MAP_FLAG_BAD) {
> +            if (cur_entry->flags & XATTR_MAP_FLAG_BAD ||
> +                cur_entry->flags & XATTR_MAP_FLAG_UNSUPPORTED) {
>                  return -ENODATA;
>              }
>              if (cur_entry->flags & XATTR_MAP_FLAG_OK) {
> Index: rhvgoyal-qemu/docs/tools/virtiofsd.rst
> ===================================================================
> --- rhvgoyal-qemu.orig/docs/tools/virtiofsd.rst	2021-09-22 08:37:15.938372097 -0400
> +++ rhvgoyal-qemu/docs/tools/virtiofsd.rst	2021-09-22 14:44:09.814188712 -0400
> @@ -183,6 +183,12 @@ Using ':' as the separator a rule is of
>    'ok' as either an explicit terminator or for special handling of certain
>    patterns.
>  
> +- 'unsupported' - If a client tries to use a name matching 'key' it's
> +  denied using ENOTSUP; when the server passes an attribute
> +  name matching 'prepend' it's hidden.  In many ways it's use is very like
> +  'ok' as either an explicit terminator or for special handling of certain
> +  patterns.
> +
>  **key** is a string tested as a prefix on an attribute name originating
>  on the client.  It maybe empty in which case a 'client' rule
>  will always match on client names.
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: virtio-fs@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Virtio-fs] [PATCH] virtiofsd: xattr mapping add a new type "unsupported"
Date: Mon, 25 Oct 2021 18:48:44 +0100	[thread overview]
Message-ID: <YXbt/JnPITP780GO@work-vm> (raw)
In-Reply-To: <YUt9qbmgAfCFfg5t@redhat.com>

* Vivek Goyal (vgoyal@redhat.com) wrote:
> Right now for xattr remapping, we support types of "prefix", "ok" or "bad".
> Type "bad" returns -EPERM on setxattr and hides xattr in listxattr. For
> getxattr, mapping code returns -EPERM but getxattr code converts it to -ENODATA.
> 
> I need a new semantics where if an xattr is unsupported, then
> getxattr()/setxattr() return -ENOTSUP and listxattr() should hide the xattr.
> This is needed to simulate that security.selinux is not supported by
> virtiofs filesystem and in that case client falls back to some default
> label specified by policy.
> 
> So add a new type "unsupported" which returns -ENOTSUP on getxattr() and
> setxattr() and hides xattrs in listxattr().
> 
> For example, one can use following mapping rule to not support
> security.selinux xattr and allow others.
> 
> "-o xattrmap=/unsupported/all/security.selinux/security.selinux//ok/all///"
> 
> Suggested-by: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Queued

> ---
>  docs/tools/virtiofsd.rst         |    6 ++++++
>  tools/virtiofsd/passthrough_ll.c |   17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===================================================================
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c	2021-09-22 08:37:16.070377732 -0400
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c	2021-09-22 14:17:09.543016250 -0400
> @@ -2465,6 +2465,11 @@ static void lo_flock(fuse_req_t req, fus
>   * Automatically reversed on read
>   */
>  #define XATTR_MAP_FLAG_PREFIX  (1 <<  2)
> +/*
> + * The attribute is unsupported;
> + * ENOTSUP on write, hidden on read.
> + */
> +#define XATTR_MAP_FLAG_UNSUPPORTED     (1 <<  3)
>  
>  /* scopes */
>  /* Apply rule to get/set/remove */
> @@ -2636,6 +2641,8 @@ static void parse_xattrmap(struct lo_dat
>              tmp_entry.flags |= XATTR_MAP_FLAG_OK;
>          } else if (strstart(map, "bad", &map)) {
>              tmp_entry.flags |= XATTR_MAP_FLAG_BAD;
> +        } else if (strstart(map, "unsupported", &map)) {
> +            tmp_entry.flags |= XATTR_MAP_FLAG_UNSUPPORTED;
>          } else if (strstart(map, "map", &map)) {
>              /*
>               * map is sugar that adds a number of rules, and must be
> @@ -2646,8 +2653,8 @@ static void parse_xattrmap(struct lo_dat
>          } else {
>              fuse_log(FUSE_LOG_ERR,
>                       "%s: Unexpected type;"
> -                     "Expecting 'prefix', 'ok', 'bad' or 'map' in rule %zu\n",
> -                     __func__, lo->xattr_map_nentries);
> +                     "Expecting 'prefix', 'ok', 'bad', 'unsupported' or 'map'"
> +                     " in rule %zu\n", __func__, lo->xattr_map_nentries);
>              exit(1);
>          }
>  
> @@ -2749,6 +2756,9 @@ static int xattr_map_client(const struct
>              if (cur_entry->flags & XATTR_MAP_FLAG_BAD) {
>                  return -EPERM;
>              }
> +            if (cur_entry->flags & XATTR_MAP_FLAG_UNSUPPORTED) {
> +                return -ENOTSUP;
> +            }
>              if (cur_entry->flags & XATTR_MAP_FLAG_OK) {
>                  /* Unmodified name */
>                  return 0;
> @@ -2788,7 +2798,8 @@ static int xattr_map_server(const struct
>  
>          if ((cur_entry->flags & XATTR_MAP_FLAG_SERVER) &&
>              (strstart(server_name, cur_entry->prepend, &end))) {
> -            if (cur_entry->flags & XATTR_MAP_FLAG_BAD) {
> +            if (cur_entry->flags & XATTR_MAP_FLAG_BAD ||
> +                cur_entry->flags & XATTR_MAP_FLAG_UNSUPPORTED) {
>                  return -ENODATA;
>              }
>              if (cur_entry->flags & XATTR_MAP_FLAG_OK) {
> Index: rhvgoyal-qemu/docs/tools/virtiofsd.rst
> ===================================================================
> --- rhvgoyal-qemu.orig/docs/tools/virtiofsd.rst	2021-09-22 08:37:15.938372097 -0400
> +++ rhvgoyal-qemu/docs/tools/virtiofsd.rst	2021-09-22 14:44:09.814188712 -0400
> @@ -183,6 +183,12 @@ Using ':' as the separator a rule is of
>    'ok' as either an explicit terminator or for special handling of certain
>    patterns.
>  
> +- 'unsupported' - If a client tries to use a name matching 'key' it's
> +  denied using ENOTSUP; when the server passes an attribute
> +  name matching 'prepend' it's hidden.  In many ways it's use is very like
> +  'ok' as either an explicit terminator or for special handling of certain
> +  patterns.
> +
>  **key** is a string tested as a prefix on an attribute name originating
>  on the client.  It maybe empty in which case a 'client' rule
>  will always match on client names.
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


  parent reply	other threads:[~2021-10-25 17:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-22 19:02 [PATCH] virtiofsd: xattr mapping add a new type "unsupported" Vivek Goyal
2021-09-22 19:02 ` [Virtio-fs] " Vivek Goyal
2021-10-05  9:43 ` Dr. David Alan Gilbert
2021-10-05  9:43   ` [Virtio-fs] " Dr. David Alan Gilbert
2021-10-25 17:48 ` Dr. David Alan Gilbert [this message]
2021-10-25 17:48   ` Dr. David Alan Gilbert

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=YXbt/JnPITP780GO@work-vm \
    --to=dgilbert@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vgoyal@redhat.com \
    --cc=virtio-fs@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.