diff mbox series

[v5,9/9] virtiofsd: Add an option to enable/disable security label

Message ID 20220202193935.268777-10-vgoyal@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtiofsd: Add support for file security context at file creation | expand

Commit Message

Vivek Goyal Feb. 2, 2022, 7:39 p.m. UTC
Provide an option "-o security_label/no_security_label" to enable/disable
security label functionality. By default these are turned off.

If enabled, server will indicate to client that it is capable of handling
one security label during file creation. Typically this is expected to
be a SELinux label. File server will set this label on the file. It will
try to set it atomically wherever possible. But its not possible in
all the cases.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 docs/tools/virtiofsd.rst         |  7 +++++++
 tools/virtiofsd/helper.c         |  1 +
 tools/virtiofsd/passthrough_ll.c | 15 +++++++++++++++
 3 files changed, 23 insertions(+)

Comments

Dr. David Alan Gilbert Feb. 7, 2022, 12:40 p.m. UTC | #1
* Vivek Goyal (vgoyal@redhat.com) wrote:
> Provide an option "-o security_label/no_security_label" to enable/disable
> security label functionality. By default these are turned off.
> 
> If enabled, server will indicate to client that it is capable of handling
> one security label during file creation. Typically this is expected to
> be a SELinux label. File server will set this label on the file. It will
> try to set it atomically wherever possible. But its not possible in
> all the cases.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  docs/tools/virtiofsd.rst         |  7 +++++++
>  tools/virtiofsd/helper.c         |  1 +
>  tools/virtiofsd/passthrough_ll.c | 15 +++++++++++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> index 07ac0be551..a2c005f4a0 100644
> --- a/docs/tools/virtiofsd.rst
> +++ b/docs/tools/virtiofsd.rst
> @@ -104,6 +104,13 @@ Options
>    * posix_acl|no_posix_acl -
>      Enable/disable posix acl support.  Posix ACLs are disabled by default.
>  
> +  * security_label|no_security_label -
> +    Enable/disable security label support. Security labels are disabled by
> +    default. This will allow client to send a MAC label of file during
                                ^ the                        ^ a
> +    file creation. Typically this is expected to be SELinux security
                                                      ^ an

> +    label. Server will try to set that label on newly created file
              ^The server
