[Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Eric Blake]

Although you generally won't use encryption with a Unix socket (after
all, everything is local, so why waste the CPU power), there are
situations in testsuites where Unix sockets are much nicer than TCP
sockets.  Since nbdkit allows encryption over both types of sockets,
it makes sense for qemu-nbd to do likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-nbd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index a8cb39e51043..ddfb6815fb69 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -931,10 +931,6 @@ int main(int argc, char **argv)
     }

     if (tlscredsid) {
-        if (sockpath) {
-            error_report("TLS is only supported with IPv4/IPv6");
-            exit(EXIT_FAILURE);
-        }
         if (device) {
             error_report("TLS is not supported with a host device");
             exit(EXIT_FAILURE);
-- 
2.20.1

Re: [Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Daniel P. Berrangé]

On Tue, Jun 25, 2019 at 09:49:42PM -0500, Eric Blake wrote:
> Although you generally won't use encryption with a Unix socket (after
> all, everything is local, so why waste the CPU power), there are
> situations in testsuites where Unix sockets are much nicer than TCP
> sockets.  Since nbdkit allows encryption over both types of sockets,
> it makes sense for qemu-nbd to do likewise.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qemu-nbd.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Do you need something on the client side too ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 1667 bytes --]

On 6/26/19 3:22 AM, Daniel P. Berrangé wrote:
> On Tue, Jun 25, 2019 at 09:49:42PM -0500, Eric Blake wrote:
>> Although you generally won't use encryption with a Unix socket (after
>> all, everything is local, so why waste the CPU power), there are
>> situations in testsuites where Unix sockets are much nicer than TCP
>> sockets.  Since nbdkit allows encryption over both types of sockets,
>> it makes sense for qemu-nbd to do likewise.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  qemu-nbd.c | 4 ----
>>  1 file changed, 4 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> Do you need something on the client side too ?

The proposal that Rich is working on for standardized NBD URIs [1] says
that we need a patch to support nbds://host/export and
nbds+unix://export?socket=/path as ways to request an encrypted client
connection with default encryption parameters. For anything more
complex, we have to use --imageopts and request an encrypted connection
by parts - but the QAPI schema already permits us to pass in an
'tls-creds' parameter for both TCP and Unix sockets, so no, I don't
think we need any client side changes at this point.

I do, however, plan to test that 'qemu-nbd --list -k socket --tls...'
works (I think it does, and it can be used even without this patch
against nbdkit as server...), prior to taking this patch through my NBD
tree.

[1] https://lists.debian.org/nbd/2019/06/msg00011.html

> 
> 
> Regards,
> Daniel
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Daniel P. Berrangé]

On Thu, Jun 27, 2019 at 09:49:13AM -0500, Eric Blake wrote:
> On 6/26/19 3:22 AM, Daniel P. Berrangé wrote:
> > On Tue, Jun 25, 2019 at 09:49:42PM -0500, Eric Blake wrote:
> >> Although you generally won't use encryption with a Unix socket (after
> >> all, everything is local, so why waste the CPU power), there are
> >> situations in testsuites where Unix sockets are much nicer than TCP
> >> sockets.  Since nbdkit allows encryption over both types of sockets,
> >> it makes sense for qemu-nbd to do likewise.
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >> ---
> >>  qemu-nbd.c | 4 ----
> >>  1 file changed, 4 deletions(-)
> > 
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > 
> > Do you need something on the client side too ?
> 
> The proposal that Rich is working on for standardized NBD URIs [1] says
> that we need a patch to support nbds://host/export and
> nbds+unix://export?socket=/path as ways to request an encrypted client
> connection with default encryption parameters. For anything more
> complex, we have to use --imageopts and request an encrypted connection
> by parts - but the QAPI schema already permits us to pass in an
> 'tls-creds' parameter for both TCP and Unix sockets, so no, I don't
> think we need any client side changes at this point.

