[v1] qemu_dbus: Escape path to dbus-daemon socket if needed

[PATCH] qemu_dbus: Escape path to dbus-daemon socket if needed

Posted by Michal Privoznik via Devel 1 month, 3 weeks ago

From: Michal Privoznik <mprivozn@redhat.com>

According to [1] there are only few characters allowed in the
path to the daemon socket (alphanum and some punctuation;
notably, space is missing on the list). The rest must be escaped
by '%NN' notation. Fortunately, g_uri_escape_string() with some
carefully selected input values is able to escape the path.
Almost - it considers tilde valid but DBus doesn't. Well, let's
hope nobody has tilde in domain name.

1: https://gitlab.freedesktop.org/dbus/dbus/-/blob/2dee5236088bcf690ba92b743a584720b8cb6f55/dbus/dbus-address.c#L332
Closes: https://gitlab.com/libvirt/libvirt/-/issues/834
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_dbus.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 625884ad46..db0757b71d 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -119,6 +119,12 @@ static int
 qemuDBusWriteConfig(const char *filename, const char *path, bool privileged)
 {
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+    /* Escape @path pointing to socket where the dbus-daemon is going to listen
+     * to. Valid characters are alphanumeric, '-', '_', '/', '\\', '*' and '.'.
+     * The rest must be URI escaped. g_uri_escape_string() treads alphanumeric,
+     * '-', '.', '_' and '~' as valid.
+     */
+    g_autofree char *escapedPath = g_uri_escape_string(path, "/\\*", false);
     g_autofree char *config = NULL;
 
     virBufferAddLit(&buf, "<!DOCTYPE busconfig PUBLIC \"-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN\"\n");
@@ -127,7 +133,7 @@ qemuDBusWriteConfig(const char *filename, const char *path, bool privileged)
     virBufferAdjustIndent(&buf, 2);
 
     virBufferAddLit(&buf, "<type>org.libvirt.qemu</type>\n");
-    virBufferAsprintf(&buf, "<listen>unix:path=%s</listen>\n", path);
+    virBufferAsprintf(&buf, "<listen>unix:path=%s</listen>\n", escapedPath);
     virBufferAddLit(&buf, "<auth>EXTERNAL</auth>\n");
 
     virBufferAddLit(&buf, "<policy context='default'>\n");
-- 
2.51.2

Re: [PATCH] qemu_dbus: Escape path to dbus-daemon socket if needed

Posted by Ján Tomko via Devel 1 month, 3 weeks ago

On a Tuesday in 2025, Michal Privoznik via Devel wrote:
>From: Michal Privoznik <mprivozn@redhat.com>
>
>According to [1] there are only few characters allowed in the
>path to the daemon socket (alphanum and some punctuation;
>notably, space is missing on the list). The rest must be escaped
>by '%NN' notation. Fortunately, g_uri_escape_string() with some
>carefully selected input values is able to escape the path.

>Almost - it considers tilde valid but DBus doesn't. Well, let's
>hope nobody has tilde in domain name.

https://www.rfc-editor.org/rfc/rfc3986.html#appendix-A
Well, g_uri_escape_string does what it says:
    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"

Whether dbus is wrong or not, libvirt should escape the tilde too.

Jano

>
>1: https://gitlab.freedesktop.org/dbus/dbus/-/blob/2dee5236088bcf690ba92b743a584720b8cb6f55/dbus/dbus-address.c#L332
>Closes: https://gitlab.com/libvirt/libvirt/-/issues/834
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_dbus.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)

Re: [PATCH] qemu_dbus: Escape path to dbus-daemon socket if needed

Posted by Michal Prívozník via Devel 1 month, 3 weeks ago

On 12/18/25 13:35, Ján Tomko wrote:
> On a Tuesday in 2025, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <mprivozn@redhat.com>
>>
>> According to [1] there are only few characters allowed in the
>> path to the daemon socket (alphanum and some punctuation;
>> notably, space is missing on the list). The rest must be escaped
>> by '%NN' notation. Fortunately, g_uri_escape_string() with some
>> carefully selected input values is able to escape the path.
> 
>> Almost - it considers tilde valid but DBus doesn't. Well, let's
>> hope nobody has tilde in domain name.
> 
> https://www.rfc-editor.org/rfc/rfc3986.html#appendix-A
> Well, g_uri_escape_string does what it says:
>    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
> 
> Whether dbus is wrong or not, libvirt should escape the tilde too.

