Re: [Qemu-devel] spec, RFC: TLS support for NBD

["Daniel P. Berrange" <berrange@redhat.com>]

On Mon, Oct 20, 2014 at 01:51:43PM +0200, Markus Armbruster wrote:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
> > On Mon, Oct 20, 2014 at 08:58:14AM +0100, Daniel P. Berrange wrote:
> >> On Sat, Oct 18, 2014 at 07:33:22AM +0100, Richard W.M. Jones wrote:
> >> > On Sat, Oct 18, 2014 at 12:03:23AM +0200, Wouter Verhelst wrote:
> >> > > Hi all,
> >> > > 
> >> > > (added rjones from nbdkit fame -- hi there)
> >> > 
> >> > [I'm happy to implement whatever you come up with, but I've added
> >> > Florian Weimer to CC who is part of Red Hat's product security group]
> >> > 
> >> > > So I think the following would make sense to allow TLS in NBD.
> >> > > 
> >> > > This would extend the newstyle negotiation by adding two options (i.e.,
> >> > > client requests), one server reply, and one server error as well as
> >> > > extend one existing reply, in the following manner:
> >> > > 
> >> > > - The two new commands are NBD_OPT_PEEK_EXPORT and NBD_OPT_STARTTLS. The
> >> > >   former would be used to verify if the server will do TLS for a given
> >> > >   export:
> >> > > 
> >> > >   C: NBD_OPT_PEEK_EXPORT
> >> > >   S: NBD_REP_SERVER, with an extra field after the export name
> >> > >      containing flags that describe the export (R/O vs R/W state,
> >> > >      whether TLS is allowed and/or required).
> >> 
> >> IMHO the server should never provide *any* information about the exported
> >> volume(s) until the TLS layer has been fully setup. ie we shouldn't only
> >> think about the actual block data transfers, we should protect the entire
> >> NBD protocol even metadata related operations.
> >
> > This makes sense.
> 
> Seconded.
> 
> > TLS is about the transport, not about a particular NBD export.  The only
> > thing that should be communicated is STARTTLS.
> 
> Furthermore, STARTTLS is vulnerable to active attacks: if you can get
> between the peers, you can make them fall back to unencrypted silently.
> How do you plan to guard against that?

Well the use of a STARTTLS message at a protocol level isn't vulnerable
per-se, rather it is the handling of it that matters. The key is what
happens if the server wants TLS and the client does not send a STARTTLS
message. If the server happily carries on with plain text that's bad. If
the server closes any connection that attempts to skip STARTTLS, that's
fine. Likewise if the client wants TLS and the server claims to not do
TLS, then the client should close the connection and not carry on. This
avoids the MITM downgrade problem.

So from the POV of QEMU / QEMU-NBD I'd expect us to have a CLI option
tls=on|off  and if the client / server are configured differently then
it would be a hard failure, never any negotiated fallback to plain text
if one requests TLS and the other doesn't.

If QEMU relies on the CLI option, then technically we do not need any
NBD protocol level changes at all. A standard TLS handshake could be
started the moment the TCP connection is established. Only once the
TLS handshake completes would the NBD protocol start running.

The real / main benefit of having a STARTTLS message would be to give
better error reporting for clients not attempting TLS. eg so they could
report a clear "This server requires TLS" error instead of just seeing
unintelligible data from the NBD server and no clue that it is a TLS
handshake.

This is how the VNC integration works at least. The VNC server advertizes
that it requires the TLS auth protocol extension. If the VNC client does
not support this, the server will drop the connection and the VNC client
can at least report to the user that the server requested use of TLS.

The key is that no data or metadata that is in any way related to remote
desktop (or NBD volume) is exchanged between server/client until after
the TLS auth protocol completes.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|