[libvirt] [v1] remote: Add extra parameter client_pki_dir for URI
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_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); + } } /* Construct the original name. */ @@ -577,7 +583,7 @@ doRemoteOpen (virConnectPtr conn, /* Connect to the remote service. */ switch (transport) { case trans_tls: - if (initialize_gnutls() == -1) goto failed; + if (initialize_gnutls(client_pki_dir) == -1) goto failed; priv->uses_tls = 1; priv->is_secure = 1; @@ -947,6 +953,7 @@ doRemoteOpen (virConnectPtr conn, } VIR_FREE(cmd_argv); } + VIR_FREE(client_pki_dir); return retcode; @@ -1139,11 +1146,14 @@ static void remote_debug_gnutls_log(int level, const char* str) { } static int -initialize_gnutls(void) +initialize_gnutls(char *client_pki_dir) { static int initialized = 0; int err; char *gnutlsdebug; + char *libvirt_cacert = NULL; + char *libvirt_clientkey = NULL; + char *libvirt_clientcert = NULL; if (initialized) return 0; @@ -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); + 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; + } + + if (check_cert_file("CA certificate", libvirt_cacert) < 0) return -1; - if (check_cert_file("client key", LIBVIRT_CLIENTKEY) < 0) + if (check_cert_file("client key", libvirt_clientkey) < 0) return -1; - if (check_cert_file("client certificate", LIBVIRT_CLIENTCERT) < 0) + if (check_cert_file("client certificate", libvirt_clientcert) < 0) return -1; /* Set the trusted CA cert. */ - DEBUG("loading CA file %s", LIBVIRT_CACERT); + DEBUG("loading CA file %s", libvirt_cacert); err = - gnutls_certificate_set_x509_trust_file (x509_cred, LIBVIRT_CACERT, + gnutls_certificate_set_x509_trust_file (x509_cred, libvirt_cacert, GNUTLS_X509_FMT_PEM); if (err < 0) { remoteError(VIR_ERR_GNUTLS_ERROR, @@ -1188,11 +1244,11 @@ initialize_gnutls(void) /* Set the client certificate and private key. */ DEBUG("loading client cert and key from files %s and %s", - LIBVIRT_CLIENTCERT, LIBVIRT_CLIENTKEY); + libvirt_clientcert, libvirt_clientkey); err = gnutls_certificate_set_x509_key_file (x509_cred, - LIBVIRT_CLIENTCERT, - LIBVIRT_CLIENTKEY, + libvirt_clientcert, + libvirt_clientkey, GNUTLS_X509_FMT_PEM); if (err < 0) { remoteError(VIR_ERR_GNUTLS_ERROR, @@ -1202,7 +1258,14 @@ initialize_gnutls(void) } initialized = 1; + VIR_FREE(libvirt_cacert); + VIR_FREE(libvirt_clientkey); + VIR_FREE(libvirt_clientcert); return 0; + +out_of_memory: + virReportOOMError(); + return -1; } static int verify_certificate (virConnectPtr conn, struct private_data *priv, gnutls_session_t session); -- 1.7.3.2
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
于 2011年01月24日 19:52, Daniel P. Berrange 写道:
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'
okay, make sense, will update.
+ 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'.
yes, will update.
+ 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
oh, yeah, checking $HOME/.pki for non-root user will be better, will update. Thanks Osier
participants (2)
-
Daniel P. Berrange -
Osier Yang