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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux