[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qga: add guest-set-admin-password command
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH v2] qga: add guest-set-admin-password command |
Date: |
Wed, 4 Feb 2015 14:10:05 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Feb 04, 2015 at 04:25:47PM +0300, Olga Krishtal wrote:
> 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 <address@hidden>
> >---
> > 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
In that scenario, I'd suggest the windows impl of the command simply report
an error if the flag was crypted==true. QEMU shouldn't try to interpret the
data in any way IMHO, so that'd rule out decryption of any kind. In any
case, 'crypt' as a concept is all about doing a one-way hash so you can't
decrypt regardless.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|