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

Re: [libvirt] [PATCH v5 4/9] conf: Introduce TLS options for VxHS block device clients




On 08/29/2017 02:39 AM, Ashish Mittal wrote:
> 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 veritas com>
> ---
> v5 changelog:
> (1) Fixed the release version for VxHS changes.
> (2) No functional changes.
> 
> v4 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.
> 
>  docs/formatdomain.html.in          | 16 ++++++++++++++++
>  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 ++
>  6 files changed, 55 insertions(+)
> 

Starting with this patch - I think there's a bit more work to do... We
just need to think logically through things based on the VxHS usage model.

> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 64397d4..41b4ea8 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2649,6 +2649,22 @@
>              transport is "unix", the socket attribute specifies the path to an
>              AF_UNIX socket.
>              </p>
> +            <p>
> +            <span class="since">Since 3.8.0,</span> the optional attribute
> +            <code>tls</code> (QEMU only) can be used to control whether a vxhs

s/(QEMU only) //

s/vxhs/VxHS/

> +            network block device would utilize a hypervisor configured
> +            TLS X.509 certificate environment in order to encrypt the data
> +            channel. For the QEMU hypervisor, usage of a TLS environment can
> +            be controlled on the host by the <code>vxhs_tls</code> and
> +            <code>vxhs_tls_x509_cert_dir</code> or
> +            <code>default_tls_x509_cert_dir</code> settings in the file
> +            /etc/libvirt/qemu.conf. If <code>vxhs_tls</code> is enabled,
> +            then unless the domain <code>tls</code> attribute is set to "no",
> +            libvirt will use the host configured TLS environment.
> +            If <code>vxhs_tls</code> is disabled, but the <code>tls</code>
> +            attribute is set to "yes" in the device domain specification,
> +            then libvirt will throw an error.
> +            </p>

I see this is pretty much a copy of the chardev TLS wording; however,
I'm not quite sure the exact same logic can apply in what you've done in
this patch.

Those last 3 lines may not be necessary - I guess that depends on what
happens with how you handle or describe the "default" environment. Is
there a reason you've added them and the check in the next patch?

BTW: If they stay, it'd be the "domain disk source specification" to be
specific.

>            </dd>
>            <dt><code>snapshot</code></dt>
>            <dd>
> 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 f977e3b..0bbcdb2 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -258,6 +258,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"
> +
> +

I don't believe you can use/rely on default like the other definitions
and that's something you're going to have to "call out" if you want to
claim someone could use the default cert_dir.

If the default environment didn't set _verify, then theoretically it
won't have the client-cert.pem and client-key.pem files. What's not 100%
clear is whether the VxHS server would require them.  What is clear is
that the QEMU block/vxhs.c code would provide them and that from what
I've read iio_open() requires them.

As for the default environment the _uuid value - that doesn't matter
because it's used to decode the server.pem file and vxhs doesn't use
that for the "client" side.

>  # 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
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index ab5f7cc..3d53a86 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -283,6 +283,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);

This sets the default to /etc/pki/qemu if /etc/pki/libvirt-vxhs doesn't
exist. The existence of /etc/pki/qemu is not checked until later...

>  
>  #undef SET_TLS_X509_CERT_DEFAULT
>  
> @@ -380,6 +381,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);
>  
> @@ -556,6 +559,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;
In the time since you've last made changes in this space and now there's
been some other adjustments which you'll need to add vxhs specific code for.

See virQEMUDriverConfigTLSDirResetDefaults and add a
CHECK_RESET_CERT_DIR_DEFAULT(vxhs);

See virQEMUDriverConfigValidate to make the appropriate changes. If
you've decided that you're going to allow usage of the default
environment, then the verify code would have to ensure that if vxhs_tls
were set that default verify was true. The default uuid has no bearing.
If those conditions aren't met, then you'd have to fail the config setup.


John

>  
>  #define GET_CONFIG_TLS_CERTINFO(val)                                        \
>      do {                                                                    \
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index d469b50..13b6f81 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -203,6 +203,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 676d48c..688e5b9 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" }
> 


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