Re: [PATCH][Take 2] VNC authentification

["Daniel P. Berrange" <berrange@redhat.com>]

On Sun, Oct 01, 2006 at 03:53:33AM +0900, Masami Watanabe wrote:
> Hi Dan,
> 
> I post patch that reflects your point.
> However, Now, I can not use standard VNC clients to server.
> therefore, I cannot do final test. It becomes possible on next Tuesday.
> Please forgive my post, it is current update.

The Python XenD bits of your latest patch all look good to me now - thanks 
for taking time to address the issues.

I've compiled the patches against latest Xen going into Fedora Core 6, 
and the password authentication does appear to be working as expected.
Only issue was that I forgot the password in the VM config file needed
to be the base64 encoded, DES-encrypted format - once I sorted that
out it worked fine. 

> --- a/tools/examples/xend-config.sxp	Wed Sep 27 17:49:22 2006 +0100
> +++ b/tools/examples/xend-config.sxp	Sun Oct 01 02:13:06 2006 +0900
> @@ -130,3 +130,7 @@
>  
>  # The tool used for initiating virtual TPM migration
>  #(external-migration-tool '')
> +
> +# The default password for VNC console on HVM domain.
> +# Empty string is no authentication.
> +(vncpasswd '')

We should add a note about this needing to be the base-64 encoded, 
DES encrypted password, rather than plain text.

> diff -r 1d0e75523636 tools/examples/xmexample.hvm
> --- a/tools/examples/xmexample.hvm	Wed Sep 27 17:49:22 2006 +0100
> +++ b/tools/examples/xmexample.hvm	Sun Oct 01 02:13:06 2006 +0900
> @@ -145,6 +145,11 @@ vnc=1
>  #vncconsole=0
>  
>  #----------------------------------------------------------------------------
> +# set password for domain's VNC console
> +# default is depents on vncpasswd in xend-config.sxp
> +vncpasswd=''

Again add a comment about the format.

> +static int make_challenge(char *random, int size)
> +{
> +    FILE *fp;
> +    int readsize;
> +
> +    fp = fopen("/dev/random", "r");
> +    if (!fp) {
> +	fprintf(stderr, "make_challenge: no OS supplied /dev/random\n");
> +	exit(1);
> +    }
> +    readsize = fread(random, size, 1, fp);
> +    fclose(fp);
> +
> +    return 0;
> +}

Using /dev/random for this is rather overkill for VNC. I very quickly 
exhausted my machine's entropy pool after connecting & disconnecting
a couple of times over.  The regular VNC server sources just call the
standard C 'random()'  function CHALLENGESIZE times over to get some
random bytes  (and seed the random pool at startup based on time & pid). 

> +
> +int xenstore_read_vncpasswd(int domid)
> +{
> +    extern char vncpasswd[64];
> +    char *buf = NULL, *path, *uuid = NULL, *passwd = NULL;
> +    unsigned int i, len, rc = 0;
> +
> +    if (xsh == NULL) {
> +	return -1;
> +    }
> +
> +    path = xs_get_domain_path(xsh, domid);
> +    if (path == NULL) {
> +        fprintf(logfile, "xs_get_domain_path() error\n");
> +        return -1;
> +    }
> +
> +    pasprintf(&buf, "%s/vm", path);
> +    uuid = xs_read(xsh, XBT_NULL, buf, &len);
> +    if (uuid == NULL) {
> +        fprintf(logfile, "xs_read(): uuid get error\n");
> +	free(path);
> +	return -1;
> +    }
> +
> +    pasprintf(&buf, "%s/vncpasswd", uuid);
> +    passwd = xs_read(xsh, XBT_NULL, buf, &len);
> +    if (passwd == NULL) {
> +        free(uuid);
> +	free(path);
> +	return rc;
> +    }
> +
> +    for (i=0; i<len; i++) {
> +        vncpasswd[i] = passwd[i];
> +        passwd[i] = '\0';
> +    }
> +    vncpasswd[len] = '\0';

Should check for buffer overflow since 'vncpasswd' is only 64 bytes
long.

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|