On Mon, Feb 14, 2022 at 01:32:38PM +0000, Dr. David Alan Gilbert wrote:
> * Vivek Goyal (vgo...@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 <vgo...@redhat.com>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>
> OK, but you have missed some of the docs typos I mentined in the last
> review; they can be cleared up any time.
Hi David,
I could not find any comments in V5 w.r.t doc typos. I am not sure which
email I have missed.
Anyway, will be nice if I can take care of these typos in a follow up
patch and these patches can be merged.
Thanks
Vivek
>
> > ---
> > docs/tools/virtiofsd.rst | 32 ++++++++++++++++++++++++++++++++
> > tools/virtiofsd/helper.c | 1 +
> > tools/virtiofsd/passthrough_ll.c | 15 +++++++++++++++
> > 3 files changed, 48 insertions(+)
> >
> > diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> > index 07ac0be551..0c0560203c 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.
> > @@ -348,6 +355,31 @@ client arguments or lists returned from the host.
> > This stops
> > the client seeing any 'security.' attributes on the server and
> > stops it setting any.
> >
> > +SELinux support
> > +---------------
> > +One can enable support for SELinux by running virtiofsd with option
> > +"-o security_label". But this will try to save guest's security context
> > +in xattr security.selinux on host and it might fail if host's SELinux
> > +policy does not permit virtiofsd to do this operation.
> > +
> > +Hence, it is preferred to remap guest's "security.selinux" xattr to say
> > +"trusted.virtiofs.security.selinux" on host.
> > +
> > +"-o xattrmap=:map:security.selinux:trusted.virtiofs.:"
> > +
> > +This will make sure that guest and host's SELinux xattrs on same file
> > +remain separate and not interfere with each other. And will allow both
> > +host and guest to implement their own separate SELinux policies.
> > +
> > +Setting trusted xattr on host requires CAP_SYS_ADMIN. So one will need
> > +add this capability to daemon.
> > +
> > +"-o modcaps=+sys_admin"
> > +
> > +Giving CAP_SYS_ADMIN increases the risk on system. Now virtiofsd is more
> > +powerful and if gets compromised, it can do lot of damage to host system.
> > +So keep this trade-off in my mind while making a decision.
> > +
> > Examples
> > --------
> >
> > 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 d49128a58d..f3ec6aafe5 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;
> > @@ -808,6 +811,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;
> > --
> > 2.34.1
> >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>
