On 08/31/2017 08:21 AM, Peter Krempa wrote: > On Wed, Aug 30, 2017 at 18:46:07 -0400, John Ferlan 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. >> >> The following describes the behavior of TLS for VxHS block device: >> >> (1) Two new options have been added 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 CA certificate and the client certificate and keys are saved. >> If this value is missing, the "default_tls_x509_cert_dir" will be >> used instead. If the environment is not configured properly the >> authentication to the VxHS server will fail. >> >> Signed-off-by: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> >> --- >> >> This is "most of" the v5 patch4 with a few adjustments: >> >> * Alter the qemu.conf description to describe what files are really >> necessary for the environment, especially if someone was going to >> assume the default environment would work. >> >> * Extracted the formatdomain.html.in and moved it to the subsequent >> patch where the domain XML is updated >> >> * Added calls/checks to CHECK_RESET_CERT_DIR_DEFAULT(vxhs); and >> virQEMUDriverConfigValidate. I think we could possibly add more >> to the validate check to ensure the "right" files are there - >> especially the client files, but I wasn't sure if we should or >> not so left it as an exercise for later >> >> src/qemu/libvirtd_qemu.aug | 4 ++++ >> src/qemu/qemu.conf | 33 +++++++++++++++++++++++++++++++++ >> src/qemu/qemu_conf.c | 16 ++++++++++++++++ >> src/qemu/qemu_conf.h | 3 +++ >> src/qemu/test_libvirtd_qemu.aug.in | 2 ++ >> 5 files changed, 58 insertions(+) > > [...] > >> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf >> index f977e3b..f92d5fe 100644 >> --- a/src/qemu/qemu.conf >> +++ b/src/qemu/qemu.conf >> @@ -258,6 +258,39 @@ >> #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 > > I find the semantics of this parameter weird since it does two things at > the same time: > > 1) it says that you CAN use TLS for vhxs > (if it's not set and you set tls='yes' in the XML you get an error) > > 2) It changes the default from tls='no' to tls='yes', if it was not > specified. > > This means that there's no way to 'opt-in' to use TLS. I don't quite see > why the first thing is necessary at all. If you set the cert dir below > and the certificates exist I don't see why users should be forced to > switch the default to be able to use TLS. > I figured once we got here there'd be things to work through. I agree with you about the oddity of having vxhs_tls = 0 and tls='yes'... Something I noted more concisely in patch12, but "left" in the code to ensure I wasn't the "only one". Following the lead of qemuDomainPrepareChardevSourceTLS (without the tlsFromConfig) was more in line with what I was thinking. If someone sets up the directory properly and sets "tls = 'yes'" in the domain XML, but doesn't set the vxhs_tls = 1, then it should be possible to use TLS. Setting "vxhs_tls = 1" would then mean that every VxHS disk would use TLS unless the property "tls = 'no'" was set (and that works as seen in patch13). Although, I think what the approach Ashish used is to "guard against" the possibility that someone doesn't set up the directory or the vxhs_tls qemu.conf variable, but sets the domain XML property to "tls = 'yes'" and expects things to work. In this case, there could be a failure when the QEMU code goes to find the files. QEMU will indicate something's missing unless of course we get lucky and the default environment has the client files. IIRC from a previous run at this w/ migration - the message for the lack of client certificate/key is rather opaque - as in, it's not intuitively obvious that the mistake was that you don't have the client files, but that could have changed since I last looked too. This is always one of those conundrums - how much checking should libvirt make to prevent what is sure to be a failure from QEMU. In this case, I think we let the failure happen in QEMU. If someone is going to change the domain XML attribute, but not set up the environment properly, then (to a degree) they deserve what they get for not following instructions (and is why I add the details for the cert_dir as seen below). John >> +# 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 block device clients expect the client certificate and key to be >> +# present in the certificate directory along with the CA master certificate. >> +# If using the default environment, default_tls_x509_verify must be configured. >> +# The server key as well as secret UUID that would decrypt it is not used. >> +# Thus a VxHS directory must contain the following: >> +# >> +# ca-cert.pem - the CA master certificate >> +# client-cert.pem - the client certificate signed with the ca-cert.pem >> +# client-key.pem - the client private key >> +# >> +#vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs" > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
