On Mon, Sep 23, 2019 at 11:03:41AM -0300, Daniel Henrique Barboza wrote:
The existing QEMU URI path validation considers that a privileged user can use a "/session" URI as well. Let's update virConnectValidateURIPath() to consider this usage, allowing us to use the function inside qemu_driver.c.
Although the existing callers of virConnectValidateURIPath() didn't consider that a privileged user could connect with "/session", it is sensible to consider that this should be the case. thus no serious harm is done.
That we allowed use of '/session' while connecting as root is a bug IMHO. Ideally we're kill that off entirely, so there's potential for us to allow a genuine session instance daemon as root at a later date. I'm just concerned about whether there might be apps relying on this bug. I certainly don't want to see the bug spread across all drivers though.
Suggested-by: Cole Robinson <crobinso@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/driver.c | 3 ++- src/qemu/qemu_driver.c | 20 ++++---------------- 2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/src/driver.c b/src/driver.c index e627b0c1d7..f9c41383dc 100644 --- a/src/driver.c +++ b/src/driver.c @@ -276,7 +276,8 @@ virConnectValidateURIPath(const char *uriPath, bool privileged) { if (privileged) { - if (STRNEQ(uriPath, "/system")) { + if (STRNEQ(uriPath, "/system") && + STRNEQ(uriPath, "/session")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected %s URI path '%s', try %s:///system"), entityName, uriPath, entityName); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0753904472..05025e7251 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1295,22 +1295,10 @@ static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, return VIR_DRV_OPEN_ERROR; }
- if (virQEMUDriverIsPrivileged(qemu_driver)) { - if (STRNEQ(conn->uri->path, "/system") && - STRNEQ(conn->uri->path, "/session")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected QEMU URI path '%s', try qemu:///system"), - conn->uri->path); - return VIR_DRV_OPEN_ERROR; - } - } else { - if (STRNEQ(conn->uri->path, "/session")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected QEMU URI path '%s', try qemu:///session"), - conn->uri->path); - return VIR_DRV_OPEN_ERROR; - } - } + if (!virConnectValidateURIPath(conn->uri->path, + "QEMU", + virQEMUDriverIsPrivileged(qemu_driver))) + return VIR_DRV_OPEN_ERROR;
if (virConnectOpenEnsureACL(conn) < 0) return VIR_DRV_OPEN_ERROR; -- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 :|