Re: [Qemu-devel] PATCH 7/8: command line args for x509 cert paths

[Anthony Liguori <anthony@codemonkey.ws>]

Daniel P. Berrange wrote:
> This final code patch adds 4 new command line arguments to QEMU to allow the
> certificate files to be specified. The '-x509cacert', '-x509cert' and '-x509key'
> parameters are mandatory if the 'x509' or 'x509verify' flags are used when
> setting up the VNC server. If the certificates are not provided, all client
> authentication attempts will be rejected.
>   

It concerns me a little to add 4 new command line options.  Perhaps just 
supply a directory and hard code the names of each file?  Then it could 
even be specified as -vnc 
[proto]:<proto-arg>[,tls[,x509[:/path/to/x509/certs]]]  with a 
reasonable default provided.

What do you think?

Regards,

Anthony Liguori

> diff -r f38519b13575 vl.c
> --- a/vl.c	Tue Jul 31 14:51:31 2007 -0400
> +++ b/vl.c	Tue Jul 31 14:51:32 2007 -0400
> @@ -6701,6 +6701,12 @@ static void help(int exitcode)
>             "-no-reboot      exit instead of rebooting\n"
>             "-loadvm file    start right away with a saved state (loadvm in monitor)\n"
>  	   "-vnc display    start a VNC server on display\n"
> +#if CONFIG_VNC_TLS
> +	   "-x509cacert FILE  x509 CA certificate for TLS services\n"
> +	   "-x509cacrl FILE   x509 CA certificate revocation list for TLS services\n"
> +	   "-x509cert FILE    x509 public certificate for TLS services\n"
> +	   "-x509key FILE     x509 private key for TLS services\n"
> +#endif
>  #ifndef _WIN32
>  	   "-daemonize      daemonize QEMU after initializing\n"
>  #endif
> @@ -6796,6 +6802,12 @@ enum {
>      QEMU_OPTION_usbdevice,
>      QEMU_OPTION_smp,
>      QEMU_OPTION_vnc,
> +#if CONFIG_VNC_TLS
> +    QEMU_OPTION_x509cacert,
> +    QEMU_OPTION_x509cacrl,
> +    QEMU_OPTION_x509cert,
> +    QEMU_OPTION_x509key,
> +#endif
>      QEMU_OPTION_no_acpi,
>      QEMU_OPTION_no_reboot,
>      QEMU_OPTION_show_cursor,
> @@ -6889,6 +6901,12 @@ const QEMUOption qemu_options[] = {
>      { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
>      { "smp", HAS_ARG, QEMU_OPTION_smp },
>      { "vnc", HAS_ARG, QEMU_OPTION_vnc },
> +#if CONFIG_VNC_TLS
> +    { "x509cacert", HAS_ARG, QEMU_OPTION_x509cacert },
> +    { "x509cacrl", HAS_ARG, QEMU_OPTION_x509cacrl },
> +    { "x509cert", HAS_ARG, QEMU_OPTION_x509cert },
> +    { "x509key", HAS_ARG, QEMU_OPTION_x509key },
> +#endif
>  
>      /* temporary options */
>      { "usb", 0, QEMU_OPTION_usb },
> @@ -7171,6 +7189,9 @@ int main(int argc, char **argv)
>      int fds[2];
>      const char *pid_file = NULL;
>      VLANState *vlan;
> +#if CONFIG_VNC_TLS
> +    const char *x509cacert = NULL, *x509cacrl = NULL, *x509cert = NULL, *x509key = NULL;
> +#endif
>  
>      LIST_INIT (&vm_change_state_head);
>  #ifndef _WIN32
> @@ -7648,6 +7669,20 @@ int main(int argc, char **argv)
>  	    case QEMU_OPTION_vnc:
>  		vnc_display = optarg;
>  		break;
> +#if CONFIG_VNC_TLS
> +	    case QEMU_OPTION_x509cacert:
> +		x509cacert = optarg;
> +		break;
> +	    case QEMU_OPTION_x509cacrl:
> +		x509cacrl = optarg;
> +		break;
> +	    case QEMU_OPTION_x509cert:
> +		x509cert = optarg;
> +		break;
> +	    case QEMU_OPTION_x509key:
> +		x509key = optarg;
> +		break;
> +#endif
>              case QEMU_OPTION_no_acpi:
>                  acpi_enabled = 0;
>                  break;
> @@ -7945,6 +7980,10 @@ int main(int argc, char **argv)
>          dumb_display_init(ds);
>      } else if (vnc_display != NULL) {
>          vnc_display_init(ds);
> +#if CONFIG_VNC_TLS
> +        if (vnc_set_x509_credentials(ds, x509cacert, x509cacrl, x509cert, x509key) < 0)
> +            exit(1);
> +#endif
>          if (vnc_display_open(ds, vnc_display, NULL) < 0)
>              exit(1);
>      } else {
> diff -r f38519b13575 vl.h
> --- a/vl.h	Tue Jul 31 14:51:31 2007 -0400
> +++ b/vl.h	Tue Jul 31 14:51:32 2007 -0400
> @@ -971,6 +971,13 @@ void vnc_display_close(DisplayState *ds)
>  void vnc_display_close(DisplayState *ds);
>  int vnc_display_open(DisplayState *ds, const char *display, const char *password);
>  void do_info_vnc(void);
> +#if CONFIG_VNC_TLS
> +int vnc_set_x509_credentials(DisplayState *ds,
> +			     const char *cacert,
> +			     const char *cacrl,
> +			     const char *cert,
> +			     const char *key);
> +#endif
>  
>  /* x_keymap.c */
>  extern uint8_t _translate_keycode(const int key);
> diff -r f38519b13575 vnc.c
> --- a/vnc.c	Tue Jul 31 14:51:31 2007 -0400
> +++ b/vnc.c	Tue Jul 31 14:51:32 2007 -0400
> @@ -142,6 +142,11 @@ struct VncState
>  #if CONFIG_VNC_TLS
>      int subauth;
>      int x509verify;
> +
> +    char *x509cacert;
> +    char *x509cacrl;
> +    char *x509cert;
> +    char *x509key;
>  #endif
>      char challenge[VNC_AUTH_CHALLENGE_SIZE];
>  
> @@ -1378,36 +1383,50 @@ static gnutls_anon_server_credentials vn
>  }
>  
>  
> -static gnutls_certificate_credentials_t vnc_tls_initialize_x509_cred(void)
> +static gnutls_certificate_credentials_t vnc_tls_initialize_x509_cred(VncState *vs)
>  {
>      gnutls_certificate_credentials_t x509_cred;
>      int ret;
> -    struct stat st;
> +
> +    if (!vs->x509cacert) {
> +	VNC_DEBUG("No CA x509 certificate specified\n");
> +	return NULL;
> +    }
> +    if (!vs->x509cert) {
> +	VNC_DEBUG("No server x509 certificate specified\n");
> +	return NULL;
> +    }
> +    if (!vs->x509key) {
> +	VNC_DEBUG("No server private key specified\n");
> +	return NULL;
> +    }
> +
>  
>      if ((ret = gnutls_certificate_allocate_credentials(&x509_cred)) < 0) {
>  	VNC_DEBUG("Cannot allocate credentials %s\n", gnutls_strerror(ret));
>  	return NULL;
>      }
> -    if ((ret = gnutls_certificate_set_x509_trust_file(x509_cred, CA_FILE, GNUTLS_X509_FMT_PEM)) < 0) {
> +    if ((ret = gnutls_certificate_set_x509_trust_file(x509_cred,
> +						      vs->x509cacert,
> +						      GNUTLS_X509_FMT_PEM)) < 0) {
>  	VNC_DEBUG("Cannot load CA certificate %s\n", gnutls_strerror(ret));
>  	gnutls_certificate_free_credentials(x509_cred);
>  	return NULL;
>      }
>  
> -    if ((ret = gnutls_certificate_set_x509_key_file (x509_cred, CERT_FILE, KEY_FILE,
> +    if ((ret = gnutls_certificate_set_x509_key_file (x509_cred,
> +						     vs->x509cert,
> +						     vs->x509key,
>  						     GNUTLS_X509_FMT_PEM)) < 0) {
>  	VNC_DEBUG("Cannot load certificate & key %s\n", gnutls_strerror(ret));
>  	gnutls_certificate_free_credentials(x509_cred);
>  	return NULL;
>      }
>  
> -    if (stat(CRL_FILE, &st) < 0) {
> -	if (errno != ENOENT) {
> -	    gnutls_certificate_free_credentials(x509_cred);
> -	    return NULL;
> -	}
> -    } else {
> -	if ((ret = gnutls_certificate_set_x509_crl_file(x509_cred, CRL_FILE, GNUTLS_X509_FMT_PEM)) < 0) {
> +    if (vs->x509cacrl) {
> +	if ((ret = gnutls_certificate_set_x509_crl_file(x509_cred,
> +							vs->x509cacrl,
> +							GNUTLS_X509_FMT_PEM)) < 0) {
>  	    VNC_DEBUG("Cannot load CRL %s\n", gnutls_strerror(ret));
>  	    gnutls_certificate_free_credentials(x509_cred);
>  	    return NULL;
> @@ -1623,7 +1642,7 @@ static int vnc_start_tls(struct VncState
>  	}
>  
>  	if (NEED_X509_AUTH(vs)) {
> -	    gnutls_certificate_server_credentials x509_cred = vnc_tls_initialize_x509_cred();
> +	    gnutls_certificate_server_credentials x509_cred = vnc_tls_initialize_x509_cred(vs);
>  	    if (!x509_cred) {
>  		gnutls_deinit(vs->tls_session);
>  		vs->tls_session = NULL;
> @@ -1888,6 +1907,43 @@ void vnc_display_init(DisplayState *ds)
>      vnc_dpy_resize(vs->ds, 640, 400);
>  }
>  
> +#if CONFIG_VNC_TLS
> +int vnc_set_x509_credentials(DisplayState *ds,
> +			     const char *cacert,
> +			     const char *cacrl,
> +			     const char *cert,
> +			     const char *key)
> +{
> +    VncState *vs = ds ? (VncState *)ds->opaque : vnc_state;
> +
> +    if (vs->x509cacert) {
> +	free(vs->x509cacert);
> +	vs->x509cacert = NULL;
> +    }
> +    if (vs->x509cacrl) {
> +	free(vs->x509cacrl);
> +	vs->x509cacrl = NULL;
> +    }
> +    if (vs->x509cert) {
> +	free(vs->x509cert);
> +	vs->x509cert = NULL;
> +    }
> +    if (vs->x509key) {
> +	free(vs->x509key);
> +	vs->x509key = NULL;
> +    }
> +    if (cacert && !(vs->x509cacert = qemu_strdup(cacert)))
> +	return -1;
> +    if (cacrl && !(vs->x509cacrl = qemu_strdup(cacrl)))
> +	return -1;
> +    if (cert && !(vs->x509cert = qemu_strdup(cert)))
> +	return -1;
> +    if (key && !(vs->x509key = qemu_strdup(key)))
> +	return -1;
> +    return 0;
> +}
> +#endif /* CONFIG_VNC_TLS */
> +
>  void vnc_display_close(DisplayState *ds)
>  {
>      VncState *vs = ds ? (VncState *)ds->opaque : vnc_state;
>
>