configure | 7 + Makefile | 16 +++ Makefile.objs | 1 + contrib/virtiofsd/Makefile.objs | 10 +- contrib/virtiofsd/dbus.h | 9 ++ contrib/virtiofsd/dbus.c | 162 +++++++++++++++++++++++ contrib/virtiofsd/passthrough_ll.c | 8 +- contrib/virtiofsd/virtiofsctl.c | 55 ++++++++ .gitignore | 1 + contrib/virtiofsd/org.qemu.Virtiofsd.xml | 7 + 10 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 contrib/virtiofsd/dbus.h create mode 100644 contrib/virtiofsd/dbus.c create mode 100644 contrib/virtiofsd/virtiofsctl.c create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml
It is likely that virtiofsd will need to support "management commands" for reconfiguring it at runtime. The first use case was proposed by Eryu Guan for getting/setting the current log level. I promised to try out DBus as the management interface because it has a rich feature set and is accessible from most programming languages. It should be able to support all the use cases we come up with. This patch series is a prototype that implements the get-log-level and set-log-level management commands via DBus. Use the new virtiofsctl tool to talk to a running virtiofsd process: # dbus-run-session ./virtiofsd ... ... Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 # ./virtiofsctl set-log-level err Most of the work is done by gdbus-codegen(1). It generates code for the org.qemu.Virtiofsd.xml interface definition. Our code can use the simple virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. I'm pretty happy with this approach because the code is straightforward. It hasn't even triggered seccomp failures yet :). Error handling is a little problematic. I noticed that virtiofsctl silently returns success even if it cannot talk to virtiofsd. This is due to the code generated by gdbus-codegen(1) which has no error reporting :(. This can be solved by writing more low-level GDBus code instead of using the high-level generated bindings. What do you think about this approach? Stefan Hajnoczi (3): virtiofsd: add org.qemu.Virtiofsd interface virtiofsd: add DBus server to handle log level changes virtiofsd: add virtiofsctl command-line management tool configure | 7 + Makefile | 16 +++ Makefile.objs | 1 + contrib/virtiofsd/Makefile.objs | 10 +- contrib/virtiofsd/dbus.h | 9 ++ contrib/virtiofsd/dbus.c | 162 +++++++++++++++++++++++ contrib/virtiofsd/passthrough_ll.c | 8 +- contrib/virtiofsd/virtiofsctl.c | 55 ++++++++ .gitignore | 1 + contrib/virtiofsd/org.qemu.Virtiofsd.xml | 7 + 10 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 contrib/virtiofsd/dbus.h create mode 100644 contrib/virtiofsd/dbus.c create mode 100644 contrib/virtiofsd/virtiofsctl.c create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml -- 2.21.0
On Thu, Sep 05, 2019 at 04:21:33PM +0100, Stefan Hajnoczi wrote: > It is likely that virtiofsd will need to support "management commands" for > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > getting/setting the current log level. > > I promised to try out DBus as the management interface because it has a rich > feature set and is accessible from most programming languages. It should be > able to support all the use cases we come up with. > > This patch series is a prototype that implements the get-log-level and > set-log-level management commands via DBus. Use the new virtiofsctl tool to > talk to a running virtiofsd process: > > # dbus-run-session ./virtiofsd ... > ... > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # ./virtiofsctl set-log-level err > > Most of the work is done by gdbus-codegen(1). It generates code for the > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > I'm pretty happy with this approach because the code is straightforward. It > hasn't even triggered seccomp failures yet :). > > Error handling is a little problematic. I noticed that virtiofsctl silently > returns success even if it cannot talk to virtiofsd. This is due to the code > generated by gdbus-codegen(1) which has no error reporting :(. This can be > solved by writing more low-level GDBus code instead of using the high-level > generated bindings. You declared a simple property for the log level, which gets mapped into GObject properties, and hence hit the error reporting limitations they have. DBus at the protocol level can report errors for properties. For log level settings I'm not worried, but more generally we may well hit cases where error reporting is functionally critical. So we have a choice I think - Enhance gdbus-codegen so that it can map DBus properties to a different impl, with explicit getters/setters, ignoring GObject's properties. This would allow for a GError ** parameter to handle/report errors - Don't use properties at the DBus protocol level. Instead simply define explicit setter & getter methods in DBus. These woudl again bypass GObject's properties Option 2 is probably easier to be honest, especially since in the code you end up calling what are setters & getters on the client side anyway. > What do you think about this approach? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Sep 05, 2019 at 04:21:33PM +0100, Stefan Hajnoczi wrote: > It is likely that virtiofsd will need to support "management commands" for > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > getting/setting the current log level. > > I promised to try out DBus as the management interface because it has a rich > feature set and is accessible from most programming languages. It should be > able to support all the use cases we come up with. > > This patch series is a prototype that implements the get-log-level and > set-log-level management commands via DBus. Use the new virtiofsctl tool to > talk to a running virtiofsd process: > > # dbus-run-session ./virtiofsd ... > ... > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # ./virtiofsctl set-log-level err > > Most of the work is done by gdbus-codegen(1). It generates code for the > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > I'm pretty happy with this approach because the code is straightforward. It > hasn't even triggered seccomp failures yet :). > > Error handling is a little problematic. I noticed that virtiofsctl silently > returns success even if it cannot talk to virtiofsd. This is due to the code > generated by gdbus-codegen(1) which has no error reporting :(. This can be > solved by writing more low-level GDBus code instead of using the high-level > generated bindings. > > What do you think about this approach? Hi Eryu Guan, Have you had a chance to try these patches? Do they meet your requirements for being able to get/set the log level at runtime? Stefan
* Stefan Hajnoczi (stefanha@redhat.com) wrote: > It is likely that virtiofsd will need to support "management commands" for > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > getting/setting the current log level. > > I promised to try out DBus as the management interface because it has a rich > feature set and is accessible from most programming languages. It should be > able to support all the use cases we come up with. > > This patch series is a prototype that implements the get-log-level and > set-log-level management commands via DBus. Use the new virtiofsctl tool to > talk to a running virtiofsd process: > > # dbus-run-session ./virtiofsd ... > ... > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > # ./virtiofsctl set-log-level err > > Most of the work is done by gdbus-codegen(1). It generates code for the > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > I'm pretty happy with this approach because the code is straightforward. It > hasn't even triggered seccomp failures yet :). Yes it's less complex than I'd worried. Now, I do think we've got to think about how qemu in general is going to use dbus as people were discussing it, because then we have to think what the security aspects are - do we need to look at some calls only available to some clients etc. Dave > Error handling is a little problematic. I noticed that virtiofsctl silently > returns success even if it cannot talk to virtiofsd. This is due to the code > generated by gdbus-codegen(1) which has no error reporting :(. This can be > solved by writing more low-level GDBus code instead of using the high-level > generated bindings. > > What do you think about this approach? > > Stefan Hajnoczi (3): > virtiofsd: add org.qemu.Virtiofsd interface > virtiofsd: add DBus server to handle log level changes > virtiofsd: add virtiofsctl command-line management tool > > configure | 7 + > Makefile | 16 +++ > Makefile.objs | 1 + > contrib/virtiofsd/Makefile.objs | 10 +- > contrib/virtiofsd/dbus.h | 9 ++ > contrib/virtiofsd/dbus.c | 162 +++++++++++++++++++++++ > contrib/virtiofsd/passthrough_ll.c | 8 +- > contrib/virtiofsd/virtiofsctl.c | 55 ++++++++ > .gitignore | 1 + > contrib/virtiofsd/org.qemu.Virtiofsd.xml | 7 + > 10 files changed, 274 insertions(+), 2 deletions(-) > create mode 100644 contrib/virtiofsd/dbus.h > create mode 100644 contrib/virtiofsd/dbus.c > create mode 100644 contrib/virtiofsd/virtiofsctl.c > create mode 100644 contrib/virtiofsd/org.qemu.Virtiofsd.xml > > -- > 2.21.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote: > * Stefan Hajnoczi (stefanha@redhat.com) wrote: > > It is likely that virtiofsd will need to support "management commands" for > > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > > getting/setting the current log level. > > > > I promised to try out DBus as the management interface because it has a rich > > feature set and is accessible from most programming languages. It should be > > able to support all the use cases we come up with. > > > > This patch series is a prototype that implements the get-log-level and > > set-log-level management commands via DBus. Use the new virtiofsctl tool to > > talk to a running virtiofsd process: > > > > # dbus-run-session ./virtiofsd ... > > ... > > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > # ./virtiofsctl set-log-level err > > > > Most of the work is done by gdbus-codegen(1). It generates code for the > > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > > > I'm pretty happy with this approach because the code is straightforward. It > > hasn't even triggered seccomp failures yet :). > > Yes it's less complex than I'd worried. > Now, I do think we've got to think about how qemu in general is going to > use dbus as people were discussing it, because then we have to think > what the security aspects are - do we need to look at some calls only > available to some clients etc. The approach I took in this patch series is to launch a session bus just for this virtiofsd. The abstract socket unix(7) namespace used by GDBus by default does not offer any security. I think any process on the host can connect to it, regardless of uid/gid. A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket permissions as the main security mechanism. I'm not enthusiastic about using SELinux or some kind of DBus-specific policy language if we can avoid it because it's complex and obscure. Stefan
* Stefan Hajnoczi (stefanha@redhat.com) wrote: > On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote: > > * Stefan Hajnoczi (stefanha@redhat.com) wrote: > > > It is likely that virtiofsd will need to support "management commands" for > > > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > > > getting/setting the current log level. > > > > > > I promised to try out DBus as the management interface because it has a rich > > > feature set and is accessible from most programming languages. It should be > > > able to support all the use cases we come up with. > > > > > > This patch series is a prototype that implements the get-log-level and > > > set-log-level management commands via DBus. Use the new virtiofsctl tool to > > > talk to a running virtiofsd process: > > > > > > # dbus-run-session ./virtiofsd ... > > > ... > > > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > > # ./virtiofsctl set-log-level err > > > > > > Most of the work is done by gdbus-codegen(1). It generates code for the > > > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > > > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > > > > > I'm pretty happy with this approach because the code is straightforward. It > > > hasn't even triggered seccomp failures yet :). > > > > Yes it's less complex than I'd worried. > > Now, I do think we've got to think about how qemu in general is going to > > use dbus as people were discussing it, because then we have to think > > what the security aspects are - do we need to look at some calls only > > available to some clients etc. > > The approach I took in this patch series is to launch a session bus just > for this virtiofsd. The abstract socket unix(7) namespace used by GDBus > by default does not offer any security. I think any process on the host > can connect to it, regardless of uid/gid. > > A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket > permissions as the main security mechanism. Yes, that I'm fine with; my worry is that there was talk of much more complex dbus setups coming out of the qemu world with many things being connected; and then we have to think about security upfront. > I'm not enthusiastic about > using SELinux or some kind of DBus-specific policy language if we can > avoid it because it's complex and obscure. Right. Dave > Stefan -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Sep 06, 2019 at 11:29:26AM +0100, Stefan Hajnoczi wrote: > On Thu, Sep 05, 2019 at 06:40:21PM +0100, Dr. David Alan Gilbert wrote: > > * Stefan Hajnoczi (stefanha@redhat.com) wrote: > > > It is likely that virtiofsd will need to support "management commands" for > > > reconfiguring it at runtime. The first use case was proposed by Eryu Guan for > > > getting/setting the current log level. > > > > > > I promised to try out DBus as the management interface because it has a rich > > > feature set and is accessible from most programming languages. It should be > > > able to support all the use cases we come up with. > > > > > > This patch series is a prototype that implements the get-log-level and > > > set-log-level management commands via DBus. Use the new virtiofsctl tool to > > > talk to a running virtiofsd process: > > > > > > # dbus-run-session ./virtiofsd ... > > > ... > > > Using dbus address unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > > # export DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-H9WBbpjk3O,guid=0be16acefb868e6025a8737f5d7124d2 > > > # ./virtiofsctl set-log-level err > > > > > > Most of the work is done by gdbus-codegen(1). It generates code for the > > > org.qemu.Virtiofsd.xml interface definition. Our code can use the simple > > > virtiofsd_get/set_log_level() APIs and it will make corresponding DBus calls. > > > > > > I'm pretty happy with this approach because the code is straightforward. It > > > hasn't even triggered seccomp failures yet :). > > > > Yes it's less complex than I'd worried. > > Now, I do think we've got to think about how qemu in general is going to > > use dbus as people were discussing it, because then we have to think > > what the security aspects are - do we need to look at some calls only > > available to some clients etc. > > The approach I took in this patch series is to launch a session bus just > for this virtiofsd. The abstract socket unix(7) namespace used by GDBus > by default does not offer any security. I think any process on the host > can connect to it, regardless of uid/gid. Other users will be able to connect(), but you'll find that the dbus policy causes their connections to be dropped immediately - even if they are the root user in fact. > A path like unix:path=/tmp/foo would allow us to use UNIX Domain Socket > permissions as the main security mechanism. I'm not enthusiastic about > using SELinux or some kind of DBus-specific policy language if we can > avoid it because it's complex and obscure. Yep, that just needs you to supply a config file when launching to specify a desired filesystem path. I don't think it is an either/or matter - I think we'll want all three in general - DAC controls on the socket, and DBus policy and SELinux policy. DAC controls on the socket alone are not sufficient if you want to separate each QEMU from each other and they're running the same UID which is common. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.