> +    atomically wherever possible.
> +
>  .. option:: --socket-path=PATH
>  
>    Listen on vhost-user UNIX domain socket at PATH.
> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> index a8295d975a..e226fc590f 100644
> --- a/tools/virtiofsd/helper.c
> +++ b/tools/virtiofsd/helper.c
> @@ -187,6 +187,7 @@ void fuse_cmdline_help(void)
>             "                               default: no_allow_direct_io\n"
>             "    -o announce_submounts      Announce sub-mount points to the guest\n"
>             "    -o posix_acl/no_posix_acl  Enable/Disable posix_acl. (default: disabled)\n"
> +           "    -o security_label/no_security_label  Enable/Disable security label. (default: disabled)\n"
>             );
>  }
>  
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 43c9b6dbe5..fe8f3ccbb6 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -181,6 +181,7 @@ struct lo_data {
>      int user_posix_acl, posix_acl;
>      /* Keeps track if /proc/<pid>/attr/fscreate should be used or not */
>      bool use_fscreate;
> +    int user_security_label;
>  };
>  
>  static const struct fuse_opt lo_opts[] = {
> @@ -215,6 +216,8 @@ static const struct fuse_opt lo_opts[] = {
>      { "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 },
> +    { "security_label", offsetof(struct lo_data, user_security_label), 1 },
> +    { "no_security_label", offsetof(struct lo_data, user_security_label), 0 },
>      FUSE_OPT_END
>  };
>  static bool use_syslog = false;
> @@ -771,6 +774,17 @@ static void lo_init(void *userdata, struct fuse_conn_info *conn)
>          fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling posix_acl\n");
>          conn->want &= ~FUSE_CAP_POSIX_ACL;
>      }
> +
> +    if (lo->user_security_label == 1) {
> +        if (!(conn->capable & FUSE_CAP_SECURITY_CTX)) {
> +            fuse_log(FUSE_LOG_ERR, "lo_init: Can not enable security label."
> +                     " kernel does not support FUSE_SECURITY_CTX capability.\n");
> +        }

Do you need to exit in this case - or at least clear the flag?

Dave

> +        conn->want |= FUSE_CAP_SECURITY_CTX;
> +    } else {
> +        fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling security label\n");
> +        conn->want &= ~FUSE_CAP_SECURITY_CTX;
> +    }
>  }
>  
>  static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
> @@ -4279,6 +4293,7 @@ int main(int argc, char *argv[])
>          .proc_self_task = -1,
>          .user_killpriv_v2 = -1,
>          .user_posix_acl = -1,
> +        .user_security_label = -1,
>      };
>      struct lo_map_elem *root_elem;
>      struct lo_map_elem *reserve_elem;
> -- 
> 2.34.1
>
Vivek Goyal Feb. 7, 2022, 2:13 p.m. UTC | #2
On Mon, Feb 07, 2022 at 12:40:21PM +0000, Dr. David Alan Gilbert wrote:
> * Vivek Goyal (vgoyal@redhat.com) wrote:
> > Provide an option "-o security_label/no_security_label" to enable/disable
> > security label functionality. By default these are turned off.
> > 
> > If enabled, server will indicate to client that it is capable of handling
> > one security label during file creation. Typically this is expected to
> > be a SELinux label. File server will set this label on the file. It will
> > try to set it atomically wherever possible. But its not possible in
> > all the cases.
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  docs/tools/virtiofsd.rst         |  7 +++++++
> >  tools/virtiofsd/helper.c         |  1 +
> >  tools/virtiofsd/passthrough_ll.c | 15 +++++++++++++++
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> > index 07ac0be551..a2c005f4a0 100644
> > --- a/docs/tools/virtiofsd.rst
> > +++ b/docs/tools/virtiofsd.rst
> > @@ -104,6 +104,13 @@ Options
> >    * posix_acl|no_posix_acl -
> >      Enable/disable posix acl support.  Posix ACLs are disabled by default.
> >  
> > +  * security_label|no_security_label -
> > +    Enable/disable security label support. Security labels are disabled by
> > +    default. This will allow client to send a MAC label of file during
>                                 ^ the                        ^ a
> > +    file creation. Typically this is expected to be SELinux security
>                                                       ^ an
> 
> > +    label. Server will try to set that label on newly created file
>               ^The server
> > +    atomically wherever possible.
> > +
> >  .. option:: --socket-path=PATH
> >  
> >    Listen on vhost-user UNIX domain socket at PATH.
> > diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> > index a8295d975a..e226fc590f 100644
> > --- a/tools/virtiofsd/helper.c
> > +++ b/tools/virtiofsd/helper.c
> > @@ -187,6 +187,7 @@ void fuse_cmdline_help(void)
> >             "                               default: no_allow_direct_io\n"
> >             "    -o announce_submounts      Announce sub-mount points to the guest\n"
> >             "    -o posix_acl/no_posix_acl  Enable/Disable posix_acl. (default: disabled)\n"
> > +           "    -o security_label/no_security_label  Enable/Disable security label. (default: disabled)\n"
> >             );
> >  }
> >  
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index 43c9b6dbe5..fe8f3ccbb6 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -181,6 +181,7 @@ struct lo_data {
> >      int user_posix_acl, posix_acl;
> >      /* Keeps track if /proc/<pid>/attr/fscreate should be used or not */
> >      bool use_fscreate;
> > +    int user_security_label;
> >  };
> >  
> >  static const struct fuse_opt lo_opts[] = {
> > @@ -215,6 +216,8 @@ static const struct fuse_opt lo_opts[] = {
> >      { "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 },
> > +    { "security_label", offsetof(struct lo_data, user_security_label), 1 },
> > +    { "no_security_label", offsetof(struct lo_data, user_security_label), 0 },
> >      FUSE_OPT_END
> >  };
> >  static bool use_syslog = false;
> > @@ -771,6 +774,17 @@ static void lo_init(void *userdata, struct fuse_conn_info *conn)
> >          fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling posix_acl\n");
> >          conn->want &= ~FUSE_CAP_POSIX_ACL;
> >      }
> > +
> > +    if (lo->user_security_label == 1) {
> > +        if (!(conn->capable & FUSE_CAP_SECURITY_CTX)) {
> > +            fuse_log(FUSE_LOG_ERR, "lo_init: Can not enable security label."
> > +                     " kernel does not support FUSE_SECURITY_CTX capability.\n");
> > +        }
> 
> Do you need to exit in this case - or at least clear the flag?

Actually we don't have to necessarily exit here because fuse_lowlevel.c
has a check which makes it exit. And that's why I do not clear the
flag from ->want to signifiy that filesystem wants that capability
but client is not ->capable so error out and exit.


    if (se->conn.want & (~se->conn.capable)) {
        fuse_log(FUSE_LOG_ERR,
                 "fuse: error: filesystem requested capabilities "
                 "0x%llx that are not supported by kernel, aborting.\n",
                 se->conn.want & (~se->conn.capable));
        fuse_reply_err(req, EPROTO);
        se->error = -EPROTO;
        fuse_session_exit(se);
        return;
    }

Thanks
Vivek

> 
> Dave
> 
> > +        conn->want |= FUSE_CAP_SECURITY_CTX;
> > +    } else {
> > +        fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling security label\n");
> > +        conn->want &= ~FUSE_CAP_SECURITY_CTX;
> > +    }
> >  }
> >  
> >  static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
> > @@ -4279,6 +4293,7 @@ int main(int argc, char *argv[])
> >          .proc_self_task = -1,
> >          .user_killpriv_v2 = -1,
> >          .user_posix_acl = -1,
> > +        .user_security_label = -1,
> >      };
> >      struct lo_map_elem *root_elem;
> >      struct lo_map_elem *reserve_elem;
> > -- 
> > 2.34.1
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
diff mbox series

