From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38482) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ0Db-0001CS-Se for qemu-devel@nongnu.org; Wed, 04 Feb 2015 08:42:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJ0DW-0000ea-1S for qemu-devel@nongnu.org; Wed, 04 Feb 2015 08:42:11 -0500 Received: from mx2.parallels.com ([199.115.105.18]:54624) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ0DV-0000Rl-Ph for qemu-devel@nongnu.org; Wed, 04 Feb 2015 08:42:05 -0500 Message-ID: <54D21DDB.6090009@parallels.com> Date: Wed, 4 Feb 2015 16:25:47 +0300 From: Olga Krishtal MIME-Version: 1.0 References: <1421078294-26234-1-git-send-email-berrange@redhat.com> In-Reply-To: <1421078294-26234-1-git-send-email-berrange@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] qga: add guest-set-admin-password command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org Cc: Michael Roth On 12/01/15 18:58, Daniel P. Berrange wrote: > Add a new 'guest-set-admin-password' command for changing the > root/administrator password. This command is needed to allow > OpenStack to support its API for changing the admin password > on a running guest. > > Accepts either the raw password string: > > $ virsh -c qemu:///system qemu-agent-command f21x86_64 \ > '{ "execute": "guest-set-admin-password", "arguments": > { "crypted": false, "password": "12345678" } }' > {"return":{}} > > Or a pre-encrypted string (recommended) > > $ virsh -c qemu:///system qemu-agent-command f21x86_64 \ > '{ "execute": "guest-set-admin-password", "arguments": > { "crypted": true, "password": > "$6$T9O/j/aGPrE...snip....rQoRN4F0.GG0MPjNUNyml." } }' > > NB windows support is desirable, but not implemented in this > patch. > > Signed-off-by: Daniel P. Berrange > --- > qga/commands-posix.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-win32.c | 6 ++++ > qga/qapi-schema.json | 19 +++++++++++ > 3 files changed, 115 insertions(+) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index f6f3e3c..4887889 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -1875,6 +1875,90 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > return processed; > } > > +void qmp_guest_set_admin_password(bool crypted, const char *password, > + Error **errp) > +{ > + Error *local_err = NULL; > + char *passwd_path = NULL; > + pid_t pid; > + int status; > + int datafd[2] = { -1, -1 }; > + char *acctpw = g_strdup_printf("root:%s\n", password); > + size_t acctpwlen = strlen(acctpw); > + > + if (strchr(password, '\n')) { > + error_setg(errp, "forbidden characters in new password"); > + goto out; > + } > + > + passwd_path = g_find_program_in_path("chpasswd"); > + > + if (!passwd_path) { > + error_setg(errp, "cannot find 'passwd' program in PATH"); > + goto out; > + } > + > + if (pipe(datafd) < 0) { > + error_setg(errp, "cannot create pipe FDs"); > + goto out; > + } > + > + pid = fork(); > + if (pid == 0) { > + close(datafd[1]); > + /* child */ > + setsid(); > + dup2(datafd[0], 0); > + reopen_fd_to_null(1); > + reopen_fd_to_null(2); > + > + if (crypted) { > + execle(passwd_path, "chpasswd", "-e", NULL, environ); > + } else { > + execle(passwd_path, "chpasswd", NULL, environ); > + } > + _exit(EXIT_FAILURE); > + } else if (pid < 0) { > + error_setg_errno(errp, errno, "failed to create child process"); > + goto out; > + } > + close(datafd[0]); > + datafd[0] = -1; > + > + if (qemu_write_full(datafd[1], acctpw, acctpwlen) != acctpwlen) { > + error_setg_errno(errp, errno, "cannot write new account password"); > + goto out; > + } > + close(datafd[1]); > + datafd[1] = -1; > + > + ga_wait_child(pid, &status, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + goto out; > + } > + > + if (!WIFEXITED(status)) { > + error_setg(errp, "child process has terminated abnormally"); > + goto out; > + } > + > + if (WEXITSTATUS(status)) { > + error_setg(errp, "child process has failed to set admin password"); > + goto out; > + } > + > +out: > + g_free(acctpw); > + g_free(passwd_path); > + if (datafd[0] != -1) { > + close(datafd[0]); > + } > + if (datafd[1] != -1) { > + close(datafd[1]); > + } > +} > + > #else /* defined(__linux__) */ > > void qmp_guest_suspend_disk(Error **errp) > @@ -1910,6 +1994,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > return -1; > } > > +void qmp_guest_set_admin_password(bool crypted, const char *password, > + Error **errp) > +{ > + error_set(errp, QERR_UNSUPPORTED); > +} > + > #endif > > #if !defined(CONFIG_FSFREEZE) > diff --git a/qga/commands-win32.c b/qga/commands-win32.c > index 3bcbeae..56854d5 100644 > --- a/qga/commands-win32.c > +++ b/qga/commands-win32.c > @@ -446,6 +446,12 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > return -1; > } > > +void qmp_guest_set_admin_password(bool crypted, const char *password, > + Error **errp) > +{ > + error_set(errp, QERR_UNSUPPORTED); > +} > + > /* add unsupported commands to the blacklist */ > GList *ga_command_blacklist_init(GList *blacklist) > { > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index 376e79f..25118e2 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -738,3 +738,22 @@ > ## > { 'command': 'guest-get-fsinfo', > 'returns': ['GuestFilesystemInfo'] } > + > +## > +# @guest-set-admin-password > +# > +# @crypted: true if password is already crypt()d, false if raw > +# @password: the new password entry > +# > +# If the @crypted flag is true, it is the callers responsibility > +# to ensure the correct crypt() encryption scheme is used. This > +# command does not attempt to interpret or report on the encryption > +# scheme. Refer to the documentation of the guest operating system > +# in question to determine what is supported. > +# > +# Returns: Nothing on success. > +# > +# Since 2.3 > +## > +{ 'command': 'guest-set-admin-password', > + 'data': { 'crypted': 'bool', 'password': 'str' } } While implementing such functionality for Windows NT we can suffer from particular problem: -The password must be passed to WinApi function as a plain text, so we would need entire the encryption mechanism if we used ctypted: true