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