All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, virtio-fs@redhat.com, stefanha@redhat.com,
	vgoyal@redhat.com
Subject: [Virtio-fs] [PATCH] virtiofsd: Allow capability restriction to be turned off
Date: Fri, 19 Jun 2020 18:18:09 +0100	[thread overview]
Message-ID: <20200619171809.30984-1-dgilbert@redhat.com> (raw)

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Commit a59feb483b8fae24d043 dropped most capabilities at startup,
since in most cases virtiofsd doesn't need them.  Unfortunately we found
a case that needs CAP_SYS_ADMIN - setting trusted extended attributes.
It's rare to need it, and the clean fix for it is going to be more
complicated, but as a way for existing users to continue, allow the
capability restriction to be turned off by adding

  -o no_restrictcaps

We still drop the capabilities by default since this seems much safer;
even if we don't have an explicit problem to fix.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/tools/virtiofsd.rst         | 3 +++
 tools/virtiofsd/helper.c         | 3 +++
 tools/virtiofsd/passthrough_ll.c | 8 +++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 378594c422..8819d7d19e 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -81,6 +81,9 @@ Options
     Enable/disable extended attributes (xattr) on files and directories.  The
     default is ``no_xattr``.
 
+  * restrictcaps|no_restrictcaps\n"
+    Restrict capabilities at startup.  The default is ``restrictcaps``.
+
 .. 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 00a1ef666a..51ed9fbed0 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -174,6 +174,9 @@ void fuse_cmdline_help(void)
            "                               default: no_writeback\n"
            "    -o xattr|no_xattr          enable/disable xattr\n"
            "                               default: no_xattr\n"
+           "    -o restrictcaps|no_restrictcaps\n"
+           "                               restrict capabilities at startup\n"
+           "                               default: restrictcaps\n"
            "    --rlimit-nofile=<num>      set maximum number of file descriptors\n"
            "                               (0 leaves rlimit unchanged)\n"
            "                               default: min(1000000, fs.file-max - 16384)\n"
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 2ce7c96085..8dee657b19 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -144,6 +144,7 @@ struct lo_data {
     int flock;
     int posix_lock;
     int xattr;
+    int restrictcaps;
     char *source;
     double timeout;
     int cache;
@@ -170,6 +171,8 @@ static const struct fuse_opt lo_opts[] = {
     { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 },
     { "xattr", offsetof(struct lo_data, xattr), 1 },
     { "no_xattr", offsetof(struct lo_data, xattr), 0 },
+    { "restrictcaps", offsetof(struct lo_data, restrictcaps), 1 },
+    { "no_restrictcaps", offsetof(struct lo_data, restrictcaps), 0 },
     { "timeout=%lf", offsetof(struct lo_data, timeout), 0 },
     { "timeout=", offsetof(struct lo_data, timeout_set), 1 },
     { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE },
@@ -2615,7 +2618,9 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se,
     setup_namespaces(lo, se);
     setup_mounts(lo->source);
     setup_seccomp(enable_syslog);
-    setup_capabilities();
+    if (lo->restrictcaps) {
+        setup_capabilities();
+    }
 }
 
 /* Set the maximum number of open file descriptors */
@@ -2764,6 +2769,7 @@ int main(int argc, char *argv[])
         .writeback = 0,
         .posix_lock = 1,
         .proc_self_fd = -1,
+        .restrictcaps = 1,
     };
     struct lo_map_elem *root_elem;
     int ret = -1;
-- 
2.26.2


WARNING: multiple messages have this Message-ID (diff)
From: "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com>
To: qemu-devel@nongnu.org, virtio-fs@redhat.com, stefanha@redhat.com,
	vgoyal@redhat.com
