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

[Michael Roth]

Quoting Daniel P. Berrange (2015-02-12 08:05:56)
> On Thu, Feb 12, 2015 at 04:21:09PM +0300, Roman Kagan wrote:
> > 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.
> 
> The JSON string typed accepts UTF-8 encoding strings, so you can
> have any of those localized names.  If the API/command that the
> agent invokes doesn't use UTF-8 (unlikely) then the agent would
> do a charset conversion as needed.
> 
> > > +    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.
> 
> guest-exec is not something that is generally supportable. By virtue
> of the fact that it is designed to allow arbitrary command execution,
> you can't provide security policy out of the box that confines its
> in any meaningfully secure way. By having a dedicated qapi comand for
> this, we can write security policy that allows precisely the command
> needed, no more, no less. This commands provides a simple mechanism
> for the feature, while the user or application can define its policy
> for using 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.
> 
> This proposed feature is a generally useful for regaining access to a
> guest for which you've lost login access. Putting it in an application
> specific agent results in every app managing QEMU reinventing the
> wheel which is just madness. We've already got QEMU guest agent and
> SPICE guest agent, adding a 3rd openstack agent is not a desirable
> path to take. For the cross hypervisor portability problem we already
> have libvirt, so again pushing it up into openstack just makes life
> worse for non-openstack apps which want this.

I agree. It's true that we don't want QGA to become too closely
tied/integrated with high-level management tasks (since they tend to be
very environment/implementation specific), but if a feature proves
to be generally useful across the board then a good argument can be made
for pushing it down into a lower-level agent like QGA that a simpler
"out of the box" solution can potentially benefit from as well: e.g.
node-level management like libvirt, kimchi, virt-manager, etc.

The interesting stuff like SSO, package-management, etc, is what should be
left to a more integrated agent (or the general-purpose/transport aspects
of QGA like guest-file-*/guest-exec*).

> 
> 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 :|