[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] qga: add guest-set-user-password command
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v3] qga: add guest-set-user-password command |
Date: |
Mon, 16 Feb 2015 19:36:03 -0600 |
User-agent: |
alot/0.3.4 |
Quoting Olga Krishtal (2015-02-13 06:26:09)
> On 11/02/15 14:26, Daniel P. Berrange wrote:
> > Add a new 'guest-set-user-password' command for changing the password
> > of guest OS user accounts. This command is needed to enable OpenStack
> > to support its API for changing the admin password of guests running
> > on KVM/QEMU. It is not practical to provide a command at the QEMU
> > level explicitly targetting administrator account password change
> > only, since different guest OS have different names for the admin
> > account. While UNIX systems use 'root', Windows systems typically
> > use 'Administrator' and even that can be renamed. Higher level apps
> > like OpenStack have the ability to figure out the correct admin
> > account name since they have info that QEMU/libvirt do not.
> >
> > The command accepts either the clear text password string, encoded
> > in base64 to make it 8-bit safe in JSON:
> >
> > $ echo -n "123456" | base64
> > MTIzNDU2
> > $ virsh -c qemu:///system qemu-agent-command f21x86_64 \
> > '{ "execute": "guest-set-user-password",
> > "arguments": { "crypted": false,
> > "username": "root",
> > "password": "MTIzNDU2" } }'
> > {"return":{}}
> >
> > Or a password that has already been run though a crypt(3) like
> > algorithm appropriate for the guest, again then base64 encoded:
> >
> > $ echo -n '$6$n01A2Tau$e...snip...DfMOP7of9AJ1I8q0' | base64
> > JDYkb...snip...YT2Ey
> > $ virsh -c qemu:///system qemu-agent-command f21x86_64 \
> > '{ "execute": "guest-set-user-password",
> > "arguments": { "crypted": true,
> > "username": "root",
> > "password": "JDYkb...snip...YT2Ey" } }'
> >
> > NB windows support is desirable, but not implemented in this
> > patch.
> >
> > Signed-off-by: Daniel P. Berrange <address@hidden>
> > ---
> > qga/commands-posix.c | 110
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > qga/commands-win32.c | 9 +++++
> > qga/qapi-schema.json | 27 +++++++++++++
> > 3 files changed, 146 insertions(+)
> >
> > In v3:
> >
> > - Renamed from guest-set-admin-password to guest-set-user-password
> > - Added 'username' argument
> > - Require 'password' to be base64 encoded
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index f6f3e3c..57409d0 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -1875,6 +1875,108 @@ int64_t
> > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
> > return processed;
> > }
> >
> > +void qmp_guest_set_user_password(const char *username,
> > + const char *password,
> > + bool crypted,
> > + Error **errp)
> > +{
> > + Error *local_err = NULL;
> > + char *passwd_path = NULL;
> > + pid_t pid;
> > + int status;
> > + int datafd[2] = { -1, -1 };
> > + char *rawpasswddata = NULL;
> > + size_t rawpasswdlen;
> > + char *chpasswddata = NULL;
> > + size_t chpasswdlen;
> > +
> > + rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
> > + rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1);
> > + rawpasswddata[rawpasswdlen] = '\0';
> > +
> > + if (strchr(rawpasswddata, '\n')) {
> > + error_setg(errp, "forbidden characters in raw password");
> > + goto out;
> > + }
> > +
> > + if (strchr(username, '\n') ||
> > + strchr(username, ':')) {
> > + error_setg(errp, "forbidden characters in username");
> > + goto out;
> > + }
> > +
> > + chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata);
> > + chpasswdlen = strlen(chpasswddata);
> > +
> > + 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], chpasswddata, chpasswdlen) !=
> > chpasswdlen) {
> > + 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 user password");
> > + goto out;
> > + }
> > +
> > +out:
> > + g_free(chpasswddata);
> > + g_free(rawpasswddata);
> > + 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 +2012,14 @@ int64_t
> > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
> > return -1;
> > }
> >
> > +void qmp_guest_set_user_password(const char *username,
> > + const char *password,
> > + bool crypted,
> > + 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..11876f6 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -446,6 +446,14 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList
> > *vcpus, Error **errp)
> > return -1;
> > }
> >
> > +void qmp_guest_set_user_password(const char *username,
> > + const char *password,
> > + bool crypted,
> > + Error **errp)
> > +{
> > + error_set(errp, QERR_UNSUPPORTED);
> > +}
> > +
> If we have decided that there is no crypted password in Windows, may be
> we should cross out this argument (with #define) or just take crypted:
> false as default.
Agreed, but I think we can wait to document that limitation once a w32
implementation exists.
> > /* add unsupported commands to the blacklist */
> > GList *ga_command_blacklist_init(GList *blacklist)
> > {
> > @@ -454,6 +462,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
> > "guest-file-write", "guest-file-seek", "guest-file-flush",
> > "guest-suspend-hybrid", "guest-network-get-interfaces",
> > "guest-get-vcpus", "guest-set-vcpus",
> > + "guest-set-user-password",
> > "guest-fsfreeze-freeze-list", "guest-get-fsinfo",
> > "guest-fstrim", NULL};
> > char **p = (char **)list_unsupported;
> > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> > index 376e79f..5117b65 100644
> > --- a/qga/qapi-schema.json
> > +++ b/qga/qapi-schema.json
> > @@ -738,3 +738,30 @@
> > ##
> > { 'command': 'guest-get-fsinfo',
> > 'returns': ['GuestFilesystemInfo'] }
> > +
> > +##
> > +# @guest-set-user-password
> > +#
> > +# @username: the user account whose password to change
> > +# @password: the new password entry string, base64 encoded
> > +# @crypted: true if password is already crypt()d, false if raw
> > +#
> > +# If the @crypted flag is true, it is the caller's 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.
> > +#
> > +# Note all guest operating systems will support use of the
> > +# @crypted flag, as they may require the clear-text password
> > +#
> > +# The @password parameter must always be base64 encoded before
> > +# transmission, even if already crypt()d, to ensure it is 8-bit
> > +# safe when passed as JSON.
> > +#
> > +# Returns: Nothing on success.
> > +#
> > +# Since 2.3
> > +##
> > +{ 'command': 'guest-set-user-password',
> > + 'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
> Everything seems fine.