The QAPI schema isn't what I was thinking about....  in block/nbd.c
we have the same restriction you lifted here

        tlscreds = nbd_get_tls_creds(s->tlscredsid, errp);
        if (!tlscreds) {
            goto error;
        }

        /* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
        if (s->saddr->type != SOCKET_ADDRESS_TYPE_INET) {
            error_setg(errp, "TLS only supported over IP sockets");
            goto error;
        }

For client side we would also need to allow a 'tls-hostname' parameter
in BlockdevOptionsNbd, so that the client can pass a hostname to use
for validating the x509 certificate, the same way we allow for live
migration.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2336 bytes --]

On 6/27/19 9:58 AM, Daniel P. Berrangé wrote:

>>>
>>> Do you need something on the client side too ?
>>
>> The proposal that Rich is working on for standardized NBD URIs [1] says
>> that we need a patch to support nbds://host/export and
>> nbds+unix://export?socket=/path as ways to request an encrypted client
>> connection with default encryption parameters. For anything more
>> complex, we have to use --imageopts and request an encrypted connection
>> by parts - but the QAPI schema already permits us to pass in an
>> 'tls-creds' parameter for both TCP and Unix sockets, so no, I don't
>> think we need any client side changes at this point.

Okay, I just tested that pre-patch, qemu-nbd --list refuses to connect,
but post-patch it works:

$ ./qemu-nbd -r -k /tmp/nbdsock --object \
  tls-creds-psk,id=tls0,endpoint=server,dir=/home/eblake/libnbd/tests \
  --tls-creds tls0 -f raw -x / ./file
$ qemu-nbd --list -k /tmp/nbdsock --object \

tls-creds-psk,id=tls0,endpoint=client,dir=/home/eblake/libnbd/tests,username=eblake
\
  --tls-creds tls0
qemu-nbd: TLS is only supported with IPv4/IPv6
$ ./qemu-nbd --list -k /tmp/nbdsock --object \

tls-creds-psk,id=tls0,endpoint=client,dir=/home/eblake/libnbd/tests,username=eblake
\
  --tls-creds tls0
exports available: 1
...

> 
> The QAPI schema isn't what I was thinking about....  in block/nbd.c
> we have the same restriction you lifted here
> 
>         tlscreds = nbd_get_tls_creds(s->tlscredsid, errp);
>         if (!tlscreds) {
>             goto error;
>         }
> 
>         /* TODO SOCKET_ADDRESS_KIND_FD where fd has AF_INET or AF_INET6 */
>         if (s->saddr->type != SOCKET_ADDRESS_TYPE_INET) {
>             error_setg(errp, "TLS only supported over IP sockets");
>             goto error;
>         }

Oh. Yeah, I'll have to fix that; it's different than qemu-nbd --list.

> 
> For client side we would also need to allow a 'tls-hostname' parameter
> in BlockdevOptionsNbd, so that the client can pass a hostname to use
> for validating the x509 certificate, the same way we allow for live
> migration.

Okay, v2 coming up later, once I've done more integration testing.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH] qemu-nbd: Permit TLS with Unix sockets

[Richard W.M. Jones]

On Tue, Jun 25, 2019 at 09:49:42PM -0500, Eric Blake wrote:
> Although you generally won't use encryption with a Unix socket (after
> all, everything is local, so why waste the CPU power), there are
> situations in testsuites where Unix sockets are much nicer than TCP
> sockets.  Since nbdkit allows encryption over both types of sockets,
> it makes sense for qemu-nbd to do likewise.

Also it's somewhat useful if using a separate tunnel process (openssh
for one can do this now).

> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  qemu-nbd.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index a8cb39e51043..ddfb6815fb69 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -931,10 +931,6 @@ int main(int argc, char **argv)
>      }
> 
>      if (tlscredsid) {
> -        if (sockpath) {
> -            error_report("TLS is only supported with IPv4/IPv6");
> -            exit(EXIT_FAILURE);
> -        }
>          if (device) {
>              error_report("TLS is not supported with a host device");
>              exit(EXIT_FAILURE);
> -- 
> 2.20.1

The patch looks very simple, just removing an unnecessary restriction,
so:

Acked-by: Richard W.M. Jones <rjones@redhat.com>

If we could have the same change on the qemu client side that would
be great because we could use it here:

https://github.com/libguestfs/nbdkit/blob/e0d324683c86455a2fe62e97d57f1313cad9c9f3/tests/functions.sh.in#L133

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org