Re: [libvirt] [PATCH v4 2/3] conf: Introduce TLS options for VxHS block device clients

On 06/29/2017 10:02 PM, Ashish Mittal wrote: > From: Ashish Mittal <ashish.mittal(a)veritas.com&gt; > > Add a new TLS X.509 certificate type - "vxhs". This will handle the > creation of a TLS certificate capability for properly configured > VxHS network block device clients. > > Signed-off-by: Ashish Mittal <ashish.mittal(a)veritas.com&gt; > --- > Changelog: > > (1) Add two new options in /etc/libvirt/qemu.conf > to control TLS behavior with VxHS block devices > "vxhs_tls" and "vxhs_tls_x509_cert_dir". > (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable > TLS for VxHS block devices. > (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the > TLS certificates and keys are saved. If this value is missing, > the "default_tls_x509_cert_dir" will be used instead. > > src/qemu/libvirtd_qemu.aug | 4 ++++ > src/qemu/qemu.conf | 23 +++++++++++++++++++++++ > src/qemu/qemu_conf.c | 7 +++++++ > src/qemu/qemu_conf.h | 3 +++ > src/qemu/test_libvirtd_qemu.aug.in | 2 ++ > 5 files changed, 39 insertions(+) > FWIW: I have another patch upstream: https://www.redhat.com/archives/libvir-list/2017-June/msg01341.html Which is making some textual and code changes to qemu.conf and qemu_conf.c - just so you are aware.

> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug > index e1983d1..c19bf3a 100644 > --- a/src/qemu/libvirtd_qemu.aug > +++ b/src/qemu/libvirtd_qemu.aug > @@ -115,6 +115,9 @@ module Libvirtd_qemu = > > let memory_entry = str_entry "memory_backing_dir" > > + let vxhs_entry = bool_entry "vxhs_tls" > + | str_entry "vxhs_tls_x509_cert_dir" > + > (* Each entry in the config is one of the following ... *) > let entry = default_tls_entry > | vnc_entry > @@ -133,6 +136,7 @@ module Libvirtd_qemu = > | nvram_entry > | gluster_debug_level_entry > | memory_entry > + | vxhs_entry > > let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] > let empty = [ label "#empty" . eol ] > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf > index e6c0832..83c2377 100644 > --- a/src/qemu/qemu.conf > +++ b/src/qemu/qemu.conf > @@ -250,6 +250,29 @@ > #chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" > > > +# Enable use of TLS encryption on the VxHS network block devices. > +# > +# When the VxHS network block device server is set up appropriately, > +# x509 certificates are used for authentication between the clients > +# (qemu processes) and the remote VxHS server. > +# > +# It is necessary to setup CA and issue client and server certificates > +# before enabling this. > +# > +#vxhs_tls = 1 > + > + > +# In order to override the default TLS certificate location for VxHS > +# device TCP certificates, supply a valid path to the certificate directory. > +# This is used to authenticate the VxHS block device clients to the VxHS > +# server. > +# > +# If the provided path does not exist then the default_tls_x509_cert_dir > +# path will be used. > +# > +#vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs" > + > + > # In order to override the default TLS certificate location for migration > # certificates, supply a valid path to the certificate directory. If the > # provided path does not exist then the default_tls_x509_cert_dir path In patch 3, the call to qemuBuildTLSx509CommandLine has a @verifypeer parameter = true, thus it's always going to expect to verify the client rather than making that determination using *_tls_x509_verify. So that means you have to describe the environment as requiring the client-cert.pem and client-key.pem files (like the description for default). Since you set the @addpasswordid to false on input, that just means the certificate server-key.pem file cannot be encrypted - something else that would need to be described. Based on these two configuration settings, that means if vxhs_tls=1, then vxhs_tls_x509_cert_dir must be set to something other than the default environment. IOW: both must be defined and that would need to be validated during config read and cause failure (e.g. vxhsTLSx509certdir != defaultTLSx509certdir IOW: You're essentially *requiring* someone to set this up and don't handle the/a default environment. Hopefully this makes sense, it's Friday afternoon and I have a TLS migraine ;-).

John > diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c > index 73c33d6..f3813d4 100644 > --- a/src/qemu/qemu_conf.c > +++ b/src/qemu/qemu_conf.c > @@ -280,6 +280,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) > SET_TLS_X509_CERT_DEFAULT(spice); > SET_TLS_X509_CERT_DEFAULT(chardev); > SET_TLS_X509_CERT_DEFAULT(migrate); > + SET_TLS_X509_CERT_DEFAULT(vxhs); > > #undef SET_TLS_X509_CERT_DEFAULT > > @@ -395,6 +396,8 @@ static void virQEMUDriverConfigDispose(void *obj) > VIR_FREE(cfg->chardevTLSx509certdir); > VIR_FREE(cfg->chardevTLSx509secretUUID); > > + VIR_FREE(cfg->vxhsTLSx509certdir); > + > VIR_FREE(cfg->migrateTLSx509certdir); > VIR_FREE(cfg->migrateTLSx509secretUUID); > > @@ -533,6 +536,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, > goto cleanup; > if (virConfGetValueBool(conf, "spice_auto_unix_socket", &cfg->spiceAutoUnixSocket) < 0) > goto cleanup; > + if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0) > + goto cleanup; > + if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir", &cfg->vxhsTLSx509certdir) < 0) > + goto cleanup; > > #define GET_CONFIG_TLS_CERTINFO(val) \ > do { \ > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h > index 1407eef..96c0225 100644 > --- a/src/qemu/qemu_conf.h > +++ b/src/qemu/qemu_conf.h > @@ -201,6 +201,9 @@ struct _virQEMUDriverConfig { > unsigned int glusterDebugLevel; > > char *memoryBackingDir; > + > + bool vxhsTLS; > + char *vxhsTLSx509certdir; > }; > > /* Main driver state */ > diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in > index 3e317bc..dfe88f4 100644 > --- a/src/qemu/test_libvirtd_qemu.aug.in > +++ b/src/qemu/test_libvirtd_qemu.aug.in > @@ -25,6 +25,8 @@ module Test_libvirtd_qemu = > { "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" } > { "chardev_tls_x509_verify" = "1" } > { "chardev_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } > +{ "vxhs_tls" = "1" } > +{ "vxhs_tls_x509_cert_dir" = "/etc/pki/libvirt-vxhs" } > { "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" } > { "migrate_tls_x509_verify" = "1" } > { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } >