| Message ID | E1q7ytt-0005Fl-JX@lizzy.crudebyte.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] 9pfs: deprecate 'proxy' backend | expand |
Hi Christian, On Sat, 10 Jun 2023 15:39:44 +0200 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > bad shape. Using the 'proxy' backend was already discouraged for safety > reasons before and we recommended to use the 'local' backend instead, > but now it is time to officially deprecate the 'proxy' backend. > For the records : The 'proxy' backend is an old thing that predates vhost-user. It really turns QEMU into a proxy : all requests go to QEMU, which forwads them to the helper and the other way around for responses. Data is copied both ways. All of this severely damages latency. If someone really wants to offload 9p stuff to a separate process, they should come up with a vhost-user-9p implementation. The whole server would be offloaded, possibly sharing most of the code with the in-QEMU server, QEMU wouldn't be involved anymore in the I/Os. No more copies. > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > v2 -> v3: > * Fix copy wasted typo (-> 'backend'). > > MAINTAINERS | 7 +++++++ > docs/about/deprecated.rst | 17 +++++++++++++++++ > docs/tools/virtfs-proxy-helper.rst | 3 +++ > fsdev/qemu-fsdev.c | 5 +++++ > fsdev/virtfs-proxy-helper.c | 5 +++++ > hw/9pfs/9p-proxy.c | 5 +++++ > hw/9pfs/9p-proxy.h | 5 +++++ > meson.build | 2 +- > qemu-options.hx | 6 +++++- > softmmu/vl.c | 5 +++++ > 10 files changed, 58 insertions(+), 2 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 436b3f0afe..185d694b2e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2118,13 +2118,20 @@ S: Odd Fixes > W: https://wiki.qemu.org/Documentation/9p > F: hw/9pfs/ > X: hw/9pfs/xen-9p* > +X: hw/9pfs/9p-proxy* > F: fsdev/ > +X: fsdev/virtfs-proxy-helper.c > F: docs/tools/virtfs-proxy-helper.rst > F: tests/qtest/virtio-9p-test.c > F: tests/qtest/libqos/virtio-9p* > T: git https://gitlab.com/gkurz/qemu.git 9p-next > T: git https://github.com/cschoenebeck/qemu.git 9p.next > > +virtio-9p-proxy > +F: hw/9pfs/9p-proxy* > +F: fsdev/virtfs-proxy-helper.c > +S: Obsolete > + > virtio-blk > M: Stefan Hajnoczi <stefanha@redhat.com> > L: qemu-block@nongnu.org > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 0743459862..9b2c780365 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now necessary to distinguish > between persistent and volatile memory backends. As such, memdev is deprecated > in favor of persistent-memdev. > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > +removed in a future version of QEMU. Please use ``-fsdev local`` or > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > + > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > +``local`` backend. The idea was to enhance security by dispatching actual low > +level filesystem operations from 9p server (QEMU process) over to a separate > +process (the virtfs-proxy-helper binary). However this alternative never gained > +momentum. The proxy backend is much slower than the local backend, hasn't seen > +any development in years, and showed to be less secure, especially due to the > +fact that its helper daemon must be run as root, whereas with the local backend > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > +mapping permissions et al. > + > > Block device options > '''''''''''''''''''' > diff --git a/docs/tools/virtfs-proxy-helper.rst b/docs/tools/virtfs-proxy-helper.rst > index 6cdeedf8e9..bd310ebb07 100644 > --- a/docs/tools/virtfs-proxy-helper.rst > +++ b/docs/tools/virtfs-proxy-helper.rst > @@ -9,6 +9,9 @@ Synopsis > Description > ----------- > > +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > +removed, along with this daemon, in a future version of QEMU! > + > Pass-through security model in QEMU 9p server needs root privilege to do > few file operations (like chown, chmod to any mode/uid:gid). There are two > issues in pass-through security model: > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > index 3da64e9f72..242f54ab49 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > } > > if (fsdriver) { > + if (strncmp(fsdriver, "proxy", 5) == 0) { > + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " > + "instead"); > + } > + > for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { > if (strcmp(FsDrivers[i].name, fsdriver) == 0) { > break; > diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > index d9511f429c..5dd5d99284 100644 > --- a/fsdev/virtfs-proxy-helper.c > +++ b/fsdev/virtfs-proxy-helper.c > @@ -9,6 +9,11 @@ > * the COPYING file in the top-level directory. > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + What about having the proxy to print a deprecation warning as well ? > #include "qemu/osdep.h" > #include <glib/gstdio.h> > #include <sys/resource.h> > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 99d115ff0d..905cae6992 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -15,6 +15,11 @@ > * https://wiki.qemu.org/Documentation/9p > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #include "qemu/osdep.h" > #include <sys/socket.h> > #include <sys/un.h> > diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h > index b84301d001..9be4718d3e 100644 > --- a/hw/9pfs/9p-proxy.h > +++ b/hw/9pfs/9p-proxy.h > @@ -10,6 +10,11 @@ > * the COPYING file in the top-level directory. > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #ifndef QEMU_9P_PROXY_H > #define QEMU_9P_PROXY_H > > diff --git a/meson.build b/meson.build > index 34306a6205..05c01b72bb 100644 > --- a/meson.build > +++ b/meson.build > @@ -4170,7 +4170,7 @@ if have_block > summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')} > summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} > summary_info += {'VirtFS (9P) support': have_virtfs} > - summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper} > + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} > summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} > summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} > summary_info += {'bochs support': get_option('bochs').allowed()} > diff --git a/qemu-options.hx b/qemu-options.hx > index b57489d7ca..3a6c7d3ef9 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1735,7 +1735,9 @@ SRST > Accesses to the filesystem are done by QEMU. > > ``proxy`` > - Accesses to the filesystem are done by virtfs-proxy-helper(1). > + Accesses to the filesystem are done by virtfs-proxy-helper(1). This > + option is deprecated (since QEMU 8.1) and will be removed in a future > + version of QEMU. Use ``local`` instead. > > ``synth`` > Synthetic filesystem, only used by QTests. > @@ -1867,6 +1869,8 @@ SRST > > ``proxy`` > Accesses to the filesystem are done by virtfs-proxy-helper(1). > + This option is deprecated (since QEMU 8.1) and will be removed in a > + future version of QEMU. Use ``local`` instead. > > ``synth`` > Synthetic filesystem, only used by QTests. > diff --git a/softmmu/vl.c b/softmmu/vl.c > index b0b96f67fa..e60648b591 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv) > error_report("Usage: -virtfs fsdriver,mount_tag=tag"); > exit(1); > } > + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) { > + warn_report("'-virtfs proxy' is deprecated, use " > + "'-virtfs local' instead"); > + } > + > fsdev = qemu_opts_create(qemu_find_opts("fsdev"), > qemu_opts_id(opts) ?: > qemu_opt_get(opts, "mount_tag"), Since an `-fsdev` is generated internally for each `-virtfs` we end up with two warnings, which seems a bit weird : $ qemu-system-x86_64-mbuild -virtfs proxy,id=fsdev0,mount_tag=9p qemu-system-x86_64: -virtfs proxy,id=fsdev0,mount_tag=9p: warning: '-virtfs proxy' is deprecated, use '-virtfs local' instead qemu-system-x86_64: -virtfs proxy,id=fsdev0,mount_tag=9p: warning: '-fsdev proxy' is deprecated, use '-fsdev local' instead ... Since it might be quite complex to guess in qemu_fsdev_add() that we're dealing with such an internally generated opt, I'd adapt the text so that it works for both cases. e.g. '-fsdev proxy' and '-virtfs proxy' are deprecated, use 'local' instead of 'proxy' Cheers,
On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > bad shape. Using the 'proxy' backend was already discouraged for safety > reasons before and we recommended to use the 'local' backend instead, > but now it is time to officially deprecate the 'proxy' backend. > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > --- > v2 -> v3: > * Fix copy wasted typo (-> 'backend'). > > MAINTAINERS | 7 +++++++ > docs/about/deprecated.rst | 17 +++++++++++++++++ > docs/tools/virtfs-proxy-helper.rst | 3 +++ > fsdev/qemu-fsdev.c | 5 +++++ > fsdev/virtfs-proxy-helper.c | 5 +++++ > hw/9pfs/9p-proxy.c | 5 +++++ > hw/9pfs/9p-proxy.h | 5 +++++ > meson.build | 2 +- > qemu-options.hx | 6 +++++- > softmmu/vl.c | 5 +++++ > 10 files changed, 58 insertions(+), 2 deletions(-) Or would it be better to split this up, e.g. into 3 separate patches (runtime messages, docs, MAINTAINERS)? > diff --git a/MAINTAINERS b/MAINTAINERS > index 436b3f0afe..185d694b2e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2118,13 +2118,20 @@ S: Odd Fixes > W: https://wiki.qemu.org/Documentation/9p > F: hw/9pfs/ > X: hw/9pfs/xen-9p* > +X: hw/9pfs/9p-proxy* > F: fsdev/ > +X: fsdev/virtfs-proxy-helper.c > F: docs/tools/virtfs-proxy-helper.rst I missed virtfs-proxy-helper.rst here. That should be moved to the new 'proxy' section below as well. > F: tests/qtest/virtio-9p-test.c > F: tests/qtest/libqos/virtio-9p* > T: git https://gitlab.com/gkurz/qemu.git 9p-next > T: git https://github.com/cschoenebeck/qemu.git 9p.next > > +virtio-9p-proxy > +F: hw/9pfs/9p-proxy* > +F: fsdev/virtfs-proxy-helper.c > +S: Obsolete > + > virtio-blk > M: Stefan Hajnoczi <stefanha@redhat.com> > L: qemu-block@nongnu.org > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 0743459862..9b2c780365 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now necessary to distinguish > between persistent and volatile memory backends. As such, memdev is deprecated > in favor of persistent-memdev. > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > +removed in a future version of QEMU. Please use ``-fsdev local`` or > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > + > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > +``local`` backend. The idea was to enhance security by dispatching actual low > +level filesystem operations from 9p server (QEMU process) over to a separate > +process (the virtfs-proxy-helper binary). However this alternative never gained > +momentum. The proxy backend is much slower than the local backend, hasn't seen > +any development in years, and showed to be less secure, especially due to the > +fact that its helper daemon must be run as root, whereas with the local backend > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > +mapping permissions et al. > + > > Block device options > '''''''''''''''''''' > diff --git a/docs/tools/virtfs-proxy-helper.rst b/docs/tools/virtfs-proxy-helper.rst > index 6cdeedf8e9..bd310ebb07 100644 > --- a/docs/tools/virtfs-proxy-helper.rst > +++ b/docs/tools/virtfs-proxy-helper.rst > @@ -9,6 +9,9 @@ Synopsis > Description > ----------- > > +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > +removed, along with this daemon, in a future version of QEMU! > + > Pass-through security model in QEMU 9p server needs root privilege to do > few file operations (like chown, chmod to any mode/uid:gid). There are two > issues in pass-through security model: > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > index 3da64e9f72..242f54ab49 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > } > > if (fsdriver) { > + if (strncmp(fsdriver, "proxy", 5) == 0) { > + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " > + "instead"); > + } > + > for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { > if (strcmp(FsDrivers[i].name, fsdriver) == 0) { > break; > diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > index d9511f429c..5dd5d99284 100644 > --- a/fsdev/virtfs-proxy-helper.c > +++ b/fsdev/virtfs-proxy-helper.c > @@ -9,6 +9,11 @@ > * the COPYING file in the top-level directory. > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #include "qemu/osdep.h" > #include <glib/gstdio.h> > #include <sys/resource.h> > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 99d115ff0d..905cae6992 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -15,6 +15,11 @@ > * https://wiki.qemu.org/Documentation/9p > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #include "qemu/osdep.h" > #include <sys/socket.h> > #include <sys/un.h> > diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h > index b84301d001..9be4718d3e 100644 > --- a/hw/9pfs/9p-proxy.h > +++ b/hw/9pfs/9p-proxy.h > @@ -10,6 +10,11 @@ > * the COPYING file in the top-level directory. > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a future version of QEMU! > + */ > + > #ifndef QEMU_9P_PROXY_H > #define QEMU_9P_PROXY_H > > diff --git a/meson.build b/meson.build > index 34306a6205..05c01b72bb 100644 > --- a/meson.build > +++ b/meson.build > @@ -4170,7 +4170,7 @@ if have_block > summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')} > summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} > summary_info += {'VirtFS (9P) support': have_virtfs} > - summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper} > + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} > summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} > summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} > summary_info += {'bochs support': get_option('bochs').allowed()} > diff --git a/qemu-options.hx b/qemu-options.hx > index b57489d7ca..3a6c7d3ef9 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1735,7 +1735,9 @@ SRST > Accesses to the filesystem are done by QEMU. > > ``proxy`` > - Accesses to the filesystem are done by virtfs-proxy-helper(1). > + Accesses to the filesystem are done by virtfs-proxy-helper(1). This > + option is deprecated (since QEMU 8.1) and will be removed in a future > + version of QEMU. Use ``local`` instead. > > ``synth`` > Synthetic filesystem, only used by QTests. > @@ -1867,6 +1869,8 @@ SRST > > ``proxy`` > Accesses to the filesystem are done by virtfs-proxy-helper(1). > + This option is deprecated (since QEMU 8.1) and will be removed in a > + future version of QEMU. Use ``local`` instead. > > ``synth`` > Synthetic filesystem, only used by QTests. > diff --git a/softmmu/vl.c b/softmmu/vl.c > index b0b96f67fa..e60648b591 100644 > --- a/softmmu/vl.c > +++ b/softmmu/vl.c > @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv) > error_report("Usage: -virtfs fsdriver,mount_tag=tag"); > exit(1); > } > + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) { > + warn_report("'-virtfs proxy' is deprecated, use " > + "'-virtfs local' instead"); > + } > + > fsdev = qemu_opts_create(qemu_find_opts("fsdev"), > qemu_opts_id(opts) ?: > qemu_opt_get(opts, "mount_tag"), >
On Thursday, June 15, 2023 11:35:05 AM CEST Christian Schoenebeck wrote: > On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > > bad shape. Using the 'proxy' backend was already discouraged for safety > > reasons before and we recommended to use the 'local' backend instead, > > but now it is time to officially deprecate the 'proxy' backend. > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Ping > > --- > > v2 -> v3: > > * Fix copy wasted typo (-> 'backend'). > > > > MAINTAINERS | 7 +++++++ > > docs/about/deprecated.rst | 17 +++++++++++++++++ > > docs/tools/virtfs-proxy-helper.rst | 3 +++ > > fsdev/qemu-fsdev.c | 5 +++++ > > fsdev/virtfs-proxy-helper.c | 5 +++++ > > hw/9pfs/9p-proxy.c | 5 +++++ > > hw/9pfs/9p-proxy.h | 5 +++++ > > meson.build | 2 +- > > qemu-options.hx | 6 +++++- > > softmmu/vl.c | 5 +++++ > > 10 files changed, 58 insertions(+), 2 deletions(-) > > Or would it be better to split this up, e.g. into 3 separate patches (runtime > messages, docs, MAINTAINERS)? > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 436b3f0afe..185d694b2e 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2118,13 +2118,20 @@ S: Odd Fixes > > W: https://wiki.qemu.org/Documentation/9p > > F: hw/9pfs/ > > X: hw/9pfs/xen-9p* > > +X: hw/9pfs/9p-proxy* > > F: fsdev/ > > +X: fsdev/virtfs-proxy-helper.c > > F: docs/tools/virtfs-proxy-helper.rst > > I missed virtfs-proxy-helper.rst here. That should be moved to the new 'proxy' > section below as well. > > > F: tests/qtest/virtio-9p-test.c > > F: tests/qtest/libqos/virtio-9p* > > T: git https://gitlab.com/gkurz/qemu.git 9p-next > > T: git https://github.com/cschoenebeck/qemu.git 9p.next > > > > +virtio-9p-proxy > > +F: hw/9pfs/9p-proxy* > > +F: fsdev/virtfs-proxy-helper.c > > +S: Obsolete > > + > > virtio-blk > > M: Stefan Hajnoczi <stefanha@redhat.com> > > L: qemu-block@nongnu.org > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > > index 0743459862..9b2c780365 100644 > > --- a/docs/about/deprecated.rst > > +++ b/docs/about/deprecated.rst > > @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now necessary to distinguish > > between persistent and volatile memory backends. As such, memdev is deprecated > > in favor of persistent-memdev. > > > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > > +removed in a future version of QEMU. Please use ``-fsdev local`` or > > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > > + > > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > > +``local`` backend. The idea was to enhance security by dispatching actual low > > +level filesystem operations from 9p server (QEMU process) over to a separate > > +process (the virtfs-proxy-helper binary). However this alternative never gained > > +momentum. The proxy backend is much slower than the local backend, hasn't seen > > +any development in years, and showed to be less secure, especially due to the > > +fact that its helper daemon must be run as root, whereas with the local backend > > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > > +mapping permissions et al. > > + > > > > Block device options > > '''''''''''''''''''' > > diff --git a/docs/tools/virtfs-proxy-helper.rst b/docs/tools/virtfs-proxy-helper.rst > > index 6cdeedf8e9..bd310ebb07 100644 > > --- a/docs/tools/virtfs-proxy-helper.rst > > +++ b/docs/tools/virtfs-proxy-helper.rst > > @@ -9,6 +9,9 @@ Synopsis > > Description > > ----------- > > > > +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > +removed, along with this daemon, in a future version of QEMU! > > + > > Pass-through security model in QEMU 9p server needs root privilege to do > > few file operations (like chown, chmod to any mode/uid:gid). There are two > > issues in pass-through security model: > > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > > index 3da64e9f72..242f54ab49 100644 > > --- a/fsdev/qemu-fsdev.c > > +++ b/fsdev/qemu-fsdev.c > > @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > > } > > > > if (fsdriver) { > > + if (strncmp(fsdriver, "proxy", 5) == 0) { > > + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " > > + "instead"); > > + } > > + > > for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { > > if (strcmp(FsDrivers[i].name, fsdriver) == 0) { > > break; > > diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > > index d9511f429c..5dd5d99284 100644 > > --- a/fsdev/virtfs-proxy-helper.c > > +++ b/fsdev/virtfs-proxy-helper.c > > @@ -9,6 +9,11 @@ > > * the COPYING file in the top-level directory. > > */ > > > > +/* > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > + * removed in a future version of QEMU! > > + */ > > + > > #include "qemu/osdep.h" > > #include <glib/gstdio.h> > > #include <sys/resource.h> > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > > index 99d115ff0d..905cae6992 100644 > > --- a/hw/9pfs/9p-proxy.c > > +++ b/hw/9pfs/9p-proxy.c > > @@ -15,6 +15,11 @@ > > * https://wiki.qemu.org/Documentation/9p > > */ > > > > +/* > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > + * removed in a future version of QEMU! > > + */ > > + > > #include "qemu/osdep.h" > > #include <sys/socket.h> > > #include <sys/un.h> > > diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h > > index b84301d001..9be4718d3e 100644 > > --- a/hw/9pfs/9p-proxy.h > > +++ b/hw/9pfs/9p-proxy.h > > @@ -10,6 +10,11 @@ > > * the COPYING file in the top-level directory. > > */ > > > > +/* > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > + * removed in a future version of QEMU! > > + */ > > + > > #ifndef QEMU_9P_PROXY_H > > #define QEMU_9P_PROXY_H > > > > diff --git a/meson.build b/meson.build > > index 34306a6205..05c01b72bb 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -4170,7 +4170,7 @@ if have_block > > summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')} > > summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} > > summary_info += {'VirtFS (9P) support': have_virtfs} > > - summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper} > > + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} > > summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} > > summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} > > summary_info += {'bochs support': get_option('bochs').allowed()} > > diff --git a/qemu-options.hx b/qemu-options.hx > > index b57489d7ca..3a6c7d3ef9 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1735,7 +1735,9 @@ SRST > > Accesses to the filesystem are done by QEMU. > > > > ``proxy`` > > - Accesses to the filesystem are done by virtfs-proxy-helper(1). > > + Accesses to the filesystem are done by virtfs-proxy-helper(1). This > > + option is deprecated (since QEMU 8.1) and will be removed in a future > > + version of QEMU. Use ``local`` instead. > > > > ``synth`` > > Synthetic filesystem, only used by QTests. > > @@ -1867,6 +1869,8 @@ SRST > > > > ``proxy`` > > Accesses to the filesystem are done by virtfs-proxy-helper(1). > > + This option is deprecated (since QEMU 8.1) and will be removed in a > > + future version of QEMU. Use ``local`` instead. > > > > ``synth`` > > Synthetic filesystem, only used by QTests. > > diff --git a/softmmu/vl.c b/softmmu/vl.c > > index b0b96f67fa..e60648b591 100644 > > --- a/softmmu/vl.c > > +++ b/softmmu/vl.c > > @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv) > > error_report("Usage: -virtfs fsdriver,mount_tag=tag"); > > exit(1); > > } > > + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) { > > + warn_report("'-virtfs proxy' is deprecated, use " > > + "'-virtfs local' instead"); > > + } > > + > > fsdev = qemu_opts_create(qemu_find_opts("fsdev"), > > qemu_opts_id(opts) ?: > > qemu_opt_get(opts, "mount_tag"), > > > > > >
On Thu, Jun 15, 2023 at 11:35:05AM +0200, Christian Schoenebeck wrote: > On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > > bad shape. Using the 'proxy' backend was already discouraged for safety > > reasons before and we recommended to use the 'local' backend instead, > > but now it is time to officially deprecate the 'proxy' backend. > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > --- > > v2 -> v3: > > * Fix copy wasted typo (-> 'backend'). > > > > MAINTAINERS | 7 +++++++ > > docs/about/deprecated.rst | 17 +++++++++++++++++ > > docs/tools/virtfs-proxy-helper.rst | 3 +++ > > fsdev/qemu-fsdev.c | 5 +++++ > > fsdev/virtfs-proxy-helper.c | 5 +++++ > > hw/9pfs/9p-proxy.c | 5 +++++ > > hw/9pfs/9p-proxy.h | 5 +++++ > > meson.build | 2 +- > > qemu-options.hx | 6 +++++- > > softmmu/vl.c | 5 +++++ > > 10 files changed, 58 insertions(+), 2 deletions(-) > > Or would it be better to split this up, e.g. into 3 separate patches (runtime > messages, docs, MAINTAINERS)? In general we've tended to do deprecations all in one patch like you have here. I don't think there's compelling benefit to split the patch. With regards, Daniel
On Wed, 21 Jun 2023 15:32:39 +0200 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Thursday, June 15, 2023 11:35:05 AM CEST Christian Schoenebeck wrote: > > On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > > > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > > > bad shape. Using the 'proxy' backend was already discouraged for safety > > > reasons before and we recommended to use the 'local' backend instead, > > > but now it is time to officially deprecate the 'proxy' backend. > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > Ping > It seems you missed the review I posted last week : https://patchew.org/QEMU/E1q7ytt-0005Fl-JX@lizzy.crudebyte.com/#20230612165742.3333ea08@bahia > > > --- > > > v2 -> v3: > > > * Fix copy wasted typo (-> 'backend'). > > > > > > MAINTAINERS | 7 +++++++ > > > docs/about/deprecated.rst | 17 +++++++++++++++++ > > > docs/tools/virtfs-proxy-helper.rst | 3 +++ > > > fsdev/qemu-fsdev.c | 5 +++++ > > > fsdev/virtfs-proxy-helper.c | 5 +++++ > > > hw/9pfs/9p-proxy.c | 5 +++++ > > > hw/9pfs/9p-proxy.h | 5 +++++ > > > meson.build | 2 +- > > > qemu-options.hx | 6 +++++- > > > softmmu/vl.c | 5 +++++ > > > 10 files changed, 58 insertions(+), 2 deletions(-) > > > > Or would it be better to split this up, e.g. into 3 separate patches (runtime > > messages, docs, MAINTAINERS)? > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 436b3f0afe..185d694b2e 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -2118,13 +2118,20 @@ S: Odd Fixes > > > W: https://wiki.qemu.org/Documentation/9p > > > F: hw/9pfs/ > > > X: hw/9pfs/xen-9p* > > > +X: hw/9pfs/9p-proxy* > > > F: fsdev/ > > > +X: fsdev/virtfs-proxy-helper.c > > > F: docs/tools/virtfs-proxy-helper.rst > > > > I missed virtfs-proxy-helper.rst here. That should be moved to the new 'proxy' > > section below as well. > > > > > F: tests/qtest/virtio-9p-test.c > > > F: tests/qtest/libqos/virtio-9p* > > > T: git https://gitlab.com/gkurz/qemu.git 9p-next > > > T: git https://github.com/cschoenebeck/qemu.git 9p.next > > > > > > +virtio-9p-proxy > > > +F: hw/9pfs/9p-proxy* > > > +F: fsdev/virtfs-proxy-helper.c > > > +S: Obsolete > > > + > > > virtio-blk > > > M: Stefan Hajnoczi <stefanha@redhat.com> > > > L: qemu-block@nongnu.org > > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > > > index 0743459862..9b2c780365 100644 > > > --- a/docs/about/deprecated.rst > > > +++ b/docs/about/deprecated.rst > > > @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now necessary to distinguish > > > between persistent and volatile memory backends. As such, memdev is deprecated > > > in favor of persistent-memdev. > > > > > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > + > > > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > > > +removed in a future version of QEMU. Please use ``-fsdev local`` or > > > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > > > + > > > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > > > +``local`` backend. The idea was to enhance security by dispatching actual low > > > +level filesystem operations from 9p server (QEMU process) over to a separate > > > +process (the virtfs-proxy-helper binary). However this alternative never gained > > > +momentum. The proxy backend is much slower than the local backend, hasn't seen > > > +any development in years, and showed to be less secure, especially due to the > > > +fact that its helper daemon must be run as root, whereas with the local backend > > > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > > > +mapping permissions et al. > > > + > > > > > > Block device options > > > '''''''''''''''''''' > > > diff --git a/docs/tools/virtfs-proxy-helper.rst b/docs/tools/virtfs-proxy-helper.rst > > > index 6cdeedf8e9..bd310ebb07 100644 > > > --- a/docs/tools/virtfs-proxy-helper.rst > > > +++ b/docs/tools/virtfs-proxy-helper.rst > > > @@ -9,6 +9,9 @@ Synopsis > > > Description > > > ----------- > > > > > > +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > > +removed, along with this daemon, in a future version of QEMU! > > > + > > > Pass-through security model in QEMU 9p server needs root privilege to do > > > few file operations (like chown, chmod to any mode/uid:gid). There are two > > > issues in pass-through security model: > > > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > > > index 3da64e9f72..242f54ab49 100644 > > > --- a/fsdev/qemu-fsdev.c > > > +++ b/fsdev/qemu-fsdev.c > > > @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > > > } > > > > > > if (fsdriver) { > > > + if (strncmp(fsdriver, "proxy", 5) == 0) { > > > + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " > > > + "instead"); > > > + } > > > + > > > for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { > > > if (strcmp(FsDrivers[i].name, fsdriver) == 0) { > > > break; > > > diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > > > index d9511f429c..5dd5d99284 100644 > > > --- a/fsdev/virtfs-proxy-helper.c > > > +++ b/fsdev/virtfs-proxy-helper.c > > > @@ -9,6 +9,11 @@ > > > * the COPYING file in the top-level directory. > > > */ > > > > > > +/* > > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > > + * removed in a future version of QEMU! > > > + */ > > > + > > > #include "qemu/osdep.h" > > > #include <glib/gstdio.h> > > > #include <sys/resource.h> > > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > > > index 99d115ff0d..905cae6992 100644 > > > --- a/hw/9pfs/9p-proxy.c > > > +++ b/hw/9pfs/9p-proxy.c > > > @@ -15,6 +15,11 @@ > > > * https://wiki.qemu.org/Documentation/9p > > > */ > > > > > > +/* > > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > > + * removed in a future version of QEMU! > > > + */ > > > + > > > #include "qemu/osdep.h" > > > #include <sys/socket.h> > > > #include <sys/un.h> > > > diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h > > > index b84301d001..9be4718d3e 100644 > > > --- a/hw/9pfs/9p-proxy.h > > > +++ b/hw/9pfs/9p-proxy.h > > > @@ -10,6 +10,11 @@ > > > * the COPYING file in the top-level directory. > > > */ > > > > > > +/* > > > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > > > + * removed in a future version of QEMU! > > > + */ > > > + > > > #ifndef QEMU_9P_PROXY_H > > > #define QEMU_9P_PROXY_H > > > > > > diff --git a/meson.build b/meson.build > > > index 34306a6205..05c01b72bb 100644 > > > --- a/meson.build > > > +++ b/meson.build > > > @@ -4170,7 +4170,7 @@ if have_block > > > summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')} > > > summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} > > > summary_info += {'VirtFS (9P) support': have_virtfs} > > > - summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper} > > > + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} > > > summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} > > > summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} > > > summary_info += {'bochs support': get_option('bochs').allowed()} > > > diff --git a/qemu-options.hx b/qemu-options.hx > > > index b57489d7ca..3a6c7d3ef9 100644 > > > --- a/qemu-options.hx > > > +++ b/qemu-options.hx > > > @@ -1735,7 +1735,9 @@ SRST > > > Accesses to the filesystem are done by QEMU. > > > > > > ``proxy`` > > > - Accesses to the filesystem are done by virtfs-proxy-helper(1). > > > + Accesses to the filesystem are done by virtfs-proxy-helper(1). This > > > + option is deprecated (since QEMU 8.1) and will be removed in a future > > > + version of QEMU. Use ``local`` instead. > > > > > > ``synth`` > > > Synthetic filesystem, only used by QTests. > > > @@ -1867,6 +1869,8 @@ SRST > > > > > > ``proxy`` > > > Accesses to the filesystem are done by virtfs-proxy-helper(1). > > > + This option is deprecated (since QEMU 8.1) and will be removed in a > > > + future version of QEMU. Use ``local`` instead. > > > > > > ``synth`` > > > Synthetic filesystem, only used by QTests. > > > diff --git a/softmmu/vl.c b/softmmu/vl.c > > > index b0b96f67fa..e60648b591 100644 > > > --- a/softmmu/vl.c > > > +++ b/softmmu/vl.c > > > @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv) > > > error_report("Usage: -virtfs fsdriver,mount_tag=tag"); > > > exit(1); > > > } > > > + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) { > > > + warn_report("'-virtfs proxy' is deprecated, use " > > > + "'-virtfs local' instead"); > > > + } > > > + > > > fsdev = qemu_opts_create(qemu_find_opts("fsdev"), > > > qemu_opts_id(opts) ?: > > > qemu_opt_get(opts, "mount_tag"), > > > > > > > > > > > > >
On Sat, Jun 10, 2023 at 03:39:44PM +0200, Christian Schoenebeck wrote: > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > + > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > +removed in a future version of QEMU. Please use ``-fsdev local`` or > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > + > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > +``local`` backend. The idea was to enhance security by dispatching actual low > +level filesystem operations from 9p server (QEMU process) over to a separate > +process (the virtfs-proxy-helper binary). However this alternative never gained > +momentum. The proxy backend is much slower than the local backend, hasn't seen > +any development in years, and showed to be less secure, especially due to the > +fact that its helper daemon must be run as root, whereas with the local backend > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > +mapping permissions et al. The fact that the helper daemon runs as root was actually an intentional design choice to improve security. When QEMU is running unprivileged, the 'local' backend is limited in what it can serve to stuff that is readable/ writable by the 'qemu' user account. Using the 'proxy' backend allowed that restriction to be lifted, such that it can serve files owned by arbitrary users. Yes, the 'proxy' backend is less secure than the 'local' backend in an unprivileged QEMU. It is massively more secure, however, than the 'local' backend in a QEMU process running as root, which was the only option if you need to serve files for many users. IOW, if someone is currently using the 'proxy' backend, the 'local' backend is quite likely NOT a viable alternative. I'm fine with deprecating this. In terms of messaging wrt replacements, we should highlight both the 9p 'local' backend, and virtiofsd as the two alternatives. The latter is likely the better choice (on linux hosts) for many. With regards, Daniel
On Wednesday, June 21, 2023 3:41:36 PM CEST Greg Kurz wrote: > On Wed, 21 Jun 2023 15:32:39 +0200 > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > > > On Thursday, June 15, 2023 11:35:05 AM CEST Christian Schoenebeck wrote: > > > On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > > > > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > > > > bad shape. Using the 'proxy' backend was already discouraged for safety > > > > reasons before and we recommended to use the 'local' backend instead, > > > > but now it is time to officially deprecate the 'proxy' backend. > > > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > > > Ping > > > > It seems you missed the review I posted last week : > > https://patchew.org/QEMU/E1q7ytt-0005Fl-JX@lizzy.crudebyte.com/ #20230612165742.3333ea08@bahia Oh, I never received your email. I'll check the logs what happened there. I'll send a v4 with your suggestions, they make sense. Do you want me to add your "for the records" comments as well in the deprecation notice? Best regards, Christian Schoenebeck
On Wednesday, June 21, 2023 3:46:39 PM CEST Daniel P. Berrangé wrote: > On Sat, Jun 10, 2023 at 03:39:44PM +0200, Christian Schoenebeck wrote: > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > + > > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > > +removed in a future version of QEMU. Please use ``-fsdev local`` or > > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > > + > > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > > +``local`` backend. The idea was to enhance security by dispatching actual low > > +level filesystem operations from 9p server (QEMU process) over to a separate > > +process (the virtfs-proxy-helper binary). However this alternative never gained > > +momentum. The proxy backend is much slower than the local backend, hasn't seen > > +any development in years, and showed to be less secure, especially due to the > > +fact that its helper daemon must be run as root, whereas with the local backend > > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > > +mapping permissions et al. > > The fact that the helper daemon runs as root was actually an intentional > design choice to improve security. When QEMU is running unprivileged, the > 'local' backend is limited in what it can serve to stuff that is readable/ > writable by the 'qemu' user account. > > Using the 'proxy' backend allowed that restriction to be lifted, such > that it can serve files owned by arbitrary users. > > Yes, the 'proxy' backend is less secure than the 'local' backend in an > unprivileged QEMU. It is massively more secure, however, than the 'local' > backend in a QEMU process running as root, which was the only option if > you need to serve files for many users. > > IOW, if someone is currently using the 'proxy' backend, the 'local' backend > is quite likely NOT a viable alternative. Depends. Some people just want to dump few files between host <-> guest, they should even be fine with unpriviliged 'passhthrough' security model with the 'local' backend. For more complex use cases, you would probably transition to 'mapped' security model with the 'local' backend. That's like transitioning from one file system to another, mounting the two, copying over once, that's it. > I'm fine with deprecating this. In terms of messaging wrt replacements, > we should highlight both the 9p 'local' backend, and virtiofsd as the > two alternatives. The latter is likely the better choice (on linux > hosts) for many. OK, I can add that to the deprecation doc, but you don't want that to be added to all the runtime warnings as well, do you? Best regards, Christian Schoenebeck
On Wed, 21 Jun 2023 16:16:46 +0200 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Wednesday, June 21, 2023 3:41:36 PM CEST Greg Kurz wrote: > > On Wed, 21 Jun 2023 15:32:39 +0200 > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > > > > > On Thursday, June 15, 2023 11:35:05 AM CEST Christian Schoenebeck wrote: > > > > On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > > > > > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is > in > > > > > bad shape. Using the 'proxy' backend was already discouraged for > safety > > > > > reasons before and we recommended to use the 'local' backend instead, > > > > > but now it is time to officially deprecate the 'proxy' backend. > > > > > > > > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > > > > > Ping > > > > > > > It seems you missed the review I posted last week : > > > > https://patchew.org/QEMU/E1q7ytt-0005Fl-JX@lizzy.crudebyte.com/ > #20230612165742.3333ea08@bahia > > Oh, I never received your email. I'll check the logs what happened there. > > I'll send a v4 with your suggestions, they make sense. > > Do you want me to add your "for the records" comments as well in the > deprecation notice? > This was more targeting qemu-devel archives or git log, but feel free to provide relevant details in the deprecation notice. I agree with Daniel that virtiofsd should also be mentioned as an alternative. > Best regards, > Christian Schoenebeck > > Cheers,
On Wed, Jun 21, 2023 at 04:27:04PM +0200, Christian Schoenebeck wrote: > On Wednesday, June 21, 2023 3:46:39 PM CEST Daniel P. Berrangé wrote: > > On Sat, Jun 10, 2023 at 03:39:44PM +0200, Christian Schoenebeck wrote: > > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > > > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > + > > > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > > > +removed in a future version of QEMU. Please use ``-fsdev local`` or > > > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > > > + > > > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > > > +``local`` backend. The idea was to enhance security by dispatching actual low > > > +level filesystem operations from 9p server (QEMU process) over to a separate > > > +process (the virtfs-proxy-helper binary). However this alternative never gained > > > +momentum. The proxy backend is much slower than the local backend, hasn't seen > > > +any development in years, and showed to be less secure, especially due to the > > > +fact that its helper daemon must be run as root, whereas with the local backend > > > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > > > +mapping permissions et al. > > > > The fact that the helper daemon runs as root was actually an intentional > > design choice to improve security. When QEMU is running unprivileged, the > > 'local' backend is limited in what it can serve to stuff that is readable/ > > writable by the 'qemu' user account. > > > > Using the 'proxy' backend allowed that restriction to be lifted, such > > that it can serve files owned by arbitrary users. > > > > Yes, the 'proxy' backend is less secure than the 'local' backend in an > > unprivileged QEMU. It is massively more secure, however, than the 'local' > > backend in a QEMU process running as root, which was the only option if > > you need to serve files for many users. > > > > IOW, if someone is currently using the 'proxy' backend, the 'local' backend > > is quite likely NOT a viable alternative. > > Depends. Some people just want to dump few files between host <-> guest, they > should even be fine with unpriviliged 'passhthrough' security model with the > 'local' backend. > > For more complex use cases, you would probably transition to 'mapped' security > model with the 'local' backend. That's like transitioning from one file system > to another, mounting the two, copying over once, that's it. > > > I'm fine with deprecating this. In terms of messaging wrt replacements, > > we should highlight both the 9p 'local' backend, and virtiofsd as the > > two alternatives. The latter is likely the better choice (on linux > > hosts) for many. > > OK, I can add that to the deprecation doc, but you don't want that to be > added to all the runtime warnings as well, do you? I'd suggest we could do mention it briefly as an option at rutime, eg warn_report("'-virtfs proxy' is deprecated, use '-virtfs local' or virtiofs instead"); With regards, Daniel
On Wednesday, June 21, 2023 5:28:33 PM CEST Daniel P. Berrangé wrote: [...] > > > I'm fine with deprecating this. In terms of messaging wrt replacements, > > > we should highlight both the 9p 'local' backend, and virtiofsd as the > > > two alternatives. The latter is likely the better choice (on linux > > > hosts) for many. > > > > OK, I can add that to the deprecation doc, but you don't want that to be > > added to all the runtime warnings as well, do you? > > I'd suggest we could do mention it briefly as an option at rutime, eg > > warn_report("'-virtfs proxy' is deprecated, use '-virtfs local' or virtiofs instead"); I asked because with what Greg already suggested, it starts to become a long runtime message, like: "'-fsdev proxy' and '-virtfs proxy' are deprecated, use 'local' instead of 'proxy' or virtiofs" And that's probably still ambiguous, becauses that might suggest people that they could simply s/-virtfs proxy/-virtfs virtiofs/. Low care rate on my end though. Best regards, Christian Schoenebeck
diff --git a/MAINTAINERS b/MAINTAINERS index 436b3f0afe..185d694b2e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2118,13 +2118,20 @@ S: Odd Fixes W: https://wiki.qemu.org/Documentation/9p F: hw/9pfs/ X: hw/9pfs/xen-9p* +X: hw/9pfs/9p-proxy* F: fsdev/ +X: fsdev/virtfs-proxy-helper.c F: docs/tools/virtfs-proxy-helper.rst F: tests/qtest/virtio-9p-test.c F: tests/qtest/libqos/virtio-9p* T: git https://gitlab.com/gkurz/qemu.git 9p-next T: git https://github.com/cschoenebeck/qemu.git 9p.next +virtio-9p-proxy +F: hw/9pfs/9p-proxy* +F: fsdev/virtfs-proxy-helper.c +S: Obsolete + virtio-blk M: Stefan Hajnoczi <stefanha@redhat.com> L: qemu-block@nongnu.org diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0743459862..9b2c780365 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now necessary to distinguish between persistent and volatile memory backends. As such, memdev is deprecated in favor of persistent-memdev. +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The 9p ``proxy`` filesystem backend driver has been deprecated and will be +removed in a future version of QEMU. Please use ``-fsdev local`` or +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. + +The 9p ``proxy`` backend was originally developed as an alternative to the 9p +``local`` backend. The idea was to enhance security by dispatching actual low +level filesystem operations from 9p server (QEMU process) over to a separate +process (the virtfs-proxy-helper binary). However this alternative never gained +momentum. The proxy backend is much slower than the local backend, hasn't seen +any development in years, and showed to be less secure, especially due to the +fact that its helper daemon must be run as root, whereas with the local backend +QEMU is typically run as unprivileged user and allows to tighten behaviour by +mapping permissions et al. + Block device options '''''''''''''''''''' diff --git a/docs/tools/virtfs-proxy-helper.rst b/docs/tools/virtfs-proxy-helper.rst index 6cdeedf8e9..bd310ebb07 100644 --- a/docs/tools/virtfs-proxy-helper.rst +++ b/docs/tools/virtfs-proxy-helper.rst @@ -9,6 +9,9 @@ Synopsis Description ----------- +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be +removed, along with this daemon, in a future version of QEMU! + Pass-through security model in QEMU 9p server needs root privilege to do few file operations (like chown, chmod to any mode/uid:gid). There are two issues in pass-through security model: diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c index 3da64e9f72..242f54ab49 100644 --- a/fsdev/qemu-fsdev.c +++ b/fsdev/qemu-fsdev.c @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) } if (fsdriver) { + if (strncmp(fsdriver, "proxy", 5) == 0) { + warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " + "instead"); + } + for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { if (strcmp(FsDrivers[i].name, fsdriver) == 0) { break; diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c index d9511f429c..5dd5d99284 100644 --- a/fsdev/virtfs-proxy-helper.c +++ b/fsdev/virtfs-proxy-helper.c @@ -9,6 +9,11 @@ * the COPYING file in the top-level directory. */ +/* + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be + * removed in a future version of QEMU! + */ + #include "qemu/osdep.h" #include <glib/gstdio.h> #include <sys/resource.h> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index 99d115ff0d..905cae6992 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -15,6 +15,11 @@ * https://wiki.qemu.org/Documentation/9p */ +/* + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be + * removed in a future version of QEMU! + */ + #include "qemu/osdep.h" #include <sys/socket.h> #include <sys/un.h> diff --git a/hw/9pfs/9p-proxy.h b/hw/9pfs/9p-proxy.h index b84301d001..9be4718d3e 100644 --- a/hw/9pfs/9p-proxy.h +++ b/hw/9pfs/9p-proxy.h @@ -10,6 +10,11 @@ * the COPYING file in the top-level directory. */ +/* + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be + * removed in a future version of QEMU! + */ + #ifndef QEMU_9P_PROXY_H #define QEMU_9P_PROXY_H diff --git a/meson.build b/meson.build index 34306a6205..05c01b72bb 100644 --- a/meson.build +++ b/meson.build @@ -4170,7 +4170,7 @@ if have_block summary_info += {'Block whitelist (ro)': get_option('block_drv_ro_whitelist')} summary_info += {'Use block whitelist in tools': get_option('block_drv_whitelist_in_tools')} summary_info += {'VirtFS (9P) support': have_virtfs} - summary_info += {'VirtFS (9P) Proxy Helper support': have_virtfs_proxy_helper} + summary_info += {'VirtFS (9P) Proxy Helper support (deprecated)': have_virtfs_proxy_helper} summary_info += {'Live block migration': config_host_data.get('CONFIG_LIVE_BLOCK_MIGRATION')} summary_info += {'replication support': config_host_data.get('CONFIG_REPLICATION')} summary_info += {'bochs support': get_option('bochs').allowed()} diff --git a/qemu-options.hx b/qemu-options.hx index b57489d7ca..3a6c7d3ef9 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1735,7 +1735,9 @@ SRST Accesses to the filesystem are done by QEMU. ``proxy`` - Accesses to the filesystem are done by virtfs-proxy-helper(1). + Accesses to the filesystem are done by virtfs-proxy-helper(1). This + option is deprecated (since QEMU 8.1) and will be removed in a future + version of QEMU. Use ``local`` instead. ``synth`` Synthetic filesystem, only used by QTests. @@ -1867,6 +1869,8 @@ SRST ``proxy`` Accesses to the filesystem are done by virtfs-proxy-helper(1). + This option is deprecated (since QEMU 8.1) and will be removed in a + future version of QEMU. Use ``local`` instead. ``synth`` Synthetic filesystem, only used by QTests. diff --git a/softmmu/vl.c b/softmmu/vl.c index b0b96f67fa..e60648b591 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -3102,6 +3102,11 @@ void qemu_init(int argc, char **argv) error_report("Usage: -virtfs fsdriver,mount_tag=tag"); exit(1); } + if (strncmp(qemu_opt_get(opts, "fsdriver"), "proxy", 5) == 0) { + warn_report("'-virtfs proxy' is deprecated, use " + "'-virtfs local' instead"); + } + fsdev = qemu_opts_create(qemu_find_opts("fsdev"), qemu_opts_id(opts) ?: qemu_opt_get(opts, "mount_tag"),
As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in bad shape. Using the 'proxy' backend was already discouraged for safety reasons before and we recommended to use the 'local' backend instead, but now it is time to officially deprecate the 'proxy' backend. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> --- v2 -> v3: * Fix copy wasted typo (-> 'backend'). MAINTAINERS | 7 +++++++ docs/about/deprecated.rst | 17 +++++++++++++++++ docs/tools/virtfs-proxy-helper.rst | 3 +++ fsdev/qemu-fsdev.c | 5 +++++ fsdev/virtfs-proxy-helper.c | 5 +++++ hw/9pfs/9p-proxy.c | 5 +++++ hw/9pfs/9p-proxy.h | 5 +++++ meson.build | 2 +- qemu-options.hx | 6 +++++- softmmu/vl.c | 5 +++++ 10 files changed, 58 insertions(+), 2 deletions(-)