On Sun, Jan 23, 2011 at 07:24:05PM +0800, Osier Yang wrote:
> This new parameter allows user specifies directory in which the client
> cerficate, client key, CA certificate reside (the directory path must
> be absolute), instead of hardcoding it in remote driver. But the hardcoded
> locations are still kept, and attempt to use them if the required files
> can not be found in the location user specified. e.g.
>
> [root@Osier client]# virsh -c qemu+tls://$hostname/system?client_pki_dir\
> > =/tmp/pki/client
> Welcome to virsh, the virtualization interactive terminal.
>
> Type: 'help' for help with commands
> 'quit' to quit
>
> virsh # quit
>
> [root@Osier client]# pwd
> /tmp/pki/client
> [root@Osier client]# ls
> cacert.pem clientcert.pem client.info clientkey.pem
>
> [root@Osier client]# mv cacert.pem ..
> mv: overwrite `../cacert.pem'? y
> [root@Osier client]# virsh -c qemu+tls://10.66.93.111/system?client_pki_dir\
> > =/tmp/pki/client
> 19:11:23.844: 7589: warning : initialize_gnutls:1186 : /tmp/pki/client/cacert.pem
> doesn't exist, try to use: /etc/pki/CA/cacert.pem
> Welcome to virsh, the virtualization interactive terminal.
>
> Type: 'help' for help with commands
> 'quit' to quit
>
> virsh #
>
> * src/remote/remote_driver.c
> ---
> src/remote/remote_driver.c | 87 ++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 75 insertions(+), 12 deletions(-)
>
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index ea119c6..7053e45 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -268,7 +268,7 @@ void remoteDomainEventQueueFlush(int timer, void *opaque);
> static char *get_transport_from_scheme (char *scheme);
>
> /* GnuTLS functions used by remoteOpen. */
> -static int initialize_gnutls(void);
> +static int initialize_gnutls(char *client_pki_dir);
> static gnutls_session_t negotiate_gnutls_on_connection (virConnectPtr conn, struct private_data *priv, int no_verify);
>
> #ifdef WITH_LIBVIRTD
> @@ -430,6 +430,7 @@ doRemoteOpen (virConnectPtr conn,
> char *port = NULL, *authtype = NULL, *username = NULL;
> int no_verify = 0, no_tty = 0;
> char **cmd_argv = NULL;
> + char *client_pki_dir = NULL;
>
> /* Return code from this function, and the private data. */
> int retcode = VIR_DRV_OPEN_ERROR;
> @@ -509,9 +510,14 @@ doRemoteOpen (virConnectPtr conn,
> priv->debugLog = stdout;
> else
> priv->debugLog = stderr;
> - } else
> + } else if (STRCASEEQ(var->name, "client_pki_dir")) {
'client' is redundant here, and it'd be nice to avoid 'dir' in the
name, since if we ever have to use NSS, it might point to a file
rather than a dir. So it would be preferable to call this 'pkipath'
> + client_pki_dir = strdup(var->value);
> + if (!client_pki_dir) goto out_of_memory;
> + var->ignore = 1;
> + } else {
> DEBUG("passing through variable '%s' ('%s') to remote end",
> var->name, var->value);
> + }
> }
> @@ -1166,18 +1176,64 @@ initialize_gnutls(void)
> return -1;
> }
>
> + if (client_pki_dir) {
> + if((virAsprintf(&libvirt_cacert, "%s/%s", client_pki_dir,
> + "cacert.pem")) < 0)
> + goto out_of_memory;
> +
> + if (!virFileExists(libvirt_cacert)) {
> + VIR_WARN(_("%s doesn't exist, try to use: %s"),
> + libvirt_cacert, LIBVIRT_CACERT);
> +
> + libvirt_cacert = strdup(LIBVIRT_CACERT);
This just leaked the memory previously assigned to 'libvirt_cacert'.
> + if (!libvirt_cacert) goto out_of_memory;
> + }
> +
> + if((virAsprintf(&libvirt_clientkey, "%s/%s", client_pki_dir,
> + "clientkey.pem")) < 0)
> + goto out_of_memory;
> +
> + if (!virFileExists(libvirt_clientkey)) {
> + VIR_WARN(_("%s doesn't exist, try to use: %s"),
> + libvirt_clientkey, LIBVIRT_CLIENTKEY);
> +
> + libvirt_clientkey = strdup(LIBVIRT_CLIENTKEY);
> + if (!libvirt_clientkey) goto out_of_memory;
> + }
> +
> + if((virAsprintf(&libvirt_clientcert, "%s/%s", client_pki_dir,
> + "clientcert.pem")) < 0)
> + goto out_of_memory;
>
> - if (check_cert_file("CA certificate", LIBVIRT_CACERT) < 0)
> + if (!virFileExists(libvirt_clientcert)) {
> + VIR_WARN(_("%s doesn't exist, try to use: %s"),
> + libvirt_clientcert, LIBVIRT_CLIENTCERT);
> +
> + libvirt_clientcert = strdup(LIBVIRT_CLIENTCERT);
> + if (!libvirt_clientcert) goto out_of_memory;
> + }
> + } else {
> + libvirt_cacert = strdup(LIBVIRT_CACERT);
> + if (!libvirt_cacert) goto out_of_memory;
> +
> + libvirt_clientkey = strdup(LIBVIRT_CLIENTKEY);
> + if (!libvirt_cacert) goto out_of_memory;
> +
> + libvirt_clientcert = strdup(LIBVIRT_CLIENTCERT);
> + if (!libvirt_cacert) goto out_of_memory;
> + }
I think I would structure this code somewhat differently. We currently
have a global location for PKI. This patch lets the user set a custom dir
per URI. I think we also want to check $HOME/.pki for non-root users.
Thus I would structure it:
- If pkipath URI parameter is set, use that. If a file does
not exist raise a fatal error, no fallback
- Try $HOME/.pki if non root. If the files don't exist, or if
root, then use /etc/pki
Daniel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list