Re: [Qemu-devel] [PATCH v2] qga: add guest-set-admin-password command

[Olga Krishtal <okrishtal@parallels.com>]

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 <berrange@redhat.com>
> ---
>   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