Patch

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 07ac0be551..a2c005f4a0 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -104,6 +104,13 @@  Options
   * posix_acl|no_posix_acl -
     Enable/disable posix acl support.  Posix ACLs are disabled by default.
 
+  * security_label|no_security_label -
+    Enable/disable security label support. Security labels are disabled by
+    default. This will allow client to send a MAC label of file during
+    file creation. Typically this is expected to be SELinux security
+    label. Server will try to set that label on newly created file
+    atomically wherever possible.
+
 .. option:: --socket-path=PATH
 
   Listen on vhost-user UNIX domain socket at PATH.
diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index a8295d975a..e226fc590f 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -187,6 +187,7 @@  void fuse_cmdline_help(void)
            "                               default: no_allow_direct_io\n"
            "    -o announce_submounts      Announce sub-mount points to the guest\n"
            "    -o posix_acl/no_posix_acl  Enable/Disable posix_acl. (default: disabled)\n"
+           "    -o security_label/no_security_label  Enable/Disable security label. (default: disabled)\n"
            );
 }
 
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 43c9b6dbe5..fe8f3ccbb6 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -181,6 +181,7 @@  struct lo_data {
     int user_posix_acl, posix_acl;
     /* Keeps track if /proc/<pid>/attr/fscreate should be used or not */
     bool use_fscreate;
+    int user_security_label;
 };
 
 static const struct fuse_opt lo_opts[] = {
@@ -215,6 +216,8 @@  static const struct fuse_opt lo_opts[] = {
     { "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 },
+    { "security_label", offsetof(struct lo_data, user_security_label), 1 },
+    { "no_security_label", offsetof(struct lo_data, user_security_label), 0 },
     FUSE_OPT_END
 };
 static bool use_syslog = false;
@@ -771,6 +774,17 @@  static void lo_init(void *userdata, struct fuse_conn_info *conn)
         fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling posix_acl\n");
         conn->want &= ~FUSE_CAP_POSIX_ACL;
     }
+
+    if (lo->user_security_label == 1) {
+        if (!(conn->capable & FUSE_CAP_SECURITY_CTX)) {
+            fuse_log(FUSE_LOG_ERR, "lo_init: Can not enable security label."
+                     " kernel does not support FUSE_SECURITY_CTX capability.\n");
+        }
+        conn->want |= FUSE_CAP_SECURITY_CTX;
+    } else {
+        fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling security label\n");
+        conn->want &= ~FUSE_CAP_SECURITY_CTX;
+    }
 }
 
 static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
@@ -4279,6 +4293,7 @@  int main(int argc, char *argv[])
         .proc_self_task = -1,
         .user_killpriv_v2 = -1,
         .user_posix_acl = -1,
+        .user_security_label = -1,
     };
     struct lo_map_elem *root_elem;
     struct lo_map_elem *reserve_elem;