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@xxxxxxxxxx>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
