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

On 07/18/2017 11:11 PM, ashish mittal wrote: After doing some more thinking about this, my initial assessment may have missed the mark. Even though you're doing libvirt/qemu/client to vxhs/server processing, you still are processing the QEMU command line arguments, in this case "-object tls-creds-x509" in order to fetch the certificate location (and whatever else that object supports). I could be wrong, but IIUC, QEMU will process those arguments generically and doesn't special case the VxHS environment to "ignore anything". I assume you know the QEMU VxHS command line processing better than I do though! I assume when you pass along the following command line option (from your example) that processing sees: -object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\ endpoint=client,verify-peer=yes \ and QEMU will look in @dir for specific files and will allow the QEMU VxHS code to utilize that certificate directory in order to authenticate to the VxHS server. If verify_peer is 'yes', then that means your properly set up VxHS server will want to do that client handshake (at least that's my understanding). The QEMU code doesn't "present" those certificates, but rather processes them. The presentation is done by the QEMU/VxHS client code using the object that was created (objvxhs_tls0). When the server certificate is presented if it was encrypted, but the QEMU command line processing didn't decrypt it, then the target server won't be able to read it. If it was encrypted, I assume the QEMU command line processing will see that 'passwordid' and get the secret to decrypt the certificate before you have to present it to your VxHS server. In any case, since you have the QEMU/VxHS environment available, it should be very easy for you to make that determination. That is what happens when "verify-peer" is "yes" and what happens when it's "no". Check your @dir - does it have the client*.pem files? If so, then the verification is going on. If not, then either the server isn't doing the verification or I'm missing something. I think the next step would be to test what happens under various scenarios that have come up during the review process: 1. TLS environment that doesn't expect passphrase and verify-peer=no (there's no client*.pem files). I assume you've been using this model for your testing) 2. Same as 1, but alter verify-peer=yes a. With no client*.pem files b. With client*.pem files 3. TLS environment that expects a passphrase a. Without providing an "-object secret" to decode it b. With an "-object secret" to decode it That would mean (at least) 5 separate directories that you can change the @dir argument to test with. There could be more test options, but those 5 came to mind quickly. Again, I suggest looking at Dan Berrange's blog for an example: https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-3-... You should be able to set up a very simple example that uses a clear text secret such as "-object secret,id=sec0,data=letmein" http://wiki.qemu.org/ChangeLog/2.6#Secret_passing_system Well, whomever configured the environment can set it up that way. That's part of the processing done outside of libvirt. Let's not overthink this... Perhaps rather than patch 2 having: + if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir", &cfg->vxhsTLSx509certdir) < 0) + goto cleanup; Let's change that to if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0) goto cleanup; GET_CONFIG_TLS_CERTINFO(vhxs); What does this do? Well if the vxhs* specific parameters aren't set, the macro will use/copy the default* values. So if you modify virQEMUDriverConfig to add vxhsTLSx509secretUUID and vxhsTLSx509verify, then you can just use them. Let's take this a step further, if someone set up a default TLS environment with a password protected certificate, then they'd have to change the default_tls_x509_secret_uuid value to provide the secret UUID for that certificate. That's part of the "processing" required. So now, someone sets up a VxHS environment and wants to use that certificate. So, they define "vxhs_tls = 1" and let the defaults take over. That's it as long as the code is then modified to pass along the vxhsTLSx509verify and vxhsTLSx509SecretUUID when creating the tls-creds-x509 object. NB: If passphrases are used, then the qemuDomainSecretDiskPrepare may need some love to recognize that the VxHS disk would be creating a secret similar to what the similar Chardev routine does. OTOH: if VxHS wants to have it's own environment, someone sets it up, does all the certificate magic, points the vxhs_tls_x509_cert_dir at the environment. If the environment doesn't care about client verification, then verify-peer would be 0. If the environment doesn't care about secrets, then it doesn't uncomment that line (although I think there's a bug in the logic that sets the value to the default if the default exists - I sent a patch today on that). Anyway, if there are VxHS specific values, that doesn't change any other code - those values then get used. After much typing and thinking about how QEMU command line probably processes things, I don't believe so. There's a lot of magic happening with the -object tls-creds-x509 and the 'passwordid' argument to reference an -object secret argument. There's lots of libvirt logic to handle creating the objects properly based on settings. There's quite a bit of code you can just copy or adjust to add VxHS processing, don't make it more complicated than it needs to be. This has nothing to do with secrets and VxHS - I think all this "magic" just happens for you. I could be wrong, but the only way to know is to actually test with the environment and the options. I'm merely pointing out you shouldn't attempt to use a default environment that may be configured in such a way that you don't pass/use the correct arguments to make all the magic happen. In that case, you'd have to direct someone to create their own VxHS TLS environment. The best I can say is to work through this logically and test, test, test. I don't believe you should add any code to check whether the default TLS env is being used. QEMU will let you know when you launch it that it cannot decode the server certificate if that's the problem! I really don't see this being much different than the ChardevTLS which does something very similar with respect to presenting certificates to some backend server. Migration is a bit "different" insomuch as it's two qemu processes (source/client and target/server), but it's still processing all values in the config file regardless of whether it's using the default or migration specific environment. At this point it's a matter of getting the code in place and testing. Again, I assume that when passing along those -object's that QEMU will still process them. You just end up *using* the @id for the tls-creds-x509 in order to present that to your server (my assumption again). It's getting very difficult to follow along - we're now 6 levels deeps w/ replies and a few weeks later than when the patches were first posted. My short term memory has other things clogging it up ;-) John