[Qemu-devel] Re: [PATCH] vnc: Fix password expiration through 'change vnc ""'

[Daniel P. Berrange]

On Mon, Feb 14, 2011 at 08:14:07AM -0600, Anthony Liguori wrote:
> On 02/14/2011 06:24 AM, Daniel P. Berrange wrote:
> >On Mon, Feb 14, 2011 at 06:10:04AM -0600, Anthony Liguori wrote:
> >>On 02/14/2011 04:57 AM, Gerd Hoffmann wrote:
> >>>On 01/31/11 21:43, Anthony Liguori wrote:
> >>>>commit 52c18be9e99dabe295321153fda7fce9f76647ac introduced a
> >>>>regression in the
> >>>>change vnc password command that changed the behavior of setting the VNC
> >>>>password to an empty string from disabling login to disabling
> >>>>authentication.
> >>>>
> >>>>This commit refactors the code to eliminate this overloaded semantics in
> >>>>vnc_display_password and instead introduces the
> >>>>vnc_display_disable_login.   The
> >>>>monitor implementation then determines the behavior of an empty
> >>>>or missing
> >>>>string.
> >>>Hmm, now about simply never ever changing vs->auth?
> >>If auth is none and you do a vnc change password "" then if we don't
> >>set vs->auth to vnc, it won't have the desired effect.  I really
> >>dislike the semantics of this command but that was a past mistake.
> >Actually blindly setting 'vs->auth' to 'vnc' is also a security flaw.
> 
> But this is the semantics of the command.  I agree it's stupid but a
> security flaw is a regression and this is not a regression.
> 
> This is why the set-password command no longer does any of this nonsense.
> 
> >If using the VeNCrypt security method, then 'vs->auth' will be VENCRYPT
> >and the 'vs->subauth' will possibly indicate the 'VNC' sub-auth scheme.
> >So we really do want the change password command to leave 'vs->auth'
> >alone completely - just change the password string, with no side effects
> >on auth methods. If an app intends to use the change password command
> >it will have already launched QEMU with neccessary -vnc flags to set the
> >desired vs->auth and vs->subauth methods.
> 
> I think I see how this could work but I'm not sure it's worth doing.
> I'd rather just leave the (bad) semantics of this command alone and
> deprecate the interface.

If you make it blindly set 'vs->auth = VNC' then you haven't fully
fixed the security flawed, because you will be downgrading from
'vencrypt+vnc' auth to just 'vnc' which loose all your data encryption
capabilities. Just reverting the previous commit 
52c18be9e99dabe295321153fda7fce9f76647ac
fully addresses the problems leaving the command side-effect free
as it was originally designed & implemented.

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