[Libguestfs] [PATCH] Fix handling of passwords in URLs
Pino Toscano
ptoscano at redhat.com
Fri May 2 10:41:15 UTC 2014
So far, passwords in URLs (eg http://user:password@host..) have been
handled as part of the username, and thus passing
add-drive path username:username:password ...
instead of
add-drive path username:username secret:password ...
Fix the parsing of URLs to handle passwords as separate elements,
properly passing it as "secret" parameter for add-drive, and properly
readd it when building URLs in the direct backend.
Furthmore, to keep curl- and ssh-based qemu drivers working with
authenticated resources, make sure they can accept secrets.
Reported in comment #1 of RHBZ#1092583.
---
customize/customize_main.ml | 5 +++--
fish/options.c | 6 ++++++
fish/options.h | 1 +
fish/test-add-uri.sh | 6 ++++++
fish/uri.c | 26 ++++++++++++++++++++++----
fish/uri.h | 1 +
mllib/uRI.ml | 1 +
mllib/uRI.mli | 1 +
mllib/uri-c.c | 13 ++++++++++++-
resize/resize.ml | 5 +++--
src/drives.c | 10 ----------
src/launch-direct.c | 34 +++++++++++++++++++++++-----------
sysprep/main.ml | 5 +++--
13 files changed, 82 insertions(+), 32 deletions(-)
diff --git a/customize/customize_main.ml b/customize/customize_main.ml
index fb38336..00d5bae 100644
--- a/customize/customize_main.ml
+++ b/customize/customize_main.ml
@@ -157,11 +157,12 @@ read the man page virt-customize(1).
List.iter (
fun (uri, format) ->
let { URI.path = path; protocol = protocol;
- server = server; username = username } = uri in
+ server = server; username = username;
+ password = password } = uri in
let discard = if readonly then None else Some "besteffort" in
g#add_drive
~readonly ?discard
- ?format ~protocol ?server ?username
+ ?format ~protocol ?server ?username ?secret:password
path
) files
in
diff --git a/fish/options.c b/fish/options.c
index 80b71ec..5e6eb73 100644
--- a/fish/options.c
+++ b/fish/options.c
@@ -67,6 +67,7 @@ option_a (const char *arg, const char *format, struct drv **drvsp)
drv->uri.protocol = uri.protocol;
drv->uri.server = uri.server;
drv->uri.username = uri.username;
+ drv->uri.password = uri.password;
drv->uri.format = format;
drv->uri.orig_uri = arg;
}
@@ -167,6 +168,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, char next_drive)
ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK;
ad_optargs.username = drv->uri.username;
}
+ if (drv->uri.password) {
+ ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
+ ad_optargs.secret = drv->uri.password;
+ }
r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs);
if (r == -1)
@@ -290,6 +295,7 @@ free_drives (struct drv *drv)
free (drv->uri.protocol);
guestfs___free_string_list (drv->uri.server);
free (drv->uri.username);
+ free (drv->uri.password);
break;
case drv_d:
/* d.filename is optarg, don't free it */
diff --git a/fish/options.h b/fish/options.h
index dbf9163..639b1fe 100644
--- a/fish/options.h
+++ b/fish/options.h
@@ -68,6 +68,7 @@ struct drv {
char *protocol; /* protocol (eg. "nbd") */
char **server; /* server(s) - can be NULL */
char *username; /* username - can be NULL */
+ char *password; /* password - can be NULL */
const char *format; /* format (NULL == autodetect) */
const char *orig_uri; /* original URI (for error messages etc.) */
} uri;
diff --git a/fish/test-add-uri.sh b/fish/test-add-uri.sh
index 1a5b067..2b7a142 100755
--- a/fish/test-add-uri.sh
+++ b/fish/test-add-uri.sh
@@ -41,6 +41,9 @@ grep -sq 'add_drive ".*/test-add-uri.img"' test-add-uri.out || fail
$VG ./guestfish -x -a ftp://user@example.com/disk.img </dev/null >test-add-uri.out 2>&1
grep -sq 'add_drive "/disk.img" "protocol:ftp" "server:tcp:example.com" "username:user"' test-add-uri.out || fail
+$VG ./guestfish -x -a ftp://user:password@example.com/disk.img </dev/null >test-add-uri.out 2>&1
+grep -sq 'add_drive "/disk.img" "protocol:ftp" "server:tcp:example.com" "username:user" "secret:password"' test-add-uri.out || fail
+
# gluster
$VG ./guestfish -x -a gluster://example.com/disk </dev/null >test-add-uri.out 2>&1
grep -sq 'add_drive "disk" "protocol:gluster" "server:tcp:example.com"' test-add-uri.out || fail
@@ -78,6 +81,9 @@ grep -sq 'add_drive "/disk.img" "protocol:ssh" "server:tcp:example.com"' test-ad
$VG ./guestfish -x -a ssh://user@example.com/disk.img </dev/null >test-add-uri.out 2>&1
grep -sq 'add_drive "/disk.img" "protocol:ssh" "server:tcp:example.com" "username:user"' test-add-uri.out || fail
+$VG ./guestfish -x -a ssh://user:password@example.com/disk.img </dev/null >test-add-uri.out 2>&1
+grep -sq 'add_drive "/disk.img" "protocol:ssh" "server:tcp:example.com" "username:user" "secret:password"' test-add-uri.out || fail
+
$VG ./guestfish -x -a ssh://user@example.com:2000/disk.img </dev/null >test-add-uri.out 2>&1
grep -sq 'add_drive "/disk.img" "protocol:ssh" "server:tcp:example.com:2000" "username:user"' test-add-uri.out || fail
diff --git a/fish/uri.c b/fish/uri.c
index 95f8cf8..f45c907 100644
--- a/fish/uri.c
+++ b/fish/uri.c
@@ -34,7 +34,7 @@
#include "uri.h"
static int is_uri (const char *arg);
-static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret);
+static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret, char **password_ret);
static char *query_get (xmlURIPtr uri, const char *search_name);
static int make_server (xmlURIPtr uri, const char *socket, char ***ret);
@@ -45,10 +45,11 @@ parse_uri (const char *arg, struct uri *uri_ret)
char *protocol = NULL;
char **server = NULL;
char *username = NULL;
+ char *password = NULL;
/* Does it look like a URI? */
if (is_uri (arg)) {
- if (parse (arg, &path, &protocol, &server, &username) == -1)
+ if (parse (arg, &path, &protocol, &server, &username, &password) == -1)
return -1;
}
else {
@@ -70,6 +71,7 @@ parse_uri (const char *arg, struct uri *uri_ret)
uri_ret->protocol = protocol;
uri_ret->server = server;
uri_ret->username = username;
+ uri_ret->password = password;
return 0;
}
@@ -99,7 +101,7 @@ is_uri (const char *arg)
static int
parse (const char *arg, char **path_ret, char **protocol_ret,
- char ***server_ret, char **username_ret)
+ char ***server_ret, char **username_ret, char **password_ret)
{
CLEANUP_XMLFREEURI xmlURIPtr uri = NULL;
CLEANUP_FREE char *socket = NULL;
@@ -150,16 +152,31 @@ parse (const char *arg, char **path_ret, char **protocol_ret,
return -1;
}
+ *password_ret = NULL;
+ *username_ret = NULL;
if (uri->user && STRNEQ (uri->user, "")) {
+ char *p = strchr (uri->user, ':');
+ if (p != NULL) {
+ if (STRNEQ (p+1, "")) {
+ *password_ret = strdup (p+1);
+ if (*password_ret == NULL) {
+ perror ("strdup: password");
+ free (*protocol_ret);
+ guestfs___free_string_list (*server_ret);
+ return -1;
+ }
+ }
+ *p = '\0';
+ }
*username_ret = strdup (uri->user);
if (*username_ret == NULL) {
perror ("strdup: username");
+ free (*password_ret);
free (*protocol_ret);
guestfs___free_string_list (*server_ret);
return -1;
}
}
- else *username_ret = NULL;
/* We may have to adjust the path depending on the protocol. For
* example ceph/rbd URIs look like rbd:///pool/disk, but the
@@ -180,6 +197,7 @@ parse (const char *arg, char **path_ret, char **protocol_ret,
free (*protocol_ret);
guestfs___free_string_list (*server_ret);
free (*username_ret);
+ free (*password_ret);
return -1;
}
diff --git a/fish/uri.h b/fish/uri.h
index 420d20c..9202a70 100644
--- a/fish/uri.h
+++ b/fish/uri.h
@@ -26,6 +26,7 @@ struct uri {
char *protocol; /* protocol (eg. "file", "nbd") */
char **server; /* server(s) - can be NULL */
char *username; /* username - can be NULL */
+ char *password; /* password - can be NULL */
};
/* Parse the '-a' option parameter 'arg', and place the result in
diff --git a/mllib/uRI.ml b/mllib/uRI.ml
index 272f339..d4f7522 100644
--- a/mllib/uRI.ml
+++ b/mllib/uRI.ml
@@ -21,6 +21,7 @@ type uri = {
protocol : string;
server : string array option;
username : string option;
+ password : string option;
}
external parse_uri : string -> uri = "virt_resize_parse_uri"
diff --git a/mllib/uRI.mli b/mllib/uRI.mli
index efd39dd..0692f95 100644
--- a/mllib/uRI.mli
+++ b/mllib/uRI.mli
@@ -23,6 +23,7 @@ type uri = {
protocol : string; (** protocol, eg. [file], [nbd] *)
server : string array option; (** list of servers *)
username : string option; (** username *)
+ password : string option; (** password *)
}
val parse_uri : string -> uri
diff --git a/mllib/uri-c.c b/mllib/uri-c.c
index 1ae7350..2ce4021 100644
--- a/mllib/uri-c.c
+++ b/mllib/uri-c.c
@@ -47,7 +47,7 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */)
caml_invalid_argument ("URI.parse_uri");
/* Convert the struct into an OCaml tuple. */
- rv = caml_alloc_tuple (4);
+ rv = caml_alloc_tuple (5);
/* path : string */
sv = caml_copy_string (uri.path);
@@ -81,5 +81,16 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */)
ov = Val_int (0);
Store_field (rv, 3, ov);
+ /* password : string option */
+ if (uri.password) {
+ sv = caml_copy_string (uri.password);
+ free (uri.password);
+ ov = caml_alloc (1, 0);
+ Store_field (ov, 0, sv);
+ }
+ else
+ ov = Val_int (0);
+ Store_field (rv, 4, ov);
+
CAMLreturn (rv);
}
diff --git a/resize/resize.ml b/resize/resize.ml
index c1794ed..c6b6c9e 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -322,8 +322,9 @@ read the man page virt-resize(1).
let g = new G.guestfs () in
if debug then g#set_trace true;
let _, { URI.path = path; protocol = protocol;
- server = server; username = username } = infile in
- g#add_drive ?format ~readonly:true ~protocol ?server ?username path;
+ server = server; username = username;
+ password = password } = infile in
+ g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password path;
(* The output disk is being created, so use cache=unsafe here. *)
g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe"
outfile;
diff --git a/src/drives.c b/src/drives.c
index 57c2471..4bd8328 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -200,11 +200,6 @@ static struct drive *
create_drive_curl (guestfs_h *g,
const struct drive_create_data *data)
{
- if (data->secret != NULL) {
- error (g, _("curl: you cannot specify a secret with this protocol"));
- return NULL;
- }
-
if (data->nr_servers != 1) {
error (g, _("curl: you must specify exactly one server"));
return NULL;
@@ -371,11 +366,6 @@ static struct drive *
create_drive_ssh (guestfs_h *g,
const struct drive_create_data *data)
{
- if (data->secret != NULL) {
- error (g, _("ssh: you cannot specify a secret with this protocol"));
- return NULL;
- }
-
if (data->nr_servers != 1) {
error (g, _("ssh: you must specify exactly one server"));
return NULL;
diff --git a/src/launch-direct.c b/src/launch-direct.c
index bb06c9e..96ae180 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -1207,12 +1207,14 @@ qemu_escape_param (guestfs_h *g, const char *param)
static char *
make_uri (guestfs_h *g, const char *scheme, const char *user,
+ const char *password,
struct drive_server *server, const char *path)
{
xmlURI uri = { .scheme = (char *) scheme,
.user = (char *) user };
CLEANUP_FREE char *query = NULL;
CLEANUP_FREE char *pathslash = NULL;
+ CLEANUP_FREE char *userauth = NULL;
/* Need to add a leading '/' to URI paths since xmlSaveUri doesn't. */
if (path[0] != '/') {
@@ -1222,6 +1224,13 @@ make_uri (guestfs_h *g, const char *scheme, const char *user,
else
uri.path = (char *) path;
+ /* Rebuild user:password. */
+ if (user != NULL && password != NULL) {
+ /* Keep the string in an own variable so it can be freed automatically. */
+ userauth = safe_asprintf (g, "%s:%s", user, password);
+ uri.user = userauth;
+ }
+
switch (server->transport) {
case drive_transport_none:
case drive_transport_tcp:
@@ -1267,34 +1276,37 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)
return path;
case drive_protocol_ftp:
- return make_uri (g, "ftp", src->username,
+ return make_uri (g, "ftp", src->username, src->secret,
&src->servers[0], src->u.exportname);
case drive_protocol_ftps:
- return make_uri (g, "ftps", src->username,
+ return make_uri (g, "ftps", src->username, src->secret,
&src->servers[0], src->u.exportname);
case drive_protocol_gluster:
switch (src->servers[0].transport) {
case drive_transport_none:
- return make_uri (g, "gluster", NULL, &src->servers[0], src->u.exportname);
+ return make_uri (g, "gluster", NULL, NULL,
+ &src->servers[0], src->u.exportname);
case drive_transport_tcp:
- return make_uri (g, "gluster+tcp",
- NULL, &src->servers[0], src->u.exportname);
+ return make_uri (g, "gluster+tcp", NULL, NULL,
+ &src->servers[0], src->u.exportname);
case drive_transport_unix:
- return make_uri (g, "gluster+unix", NULL, &src->servers[0], NULL);
+ return make_uri (g, "gluster+unix", NULL, NULL,
+ &src->servers[0], NULL);
}
case drive_protocol_http:
- return make_uri (g, "http", src->username,
+ return make_uri (g, "http", src->username, src->secret,
&src->servers[0], src->u.exportname);
case drive_protocol_https:
- return make_uri (g, "https", src->username,
+ return make_uri (g, "https", src->username, src->secret,
&src->servers[0], src->u.exportname);
case drive_protocol_iscsi:
- return make_uri (g, "iscsi", NULL, &src->servers[0], src->u.exportname);
+ return make_uri (g, "iscsi", NULL, NULL,
+ &src->servers[0], src->u.exportname);
case drive_protocol_nbd: {
CLEANUP_FREE char *p = NULL;
@@ -1380,11 +1392,11 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)
src->u.exportname);
case drive_protocol_ssh:
- return make_uri (g, "ssh", src->username,
+ return make_uri (g, "ssh", src->username, src->secret,
&src->servers[0], src->u.exportname);
case drive_protocol_tftp:
- return make_uri (g, "tftp", src->username,
+ return make_uri (g, "tftp", src->username, src->secret,
&src->servers[0], src->u.exportname);
}
diff --git a/sysprep/main.ml b/sysprep/main.ml
index 71abfa7..37e4dc8 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -203,11 +203,12 @@ read the man page virt-sysprep(1).
List.iter (
fun (uri, format) ->
let { URI.path = path; protocol = protocol;
- server = server; username = username } = uri in
+ server = server; username = username;
+ password = password } = uri in
let discard = if readonly then None else Some "besteffort" in
g#add_drive
~readonly ?discard
- ?format ~protocol ?server ?username
+ ?format ~protocol ?server ?username ?secret:password
path
) files
in
--
1.9.0
More information about the Libguestfs
mailing list