On Fri, Jun 30, 2017 at 1:29 PM, John Ferlan <jferlan@xxxxxxxxxx> wrote: > > > On 06/29/2017 10:02 PM, Ashish Mittal wrote: >> From: Ashish Mittal <ashish.mittal@xxxxxxxxxxx> >> >> 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@xxxxxxxxxxx> >> --- >> 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. > I guess that means I might have to rebase and resolve any potential conflicts in the near future :) It was for the same reason I was hoping that the base VxHS functionality (patch 1) could be merged first. I could then add TLS support in subsequent patches. QEMU vxhs code does support using VxHS devices without TLS. > >> 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 ;-). > Thanks for that insight! I will mull over this for a bit and see how I can rework the code. Will get back on this particular point. > 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" } >> -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
