[Libguestfs] [libnbd PATCH 2/2] uri: Reject nbd:unix:/path/to/socket as invalid URI
libxml2 parses it as valid per RFC 3986, as the nbd: scheme with no
authority and a relative path. This string has been used with qemu to
request a Unix socket, such that nbdkit --run produces it for $nbd
(compared to $unixsocket), but accepting it as a URI means that we
instead try to connect to a TCP socket with a default authority
(localhost, port 10809), and nbdkit ignores the path element
'unix:/path/to/socket' and you end up with a connection (or an
attempt) to a completely different server.
It's easiest to just reject all URIs that do not use the
scheme://authority form, at which point any path element is either
empty or begins with '/'. This rejects 'nbd:unix:/path/to/socket'
which nbdkit produces, and also rejects 'nbd:/path/to/socket' (with no
authority but an absolute path) even though that form is less likely
to appear in the wild as qemu did not use it.
Reported-by: Martin Kletzander <mkletzan(a)redhat.com>
---
lib/connect.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/lib/connect.c b/lib/connect.c
index 8b19e1e..e9d76ff 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -28,6 +28,7 @@
#include <netinet/tcp.h>
#include <sys/types.h>
#include <sys/socket.h>
+#include <assert.h>
#ifdef HAVE_LIBXML2
#include <libxml/uri.h>
@@ -275,6 +276,12 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char
*raw_uri)
goto cleanup;
}
+ /* Insist on the scheme://[authority][/absname] form. */
+ if (strncmp (raw_uri + strlen (uri->scheme), "://", 3)) {
+ set_error (EINVAL, "URI must begin with '%s://'", uri->scheme);
+ goto cleanup;
+ }
+
for (i = 0; i < nqueries; i++) {
if (strcmp (queries[i].name, "socket") == 0)
unixsocket = queries[i].value;
@@ -291,14 +298,11 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char
*raw_uri)
* nbd_unlocked_set_tls_* to match...
*/
- /* Export name. Insist on the scheme://[authority][/absname] form. */
+ /* Export name. */
if (uri->path) {
- if (uri->path[0] == '/')
- r = nbd_unlocked_set_export_name (h, &uri->path[1]);
- else {
- set_error (EINVAL, "URI must begin with '%s://'",
uri->scheme);
- goto cleanup;
- }
+ /* Since we require scheme://authority above, any path is absolute */
+ assert (uri->path[0] == '/');
+ r = nbd_unlocked_set_export_name (h, &uri->path[1]);
}
else
r = nbd_unlocked_set_export_name (h, "");
--
2.20.1