Re: [PATCH 04/12] qemu-nbd: add --tls-hostname option for TLS certificate validation

[Eric Blake]

On Thu, Mar 03, 2022 at 04:03:22PM +0000, Daniel P. Berrangé wrote:
> When using the --list option, qemu-nbd acts as an NBD client rather
> than a server. As such when using TLS, it has a need to validate
> the server certificate. This adds a --tls-hostname option which can
> be used to override the default hostname used for certificate
> validation.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/tools/qemu-nbd.rst | 14 ++++++++++++++
>  qemu-nbd.c              | 17 ++++++++++++++++-
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/tools/qemu-nbd.rst b/docs/tools/qemu-nbd.rst
> index 6031f96893..acce54a39d 100644
> --- a/docs/tools/qemu-nbd.rst
> +++ b/docs/tools/qemu-nbd.rst
> @@ -169,6 +169,20 @@ driver options if ``--image-opts`` is specified.
>    option; or provide the credentials needed for connecting as a client
>    in list mode.
>  
> +.. option:: --tls-hostname=hostname
> +
> +  When validating an x509 certificate received over a TLS connection,
> +  the hostname that the NBD client used to connect will be checked
> +  against information in the server provided certificate. Sometimes
> +  it might be required to override the hostname used to perform this
> +  check. For example if the NBD client is using a tunnel from localhost
> +  to connect to the remote server. In this case the `--tls-hostname`

For example, if the ... to the remote server, the `--tls-hostname`


> +  option should be used to set the officially expected hostname of
> +  the remote NBD server. This can also be used if accessing NBD over
> +  a UNIX socket where there is no inherant hostname available. This

inherent

> +  only is only permitted when acting as a NBD client with the `--list`

s/only is/is/

> +  option.
> +
>  .. option:: --fork
>  

> @@ -835,6 +841,10 @@ int main(int argc, char **argv)
>              error_report("TLS authorization is incompatible with export 
> list");
>              exit(EXIT_FAILURE);
>          }
> +        if (tlshostname && !list) {
> +            error_report("TLS hostname is only required with export list");

maybe s/required/supported/

> +            exit(EXIT_FAILURE);
> +        }
>          tlscreds = nbd_get_tls_creds(tlscredsid, list, &local_err);
>          if (local_err) {
>              error_reportf_err(local_err, "Failed to get TLS creds: ");
> @@ -845,6 +855,10 @@ int main(int argc, char **argv)
>              error_report("--tls-authz is not permitted without --tls-creds");
>              exit(EXIT_FAILURE);
>          }
> +        if (tlshostname) {
> +            error_report("--tls-hostname is not permitted without 
> --tls-creds");
> +            exit(EXIT_FAILURE);
> +        }
>      }
>  
>      if (selinux_label) {
> @@ -861,7 +875,8 @@ int main(int argc, char **argv)
>  
>      if (list) {
>          saddr = nbd_build_socket_address(sockpath, bindto, port);
> -        return qemu_nbd_client_list(saddr, tlscreds, bindto);
> +        return qemu_nbd_client_list(saddr, tlscreds,
> +                                    tlshostname ? tlshostname : bindto);

With the grammar fixes,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org