Re: [PATCH][Take 3] VNC authentification

[Anthony Liguori <aliguori@us.ibm.com>]

Masami Watanabe wrote:
> Hi all,
>
> My understanding of main_loop of vl.c was insufficient. 
> I have improved the point to have set the handler doubly.
> The protocol of protocol 3.3 and (UltraVNC's ?)3.4 did not actually
> need the response of the authentification type from the client.
> Therefore, protocol_authtype operated without being called.
>   

Yeah, I noticed that myself. That why your code was working. Your
authtype (which would have been right for 3.7+) was not being invoked
which happened to make it work for 3.3 :-)

> I will post the take 4 patch after this.
>
> The patch has limited  RFB protocol to 3.3 and 3.4.
> I also think that it should support 3.8 without being support 3.3 in
> the future.
>   

There is no official RFB 3.4.

The only documented protocols are 3.3, 3.7, and 3.8. Leaving no
authentication is definitely important. I can live with not having 3.8
support right away but I think it's key to allow for no authentication.

Regards,

Anthony Liguori

> However, Leaving like non authetification is not good.
> You will think the patch to be provisional action till then.
>
> Another.
> ill the RFB protocol still keep evolving in the future?
>
>
> Best regards,
> Watanabe
>
>
> On Tue, 03 Oct 2006 13:49:37 -0500, Anthony Liguori wrote:
>   
>> Daniel P. Berrange wrote:
>>     
>>> On Tue, Oct 03, 2006 at 12:56:31PM -0500, Anthony Liguori wrote:
>>>   
>>>       
>>>> Masami Watanabe wrote:
>>>>     
>>>>         
>>>>> +static int vnc_auth(VncState *vs)
>>>>> +{
>>>>> +    extern char vncpasswd[64];
>>>>> +    extern unsigned char challenge[AUTHCHALLENGESIZE];
>>>>> +
>>>>> +    if (*vncpasswd == '\0') {
>>>>> +	/* AuthType is None */
>>>>> +	vnc_write_u32(vs, 1);
>>>>> +	vnc_flush(vs);
>>>>> +	vnc_read_when(vs, protocol_client_init, 1);
>>>>> +    } else {
>>>>> +	/* AuthType is VncAuth */
>>>>> +	vnc_write_u32(vs, 2);
>>>>> +	vnc_flush(vs);
>>>>> +
>>>>> +	/* Read AuthType */
>>>>> +	vnc_read_when(vs, protocol_authtype, 1);
>>>>>  
>>>>>       
>>>>>           
>>>> As I mentioned before, you cannot have to vnc_read_when()'s execution 
>>>> path without returning the the mainloop.
>>>>
>>>> protocol_authtype() cannot possibly be invoked.  If the code is working 
>>>> now, it's pure luck.
>>>>     
>>>>         
>>> Yeah, the impl of protocol_authtype() in there is a no-op too - it should
>>> be rejecting auth types which aren't supported, even if it was being invoked.
>>> With the code as it is, protocol_authtype never runs & the server starts
>>> doing VNCAuth regardless of what the client says it wants to do, which is
>>> clearly not correct.
>>>   
>>>       
>> Another thing to keep in mind, is that the reason I did 3.3 instead of 
>> 3.8 is that I knew there was only one auth type we would be supporting.  
>> If we do support multiple auth types, we really ought to move to using 
>> the 3.8 protocol since that provides a negotiation mechanism.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>     
>>> Dan.
>>>   
>>>       
>
>