Subject: [PATCH] virtiofsd: Allow capability restriction to be turned off
Date: Fri, 19 Jun 2020 18:18:09 +0100	[thread overview]
Message-ID: <20200619171809.30984-1-dgilbert@redhat.com> (raw)

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Commit a59feb483b8fae24d043 dropped most capabilities at startup,
since in most cases virtiofsd doesn't need them.  Unfortunately we found
a case that needs CAP_SYS_ADMIN - setting trusted extended attributes.
It's rare to need it, and the clean fix for it is going to be more
complicated, but as a way for existing users to continue, allow the
capability restriction to be turned off by adding

  -o no_restrictcaps

We still drop the capabilities by default since this seems much safer;
even if we don't have an explicit problem to fix.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 docs/tools/virtiofsd.rst         | 3 +++
 tools/virtiofsd/helper.c         | 3 +++
 tools/virtiofsd/passthrough_ll.c | 8 +++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
index 378594c422..8819d7d19e 100644
--- a/docs/tools/virtiofsd.rst
+++ b/docs/tools/virtiofsd.rst
@@ -81,6 +81,9 @@ Options
     Enable/disable extended attributes (xattr) on files and directories.  The
     default is ``no_xattr``.
 
+  * restrictcaps|no_restrictcaps\n"
+    Restrict capabilities at startup.  The default is ``restrictcaps``.
+
 .. 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 00a1ef666a..51ed9fbed0 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -174,6 +174,9 @@ void fuse_cmdline_help(void)
            "                               default: no_writeback\n"
            "    -o xattr|no_xattr          enable/disable xattr\n"
            "                               default: no_xattr\n"
+           "    -o restrictcaps|no_restrictcaps\n"
+           "                               restrict capabilities at startup\n"
+           "                               default: restrictcaps\n"
            "    --rlimit-nofile=<num>      set maximum number of file descriptors\n"
            "                               (0 leaves rlimit unchanged)\n"
            "                               default: min(1000000, fs.file-max - 16384)\n"
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 2ce7c96085..8dee657b19 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -144,6 +144,7 @@ struct lo_data {
     int flock;
     int posix_lock;
     int xattr;
+    int restrictcaps;
     char *source;
     double timeout;
     int cache;
@@ -170,6 +171,8 @@ static const struct fuse_opt lo_opts[] = {
     { "no_posix_lock", offsetof(struct lo_data, posix_lock), 0 },
     { "xattr", offsetof(struct lo_data, xattr), 1 },
     { "no_xattr", offsetof(struct lo_data, xattr), 0 },
+    { "restrictcaps", offsetof(struct lo_data, restrictcaps), 1 },
+    { "no_restrictcaps", offsetof(struct lo_data, restrictcaps), 0 },
     { "timeout=%lf", offsetof(struct lo_data, timeout), 0 },
     { "timeout=", offsetof(struct lo_data, timeout_set), 1 },
     { "cache=none", offsetof(struct lo_data, cache), CACHE_NONE },
@@ -2615,7 +2618,9 @@ static void setup_sandbox(struct lo_data *lo, struct fuse_session *se,
     setup_namespaces(lo, se);
     setup_mounts(lo->source);
     setup_seccomp(enable_syslog);
-    setup_capabilities();
+    if (lo->restrictcaps) {
+        setup_capabilities();
+    }
 }
 
 /* Set the maximum number of open file descriptors */
@@ -2764,6 +2769,7 @@ int main(int argc, char *argv[])
         .writeback = 0,
         .posix_lock = 1,
         .proc_self_fd = -1,
+        .restrictcaps = 1,
     };
     struct lo_map_elem *root_elem;
     int ret = -1;
-- 
2.26.2



             reply	other threads:[~2020-06-19 17:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 17:18 Dr. David Alan Gilbert (git) [this message]
2020-06-19 17:18 ` [PATCH] virtiofsd: Allow capability restriction to be turned off Dr. David Alan Gilbert (git)
2020-06-19 17:46 ` [Virtio-fs] " Vivek Goyal
2020-06-19 17:46   ` Vivek Goyal
2020-06-19 17:52 ` [Virtio-fs] " no-reply
2020-06-19 17:52   ` no-reply

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=20200619171809.30984-1-dgilbert@redhat.com \
    --to=dgilbert@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --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.