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

[Anthony Liguori <aliguori@linux.vnet.ibm.com>]

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.

Regards,

Anthony Liguori

> Regards,
> Daniel
>