[libvirt] [PATCH 0/9] Make TLS priority choice configurable
Historically libvirt has used gnutls_set_default_priority() to tell GNUTLS to use its standard protocol/cipher config settings. Since Fedora >= 21, this has caused gnutls to lookup the conf in /etc/crypto-policies/back-end/gnutls.conf, while previously it was hardcoded at gnutls build time. Using the global config is good, but sometimes there might be a need to have libvirt use a different config than everything else on the host. eg the global config must need to be weakened for back-compat usage in non-libvirt apps. We should allow libvirt to maintain a strong config despite this. Ideally gnutls would let us express a preference for multiple config file settings, and would pick the first one it found. That would let us request "@LIBVIRT,SYSTEM" to say use the "LIBVIRT" priority if set, otherwise use the "SYSTEM" priority. This is proposed in upstream GNUTLS http://lists.gnutls.org/pipermail/gnutls-devel/2016-June/008007.html and if accepted will be the best way to configure things. Until that feature is accepted though, we should allow a local override in libvirtd.conf (servers) and libvirt.conf (clients). This series of patches does that. NB, we also need to do similar for the QEMU VNC TLS configuration but that's going to be a followup series. Daniel P. Berrange (9): tls: remove support for gnutls 1.x.x, require 2.2.0 rpc: set gnutls log function at global init time configure: allow setting default TLS priority string rpc: allow priority string to be passed to TLS context libvirtd: add config option for TLS priority remote: allow TLS protocol/cipher priority override in URI Pass config file object through to driver open methods remote: allow TLS priority to be customized Use @SYSTEM priority for TLS on Fedora >= 21 configure.ac | 12 ++++++++- daemon/libvirtd-config.c | 2 ++ daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 2 ++ daemon/libvirtd.conf | 9 ++++++- daemon/test_libvirtd.aug.in | 1 + docs/remote.html.in | 13 ++++++++++ libvirt.spec.in | 7 ++++++ src/Makefile.am | 1 - src/bhyve/bhyve_driver.c | 1 + src/driver-hypervisor.h | 1 + src/esx/esx_driver.c | 1 + src/gnutls_1_0_compat.h | 43 -------------------------------- src/hyperv/hyperv_driver.c | 4 ++- src/libvirt.c | 2 +- src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 4 ++- src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 20 ++++++++++++++- src/rpc/virnettlscontext.c | 59 ++++++++++++++++++++++---------------------- src/rpc/virnettlscontext.h | 4 +++ src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_common.c | 1 + src/vbox/vbox_driver.c | 1 + src/vmware/vmware_driver.c | 1 + src/vz/vz_driver.c | 1 + src/xen/xen_driver.c | 4 ++- tests/virnettlscontexttest.c | 2 ++ tests/virnettlshelpers.h | 1 - tests/virnettlssessiontest.c | 2 ++ 34 files changed, 126 insertions(+), 81 deletions(-) delete mode 100644 src/gnutls_1_0_compat.h -- 2.5.5
We need to use the gnutls_priority_set_direct method which was not introduced until 2.1.7, so bump version to 2.2.0 which is the first stable release with it included. This release dates from Dec 2007 so it is reasonable to ditch support for the 1.x.x series for gnutls releases entirely. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 2 +- src/Makefile.am | 1 - src/gnutls_1_0_compat.h | 43 ------------------------------------------- src/rpc/virnettlscontext.c | 11 ----------- tests/virnettlshelpers.h | 1 - 5 files changed, 1 insertion(+), 57 deletions(-) delete mode 100644 src/gnutls_1_0_compat.h diff --git a/configure.ac b/configure.ac index 74c33b3..42eaa82 100644 --- a/configure.ac +++ b/configure.ac @@ -117,7 +117,7 @@ fi dnl Required minimum versions of all libs we depend on LIBXML_REQUIRED="2.6.0" -GNUTLS_REQUIRED="1.0.25" +GNUTLS_REQUIRED="2.2.0" POLKIT_REQUIRED="0.6" PARTED_REQUIRED="1.8.0" DEVMAPPER_REQUIRED=1.0.0 diff --git a/src/Makefile.am b/src/Makefile.am index 12b66c2..2ad400d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -436,7 +436,6 @@ remote/qemu_client_bodies.h: $(srcdir)/rpc/gendispatch.pl \ > $(srcdir)/remote/qemu_client_bodies.h REMOTE_DRIVER_SOURCES = \ - gnutls_1_0_compat.h \ remote/remote_driver.c remote/remote_driver.h \ $(REMOTE_DRIVER_GENERATED) diff --git a/src/gnutls_1_0_compat.h b/src/gnutls_1_0_compat.h deleted file mode 100644 index b006e2b..0000000 --- a/src/gnutls_1_0_compat.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * gnutls_1_0_compat.h: GnuTLS 1.0 compatibility - * - * Copyright (C) 2007, 2013 Red Hat, Inc. - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see - * <http://www.gnu.org/licenses/>. - * - * Author: Richard W.M. Jones <rjones@redhat.com> - */ - -#ifndef LIBVIRT_GNUTLS_1_0_COMPAT_H__ -# define LIBVIRT_GNUTLS_1_0_COMPAT_H__ - -# include <gnutls/gnutls.h> - -/* enable backward compatibility macros for gnutls 1.x.y */ -# if LIBGNUTLS_VERSION_MAJOR < 2 -# define GNUTLS_1_0_COMPAT -# endif - -# ifdef GNUTLS_1_0_COMPAT -# define gnutls_session_t gnutls_session -# define gnutls_x509_crt_t gnutls_x509_crt -# define gnutls_dh_params_t gnutls_dh_params -# define gnutls_transport_ptr_t gnutls_transport_ptr -# define gnutls_datum_t gnutls_datum -# define gnutls_certificate_credentials_t gnutls_certificate_credentials -# define gnutls_cipher_algorithm_t gnutls_cipher_algorithm -# endif - -#endif /* LIBVIRT_GNUTLS_1_0_COMPAT_H__ */ diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 6e78623..fa9ca41 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -29,7 +29,6 @@ # include <gnutls/crypto.h> #endif #include <gnutls/x509.h> -#include "gnutls_1_0_compat.h" #include "virnettlscontext.h" #include "virstring.h" @@ -170,7 +169,6 @@ static int virNetTLSContextCheckCertTimes(gnutls_x509_crt_t cert, } -#ifndef GNUTLS_1_0_COMPAT /* * The gnutls_x509_crt_get_basic_constraints function isn't * available in GNUTLS 1.0.x branches. This isn't critical @@ -219,7 +217,6 @@ static int virNetTLSContextCheckCertBasicConstraints(gnutls_x509_crt_t cert, return 0; } -#endif static int virNetTLSContextCheckCertKeyUsage(gnutls_x509_crt_t cert, @@ -438,11 +435,9 @@ static int virNetTLSContextCheckCert(gnutls_x509_crt_t cert, isServer, isCA) < 0) return -1; -#ifndef GNUTLS_1_0_COMPAT if (virNetTLSContextCheckCertBasicConstraints(cert, certFile, isServer, isCA) < 0) return -1; -#endif if (virNetTLSContextCheckCertKeyUsage(cert, certFile, isCA) < 0) @@ -489,10 +484,8 @@ static int virNetTLSContextCheckCertPair(gnutls_x509_crt_t cert, if (status & GNUTLS_CERT_REVOKED) reason = _("The certificate has been revoked."); -#ifndef GNUTLS_1_0_COMPAT if (status & GNUTLS_CERT_INSECURE_ALGORITHM) reason = _("The certificate uses an insecure algorithm"); -#endif virReportError(VIR_ERR_SYSTEM_ERROR, _("Our own certificate %s failed validation against %s: %s"), @@ -1022,10 +1015,8 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, if (status & GNUTLS_CERT_REVOKED) reason = _("The certificate has been revoked."); -#ifndef GNUTLS_1_0_COMPAT if (status & GNUTLS_CERT_INSECURE_ALGORITHM) reason = _("The certificate uses an insecure algorithm"); -#endif virReportError(VIR_ERR_SYSTEM_ERROR, _("Certificate failed validation: %s"), @@ -1088,13 +1079,11 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, /* !sess->isServer, since on the client, we're validating the * server's cert, and on the server, the client's cert */ -#ifndef GNUTLS_1_0_COMPAT if (virNetTLSContextCheckCertBasicConstraints(cert, "[session]", !sess->isServer, false) < 0) { gnutls_x509_crt_deinit(cert); goto authdeny; } -#endif if (virNetTLSContextCheckCertKeyUsage(cert, "[session]", false) < 0) { diff --git a/tests/virnettlshelpers.h b/tests/virnettlshelpers.h index 3f6afb9..48e7431 100644 --- a/tests/virnettlshelpers.h +++ b/tests/virnettlshelpers.h @@ -22,7 +22,6 @@ #include <gnutls/x509.h> #if !defined WIN32 && HAVE_LIBTASN1_H && LIBGNUTLS_VERSION_NUMBER >= 0x020600 -# include "gnutls_1_0_compat.h" # include <libtasn1.h> -- 2.5.5
On Mon, Jun 06, 2016 at 16:08:55 +0100, Daniel Berrange wrote:
We need to use the gnutls_priority_set_direct method which was not introduced until 2.1.7, so bump version to 2.2.0 which is the first stable release with it included. This release dates from Dec 2007 so it is reasonable to ditch support for the 1.x.x series for gnutls releases entirely.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 2 +- src/Makefile.am | 1 - src/gnutls_1_0_compat.h | 43 ------------------------------------------- src/rpc/virnettlscontext.c | 11 ----------- tests/virnettlshelpers.h | 1 - 5 files changed, 1 insertion(+), 57 deletions(-) delete mode 100644 src/gnutls_1_0_compat.h
There's one remaining comment mentioning gnutls 1 in src/rpc/virnettlscontext.c, line 172 ACK with or without the comment fixed.
Currently we set the gnutls log function when creating a TLS context, however, the setting is in fact global, not per context. So we should be setting it when we first call gnutls_global_init() instead. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnettlscontext.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index fa9ca41..425f7ff 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -708,7 +708,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, bool isServer) { virNetTLSContextPtr ctxt; - const char *gnutlsdebug; int err; if (virNetTLSContextInitialize() < 0) @@ -717,16 +716,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, if (!(ctxt = virObjectLockableNew(virNetTLSContextClass))) return NULL; - if ((gnutlsdebug = virGetEnvAllowSUID("LIBVIRT_GNUTLS_DEBUG")) != NULL) { - int val; - if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) - val = 10; - gnutls_global_set_log_level(val); - gnutls_global_set_log_function(virNetTLSLog); - VIR_DEBUG("Enabled GNUTLS debug"); - } - - err = gnutls_certificate_allocate_credentials(&ctxt->x509cred); if (err) { virReportError(VIR_ERR_SYSTEM_ERROR, @@ -1440,5 +1429,15 @@ void virNetTLSSessionDispose(void *obj) */ void virNetTLSInit(void) { + const char *gnutlsdebug; + if ((gnutlsdebug = virGetEnvAllowSUID("LIBVIRT_GNUTLS_DEBUG")) != NULL) { + int val; + if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) + val = 10; + gnutls_global_set_log_level(val); + gnutls_global_set_log_function(virNetTLSLog); + VIR_DEBUG("Enabled GNUTLS debug"); + } + gnutls_global_init(); } -- 2.5.5
On Mon, Jun 06, 2016 at 16:08:56 +0100, Daniel Berrange wrote:
Currently we set the gnutls log function when creating a TLS context, however, the setting is in fact global, not per context. So we should be setting it when we first call gnutls_global_init() instead.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnettlscontext.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
ACK
Currently libvirt calls gnutls_set_default_priority() which on old systems resolves to "NORMAL" while new systems it resolves to "@SYSTEM". Either way, this is a global default that is identical across all apps. We want to allow distros to flexibility to define a custom default string for libvirt priority, so add a --tls-priority=STRING flag to configure to enable this to be set. It is expected that distros would use this when creating RPM/Deb/etc packages, according to their preferred crypto handling policies. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 10 ++++++++++ src/rpc/virnettlscontext.c | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 42eaa82..c4fc8be 100644 --- a/configure.ac +++ b/configure.ac @@ -1277,6 +1277,16 @@ AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS]) +AC_ARG_WITH([tls-priority], + [AS_HELP_STRING([--with-tls-priority], + [set the default TLS session priority string @<:@default=NORMAL@:>@])], + [], + [with_tls_priority=NORMAL]) + +AC_DEFINE_UNQUOTED([TLS_PRIORITY], ["$with_tls_priority"], + [TLS default priority string]) + + dnl PolicyKit library POLKIT_CFLAGS= POLKIT_LIBS= diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 425f7ff..975b5b8 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1204,10 +1204,10 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, /* avoid calling all the priority functions, since the defaults * are adequate. */ - if ((err = gnutls_set_default_priority(sess->session)) != 0) { + if ((err = gnutls_priority_set_direct(sess->session, TLS_PRIORITY, NULL)) != 0) { virReportError(VIR_ERR_SYSTEM_ERROR, - _("Failed to set TLS session priority %s"), - gnutls_strerror(err)); + _("Failed to set TLS session priority to %s: %s"), + TLS_PRIORITY, gnutls_strerror(err)); goto error; } -- 2.5.5
On Mon, Jun 06, 2016 at 16:08:57 +0100, Daniel Berrange wrote:
Currently libvirt calls gnutls_set_default_priority() which on old systems resolves to "NORMAL" while new systems it resolves to "@SYSTEM". Either way, this is a global default that is identical across all apps.
We want to allow distros to flexibility to define a custom default string for libvirt priority, so add a --tls-priority=STRING flag to configure to enable this to be set.
It is expected that distros would use this when creating RPM/Deb/etc packages, according to their preferred crypto handling policies.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 10 ++++++++++ src/rpc/virnettlscontext.c | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 42eaa82..c4fc8be 100644 --- a/configure.ac +++ b/configure.ac @@ -1277,6 +1277,16 @@ AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS])
+AC_ARG_WITH([tls-priority], + [AS_HELP_STRING([--with-tls-priority], + [set the default TLS session priority string @<:@default=NORMAL@:>@])], + [], + [with_tls_priority=NORMAL]) + +AC_DEFINE_UNQUOTED([TLS_PRIORITY], ["$with_tls_priority"], + [TLS default priority string]) + + dnl PolicyKit library POLKIT_CFLAGS= POLKIT_LIBS=
I think the setting should also be added to the "Configuration summary" section in configure output.
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
ACK
On Wed, Jun 08, 2016 at 12:58:05PM +0200, Peter Krempa wrote:
On Mon, Jun 06, 2016 at 16:08:57 +0100, Daniel Berrange wrote:
Currently libvirt calls gnutls_set_default_priority() which on old systems resolves to "NORMAL" while new systems it resolves to "@SYSTEM". Either way, this is a global default that is identical across all apps.
We want to allow distros to flexibility to define a custom default string for libvirt priority, so add a --tls-priority=STRING flag to configure to enable this to be set.
It is expected that distros would use this when creating RPM/Deb/etc packages, according to their preferred crypto handling policies.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 10 ++++++++++ src/rpc/virnettlscontext.c | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac index 42eaa82..c4fc8be 100644 --- a/configure.ac +++ b/configure.ac @@ -1277,6 +1277,16 @@ AC_SUBST([GNUTLS_CFLAGS]) AC_SUBST([GNUTLS_LIBS])
+AC_ARG_WITH([tls-priority], + [AS_HELP_STRING([--with-tls-priority], + [set the default TLS session priority string @<:@default=NORMAL@:>@])], + [], + [with_tls_priority=NORMAL]) + +AC_DEFINE_UNQUOTED([TLS_PRIORITY], ["$with_tls_priority"], + [TLS default priority string]) + + dnl PolicyKit library POLKIT_CFLAGS= POLKIT_LIBS=
I think the setting should also be added to the "Configuration summary" section in configure output.
Good idea, will do that.
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
ACK
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Extend the virNetTLSContextNew* constructors to allow the TLS priority string to be passed in, overriding the compile time default. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 ++ src/remote/remote_driver.c | 1 + src/rpc/virnettlscontext.c | 27 ++++++++++++++++++++------- src/rpc/virnettlscontext.h | 4 ++++ tests/virnettlscontexttest.c | 2 ++ tests/virnettlssessiontest.c | 2 ++ 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5617e42..b844af4 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -585,6 +585,7 @@ daemonSetupNetworking(virNetServerPtr srv, config->cert_file, config->key_file, (const char *const*)config->tls_allowed_dn_list, + NULL, config->tls_no_sanity_certificate ? false : true, config->tls_no_verify_certificate ? false : true))) goto cleanup; @@ -592,6 +593,7 @@ daemonSetupNetworking(virNetServerPtr srv, if (!(ctxt = virNetTLSContextNewServerPath(NULL, !privileged, (const char *const*)config->tls_allowed_dn_list, + NULL, config->tls_no_sanity_certificate ? false : true, config->tls_no_verify_certificate ? false : true))) goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e3cf5fb..219cf47 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -845,6 +845,7 @@ doRemoteOpen(virConnectPtr conn, #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false, + NULL, sanity, verify); if (!priv->tls) goto failed; diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 975b5b8..bc15890 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -65,6 +65,7 @@ struct _virNetTLSContext { bool isServer; bool requireValidCert; const char *const*x509dnWhitelist; + char *priority; }; struct _virNetTLSSession { @@ -703,6 +704,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, const char *cert, const char *key, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert, bool isServer) @@ -716,6 +718,9 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, if (!(ctxt = virObjectLockableNew(virNetTLSContextClass))) return NULL; + if (VIR_STRDUP(ctxt->priority, priority) < 0) + goto error; + err = gnutls_certificate_allocate_credentials(&ctxt->x509cred); if (err) { virReportError(VIR_ERR_SYSTEM_ERROR, @@ -903,6 +908,7 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath, bool tryUserPkiPath, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert, bool isServer) @@ -915,7 +921,7 @@ static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath, return NULL; ctxt = virNetTLSContextNew(cacert, cacrl, cert, key, - x509dnWhitelist, sanityCheckCert, + x509dnWhitelist, priority, sanityCheckCert, requireValidCert, isServer); VIR_FREE(cacert); @@ -929,19 +935,21 @@ static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath, virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath, bool tryUserPkiPath, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNewPath(pkipath, tryUserPkiPath, x509dnWhitelist, + return virNetTLSContextNewPath(pkipath, tryUserPkiPath, x509dnWhitelist, priority, sanityCheckCert, requireValidCert, true); } virNetTLSContextPtr virNetTLSContextNewClientPath(const char *pkipath, bool tryUserPkiPath, + const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNewPath(pkipath, tryUserPkiPath, NULL, + return virNetTLSContextNewPath(pkipath, tryUserPkiPath, NULL, priority, sanityCheckCert, requireValidCert, false); } @@ -951,10 +959,11 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char *cacert, const char *cert, const char *key, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNew(cacert, cacrl, cert, key, x509dnWhitelist, + return virNetTLSContextNew(cacert, cacrl, cert, key, x509dnWhitelist, priority, sanityCheckCert, requireValidCert, true); } @@ -963,10 +972,11 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert, const char *cacrl, const char *cert, const char *key, + const char *priority, bool sanityCheckCert, bool requireValidCert) { - return virNetTLSContextNew(cacert, cacrl, cert, key, NULL, + return virNetTLSContextNew(cacert, cacrl, cert, key, NULL, priority, sanityCheckCert, requireValidCert, false); } @@ -1145,6 +1155,7 @@ void virNetTLSContextDispose(void *obj) PROBE(RPC_TLS_CONTEXT_DISPOSE, "ctxt=%p", ctxt); + VIR_FREE(ctxt->priority); gnutls_dh_params_deinit(ctxt->dhParams); gnutls_certificate_free_credentials(ctxt->x509cred); } @@ -1204,10 +1215,12 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, /* avoid calling all the priority functions, since the defaults * are adequate. */ - if ((err = gnutls_priority_set_direct(sess->session, TLS_PRIORITY, NULL)) != 0) { + if ((err = gnutls_priority_set_direct(sess->session, + ctxt->priority ? : TLS_PRIORITY, + NULL)) != 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Failed to set TLS session priority to %s: %s"), - TLS_PRIORITY, gnutls_strerror(err)); + ctxt->priority ? : TLS_PRIORITY, gnutls_strerror(err)); goto error; } diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 21539ad..6100b45 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -36,11 +36,13 @@ void virNetTLSInit(void); virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath, bool tryUserPkiPath, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert); virNetTLSContextPtr virNetTLSContextNewClientPath(const char *pkipath, bool tryUserPkiPath, + const char *priority, bool sanityCheckCert, bool requireValidCert); @@ -49,6 +51,7 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char *cacert, const char *cert, const char *key, const char *const*x509dnWhitelist, + const char *priority, bool sanityCheckCert, bool requireValidCert); @@ -56,6 +59,7 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert, const char *cacrl, const char *cert, const char *key, + const char *priority, bool sanityCheckCert, bool requireValidCert); diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index d33b896..42c8b0c 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -72,6 +72,7 @@ static int testTLSContextInit(const void *opaque) data->crt, KEYFILE, NULL, + NULL, true, true); } else { @@ -79,6 +80,7 @@ static int testTLSContextInit(const void *opaque) NULL, data->crt, KEYFILE, + NULL, true, true); } diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 3af948a..8b79a1e 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -113,6 +113,7 @@ static int testTLSSessionInit(const void *opaque) data->servercrt, KEYFILE, data->wildcards, + NULL, false, true); @@ -120,6 +121,7 @@ static int testTLSSessionInit(const void *opaque) NULL, data->clientcrt, KEYFILE, + NULL, false, true); -- 2.5.5
On Mon, Jun 06, 2016 at 16:08:58 +0100, Daniel Berrange wrote:
Extend the virNetTLSContextNew* constructors to allow the TLS priority string to be passed in, overriding the compile time default.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 ++ src/remote/remote_driver.c | 1 + src/rpc/virnettlscontext.c | 27 ++++++++++++++++++++------- src/rpc/virnettlscontext.h | 4 ++++ tests/virnettlscontexttest.c | 2 ++ tests/virnettlssessiontest.c | 2 ++ 6 files changed, 31 insertions(+), 7 deletions(-)
[...]
@@ -1204,10 +1215,12 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, /* avoid calling all the priority functions, since the defaults * are adequate. */ - if ((err = gnutls_priority_set_direct(sess->session, TLS_PRIORITY, NULL)) != 0) { + if ((err = gnutls_priority_set_direct(sess->session, + ctxt->priority ? : TLS_PRIORITY,
Ternary with the second argument missing is a GNU extension.
+ NULL)) != 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Failed to set TLS session priority to %s: %s"), - TLS_PRIORITY, gnutls_strerror(err)); + ctxt->priority ? : TLS_PRIORITY, gnutls_strerror(err));
... same here.
goto error; }
ACK
Add a "tls_priority" config option to /etc/libvirt/libvirtd.conf to allow the administrator to override the built-in default setting. This only affects the server side configuration. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd-config.c | 2 ++ daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 4 ++-- daemon/libvirtd.conf | 9 ++++++++- daemon/test_libvirtd.aug.in | 1 + 6 files changed, 15 insertions(+), 3 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index 45280e9..940bd4b 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -367,6 +367,7 @@ daemonConfigFree(struct daemonConfig *data) tmp++; } VIR_FREE(data->sasl_allowed_username_list); + VIR_FREE(data->tls_priority); VIR_FREE(data->key_file); VIR_FREE(data->ca_file); @@ -442,6 +443,7 @@ daemonConfigLoadOptions(struct daemonConfig *data, &data->sasl_allowed_username_list, filename) < 0) goto error; + GET_CONF_STR(conf, filename, tls_priority); GET_CONF_UINT(conf, filename, min_workers); GET_CONF_UINT(conf, filename, max_workers); diff --git a/daemon/libvirtd-config.h b/daemon/libvirtd-config.h index 672e9ad..b9098a8 100644 --- a/daemon/libvirtd-config.h +++ b/daemon/libvirtd-config.h @@ -56,6 +56,7 @@ struct daemonConfig { int tls_no_sanity_certificate; char **tls_allowed_dn_list; char **sasl_allowed_username_list; + char *tls_priority; char *key_file; char *cert_file; diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug index 7a81723..2b8df66 100644 --- a/daemon/libvirtd.aug +++ b/daemon/libvirtd.aug @@ -53,6 +53,7 @@ module Libvirtd = | str_array_entry "tls_allowed_dn_list" | str_array_entry "sasl_allowed_username_list" | str_array_entry "access_drivers" + | str_entry "tls_priority" let processing_entry = int_entry "min_workers" | int_entry "max_workers" diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index b844af4..a1e2015 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -585,7 +585,7 @@ daemonSetupNetworking(virNetServerPtr srv, config->cert_file, config->key_file, (const char *const*)config->tls_allowed_dn_list, - NULL, + config->tls_priority, config->tls_no_sanity_certificate ? false : true, config->tls_no_verify_certificate ? false : true))) goto cleanup; @@ -593,7 +593,7 @@ daemonSetupNetworking(virNetServerPtr srv, if (!(ctxt = virNetTLSContextNewServerPath(NULL, !privileged, (const char *const*)config->tls_allowed_dn_list, - NULL, + config->tls_priority, config->tls_no_sanity_certificate ? false : true, config->tls_no_verify_certificate ? false : true))) goto cleanup; diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 1c1fa7f..3b957e5 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -242,7 +242,7 @@ #tls_allowed_dn_list = ["DN1", "DN2"] -# A whitelist of allowed SASL usernames. The format for usernames +# A whitelist of allowed SASL usernames. The format for username # depends on the SASL authentication mechanism. Kerberos usernames # look like username@REALM # @@ -259,6 +259,13 @@ #sasl_allowed_username_list = ["joe@EXAMPLE.COM", "fred@EXAMPLE.COM" ] +# Override the compile time default TLS priority string. The +# default is usually "NORMAL" unless overridden at build time. +# Only set this is it is desired for libvirt to deviate from +# the global default settings. +# +#tls_priority="NORMAL" + ################################################################# # diff --git a/daemon/test_libvirtd.aug.in b/daemon/test_libvirtd.aug.in index 7a03603..1fb182c 100644 --- a/daemon/test_libvirtd.aug.in +++ b/daemon/test_libvirtd.aug.in @@ -35,6 +35,7 @@ module Test_libvirtd = { "1" = "joe@EXAMPLE.COM" } { "2" = "fred@EXAMPLE.COM" } } + { "tls_priority" = "NORMAL" } { "max_clients" = "5000" } { "max_queued_clients" = "1000" } { "max_anonymous_clients" = "20" } -- 2.5.5
On Mon, Jun 06, 2016 at 16:08:59 +0100, Daniel Berrange wrote:
Add a "tls_priority" config option to /etc/libvirt/libvirtd.conf to allow the administrator to override the built-in default setting. This only affects the server side configuration.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd-config.c | 2 ++ daemon/libvirtd-config.h | 1 + daemon/libvirtd.aug | 1 + daemon/libvirtd.c | 4 ++-- daemon/libvirtd.conf | 9 ++++++++- daemon/test_libvirtd.aug.in | 1 + 6 files changed, 15 insertions(+), 3 deletions(-)
ACK
Add support for a "tls_priority" URI parameter in remote driver URIs. eg qemu+tls://localhost/session?tls_priority=NORMAL:-VERS-SSL3.0 Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/remote.html.in | 13 +++++++++++++ src/remote/remote_driver.c | 5 ++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/remote.html.in b/docs/remote.html.in index 638fdae..9b132f1 100644 --- a/docs/remote.html.in +++ b/docs/remote.html.in @@ -230,6 +230,19 @@ Note that parameter values must be </tr> <tr> <td> + <code>tls_priority</code> + </td> + <td> tls </td> + <td> + A vaid GNUTLS priority string +</td> + </tr> + <tr> + <td colspan="2"/> + <td> Example: <code>tls_priority=NORMAL:-VERS-SSL3.0</code> </td> + </tr> + <tr> + <td> <code>command</code> </td> <td> ssh, ext </td> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 219cf47..5f02169 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -652,6 +652,7 @@ doRemoteOpen(virConnectPtr conn, #ifndef WIN32 char *daemonPath = NULL; #endif + char *tls_priority = NULL; /* We handle *ALL* URIs here. The caller has rejected any * URIs we don't care about */ @@ -774,6 +775,7 @@ doRemoteOpen(virConnectPtr conn, EXTRACT_URI_ARG_STR("pkipath", pkipath); EXTRACT_URI_ARG_STR("known_hosts", knownHosts); EXTRACT_URI_ARG_STR("known_hosts_verify", knownHostsVerify); + EXTRACT_URI_ARG_STR("tls_priority", tls_priority); EXTRACT_URI_ARG_BOOL("no_sanity", sanity); EXTRACT_URI_ARG_BOOL("no_verify", verify); @@ -845,12 +847,13 @@ doRemoteOpen(virConnectPtr conn, #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false, - NULL, + tls_priority, sanity, verify); if (!priv->tls) goto failed; priv->is_secure = 1; #else + (void)tls_priority; (void)sanity; (void)verify; virReportError(VIR_ERR_INVALID_ARG, "%s", -- 2.5.5
On Mon, Jun 06, 2016 at 16:09:00 +0100, Daniel Berrange wrote:
Add support for a "tls_priority" URI parameter in remote driver URIs. eg
qemu+tls://localhost/session?tls_priority=NORMAL:-VERS-SSL3.0
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- docs/remote.html.in | 13 +++++++++++++ src/remote/remote_driver.c | 5 ++++- 2 files changed, 17 insertions(+), 1 deletion(-)
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 219cf47..5f02169 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c
[...]
@@ -774,6 +775,7 @@ doRemoteOpen(virConnectPtr conn, EXTRACT_URI_ARG_STR("pkipath", pkipath); EXTRACT_URI_ARG_STR("known_hosts", knownHosts); EXTRACT_URI_ARG_STR("known_hosts_verify", knownHostsVerify); + EXTRACT_URI_ARG_STR("tls_priority", tls_priority);
This copies the string from the URI.
EXTRACT_URI_ARG_BOOL("no_sanity", sanity); EXTRACT_URI_ARG_BOOL("no_verify", verify); @@ -845,12 +847,13 @@ doRemoteOpen(virConnectPtr conn, #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false, - NULL, + tls_priority, sanity, verify); if (!priv->tls) goto failed; priv->is_secure = 1; #else + (void)tls_priority; (void)sanity; (void)verify; virReportError(VIR_ERR_INVALID_ARG, "%s",
So tls_priority is leaked in doRemoteOpen. ACK with the above fixed.
The virConnectOpenInternal method opens the libvirt client config file and uses it to resolve things like URI aliases. There may be driver specific things that are useful to store in the config file too, so rather than have them re-parse the same file, pass the virConfPtr down to the drivers. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 1 + src/driver-hypervisor.h | 1 + src/esx/esx_driver.c | 1 + src/hyperv/hyperv_driver.c | 4 +++- src/libvirt.c | 2 +- src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 4 +++- src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_common.c | 1 + src/vbox/vbox_driver.c | 1 + src/vmware/vmware_driver.c | 1 + src/vz/vz_driver.c | 1 + src/xen/xen_driver.c | 4 +++- 18 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index c4051a1..1885bdf 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -178,6 +178,7 @@ bhyveDomObjFromDomain(virDomainPtr domain) static virDrvOpenStatus bhyveConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index d11ff7f..5ab5775 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -28,6 +28,7 @@ typedef virDrvOpenStatus (*virDrvConnectOpen)(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf, unsigned int flags); typedef int diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 031c666..c5f5ed8 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -841,6 +841,7 @@ esxConnectToVCenter(esxPrivate *priv, */ static virDrvOpenStatus esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index b95c549..9c7faf0 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -58,7 +58,9 @@ hypervFreePrivate(hypervPrivate **priv) static virDrvOpenStatus -hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) +hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) { virDrvOpenStatus result = VIR_DRV_OPEN_ERROR; char *plus; diff --git a/src/libvirt.c b/src/libvirt.c index 6d9dba8..a5e0e41 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1084,7 +1084,7 @@ virConnectOpenInternal(const char *name, ret->secretDriver = virConnectDriverTab[i]->secretDriver; ret->storageDriver = virConnectDriverTab[i]->storageDriver; - res = virConnectDriverTab[i]->hypervisorDriver->connectOpen(ret, auth, flags); + res = virConnectDriverTab[i]->hypervisorDriver->connectOpen(ret, auth, conf, flags); VIR_DEBUG("driver %zu %s returned %s", i, virConnectDriverTab[i]->hypervisorDriver->name, res == VIR_DRV_OPEN_SUCCESS ? "SUCCESS" : diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 95dfc01..82a05e0 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -763,6 +763,7 @@ libxlStateReload(void) static virDrvOpenStatus libxlConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f0948ea..ad866f9 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -155,6 +155,7 @@ lxcDomObjFromDomain(virDomainPtr domain) static virDrvOpenStatus lxcConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index a7474ff..489a607 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1423,6 +1423,7 @@ openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) static virDrvOpenStatus openvzConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { struct openvz_driver *driver; diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index da87686..5a9729d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1123,7 +1123,9 @@ virDomainDefParserConfig virPhypDriverDomainDefParserConfig = { static virDrvOpenStatus phypConnectOpen(virConnectPtr conn, - virConnectAuthPtr auth, unsigned int flags) + virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) { LIBSSH2_SESSION *session = NULL; int internal_socket = -1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 10d3e3d..eb0b804 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1120,6 +1120,7 @@ qemuStateCleanup(void) static virDrvOpenStatus qemuConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virQEMUDriverConfigPtr cfg = NULL; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5f02169..b42d1d1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1179,6 +1179,7 @@ remoteAllocPrivateData(void) static virDrvOpenStatus remoteConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { struct private_data *priv; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a51eb09..2c7fbd1 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1411,6 +1411,7 @@ testConnectAuthenticate(virConnectPtr conn, static virDrvOpenStatus testConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { int ret; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 923c3f6..de28705 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1200,6 +1200,7 @@ static void umlShutdownVMDaemon(struct uml_driver *driver, static virDrvOpenStatus umlConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index ee2b6ad..01748e8 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -391,6 +391,7 @@ static void vboxUninitialize(vboxGlobalData *data) static virDrvOpenStatus vboxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { vboxGlobalData *data = NULL; diff --git a/src/vbox/vbox_driver.c b/src/vbox/vbox_driver.c index d363fef..a5f3721 100644 --- a/src/vbox/vbox_driver.c +++ b/src/vbox/vbox_driver.c @@ -53,6 +53,7 @@ VIR_LOG_INIT("vbox.vbox_driver"); #if !defined(WITH_DRIVER_MODULES) || defined(VBOX_DRIVER) static virDrvOpenStatus dummyConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { uid_t uid = geteuid(); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 93f21c9..37c7353 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -118,6 +118,7 @@ vmwareDomainXMLConfigInit(void) static virDrvOpenStatus vmwareConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { struct vmware_driver *driver; diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 177a57a..6dc2ecd 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -333,6 +333,7 @@ vzDriverObjNew(void) static virDrvOpenStatus vzConnectOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf ATTRIBUTE_UNUSED, unsigned int flags) { vzDriverPtr driver = NULL; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 3f5d80d..63b5b58 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -404,7 +404,9 @@ xenDomainXMLConfInit(void) static virDrvOpenStatus -xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) +xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, + virConfPtr conf ATTRIBUTE_UNUSED, + unsigned int flags) { xenUnifiedPrivatePtr priv; char ebuf[1024]; -- 2.5.5
On Mon, Jun 06, 2016 at 16:09:01 +0100, Daniel Berrange wrote:
The virConnectOpenInternal method opens the libvirt client config file and uses it to resolve things like URI aliases.
There may be driver specific things that are useful to store in the config file too, so rather than have them re-parse the same file, pass the virConfPtr down to the drivers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/bhyve/bhyve_driver.c | 1 + src/driver-hypervisor.h | 1 + src/esx/esx_driver.c | 1 + src/hyperv/hyperv_driver.c | 4 +++- src/libvirt.c | 2 +- src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 4 +++- src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_common.c | 1 + src/vbox/vbox_driver.c | 1 + src/vmware/vmware_driver.c | 1 + src/vz/vz_driver.c | 1 + src/xen/xen_driver.c | 4 +++-
Missing change to 'xenapiConnectOpen'.
18 files changed, 24 insertions(+), 4 deletions(-)
ACK with the above resolved.
Support reading the TLS priority from the client configuration file via the "tls_priority" config option, eg $ cat $HOME/.config/libvirt/libvirt.conf tls_priority="NORMAL:-VERS-SSL3.0" Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/remote/remote_driver.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b42d1d1..367f46e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -638,6 +638,7 @@ static int doRemoteOpen(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf, unsigned int flags) { char *transport_str = NULL; @@ -844,6 +845,18 @@ doRemoteOpen(virConnectPtr conn, /* Connect to the remote service. */ switch (transport) { case trans_tls: + if (conf && !tls_priority) { + virConfValuePtr val = virConfGetValue(conf, "tls_priority"); + if (val) { + if (val->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Config file 'tls_priority' must be a string")); + goto failed; + } + tls_priority = val->str; + } + } + #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false, @@ -1179,7 +1192,7 @@ remoteAllocPrivateData(void) static virDrvOpenStatus remoteConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, - virConfPtr conf ATTRIBUTE_UNUSED, + virConfPtr conf, unsigned int flags) { struct private_data *priv; @@ -1238,7 +1251,7 @@ remoteConnectOpen(virConnectPtr conn, #endif } - ret = doRemoteOpen(conn, priv, auth, rflags); + ret = doRemoteOpen(conn, priv, auth, conf, rflags); if (ret != VIR_DRV_OPEN_SUCCESS) { conn->privateData = NULL; remoteDriverUnlock(priv); -- 2.5.5
On Mon, Jun 06, 2016 at 16:09:02 +0100, Daniel Berrange wrote:
Support reading the TLS priority from the client configuration file via the "tls_priority" config option, eg
$ cat $HOME/.config/libvirt/libvirt.conf tls_priority="NORMAL:-VERS-SSL3.0"
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/remote/remote_driver.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index b42d1d1..367f46e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -638,6 +638,7 @@ static int doRemoteOpen(virConnectPtr conn, struct private_data *priv, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virConfPtr conf, unsigned int flags) { char *transport_str = NULL; @@ -844,6 +845,18 @@ doRemoteOpen(virConnectPtr conn, /* Connect to the remote service. */ switch (transport) { case trans_tls: + if (conf && !tls_priority) { + virConfValuePtr val = virConfGetValue(conf, "tls_priority");
This does not copy the string ...
+ if (val) { + if (val->type != VIR_CONF_STRING) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("Config file 'tls_priority' must be a string")); + goto failed; + } + tls_priority = val->str;
... so make sure you copy it here due to the previously requested change.
+ } + } + #ifdef WITH_GNUTLS priv->tls = virNetTLSContextNewClientPath(pkipath, geteuid() != 0 ? true : false,
ACK
In Fedora >= 21, there is a new crypto priority framework that sets TLS policies globally for all apps. To activate this with GNUTLS we must request "@SYSTEM" instead of the traditional "NORMAL" string. The '@' causes gnutls todo a lookup in its config file for the 'SYSTEM' keyword entry. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 8b88eef..2d138b0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -208,6 +208,12 @@ %define enable_werror --disable-werror %endif +%if 0%{?fedora} >= 21 + %define tls_priority "@SYSTEM" +%else + %define tls_priority "NORMAL" +%endif + Summary: Library providing a simple virtualization API Name: libvirt @@ -1164,6 +1170,7 @@ rm -f po/stamp-po %{arg_packager_version} \ --with-qemu-user=%{qemu_user} \ --with-qemu-group=%{qemu_group} \ + --with-tls-priority=%{tls_priority} \ %{?arg_loader_nvram} \ %{?enable_werror} \ --enable-expensive-tests \ -- 2.5.5
On Mon, Jun 06, 2016 at 16:09:03 +0100, Daniel Berrange wrote:
In Fedora >= 21, there is a new crypto priority framework that sets TLS policies globally for all apps. To activate this with GNUTLS we must request "@SYSTEM" instead of the traditional "NORMAL" string. The '@' causes gnutls todo a lookup in its config file for the 'SYSTEM' keyword entry.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in index 8b88eef..2d138b0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -208,6 +208,12 @@ %define enable_werror --disable-werror %endif
+%if 0%{?fedora} >= 21 + %define tls_priority "@SYSTEM" +%else + %define tls_priority "NORMAL" +%endif +
Summary: Library providing a simple virtualization API Name: libvirt @@ -1164,6 +1170,7 @@ rm -f po/stamp-po %{arg_packager_version} \ --with-qemu-user=%{qemu_user} \ --with-qemu-group=%{qemu_group} \ + --with-tls-priority=%{tls_priority} \
This looks misalidned in git show output.
%{?arg_loader_nvram} \ %{?enable_werror} \ --enable-expensive-tests \
ACK
participants (2)
-
Daniel P. Berrange -
Peter Krempa