| Message ID | 20190626024942.29758-1-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-nbd: Permit TLS with Unix sockets | expand |
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
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.
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 >
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
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.
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);
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(-)