Re: [Qemu-devel] PATCH 4/8: VeNCrypt basic TLS support

[Anthony Liguori <anthony@codemonkey.ws>]

Daniel P. Berrange wrote:
> This patch introduces minimal support for the VeNCrypt protocol
> extension. This layers use of TLS (aka SSL) into the VNC data stream,
> providing session encryption. This patch is the bare minimum protocol
> support. It is enabled by using the 'tls' option flag eg "-vnc :1,tls'
> This is not secure on its own since it uses anonymous credentials.
> The next patches will introduce x509 certificate credentials.
>
> The configure script is setup to that TLS is only compiled in if the
> --enable-vnc-tls flag is provided. This should avoid any breakage on
> platforms without the GNU TLS libraries.
>
> diff -r a1fa771c6cf9 Makefile.target
> --- a/Makefile.target	Tue Jul 31 14:50:01 2007 -0400
> +++ b/Makefile.target	Tue Jul 31 14:50:03 2007 -0400
> @@ -402,6 +402,11 @@ endif
>  endif
>  AUDIODRV+= wavcapture.o
>  
> +ifdef CONFIG_VNC_TLS
> +CPPFLAGS += $(CONFIG_VNC_TLS_CFLAGS)
> +LIBS += $(CONFIG_VNC_TLS_LIBS)
> +endif
> +
>  VL_OBJS += i2c.o smbus.o
>  
>  # SCSI layer
> diff -r a1fa771c6cf9 configure
> --- a/configure	Tue Jul 31 14:50:01 2007 -0400
> +++ b/configure	Tue Jul 31 14:50:03 2007 -0400
> @@ -89,6 +89,7 @@ fmod="no"
>  fmod="no"
>  fmod_lib=""
>  fmod_inc=""
> +vnc_tls="no"
>  bsd="no"
>  linux="no"
>  kqemu="no"
> @@ -252,6 +253,8 @@ for opt do
>    ;;
>    --fmod-inc=*) fmod_inc="$optarg"
>    ;;
> +  --enable-vnc-tls) vnc_tls="yes"
> +  ;;
>    --enable-mingw32) mingw32="yes" ; cross_prefix="i386-mingw32-" ; linux_user="no"
>    ;;
>    --disable-slirp) slirp="no"
> @@ -362,6 +365,7 @@ echo "  --enable-alsa            enable 
>  echo "  --enable-alsa            enable ALSA audio driver"
>  echo "  --enable-fmod            enable FMOD audio driver"
>  echo "  --enable-dsound          enable DirectSound audio driver"
> +echo "  --enable-vnc-tls         enable TLS encryption for VNC server"
>  echo "  --enable-system          enable all system emulation targets"
>  echo "  --disable-system         disable all system emulation targets"
>  echo "  --enable-linux-user      enable all linux usermode emulation targets"
> @@ -589,6 +593,16 @@ fi # -z $sdl
>  fi # -z $sdl
>  
>  ##########################################
> +# VNC TLS detection
> +if test "$vnc_tls" = "yes" ; then
> +  `pkg-config gnutls` || vnc_tls="no"
> +fi
> +if test "$vnc_tls" = "yes" ; then
> +  vnc_tls_cflags=`pkg-config --cflags gnutls`
> +  vnc_tls_libs=`pkg-config --libs gnutls`
> +fi
> +
> +##########################################
>  # alsa sound support libraries

Since it's possible to probe for gnutls support, why not just enable it 
by default and disable it if it's not available?

> diff -r a1fa771c6cf9 vl.c
> --- a/vl.c	Tue Jul 31 14:50:01 2007 -0400
> +++ b/vl.c	Tue Jul 31 14:50:03 2007 -0400
> @@ -6458,7 +6458,7 @@ void main_loop_wait(int timeout)
>              if (FD_ISSET(ioh->fd, &rfds)) {
>                  ioh->fd_read(ioh->opaque);
>              }
> -            if (FD_ISSET(ioh->fd, &wfds)) {
> +            if (!ioh->deleted && ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
>                  ioh->fd_write(ioh->opaque);
>              }
>          }
>   

I thought this was fixed already.  At any rate, it should be a separate 
patch.

> +#if CONFIG_VNC_TLS
> +ssize_t vnc_tls_push(gnutls_transport_ptr_t transport,
> +		     const void *data,
> +		     size_t len) {
> +    struct VncState *vs = (struct VncState *)transport;
> +    int ret, lastErrno;
>   

s/lastErrno/last_errno/g

> + retry:
> +    ret = send(vs->csock, data, len, 0);
> +    lastErrno = errno;
> +    VNC_DEBUG("Send %d errno %d\n", ret, ret < 0 ? lastErrno : 0);
> +    if (ret < 0) {
> +	if (lastErrno == EINTR)
> +	    goto retry;
> +	errno = lastErrno;
> +	return -1;
> +    }
> +    return ret;
> +}
>   

Regards,

Anthony Liguor