Re: [Qemu-devel] [PATCH v3] qga: add guest-set-user-password command

[Roman Kagan]

On Wed, Feb 11, 2015 at 11:26:12AM +0000, 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" } }'
> 

So how would it look like for user "Daniel P. Berrangé" (with accent
aigu :), or "Роман Каган" (my name in cyrillic letters)?  What I'm
trying to say is that if we assume localized usernames we may have
trouble with character encoding.

> +    passwd_path = g_find_program_in_path("chpasswd");
> +
> +    if (!passwd_path) {
> +        error_setg(errp, "cannot find 'passwd' program in PATH");

Minor nitpick: s/passwd/chpasswd/

The patch looks technically correct; however I'm not sold on the
approach, which assumes a responsibility split between the upper
management layer, which is supposed to know the guest OS, the username,
the encryption scheme, and the guest agent which takes care of the
os-specific command to actually change the password.  I still tend to
like more the scheme with the management layer having all the necessary
information, including the command to change the password and the proper
way to call it, and using guest-exec to perform it.

IMO the question is how low the bar is to extend the qga protocol with
yet another general-purpose (i.e. not virtual machine-specific) OS
management task.  I'd rather see it out of scope for qga.  Instead, such
an upper management layer, if necessary, would bring its own agent, with
qga acting as a transport.  This way e.g. OpenStack would be able to
uniformly change admin passwords also in ESXi, Parallels Server, LXC,
OpenVz, physical servers, etc.

Roman.