Okay, I'll try to fix that in v2. But honestly, I think this is so
narrow corner case that it feels almost useless work.

Michal

Re: [PATCH] qemu_dbus: Escape path to dbus-daemon socket if needed

Posted by Daniel P. Berrangé via Devel 1 month, 3 weeks ago

On Thu, Dec 18, 2025 at 02:20:28PM +0100, Michal Prívozník via Devel wrote:
> On 12/18/25 13:35, Ján Tomko wrote:
> > On a Tuesday in 2025, Michal Privoznik via Devel wrote:
> >> From: Michal Privoznik <mprivozn@redhat.com>
> >>
> >> According to [1] there are only few characters allowed in the
> >> path to the daemon socket (alphanum and some punctuation;
> >> notably, space is missing on the list). The rest must be escaped
> >> by '%NN' notation. Fortunately, g_uri_escape_string() with some
> >> carefully selected input values is able to escape the path.
> > 
> >> Almost - it considers tilde valid but DBus doesn't. Well, let's
> >> hope nobody has tilde in domain name.
> > 
> > https://www.rfc-editor.org/rfc/rfc3986.html#appendix-A
> > Well, g_uri_escape_string does what it says:
> >    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
> > 
> > Whether dbus is wrong or not, libvirt should escape the tilde too.
> 
> Okay, I'll try to fix that in v2. But honestly, I think this is so
> narrow corner case that it feels almost useless work.

IIUC,  systemd's naming rules were designed to match the dbus naming
rules, since systemd units are exposed over dbus.

We already have virSystemdEscapeName that escapes everything that is
NOT a-z, A-Z, 0-9, :, -, _, ., \   which seems like what we need here
too.

Rename virSystemdEscapeName to virDBusEscapeName and use it everywhere ?

With 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 :|

Re: [PATCH] qemu_dbus: Escape path to dbus-daemon socket if needed

Posted by Michal Prívozník via Devel 1 month, 3 weeks ago

On 12/18/25 14:42, Daniel P. Berrangé wrote:
> On Thu, Dec 18, 2025 at 02:20:28PM +0100, Michal Prívozník via Devel wrote:
>> On 12/18/25 13:35, Ján Tomko wrote:
>>> On a Tuesday in 2025, Michal Privoznik via Devel wrote:
>>>> From: Michal Privoznik <mprivozn@redhat.com>
>>>>
>>>> According to [1] there are only few characters allowed in the
>>>> path to the daemon socket (alphanum and some punctuation;
>>>> notably, space is missing on the list). The rest must be escaped
>>>> by '%NN' notation. Fortunately, g_uri_escape_string() with some
>>>> carefully selected input values is able to escape the path.
>>>
>>>> Almost - it considers tilde valid but DBus doesn't. Well, let's
>>>> hope nobody has tilde in domain name.
>>>
>>> https://www.rfc-editor.org/rfc/rfc3986.html#appendix-A
>>> Well, g_uri_escape_string does what it says:
>>>    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
>>>
>>> Whether dbus is wrong or not, libvirt should escape the tilde too.
>>
>> Okay, I'll try to fix that in v2. But honestly, I think this is so
>> narrow corner case that it feels almost useless work.
> 
> IIUC,  systemd's naming rules were designed to match the dbus naming
> rules, since systemd units are exposed over dbus.
> 
> We already have virSystemdEscapeName that escapes everything that is
> NOT a-z, A-Z, 0-9, :, -, _, ., \   which seems like what we need here
> too.
> 
> Rename virSystemdEscapeName to virDBusEscapeName and use it everywhere ?

The former escapes using \xNN (which could be changed by passing an
argument), but what is worse, it escapes forward slash and a dot if it's
at the beginning. But I think I have implementation that matches DBus.

Michal