All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] NBD TLS support in QEMU
@ 2014-09-03 16:44 Stefan Hajnoczi
  2014-09-04 14:19 ` Benoît Canet
                   ` (3 more replies)
  0 siblings, 4 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-09-03 16:44 UTC (permalink / raw)
  To: qemu-devel
  Cc: libvir-list, nick, Max Reitz, Hani Benhabiles, Paolo Bonzini, w

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Hi,
QEMU offers both NBD client and server functionality.  The NBD protocol
runs unencrypted, which is a problem when the client and server
communicate over an untrusted network.

The particular use case that prompted this mail is storage migration in
OpenStack.  The goal is to encrypt the NBD connection between source and
destination hosts during storage migration.

I think we can integrate TLS into the NBD protocol as an optional flag.
A quick web search does not reveal existing open source SSL/TLS NBD
implementations.  I do see a VMware NBDSSL protocol but there is no
specification so I guess it is proprietary.

The NBD protocol starts with a negotiation phase.  This would be the
appropriate place to indicate that TLS will be used.  After client and
server complete TLS setup the connection can continue as normal.

Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
extended to support TLS.  In this case the kernel needs a localhost
socket and userspace handles TLS.

Thoughts?

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-03 16:44 [Qemu-devel] NBD TLS support in QEMU Stefan Hajnoczi
@ 2014-09-04 14:19 ` Benoît Canet
  2014-09-04 14:34   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
  2014-09-04 22:07   ` [Qemu-devel] " Wouter Verhelst
  2014-09-04 22:02 ` Wouter Verhelst
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 44+ messages in thread
From: Benoît Canet @ 2014-09-04 14:19 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: libvir-list, qemu-devel, Max Reitz, Hani Benhabiles, nick, w,
	Paolo Bonzini

The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> Hi,
> QEMU offers both NBD client and server functionality.  The NBD protocol
> runs unencrypted, which is a problem when the client and server
> communicate over an untrusted network.
> 
> The particular use case that prompted this mail is storage migration in
> OpenStack.  The goal is to encrypt the NBD connection between source and
> destination hosts during storage migration.

I agree this would be usefull.

> 
> I think we can integrate TLS into the NBD protocol as an optional flag.
> A quick web search does not reveal existing open source SSL/TLS NBD
> implementations.  I do see a VMware NBDSSL protocol but there is no
> specification so I guess it is proprietary.
> 
> The NBD protocol starts with a negotiation phase.  This would be the
> appropriate place to indicate that TLS will be used.  After client and
> server complete TLS setup the connection can continue as normal.

Prenegociating TLS look like we will accidentaly introduce some security hole.
Why not just using a dedicated port and let the TLS handshake happen normaly ?

Best regards

Benoît
> 
> Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> extended to support TLS.  In this case the kernel needs a localhost
> socket and userspace handles TLS.
> 
> Thoughts?
> 
> Stefan

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt]  NBD TLS support in QEMU
  2014-09-04 14:19 ` Benoît Canet
@ 2014-09-04 14:34   ` Daniel P. Berrange
  2014-09-04 15:04     ` Benoît Canet
  2014-09-04 15:54     ` John Snow
  2014-09-04 22:07   ` [Qemu-devel] " Wouter Verhelst
  1 sibling, 2 replies; 44+ messages in thread
From: Daniel P. Berrange @ 2014-09-04 14:34 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Stefan Hajnoczi, libvir-list, qemu-devel, Max Reitz,
	Paolo Bonzini, Hani Benhabiles, nick, w

On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > Hi,
> > QEMU offers both NBD client and server functionality.  The NBD protocol
> > runs unencrypted, which is a problem when the client and server
> > communicate over an untrusted network.
> > 
> > The particular use case that prompted this mail is storage migration in
> > OpenStack.  The goal is to encrypt the NBD connection between source and
> > destination hosts during storage migration.
> 
> I agree this would be usefull.
> 
> > 
> > I think we can integrate TLS into the NBD protocol as an optional flag.
> > A quick web search does not reveal existing open source SSL/TLS NBD
> > implementations.  I do see a VMware NBDSSL protocol but there is no
> > specification so I guess it is proprietary.
> > 
> > The NBD protocol starts with a negotiation phase.  This would be the
> > appropriate place to indicate that TLS will be used.  After client and
> > server complete TLS setup the connection can continue as normal.
> 
> Prenegociating TLS look like we will accidentaly introduce some security hole.
> Why not just using a dedicated port and let the TLS handshake happen normaly ?

The mgmt app (libvirt in this case) chooses an arbitrary port when
telling QEMU to setup NBD, so we don't need to specify any alternate
port. I'd expect that libvirt just tell QEMU to enable NBD at both
ends, and we immediately do the TLS handshake upon opening the
connection.  Only once TLS is established, should the NBD protocol
start running. IOW we don't need to modify the NBD protocol at all.

If the mgmt app tells QEMU to enable TLS at one end and not the
other, the mgmt app gets what it deserves (a failed TLS handshake).
We certainly would not want QEMU to auto-negotiate and fallback
to plain text in this case.

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt]  NBD TLS support in QEMU
  2014-09-04 14:34   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
@ 2014-09-04 15:04     ` Benoît Canet
  2014-09-04 15:45       ` Stefan Hajnoczi
  2014-09-04 15:54     ` John Snow
  1 sibling, 1 reply; 44+ messages in thread
From: Benoît Canet @ 2014-09-04 15:04 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Benoît Canet, Hani Benhabiles, libvir-list, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick, w, Paolo Bonzini

The Thursday 04 Sep 2014 à 15:34:59 (+0100), Daniel P. Berrange wrote :
> On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> > The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > > Hi,
> > > QEMU offers both NBD client and server functionality.  The NBD protocol
> > > runs unencrypted, which is a problem when the client and server
> > > communicate over an untrusted network.
> > > 
> > > The particular use case that prompted this mail is storage migration in
> > > OpenStack.  The goal is to encrypt the NBD connection between source and
> > > destination hosts during storage migration.
> > 
> > I agree this would be usefull.
> > 
> > > 
> > > I think we can integrate TLS into the NBD protocol as an optional flag.
> > > A quick web search does not reveal existing open source SSL/TLS NBD
> > > implementations.  I do see a VMware NBDSSL protocol but there is no
> > > specification so I guess it is proprietary.
> > > 
> > > The NBD protocol starts with a negotiation phase.  This would be the
> > > appropriate place to indicate that TLS will be used.  After client and
> > > server complete TLS setup the connection can continue as normal.
> > 
> > Prenegociating TLS look like we will accidentaly introduce some security hole.
> > Why not just using a dedicated port and let the TLS handshake happen normaly ?
> 
> The mgmt app (libvirt in this case) chooses an arbitrary port when
> telling QEMU to setup NBD, so we don't need to specify any alternate
> port. I'd expect that libvirt just tell QEMU to enable NBD at both
> ends, and we immediately do the TLS handshake upon opening the
> connection.  Only once TLS is established, should the NBD protocol
> start running. IOW we don't need to modify the NBD protocol at all.
> 
> If the mgmt app tells QEMU to enable TLS at one end and not the
> other, the mgmt app gets what it deserves (a failed TLS handshake).
> We certainly would not want QEMU to auto-negotiate and fallback
> to plain text in this case.

I agree.

Best regards

Benoît

> 
> 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 :|
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt]    NBD TLS support in QEMU
  2014-09-04 15:04     ` Benoît Canet
@ 2014-09-04 15:45       ` Stefan Hajnoczi
  0 siblings, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-09-04 15:45 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Hani Benhabiles, libvir-list, qemu-devel, Max Reitz,
	Paolo Bonzini, Stefan Hajnoczi, nick, w

[-- Attachment #1: Type: text/plain, Size: 2226 bytes --]

On Thu, Sep 04, 2014 at 05:04:06PM +0200, Benoît Canet wrote:
> The Thursday 04 Sep 2014 à 15:34:59 (+0100), Daniel P. Berrange wrote :
> > On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> > > The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > > > Hi,
> > > > QEMU offers both NBD client and server functionality.  The NBD protocol
> > > > runs unencrypted, which is a problem when the client and server
> > > > communicate over an untrusted network.
> > > > 
> > > > The particular use case that prompted this mail is storage migration in
> > > > OpenStack.  The goal is to encrypt the NBD connection between source and
> > > > destination hosts during storage migration.
> > > 
> > > I agree this would be usefull.
> > > 
> > > > 
> > > > I think we can integrate TLS into the NBD protocol as an optional flag.
> > > > A quick web search does not reveal existing open source SSL/TLS NBD
> > > > implementations.  I do see a VMware NBDSSL protocol but there is no
> > > > specification so I guess it is proprietary.
> > > > 
> > > > The NBD protocol starts with a negotiation phase.  This would be the
> > > > appropriate place to indicate that TLS will be used.  After client and
> > > > server complete TLS setup the connection can continue as normal.
> > > 
> > > Prenegociating TLS look like we will accidentaly introduce some security hole.
> > > Why not just using a dedicated port and let the TLS handshake happen normaly ?
> > 
> > The mgmt app (libvirt in this case) chooses an arbitrary port when
> > telling QEMU to setup NBD, so we don't need to specify any alternate
> > port. I'd expect that libvirt just tell QEMU to enable NBD at both
> > ends, and we immediately do the TLS handshake upon opening the
> > connection.  Only once TLS is established, should the NBD protocol
> > start running. IOW we don't need to modify the NBD protocol at all.
> > 
> > If the mgmt app tells QEMU to enable TLS at one end and not the
> > other, the mgmt app gets what it deserves (a failed TLS handshake).
> > We certainly would not want QEMU to auto-negotiate and fallback
> > to plain text in this case.
> 
> I agree.

Sounds good.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt]  NBD TLS support in QEMU
  2014-09-04 14:34   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
  2014-09-04 15:04     ` Benoît Canet
@ 2014-09-04 15:54     ` John Snow
  1 sibling, 0 replies; 44+ messages in thread
From: John Snow @ 2014-09-04 15:54 UTC (permalink / raw)
  To: Daniel P. Berrange, Benoît Canet
  Cc: Stefan Hajnoczi, libvir-list, qemu-devel, Max Reitz,
	Hani Benhabiles, nick, w, Paolo Bonzini



On 09/04/2014 10:34 AM, Daniel P. Berrange wrote:
> On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
>> The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
>>> Hi,
>>> QEMU offers both NBD client and server functionality.  The NBD protocol
>>> runs unencrypted, which is a problem when the client and server
>>> communicate over an untrusted network.
>>>
>>> The particular use case that prompted this mail is storage migration in
>>> OpenStack.  The goal is to encrypt the NBD connection between source and
>>> destination hosts during storage migration.
>>
>> I agree this would be usefull.
>>
>>>
>>> I think we can integrate TLS into the NBD protocol as an optional flag.
>>> A quick web search does not reveal existing open source SSL/TLS NBD
>>> implementations.  I do see a VMware NBDSSL protocol but there is no
>>> specification so I guess it is proprietary.
>>>
>>> The NBD protocol starts with a negotiation phase.  This would be the
>>> appropriate place to indicate that TLS will be used.  After client and
>>> server complete TLS setup the connection can continue as normal.
>>
>> Prenegociating TLS look like we will accidentaly introduce some security hole.
>> Why not just using a dedicated port and let the TLS handshake happen normaly ?
>
> The mgmt app (libvirt in this case) chooses an arbitrary port when
> telling QEMU to setup NBD, so we don't need to specify any alternate
> port. I'd expect that libvirt just tell QEMU to enable NBD at both
> ends, and we immediately do the TLS handshake upon opening the
> connection.  Only once TLS is established, should the NBD protocol
> start running. IOW we don't need to modify the NBD protocol at all.

This is my understanding of how, for example, the IRC protocol added SSL 
support. the SSL/TLS handshake happens first, but the very next thing 
the client/server expects to see is the usual IRC protocol talk, encrypted.

If it sees incorrect magic after the SSL shake, both ends hang up.

If it sees IRC magic prior to the SSL shake, it either allows an 
unencrypted session, or if the user or server has requested SSL-only, 
one or both ends hang up.

>
> If the mgmt app tells QEMU to enable TLS at one end and not the
> other, the mgmt app gets what it deserves (a failed TLS handshake).
> We certainly would not want QEMU to auto-negotiate and fallback
> to plain text in this case.
>
> Regards,
> Daniel
>

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-03 16:44 [Qemu-devel] NBD TLS support in QEMU Stefan Hajnoczi
  2014-09-04 14:19 ` Benoît Canet
@ 2014-09-04 22:02 ` Wouter Verhelst
  2014-09-05  8:13   ` Daniel P. Berrange
  2014-09-05 12:21   ` Stefan Hajnoczi
  2014-09-05  6:23 ` [Qemu-devel] [libvirt] " Michal Privoznik
  2014-09-05  8:46 ` [Qemu-devel] " Hani Benhabiles
  3 siblings, 2 replies; 44+ messages in thread
From: Wouter Verhelst @ 2014-09-04 22:02 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: libvir-list, nbd-general, qemu-devel, Max Reitz, Hani Benhabiles,
	nick, Paolo Bonzini

[Cc: to nbd-general list added]

On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> Hi,
> QEMU offers both NBD client and server functionality.  The NBD protocol
> runs unencrypted, which is a problem when the client and server
> communicate over an untrusted network.
> 
> The particular use case that prompted this mail is storage migration in
> OpenStack.  The goal is to encrypt the NBD connection between source and
> destination hosts during storage migration.

I've never given encrypted NBD high priority, since I don't think
encryption without authentication serves much purpose -- and I haven't
gotten around to adding authentication yet (for which I have plans; but
other things have priority).

> I think we can integrate TLS into the NBD protocol as an optional flag.
> A quick web search does not reveal existing open source SSL/TLS NBD
> implementations.  I do see a VMware NBDSSL protocol but there is no
> specification so I guess it is proprietary.
> 
> The NBD protocol starts with a negotiation phase.  This would be the
> appropriate place to indicate that TLS will be used.  After client and
> server complete TLS setup the connection can continue as normal.
> 
> Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> extended to support TLS.  In this case the kernel needs a localhost
> socket and userspace handles TLS.

That introduces a possibility for a deadlock, since now your network
socket isn't on the PF_MEMALLOC-protected socket anymore, which will
cause the kernel to throw away packets which are needed for your nbd
connection, in hopes of clearing some memory.

I suppose you could theoretically do the encryption in kernel space.
Not convinced that trying TLS in kernel space is a good idea, though.

I have heard of people using stunnel or the likes to pipe the NBD
protocol over a secure channel, with various levels of success.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 14:19 ` Benoît Canet
  2014-09-04 14:34   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
@ 2014-09-04 22:07   ` Wouter Verhelst
  2014-09-04 22:54     ` Benoît Canet
  1 sibling, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-09-04 22:07 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Stefan Hajnoczi, libvir-list, nbd-generic, qemu-devel, Max Reitz,
	Hani Benhabiles, nick, Paolo Bonzini

On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > Hi,
> > QEMU offers both NBD client and server functionality.  The NBD protocol
> > runs unencrypted, which is a problem when the client and server
> > communicate over an untrusted network.
> > 
> > The particular use case that prompted this mail is storage migration in
> > OpenStack.  The goal is to encrypt the NBD connection between source and
> > destination hosts during storage migration.
> 
> I agree this would be usefull.
> 
> > 
> > I think we can integrate TLS into the NBD protocol as an optional flag.
> > A quick web search does not reveal existing open source SSL/TLS NBD
> > implementations.  I do see a VMware NBDSSL protocol but there is no
> > specification so I guess it is proprietary.
> > 
> > The NBD protocol starts with a negotiation phase.  This would be the
> > appropriate place to indicate that TLS will be used.  After client and
> > server complete TLS setup the connection can continue as normal.
> 
> Prenegociating TLS look like we will accidentaly introduce some security hole.

Can you elaborate on that? How would it be a security hole?

> Why not just using a dedicated port and let the TLS handshake happen normaly ?

Because STARTTLS(-like) protocols are much cleaner; no need to open two
firewall ports. Also, when I made the request for a port number at IANA,
I was told I wouldn't get another port for a "secure" variant -- which
makes sense. As such, if the reference implementation is ever going to
support TLS, it has to be in a way where it is negotiated at setup time.

SMTP can do this safely. So can LDAP. I'm sure we can come up with a
safe way of negotiating TLS.

If you want to disallow nonencrypted communication, I'm sure it can be
made possible to require TLS for (some of) your exports.

(my objections on userspace/kernelspace issues still stand, however)

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 22:07   ` [Qemu-devel] " Wouter Verhelst
@ 2014-09-04 22:54     ` Benoît Canet
  2014-09-05  8:42       ` Wouter Verhelst
  2014-09-05 12:15       ` Stefan Hajnoczi
  0 siblings, 2 replies; 44+ messages in thread
From: Benoît Canet @ 2014-09-04 22:54 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Benoît Canet, Hani Benhabiles, libvir-list, nick,
	qemu-devel, Max Reitz, Stefan Hajnoczi, nbd-generic,
	Paolo Bonzini

The Friday 05 Sep 2014 à 00:07:04 (+0200), Wouter Verhelst wrote :
> On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> > The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > > Hi,
> > > QEMU offers both NBD client and server functionality.  The NBD protocol
> > > runs unencrypted, which is a problem when the client and server
> > > communicate over an untrusted network.
> > > 
> > > The particular use case that prompted this mail is storage migration in
> > > OpenStack.  The goal is to encrypt the NBD connection between source and
> > > destination hosts during storage migration.
> > 
> > I agree this would be usefull.
> > 
> > > 
> > > I think we can integrate TLS into the NBD protocol as an optional flag.
> > > A quick web search does not reveal existing open source SSL/TLS NBD
> > > implementations.  I do see a VMware NBDSSL protocol but there is no
> > > specification so I guess it is proprietary.
> > > 
> > > The NBD protocol starts with a negotiation phase.  This would be the
> > > appropriate place to indicate that TLS will be used.  After client and
> > > server complete TLS setup the connection can continue as normal.
> > 
> > Prenegociating TLS look like we will accidentaly introduce some security hole.

I was thinking of the fallback to cleartext case.

As a regular developper I am afraid of doing something creative with
cryptography.

> 
> Can you elaborate on that? How would it be a security hole?
> 
> > Why not just using a dedicated port and let the TLS handshake happen normaly ?
> 
> Because STARTTLS(-like) protocols are much cleaner; no need to open two
> firewall ports. Also, when I made the request for a port number at IANA,
> I was told I wouldn't get another port for a "secure" variant -- which
> makes sense. As such, if the reference implementation is ever going to
> support TLS, it has to be in a way where it is negotiated at setup time.
> 
> SMTP can do this safely. So can LDAP. I'm sure we can come up with a
> safe way of negotiating TLS.
> 
> If you want to disallow nonencrypted communication, I'm sure it can be
> made possible to require TLS for (some of) your exports.
> 
> (my objections on userspace/kernelspace issues still stand, however)
> 
> -- 
> It is easy to love a country that is famous for chocolate and beer
> 
>   -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26
> 

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt] NBD TLS support in QEMU
  2014-09-03 16:44 [Qemu-devel] NBD TLS support in QEMU Stefan Hajnoczi
  2014-09-04 14:19 ` Benoît Canet
  2014-09-04 22:02 ` Wouter Verhelst
@ 2014-09-05  6:23 ` Michal Privoznik
  2014-09-05  8:10   ` Daniel P. Berrange
  2014-09-05  8:46 ` [Qemu-devel] " Hani Benhabiles
  3 siblings, 1 reply; 44+ messages in thread
From: Michal Privoznik @ 2014-09-05  6:23 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: libvir-list, nick, Max Reitz, Hani Benhabiles, w, Paolo Bonzini

On 03.09.2014 18:44, Stefan Hajnoczi wrote:
> Hi,
> QEMU offers both NBD client and server functionality.  The NBD protocol
> runs unencrypted, which is a problem when the client and server
> communicate over an untrusted network.

This is not problem for NBD only, but for the rest of data that qemu 
sends over  network, i.e. migration stream, VNC/SPICE, ...

>
> The particular use case that prompted this mail is storage migration in
> OpenStack.  The goal is to encrypt the NBD connection between source and
> destination hosts during storage migration.
>
> I think we can integrate TLS into the NBD protocol as an optional flag.
> A quick web search does not reveal existing open source SSL/TLS NBD
> implementations.  I do see a VMware NBDSSL protocol but there is no
> specification so I guess it is proprietary.

In case of libvirt, we have so called tunnelled migration (both spelled 
& misspelled :P) in which libvirt opens a local ports on both src & dst 
side and then sets up secured forwarding pipe to the other side. Or a 
insecured one if user wishes so. The only problem is that when I adapted 
libvirt for NBD, I intentionally forbade NBD in tunnelled migration as 
it requires one more data stream for which libvirt migration protocol is 
not ready yet. Having saidy that, I feel that libvirt is the show 
stopper here, not QEMU.

I'm not saying that I'm against this. I've heard rumors that not 
everybody out there uses libvirt and thus they might appreciate this 
ability.

>
> The NBD protocol starts with a negotiation phase.  This would be the
> appropriate place to indicate that TLS will be used.  After client and
> server complete TLS setup the connection can continue as normal.

Yep, that's how most of the secured protocols run. Somebody mentions 
STARTTLS for which I vote as well.

>
> Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> extended to support TLS.  In this case the kernel needs a localhost
> socket and userspace handles TLS.

Michal

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [libvirt] NBD TLS support in QEMU
  2014-09-05  6:23 ` [Qemu-devel] [libvirt] " Michal Privoznik
@ 2014-09-05  8:10   ` Daniel P. Berrange
  0 siblings, 0 replies; 44+ messages in thread
From: Daniel P. Berrange @ 2014-09-05  8:10 UTC (permalink / raw)
  To: Michal Privoznik
  Cc: Stefan Hajnoczi, libvir-list, qemu-devel, Max Reitz,
	Paolo Bonzini, Hani Benhabiles, nick, w

On Fri, Sep 05, 2014 at 08:23:17AM +0200, Michal Privoznik wrote:
> On 03.09.2014 18:44, Stefan Hajnoczi wrote:
> >Hi,
> >QEMU offers both NBD client and server functionality.  The NBD protocol
> >runs unencrypted, which is a problem when the client and server
> >communicate over an untrusted network.
> 
> This is not problem for NBD only, but for the rest of data that qemu sends
> over  network, i.e. migration stream, VNC/SPICE, ...

We already have TLS support for VNC and SPICE.  We do need it for NBD,
migration and the chardev TCP backend.

I'd suggest that it is likely possible to add support for NBD, migration
and chardev all at the same time by doing a general purpose TLS socket
wrapper in QEMU that all those areas can use.

> >The particular use case that prompted this mail is storage migration in
> >OpenStack.  The goal is to encrypt the NBD connection between source and
> >destination hosts during storage migration.
> >
> >I think we can integrate TLS into the NBD protocol as an optional flag.
> >A quick web search does not reveal existing open source SSL/TLS NBD
> >implementations.  I do see a VMware NBDSSL protocol but there is no
> >specification so I guess it is proprietary.
> 
> In case of libvirt, we have so called tunnelled migration (both spelled &
> misspelled :P) in which libvirt opens a local ports on both src & dst side
> and then sets up secured forwarding pipe to the other side. Or a insecured
> one if user wishes so. The only problem is that when I adapted libvirt for
> NBD, I intentionally forbade NBD in tunnelled migration as it requires one
> more data stream for which libvirt migration protocol is not ready yet.
> Having saidy that, I feel that libvirt is the show stopper here, not QEMU.

While tunnelled migration via libvirt is doable, it is very much
sub-optimal as it involves a great many data copies which is bad
for performance of migration. This is the main reason that having
direct TLS support in the QEMU migration and NBD data channels is
desired.

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 22:02 ` Wouter Verhelst
@ 2014-09-05  8:13   ` Daniel P. Berrange
  2014-09-05  8:34     ` Wouter Verhelst
  2014-09-05 12:21   ` Stefan Hajnoczi
  1 sibling, 1 reply; 44+ messages in thread
From: Daniel P. Berrange @ 2014-09-05  8:13 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Stefan Hajnoczi, libvir-list, nbd-general, qemu-devel, Max Reitz,
	Hani Benhabiles, nick, Paolo Bonzini

On Fri, Sep 05, 2014 at 12:02:18AM +0200, Wouter Verhelst wrote:
> [Cc: to nbd-general list added]
> 
> On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> > Hi,
> > QEMU offers both NBD client and server functionality.  The NBD protocol
> > runs unencrypted, which is a problem when the client and server
> > communicate over an untrusted network.
> > 
> > The particular use case that prompted this mail is storage migration in
> > OpenStack.  The goal is to encrypt the NBD connection between source and
> > destination hosts during storage migration.
> 
> I've never given encrypted NBD high priority, since I don't think
> encryption without authentication serves much purpose -- and I haven't
> gotten around to adding authentication yet (for which I have plans; but
> other things have priority).

While have an authentication layer like SASL wired into the NBD protocol
would be nice, it shouldn't be considered a blocker / pre-requisite. It
is pretty straightforward for the server to require x509 certificates
from the client and validate those as a means of authentication. We've
used that as an authentication mechanism in libvirt and VNC with success,
though we did later add SASL integration as an option too.

> > I think we can integrate TLS into the NBD protocol as an optional flag.
> > A quick web search does not reveal existing open source SSL/TLS NBD
> > implementations.  I do see a VMware NBDSSL protocol but there is no
> > specification so I guess it is proprietary.
> > 
> > The NBD protocol starts with a negotiation phase.  This would be the
> > appropriate place to indicate that TLS will be used.  After client and
> > server complete TLS setup the connection can continue as normal.
> > 
> > Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> > extended to support TLS.  In this case the kernel needs a localhost
> > socket and userspace handles TLS.
> 
> That introduces a possibility for a deadlock, since now your network
> socket isn't on the PF_MEMALLOC-protected socket anymore, which will
> cause the kernel to throw away packets which are needed for your nbd
> connection, in hopes of clearing some memory.
> 
> I suppose you could theoretically do the encryption in kernel space.
> Not convinced that trying TLS in kernel space is a good idea, though.
> 
> I have heard of people using stunnel or the likes to pipe the NBD
> protocol over a secure channel, with various levels of success.

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-05  8:13   ` Daniel P. Berrange
@ 2014-09-05  8:34     ` Wouter Verhelst
  0 siblings, 0 replies; 44+ messages in thread
From: Wouter Verhelst @ 2014-09-05  8:34 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Stefan Hajnoczi, libvir-list, nbd-general, nick, qemu-devel,
	Max Reitz, Hani Benhabiles, Paolo Bonzini, Wouter Verhelst

On Fri, Sep 05, 2014 at 09:13:26AM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 05, 2014 at 12:02:18AM +0200, Wouter Verhelst wrote:
> > [Cc: to nbd-general list added]
> > 
> > On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> > > Hi,
> > > QEMU offers both NBD client and server functionality.  The NBD protocol
> > > runs unencrypted, which is a problem when the client and server
> > > communicate over an untrusted network.
> > > 
> > > The particular use case that prompted this mail is storage migration in
> > > OpenStack.  The goal is to encrypt the NBD connection between source and
> > > destination hosts during storage migration.
> > 
> > I've never given encrypted NBD high priority, since I don't think
> > encryption without authentication serves much purpose -- and I haven't
> > gotten around to adding authentication yet (for which I have plans; but
> > other things have priority).
> 
> While have an authentication layer like SASL wired into the NBD protocol
> would be nice, it shouldn't be considered a blocker / pre-requisite.

I'm not saying that. If someone were to come up with patches for TLS
support in NBD, I'm not going to say no (provided they're sensible etc,
yada yada). It's just that when I look at things to do for NBD and
prioritize, I intend work on authentication (with SASL, indeed) before I
work on TLS.

-- 
<Lo-lan-do> Home is where you have to wash the dishes.
  -- #debian-devel, Freenode, 2004-09-22

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 22:54     ` Benoît Canet
@ 2014-09-05  8:42       ` Wouter Verhelst
  2014-09-05 12:15       ` Stefan Hajnoczi
  1 sibling, 0 replies; 44+ messages in thread
From: Wouter Verhelst @ 2014-09-05  8:42 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Hani Benhabiles, libvir-list, nick, qemu-devel, Max Reitz,
	Paolo Bonzini, Stefan Hajnoczi, nbd-generic, Wouter Verhelst

On Fri, Sep 05, 2014 at 12:54:45AM +0200, Benoît Canet wrote:
> The Friday 05 Sep 2014 à 00:07:04 (+0200), Wouter Verhelst wrote :
> > On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> > > Prenegociating TLS look like we will accidentaly introduce some security hole.
> 
> I was thinking of the fallback to cleartext case.
> 
> As a regular developper I am afraid of doing something creative with
> cryptography.

STARTTLS-like schemes is not being "creative with cryptography", it's an
accepted way of doing things. Yes, there are pitfalls, but those always
exist; that doesn't mean you should fall into the trap of remaking the
errors HTTP made with HTTPS. It's taken years for HTTPS to be able to
deal with the fact that you couldn't have multiple HTTPS sites on the
same IP; I don't want to go there.

"fallback to cleartext" is a problem, but it should not be too hard to
have crypto be enabled by way of a tri-state variable ("disabled",
"available if client wants it", "required").

-- 
<Lo-lan-do> Home is where you have to wash the dishes.
  -- #debian-devel, Freenode, 2004-09-22

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-03 16:44 [Qemu-devel] NBD TLS support in QEMU Stefan Hajnoczi
                   ` (2 preceding siblings ...)
  2014-09-05  6:23 ` [Qemu-devel] [libvirt] " Michal Privoznik
@ 2014-09-05  8:46 ` Hani Benhabiles
  2014-09-05 12:31   ` Stefan Hajnoczi
  2014-09-05 13:26   ` Wouter Verhelst
  3 siblings, 2 replies; 44+ messages in thread
From: Hani Benhabiles @ 2014-09-05  8:46 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: nbd-generic, libvir-list, mprivozn, nick, qemu-devel, Max Reitz,
	Paolo Bonzini, w

On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> Hi,
> QEMU offers both NBD client and server functionality.  The NBD protocol
> runs unencrypted, which is a problem when the client and server
> communicate over an untrusted network.
> 
> The particular use case that prompted this mail is storage migration in
> OpenStack.  The goal is to encrypt the NBD connection between source and
> destination hosts during storage migration.
> 
> I think we can integrate TLS into the NBD protocol as an optional flag.
> A quick web search does not reveal existing open source SSL/TLS NBD
> implementations.  I do see a VMware NBDSSL protocol but there is no
> specification so I guess it is proprietary.

Not sold on this because:
- It requires (unnecessary) changes to the NBD specification.
- It would still be vulnerable to a protocol downgrade attack: ie. An attacker
  could strip the TLS flag from the server's response resulting in either:
  * Connection fallback to cleartext, if both ends don't force TLS.
  * Loss of backward compatibility, if one of the ends forces TLS, making the
    reason for which such a flag is added moot IIUC.

IMO, it is more fool proof add some --use-tls flag to the client and server
implementations to wrap the data in TLS, just like HTTPS does for instance.

Also, so mean of verification is required (otherwise, back to point 0 being
vulnerable to sslstrip style attacks) either that the server's cert is signed
with a certain (self-generated) CA certificate or that it matches a certain
fingerprint. Doing it similarly on the server-side would allow hitting a 2nd
bird (authentication.)

>
> The NBD protocol starts with a negotiation phase.  This would be the
> appropriate place to indicate that TLS will be used.  After client and
> server complete TLS setup the connection can continue as normal.
> 
> Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> extended to support TLS.  In this case the kernel needs a localhost
> socket and userspace handles TLS.
> 
> Thoughts?
> 
> Stefan

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 22:54     ` Benoît Canet
  2014-09-05  8:42       ` Wouter Verhelst
@ 2014-09-05 12:15       ` Stefan Hajnoczi
  1 sibling, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-09-05 12:15 UTC (permalink / raw)
  To: Benoît Canet
  Cc: Stefan Hajnoczi, libvir-list, nbd-generic, qemu-devel, Max Reitz,
	Paolo Bonzini, Hani Benhabiles, nick, Wouter Verhelst

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On Fri, Sep 05, 2014 at 12:54:45AM +0200, Benoît Canet wrote:
> The Friday 05 Sep 2014 à 00:07:04 (+0200), Wouter Verhelst wrote :
> > On Thu, Sep 04, 2014 at 04:19:17PM +0200, Benoît Canet wrote:
> > > The Wednesday 03 Sep 2014 à 17:44:17 (+0100), Stefan Hajnoczi wrote :
> > > > Hi,
> > > > QEMU offers both NBD client and server functionality.  The NBD protocol
> > > > runs unencrypted, which is a problem when the client and server
> > > > communicate over an untrusted network.
> > > > 
> > > > The particular use case that prompted this mail is storage migration in
> > > > OpenStack.  The goal is to encrypt the NBD connection between source and
> > > > destination hosts during storage migration.
> > > 
> > > I agree this would be usefull.
> > > 
> > > > 
> > > > I think we can integrate TLS into the NBD protocol as an optional flag.
> > > > A quick web search does not reveal existing open source SSL/TLS NBD
> > > > implementations.  I do see a VMware NBDSSL protocol but there is no
> > > > specification so I guess it is proprietary.
> > > > 
> > > > The NBD protocol starts with a negotiation phase.  This would be the
> > > > appropriate place to indicate that TLS will be used.  After client and
> > > > server complete TLS setup the connection can continue as normal.
> > > 
> > > Prenegociating TLS look like we will accidentaly introduce some security hole.
> 
> I was thinking of the fallback to cleartext case.

If the server or client were given a nbds:// URL then don't allow
downgrading to unencrypted.  I think we can implement this without a
security hole.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-04 22:02 ` Wouter Verhelst
  2014-09-05  8:13   ` Daniel P. Berrange
@ 2014-09-05 12:21   ` Stefan Hajnoczi
  1 sibling, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-09-05 12:21 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Stefan Hajnoczi, libvir-list, nbd-general, qemu-devel, Max Reitz,
	Hani Benhabiles, nick, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

On Fri, Sep 05, 2014 at 12:02:18AM +0200, Wouter Verhelst wrote:
> [Cc: to nbd-general list added]
> 
> On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> > Besides QEMU, the userspace NBD tools (http://nbd.sf.net/) can also be
> > extended to support TLS.  In this case the kernel needs a localhost
> > socket and userspace handles TLS.
> 
> That introduces a possibility for a deadlock, since now your network
> socket isn't on the PF_MEMALLOC-protected socket anymore, which will
> cause the kernel to throw away packets which are needed for your nbd
> connection, in hopes of clearing some memory.

Understood but there are plenty of use cases where this doesn't matter.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-05  8:46 ` [Qemu-devel] " Hani Benhabiles
@ 2014-09-05 12:31   ` Stefan Hajnoczi
  2014-09-05 13:26   ` Wouter Verhelst
  1 sibling, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-09-05 12:31 UTC (permalink / raw)
  To: Hani Benhabiles
  Cc: libvir-list, mprivozn, nbd-generic, qemu-devel, Max Reitz,
	Stefan Hajnoczi, nick, w, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

On Fri, Sep 05, 2014 at 09:46:18AM +0100, Hani Benhabiles wrote:
> On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> Also, so mean of verification is required (otherwise, back to point 0 being
> vulnerable to sslstrip style attacks) either that the server's cert is signed
> with a certain (self-generated) CA certificate or that it matches a certain
> fingerprint. Doing it similarly on the server-side would allow hitting a 2nd
> bird (authentication.)

Yes, client and server side certificates are needed.

Here are the SPICE TLS options in QEMU:

  tls-port=<nr>
      Set the TCP port spice is listening on for encrypted channels.

  x509-dir=<dir>
      Set the x509 file directory. Expects same filenames as -vnc $display,x509=$dir

  x509-key-file=<file>
  x509-key-password=<file>
  x509-cert-file=<file>
  x509-cacert-file=<file>
  x509-dh-key-file=<file>
      The x509 file names can also be configured individually.

  tls-ciphers=<list>
      Specify which ciphers to use.

I guess NBD would need similar options althoug I haven't investigated
TLS in depth yet.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-05  8:46 ` [Qemu-devel] " Hani Benhabiles
  2014-09-05 12:31   ` Stefan Hajnoczi
@ 2014-09-05 13:26   ` Wouter Verhelst
  2014-10-01 20:23     ` Wouter Verhelst
  1 sibling, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-09-05 13:26 UTC (permalink / raw)
  To: Hani Benhabiles
  Cc: nbd-generic, libvir-list, mprivozn, nick, qemu-devel, Max Reitz,
	Stefan Hajnoczi, Paolo Bonzini, w

On Fri, Sep 05, 2014 at 09:46:18AM +0100, Hani Benhabiles wrote:
> On Wed, Sep 03, 2014 at 05:44:17PM +0100, Stefan Hajnoczi wrote:
> > Hi,
> > QEMU offers both NBD client and server functionality.  The NBD protocol
> > runs unencrypted, which is a problem when the client and server
> > communicate over an untrusted network.
> > 
> > The particular use case that prompted this mail is storage migration in
> > OpenStack.  The goal is to encrypt the NBD connection between source and
> > destination hosts during storage migration.
> > 
> > I think we can integrate TLS into the NBD protocol as an optional flag.
> > A quick web search does not reveal existing open source SSL/TLS NBD
> > implementations.  I do see a VMware NBDSSL protocol but there is no
> > specification so I guess it is proprietary.
> 
> Not sold on this because:
> - It requires (unnecessary) changes to the NBD specification.

They may not be necessary from a libvirt/qemu point of view, but I've
had requests to implement encryption from other people, too. So while
they're not very high on my priority list, I do think the changes are
necessary.

> - It would still be vulnerable to a protocol downgrade attack: ie. An attacker
>   could strip the TLS flag from the server's response resulting in either:
>   * Connection fallback to cleartext, if both ends don't force TLS.
>   * Loss of backward compatibility, if one of the ends forces TLS, making the
>     reason for which such a flag is added moot IIUC.

Tunneling the entire protocol inside an SSL connection doesn't fix that;
if an attacker is able to hijack your TCP connections and change flags,
then this attacker is also able to hijack your TCP connection and
redirect it to a decrypting/encrypting proxy.

I agree that preventing a possible SSL downgrade attack (and other forms
of MITM) should be high on the priority list, but "tunnel the whole
thing in SSL" doesn't do that.

[...]

-- 
<Lo-lan-do> Home is where you have to wash the dishes.
  -- #debian-devel, Freenode, 2004-09-22

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-09-05 13:26   ` Wouter Verhelst
@ 2014-10-01 20:23     ` Wouter Verhelst
  2014-10-02 11:00       ` Paolo Bonzini
  2014-10-02 11:05       ` Daniel P. Berrange
  0 siblings, 2 replies; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-01 20:23 UTC (permalink / raw)
  To: Hani Benhabiles
  Cc: libvir-list, mprivozn, nbd-general, qemu-devel, Max Reitz,
	Stefan Hajnoczi, nick, Paolo Bonzini

Hi,

On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
> Tunneling the entire protocol inside an SSL connection doesn't fix that;
> if an attacker is able to hijack your TCP connections and change flags,
> then this attacker is also able to hijack your TCP connection and
> redirect it to a decrypting/encrypting proxy.
> 
> I agree that preventing a possible SSL downgrade attack (and other forms
> of MITM) should be high on the priority list, but "tunnel the whole
> thing in SSL" doesn't do that.

So, having given this some thought, I wanted to come up with a spec just
so that we had something we could all agree on. As part of that, I had a
look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
protocol (on port 10809 by default -- ew, please don't do that).

I had to change the protocol incompatibly a few years back, because the
oldstyle protocol is broken by design; in the oldstyle negotiation
protocol, the server dumps all information it has on the export to the
client, and then moves on to the data negotiation phase, without waiting
for any reply from the client. This means the oldstyle protocol can't be
used for any sort of negotiation[1].

As such, I strongly suggest that qemu-nbd move to the newstyle protocol.
This would also allow you to use named exports and various other things,
and would allow negotiation of TLS or SASL in a clean and proper way.

[1] That sole issue is the reason I broke backwards compatibility with
    the newstyle handshake protocol, and is also why I reserved 10809,
    the port assigned to nbd by IANA at my request, to be for newstyle
    handshakes only.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-01 20:23     ` Wouter Verhelst
@ 2014-10-02 11:00       ` Paolo Bonzini
  2014-10-02 13:50         ` Wouter Verhelst
  2014-10-02 11:05       ` Daniel P. Berrange
  1 sibling, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2014-10-02 11:00 UTC (permalink / raw)
  To: Wouter Verhelst, Hani Benhabiles
  Cc: libvir-list, mprivozn, nbd-general, qemu-devel, Max Reitz,
	Stefan Hajnoczi, nick

Il 01/10/2014 22:23, Wouter Verhelst ha scritto:
> Hi,
> 
> On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
>> Tunneling the entire protocol inside an SSL connection doesn't fix that;
>> if an attacker is able to hijack your TCP connections and change flags,
>> then this attacker is also able to hijack your TCP connection and
>> redirect it to a decrypting/encrypting proxy.
>>
>> I agree that preventing a possible SSL downgrade attack (and other forms
>> of MITM) should be high on the priority list, but "tunnel the whole
>> thing in SSL" doesn't do that.
> 
> So, having given this some thought, I wanted to come up with a spec just
> so that we had something we could all agree on. As part of that, I had a
> look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
> protocol (on port 10809 by default -- ew, please don't do that).

Can you use new-style handshake with a single unnamed export?  Export
names are a useless complication for qemu-nbd.

Paolo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-01 20:23     ` Wouter Verhelst
  2014-10-02 11:00       ` Paolo Bonzini
@ 2014-10-02 11:05       ` Daniel P. Berrange
  2014-10-02 11:28         ` Paolo Bonzini
  1 sibling, 1 reply; 44+ messages in thread
From: Daniel P. Berrange @ 2014-10-02 11:05 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick, Paolo Bonzini

On Wed, Oct 01, 2014 at 10:23:26PM +0200, Wouter Verhelst wrote:
> Hi,
> 
> On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
> > Tunneling the entire protocol inside an SSL connection doesn't fix that;
> > if an attacker is able to hijack your TCP connections and change flags,
> > then this attacker is also able to hijack your TCP connection and
> > redirect it to a decrypting/encrypting proxy.
> > 
> > I agree that preventing a possible SSL downgrade attack (and other forms
> > of MITM) should be high on the priority list, but "tunnel the whole
> > thing in SSL" doesn't do that.
> 
> So, having given this some thought, I wanted to come up with a spec just
> so that we had something we could all agree on. As part of that, I had a
> look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
> protocol (on port 10809 by default -- ew, please don't do that).
> 
> I had to change the protocol incompatibly a few years back, because the
> oldstyle protocol is broken by design; in the oldstyle negotiation
> protocol, the server dumps all information it has on the export to the
> client, and then moves on to the data negotiation phase, without waiting
> for any reply from the client. This means the oldstyle protocol can't be
> used for any sort of negotiation[1].
> 
> As such, I strongly suggest that qemu-nbd move to the newstyle protocol.

Even if we added support for the newstyle protocol I don't see us being
able to drop the oldstyle protocol. NBD is used during migration of block
storage, and we need to be able to migrate from old QEMU to new QEMU and
vica-verca, so can't just switch protocol in a new QEMU without retaining
a way to use the old protocol.

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-02 11:05       ` Daniel P. Berrange
@ 2014-10-02 11:28         ` Paolo Bonzini
  2014-10-17 22:03           ` [Qemu-devel] spec, RFC: TLS support for NBD Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Paolo Bonzini @ 2014-10-02 11:28 UTC (permalink / raw)
  To: Daniel P. Berrange, Wouter Verhelst
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick

Il 02/10/2014 13:05, Daniel P. Berrange ha scritto:
> On Wed, Oct 01, 2014 at 10:23:26PM +0200, Wouter Verhelst wrote:
>> Hi,
>>
>> On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
>>> Tunneling the entire protocol inside an SSL connection doesn't fix that;
>>> if an attacker is able to hijack your TCP connections and change flags,
>>> then this attacker is also able to hijack your TCP connection and
>>> redirect it to a decrypting/encrypting proxy.
>>>
>>> I agree that preventing a possible SSL downgrade attack (and other forms
>>> of MITM) should be high on the priority list, but "tunnel the whole
>>> thing in SSL" doesn't do that.
>>
>> So, having given this some thought, I wanted to come up with a spec just
>> so that we had something we could all agree on. As part of that, I had a
>> look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
>> protocol (on port 10809 by default -- ew, please don't do that).
>>
>> I had to change the protocol incompatibly a few years back, because the
>> oldstyle protocol is broken by design; in the oldstyle negotiation
>> protocol, the server dumps all information it has on the export to the
>> client, and then moves on to the data negotiation phase, without waiting
>> for any reply from the client. This means the oldstyle protocol can't be
>> used for any sort of negotiation[1].
>>
>> As such, I strongly suggest that qemu-nbd move to the newstyle protocol.
> 
> Even if we added support for the newstyle protocol I don't see us being
> able to drop the oldstyle protocol. NBD is used during migration of block
> storage, and we need to be able to migrate from old QEMU to new QEMU and
> vica-verca, so can't just switch protocol in a new QEMU without retaining
> a way to use the old protocol.

For that you don't use qemu-nbd, we use the NBD server that is embedded
in the QEMU executable.  That one shares almost all the code with
qemu-nbd but, because we are exporting multiple disks over a single
port, ends up using the new-style protocol.

qemu-nbd uses the old-style protocol only because it has a single,
unnamed export.

QEMU's NBD client will use the old-style protocol if given URLs like
nbd://HOST:PORT/, and the new-style protocol for nbd://HOST:PORT/NAME.

Paolo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-02 11:00       ` Paolo Bonzini
@ 2014-10-02 13:50         ` Wouter Verhelst
  2014-10-08 18:16           ` Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-02 13:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick

On Thu, Oct 02, 2014 at 01:00:04PM +0200, Paolo Bonzini wrote:
> Il 01/10/2014 22:23, Wouter Verhelst ha scritto:
> > Hi,
> > 
> > On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
> >> Tunneling the entire protocol inside an SSL connection doesn't fix that;
> >> if an attacker is able to hijack your TCP connections and change flags,
> >> then this attacker is also able to hijack your TCP connection and
> >> redirect it to a decrypting/encrypting proxy.
> >>
> >> I agree that preventing a possible SSL downgrade attack (and other forms
> >> of MITM) should be high on the priority list, but "tunnel the whole
> >> thing in SSL" doesn't do that.
> > 
> > So, having given this some thought, I wanted to come up with a spec just
> > so that we had something we could all agree on. As part of that, I had a
> > look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
> > protocol (on port 10809 by default -- ew, please don't do that).
> 
> Can you use new-style handshake with a single unnamed export?  Export
> names are a useless complication for qemu-nbd.

Not currently, but I don't think you need that. You could have a default
name, which would be used if no name was otherwise specified. It's not
much of a stretch to make that name part of the protocol spec, either.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-02 13:50         ` Wouter Verhelst
@ 2014-10-08 18:16           ` Wouter Verhelst
  2014-10-09 12:42             ` Paolo Bonzini
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-08 18:16 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick

On Thu, Oct 02, 2014 at 03:50:57PM +0200, Wouter Verhelst wrote:
> On Thu, Oct 02, 2014 at 01:00:04PM +0200, Paolo Bonzini wrote:
> > Il 01/10/2014 22:23, Wouter Verhelst ha scritto:
> > > Hi,
> > > 
> > > On Fri, Sep 05, 2014 at 03:26:09PM +0200, Wouter Verhelst wrote:
> > >> Tunneling the entire protocol inside an SSL connection doesn't fix that;
> > >> if an attacker is able to hijack your TCP connections and change flags,
> > >> then this attacker is also able to hijack your TCP connection and
> > >> redirect it to a decrypting/encrypting proxy.
> > >>
> > >> I agree that preventing a possible SSL downgrade attack (and other forms
> > >> of MITM) should be high on the priority list, but "tunnel the whole
> > >> thing in SSL" doesn't do that.
> > > 
> > > So, having given this some thought, I wanted to come up with a spec just
> > > so that we had something we could all agree on. As part of that, I had a
> > > look at qemu-nbd, and noticed that it uses the "oldstyle" handshake
> > > protocol (on port 10809 by default -- ew, please don't do that).
> > 
> > Can you use new-style handshake with a single unnamed export?  Export
> > names are a useless complication for qemu-nbd.
> 
> Not currently, but I don't think you need that. You could have a default
> name, which would be used if no name was otherwise specified. It's not
> much of a stretch to make that name part of the protocol spec, either.

So. I think this makes sense, and as such changed the proto.txt file as
follows:

diff --git a/doc/proto.txt b/doc/proto.txt
index e0a4fb1..990d012 100644
--- a/doc/proto.txt
+++ b/doc/proto.txt
@@ -242,10 +242,13 @@ Option types
 * NBD_OPT_EXPORT_NAME (1)
   Choose the export which the client would like to use, and end option
   haggling. Data: name of the export, free-form UTF8 text (subject to
   limitations by server implementation). If the chosen export does not
   exist, the server closes the connection.
+  A special, "empty", name (i.e., the length field is zero and no name
+  is specified), is reserved for a "default" export, to be used in cases
+  where explicitly specifying an export name makes no sense.
 
 * NBD_OPT_ABORT (2)
   Abort negotiation and close the connection. Optional.
 
 * NBD_OPT_LIST (3)

That is, specify an empty name to specify a default.

Thoughts?

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] NBD TLS support in QEMU
  2014-10-08 18:16           ` Wouter Verhelst
@ 2014-10-09 12:42             ` Paolo Bonzini
  0 siblings, 0 replies; 44+ messages in thread
From: Paolo Bonzini @ 2014-10-09 12:42 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general, qemu-devel,
	Max Reitz, Stefan Hajnoczi, nick

Il 08/10/2014 20:16, Wouter Verhelst ha scritto:
> @@ -242,10 +242,13 @@ Option types
>  * NBD_OPT_EXPORT_NAME (1)
>    Choose the export which the client would like to use, and end option
>    haggling. Data: name of the export, free-form UTF8 text (subject to
>    limitations by server implementation). If the chosen export does not
>    exist, the server closes the connection.
> +  A special, "empty", name (i.e., the length field is zero and no name
> +  is specified), is reserved for a "default" export, to be used in cases
> +  where explicitly specifying an export name makes no sense.

Thanks, this looks good!

Paolo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-02 11:28         ` Paolo Bonzini
@ 2014-10-17 22:03           ` Wouter Verhelst
  2014-10-18  6:33             ` Richard W.M. Jones
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-17 22:03 UTC (permalink / raw)
  To: Paolo Bonzini, Daniel P. Berrange, Hani Benhabiles,
	Stefan Hajnoczi, qemu-devel, Max Reitz, nick, libvir-list,
	mprivozn, nbd-general, rjones

[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]

Hi all,

(added rjones from nbdkit fame -- hi there)

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).
  
  If the server indicates that TLS is allowed, the client may now issue
  NBD_OPT_STARTTLS:

  C: NBD_OPT_STARTTLS
  S: NBD_REP_STARTTLS # or NBD_REP_ERR_POLICY, if unwilling
  C: <initiate TLS handshake>

  Once the TLS handshake has completed, negotiation should continue over
  the secure channel. The client should initiate that by sending an
  NBD_OPT_* message.

- The server may reply to any and all negotiation request with
  NBD_REP_ERR_TLS_REQD if it does not want to do anything without TLS.
  However, if at least one export is supported without encryption, the
  server must not in any case use this reply.

There is no command to "exit" TLS again. I don't think that makes sense,
but I could be persuaded otherwise with sound technical arguments.

Thoughts?

(full spec (with numbers etc) exists as an (uncommitted) diff to
doc/proto.txt on my laptop, ...)

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-17 22:03           ` [Qemu-devel] spec, RFC: TLS support for NBD Wouter Verhelst
@ 2014-10-18  6:33             ` Richard W.M. Jones
  2014-10-20  7:58               ` Daniel P. Berrange
  0 siblings, 1 reply; 44+ messages in thread
From: Richard W.M. Jones @ 2014-10-18  6:33 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Florian Weimer, Hani Benhabiles, libvir-list, mprivozn,
	nbd-general, qemu-devel, Max Reitz, Stefan Hajnoczi, nick,
	Paolo Bonzini

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).
>   
>   If the server indicates that TLS is allowed, the client may now issue
>   NBD_OPT_STARTTLS:
> 
>   C: NBD_OPT_STARTTLS
>   S: NBD_REP_STARTTLS # or NBD_REP_ERR_POLICY, if unwilling
>   C: <initiate TLS handshake>
> 
>   Once the TLS handshake has completed, negotiation should continue over
>   the secure channel. The client should initiate that by sending an
>   NBD_OPT_* message.
> 
> - The server may reply to any and all negotiation request with
>   NBD_REP_ERR_TLS_REQD if it does not want to do anything without TLS.
>   However, if at least one export is supported without encryption, the
>   server must not in any case use this reply.
> 
> There is no command to "exit" TLS again. I don't think that makes sense,
> but I could be persuaded otherwise with sound technical arguments.
> 
> Thoughts?
> 
> (full spec (with numbers etc) exists as an (uncommitted) diff to
> doc/proto.txt on my laptop, ...)
> 
> -- 
> It is easy to love a country that is famous for chocolate and beer
> 
>   -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26



-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-18  6:33             ` Richard W.M. Jones
@ 2014-10-20  7:58               ` Daniel P. Berrange
  2014-10-20  9:56                 ` Stefan Hajnoczi
  0 siblings, 1 reply; 44+ messages in thread
From: Daniel P. Berrange @ 2014-10-20  7:58 UTC (permalink / raw)
  To: Richard W.M. Jones
  Cc: Florian Weimer, Hani Benhabiles, libvir-list, mprivozn,
	nbd-general, nick, qemu-devel, Max Reitz, Stefan Hajnoczi,
	Paolo Bonzini, Wouter Verhelst

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.

> >   If the server indicates that TLS is allowed, the client may now issue
> >   NBD_OPT_STARTTLS:
> > 
> >   C: NBD_OPT_STARTTLS
> >   S: NBD_REP_STARTTLS # or NBD_REP_ERR_POLICY, if unwilling
> >   C: <initiate TLS handshake>
> > 
> >   Once the TLS handshake has completed, negotiation should continue over
> >   the secure channel. The client should initiate that by sending an
> >   NBD_OPT_* message.
> > 
> > - The server may reply to any and all negotiation request with
> >   NBD_REP_ERR_TLS_REQD if it does not want to do anything without TLS.
> >   However, if at least one export is supported without encryption, the
> >   server must not in any case use this reply.
> > 
> > There is no command to "exit" TLS again. I don't think that makes sense,
> > but I could be persuaded otherwise with sound technical arguments.
> > 
> > Thoughts?
> > 
> > (full spec (with numbers etc) exists as an (uncommitted) diff to
> > doc/proto.txt on my laptop, ...)

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20  7:58               ` Daniel P. Berrange
@ 2014-10-20  9:56                 ` Stefan Hajnoczi
  2014-10-20 11:51                   ` Markus Armbruster
  0 siblings, 1 reply; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-10-20  9:56 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Florian Weimer, Richard W.M. Jones, libvir-list, mprivozn,
	nbd-general, nick, qemu-devel, Max Reitz, Hani Benhabiles,
	Paolo Bonzini, Wouter Verhelst

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

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.

TLS is about the transport, not about a particular NBD export.  The only
thing that should be communicated is STARTTLS.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20  9:56                 ` Stefan Hajnoczi
@ 2014-10-20 11:51                   ` Markus Armbruster
  2014-10-20 11:56                     ` Florian Weimer
                                       ` (2 more replies)
  0 siblings, 3 replies; 44+ messages in thread
From: Markus Armbruster @ 2014-10-20 11:51 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Florian Weimer, libvir-list, mprivozn, nbd-general,
	Richard W.M. Jones, qemu-devel, Hani Benhabiles, nick,
	Wouter Verhelst, Paolo Bonzini, Max Reitz

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?

See also https://www.agwa.name/blog/post/starttls_considered_harmful

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20 11:51                   ` Markus Armbruster
@ 2014-10-20 11:56                     ` Florian Weimer
  2014-10-20 12:48                       ` Richard W.M. Jones
  2014-10-20 22:10                       ` Wouter Verhelst
  2014-10-20 12:08                     ` Daniel P. Berrange
  2014-10-20 21:53                     ` [Qemu-devel] spec, RFC: TLS support for NBDµ Wouter Verhelst
  2 siblings, 2 replies; 44+ messages in thread
From: Florian Weimer @ 2014-10-20 11:56 UTC (permalink / raw)
  To: Markus Armbruster, Stefan Hajnoczi
  Cc: libvir-list, mprivozn, nbd-general, Richard W.M. Jones,
	qemu-devel, Hani Benhabiles, nick, Wouter Verhelst,
	Paolo Bonzini, Max Reitz

On 10/20/2014 01:51 PM, Markus Armbruster wrote:
> 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?

The usual way to deal with this is to use different syntax for 
TLS-enabled and non-TLS addresses (e.g., https:// and http://).  With a 
TLS address, the client must enforce that only TLS-enabled connections 
are possible.  STARTTLS isn't the problem here, it's just an accident of 
history that many STARTTLS client implementations do not require a TLS 
handshake before proceeding.

I cannot comment on whether the proposed STARTTLS command is at the 
correct stage of the NBD protocol.  If there is a protocol description 
for NBD, I can have a look.

-- 
Florian Weimer / Red Hat Product Security

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20 11:51                   ` Markus Armbruster
  2014-10-20 11:56                     ` Florian Weimer
@ 2014-10-20 12:08                     ` Daniel P. Berrange
  2014-10-20 21:53                     ` [Qemu-devel] spec, RFC: TLS support for NBDµ Wouter Verhelst
  2 siblings, 0 replies; 44+ messages in thread
From: Daniel P. Berrange @ 2014-10-20 12:08 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Florian Weimer, Hani Benhabiles, libvir-list, mprivozn,
	nbd-general, Richard W.M. Jones, qemu-devel, Stefan Hajnoczi,
	nick, Wouter Verhelst, Paolo Bonzini, Max Reitz

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20 11:56                     ` Florian Weimer
@ 2014-10-20 12:48                       ` Richard W.M. Jones
  2014-10-20 22:10                       ` Wouter Verhelst
  1 sibling, 0 replies; 44+ messages in thread
From: Richard W.M. Jones @ 2014-10-20 12:48 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Hani Benhabiles, libvir-list, mprivozn, nbd-general,
	Markus Armbruster, qemu-devel, Stefan Hajnoczi, nick,
	Wouter Verhelst, Paolo Bonzini, Max Reitz

On Mon, Oct 20, 2014 at 01:56:43PM +0200, Florian Weimer wrote:
> On 10/20/2014 01:51 PM, Markus Armbruster wrote:
> >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?
> 
> The usual way to deal with this is to use different syntax for
> TLS-enabled and non-TLS addresses (e.g., https:// and http://).
> With a TLS address, the client must enforce that only TLS-enabled
> connections are possible.  STARTTLS isn't the problem here, it's
> just an accident of history that many STARTTLS client
> implementations do not require a TLS handshake before proceeding.
> 
> I cannot comment on whether the proposed STARTTLS command is at the
> correct stage of the NBD protocol.  If there is a protocol
> description for NBD, I can have a look.

Two actually :-)  Both are covered here:

http://sourceforge.net/p/nbd/code/ci/master/tree/doc/proto.txt

I believe that the proposed changes only cover the new style
protocol.

There's no common syntax for nbd URLs that I'm aware of.  At least,
both qemu & guestfish have nbd:... strings that they can parse, but
both have a completely different syntax.  But we could still have a
client-side indication (flag or nbds:..) to say that we want to force
TLS.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBDµ
  2014-10-20 11:51                   ` Markus Armbruster
  2014-10-20 11:56                     ` Florian Weimer
  2014-10-20 12:08                     ` Daniel P. Berrange
@ 2014-10-20 21:53                     ` Wouter Verhelst
  2014-10-21  8:17                       ` Markus Armbruster
  2 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-20 21:53 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Florian Weimer, Hani Benhabiles, libvir-list, mprivozn,
	nbd-general, Richard W.M. Jones, qemu-devel, Stefan Hajnoczi,
	nick, Paolo Bonzini, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 4228 bytes --]

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.

Mmm. I suppose the NBD_OPT_PEEK_EXPORT message could be defined so that
it is fine for an export which has the "TLS required" bit set to provide
differing information after TLS has been negotiated.

> > 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?

As I've said before in this discussion, encryption downgrade attacks are
not specific to STARTTLS; as soon as you have have an "encrypted" and an
"unencrypted" variant of a protocol, that becomes a problem. After all,
if an attacker can modify the communication so that STARTTLS is filtered
out of the communication, they can most likely also redirect all traffic
to a decrypting/encrypting proxy.

The only way to fix that is through userspace; make "opportunistic" TLS
(i.e., use it if available, but move on if not) difficult to achieve.

> See also https://www.agwa.name/blog/post/starttls_considered_harmful

A random blog post by an author who is speaking about STARTTLS in
general terms is not a good technical argument for why STARTTLS is a bad
idea in *this* specific case.

If I was defining a new protocol from scratch, I might dump the whole
thing in TLS to begin with. But that's just not the case, so I have to
deal with what exists already.

In addition, with the current state of affairs, it is *not possible* to
swap to an NBD device if you need to pipe its data through a separate
socket than the one you're handing to the kernel. The result of that is
that you can't do TLS on a device you want to swap to. This means we
need to continue to support a protocol that can do TLS for some exports,
and plain (unencrypted) traffic for other exports, *in the same running
nbd-server instance*.

I did add the NBD_REP_ERR_TLS_REQD message for a server which does not
export anything unencrypted, so that it can (after the initial few
exchanges) reply to anything except for STARTTLS with "lalala, I'm not
listening until you encrypt things". However, unless it's fine to ditch
support for swapping to an NBD device (not an option from where I'm
standing), dropping support for unencrypted communication is not an
option.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20 11:56                     ` Florian Weimer
  2014-10-20 12:48                       ` Richard W.M. Jones
@ 2014-10-20 22:10                       ` Wouter Verhelst
  2014-10-21  9:35                         ` Daniel P. Berrange
  1 sibling, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-20 22:10 UTC (permalink / raw)
  To: Florian Weimer
  Cc: qemu-devel, Hani Benhabiles, libvir-list, mprivozn, nbd-general,
	Markus Armbruster, Richard W.M. Jones, Stefan Hajnoczi, nick,
	Paolo Bonzini, Max Reitz

On Mon, Oct 20, 2014 at 01:56:43PM +0200, Florian Weimer wrote:
> I cannot comment on whether the proposed STARTTLS command is at the correct
> stage of the NBD protocol.  If there is a protocol description for NBD, I
> can have a look.

To make this discussion in that regard a bit easier, I've committed the
proposed spec to a separate branch:

https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt

Thanks,

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBDµ
  2014-10-20 21:53                     ` [Qemu-devel] spec, RFC: TLS support for NBDµ Wouter Verhelst
@ 2014-10-21  8:17                       ` Markus Armbruster
  2014-10-21 18:30                         ` Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Markus Armbruster @ 2014-10-21  8:17 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Florian Weimer, Stefan Hajnoczi, libvir-list, mprivozn,
	nbd-general, Richard W.M. Jones, qemu-devel, Hani Benhabiles,
	nick, Paolo Bonzini, Max Reitz

Wouter Verhelst <w@uter.be> writes:

> 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.
>
> Mmm. I suppose the NBD_OPT_PEEK_EXPORT message could be defined so that
> it is fine for an export which has the "TLS required" bit set to provide
> differing information after TLS has been negotiated.

Why is it useful to have a per-export "TLS required" bit?

Stefan's argument:

>> > TLS is about the transport, not about a particular NBD export.  The only
>> > thing that should be communicated is STARTTLS.

makes sense to me.

Wouldn't a per-export "TLS required" bit enlarge the attack surface?  It
puts the "peek export" operation into the "without authentication"
bucket *and* makes it more complex.

The least complex STARTTLS solution seems to be "if server wants TLS,
absolutely nothing works but switching to TLS and any feature
negotiation necessary for the client to recognize the need to switch".

Hmm, further down you give a reason why you want to mix encrypted and
unencrypted exports.

>> 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?
>
> As I've said before in this discussion, encryption downgrade attacks are
> not specific to STARTTLS; as soon as you have have an "encrypted" and an
> "unencrypted" variant of a protocol, that becomes a problem. After all,
> if an attacker can modify the communication so that STARTTLS is filtered
> out of the communication, they can most likely also redirect all traffic
> to a decrypting/encrypting proxy.
>
> The only way to fix that is through userspace; make "opportunistic" TLS
> (i.e., use it if available, but move on if not) difficult to achieve.
>
>> See also https://www.agwa.name/blog/post/starttls_considered_harmful
>
> A random blog post by an author who is speaking about STARTTLS in
> general terms is not a good technical argument for why STARTTLS is a bad
> idea in *this* specific case.

Misunderstanding.  I didn't mean to claim "STARTTLS is bad".  If I
wanted to say that, I would've said it directly.  I was merely asking
how you plan to guard against downgrade attacks.  I gather your advice
is to make the client (QEMU) insist on TLS, and check the server's
certificate.  Correct?

> If I was defining a new protocol from scratch, I might dump the whole
> thing in TLS to begin with. But that's just not the case, so I have to
> deal with what exists already.

Yes.  You can still start over on a separate port.  However:

> In addition, with the current state of affairs, it is *not possible* to
> swap to an NBD device if you need to pipe its data through a separate
> socket than the one you're handing to the kernel. The result of that is
> that you can't do TLS on a device you want to swap to. This means we
> need to continue to support a protocol that can do TLS for some exports,
> and plain (unencrypted) traffic for other exports, *in the same running
> nbd-server instance*.

I don't understand enough about NBD to challenge this argument.  The sad
consequence is more complexity and a larger attack surface.

Out of curiosity: is this "merely" an implementation restriction, or is
it more fundamental?

> I did add the NBD_REP_ERR_TLS_REQD message for a server which does not
> export anything unencrypted, so that it can (after the initial few
> exchanges) reply to anything except for STARTTLS with "lalala, I'm not
> listening until you encrypt things". However, unless it's fine to ditch
> support for swapping to an NBD device (not an option from where I'm
> standing), dropping support for unencrypted communication is not an
> option.

That's a good idea.  It doesn't make things less complex, though.

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-20 22:10                       ` Wouter Verhelst
@ 2014-10-21  9:35                         ` Daniel P. Berrange
  2014-10-21 18:02                           ` Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Daniel P. Berrange @ 2014-10-21  9:35 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Florian Weimer, qemu-devel, Hani Benhabiles, libvir-list,
	mprivozn, nbd-general, Markus Armbruster, Richard W.M. Jones,
	Stefan Hajnoczi, nick, Paolo Bonzini, Max Reitz

On Tue, Oct 21, 2014 at 12:10:39AM +0200, Wouter Verhelst wrote:
> On Mon, Oct 20, 2014 at 01:56:43PM +0200, Florian Weimer wrote:
> > I cannot comment on whether the proposed STARTTLS command is at the correct
> > stage of the NBD protocol.  If there is a protocol description for NBD, I
> > can have a look.
> 
> To make this discussion in that regard a bit easier, I've committed the
> proposed spec to a separate branch:
> 
> https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt

So, if I'm understanding correctly the new style "fixed" handshake
currently works like this:

Server starts transmission with version info

  - S: "NBDMAGIC"
  - S: 0x49484156454F5054
  - S: 0x1

And the client acknowledges with a 32 bits of and first bit set

  - C: 0x1


Now the client has to request one or more options, of which the
NBD_OPT_EXPORT_NAME (0x1) option is mandatory, so it will be the
first thing the client sends next . So it would send

  - C: 0x49484156454F5054
  - C: 0x1
  - C: 0xa  (length of name)
  - C: "volumename"



You're proposing to add a new option NBD_OPT_STARTTLS (0x5). For this to
work we would need to update the language to say that the NBD_OPT_STARTTLS
must be the first option that the client sends to the server, because we
want the option name to transmit in ciphertext.

So the new protocol startup would be

Server starts transmission with version info

  - S: "NBDMAGIC"
  - S: 0x49484156454F5054
  - S: 0x1

And the client acknowledges with a 32 bits of zero

  - C: 0x1

Client requests TLS

  - C: 0x49484156454F5054
  - C: 0x5

Server acknowledges TLS:

  - S: 0x3e889045565a9
  - S: 0x5
  - S: 0x1 (REP_ACK)
  - S: 0x0

Now the TLS handshake is performed by client + server



The client can now set other options using the secure
channel, of which the NBD_OPT_EXPORT_NAME (0x1) option
is mandatory, so it will be the first thing the client
sends next . So they would send

  - C: "0x49484156454F5054"
  - C: 0x1
  - C: 0xa  (length of name)
  - C: "volumename"

...etc...


This is secure when both client and server want to use TLS.

This is secure when the client wants TLS and the server does
not want TLS, because the server will reject TLS and the client
will then close the connection.

My concern is the case where the client does not want TLS but
the server does want TLS. In this case the client will immediately
send the NBD_OPT_EXPORT_NAME in clear text over the wire, not giving
the server any chance to reject the use of a clear text channel.

This problem is inherant to the approach of using the NBD protocol
options to negotiate TLS.



One way I see to solve that insecurity, would be for the server
to make use of one of the extra reserved bits in the very first
message it sends. ie we need to negotiate TLS immediately after the
version number / magic acknowledgment, before we actually start
the main body of the protocol

ie, with the new style fixed handshake the server should send

Server starts transmission with version info

  - S: "NBDMAGIC"
  - S: 0x49484156454F5054
  - S: 0x2 

And the client acknowledges with a 32bits and first two bits set

  - C: 0x2

The questionmark here is what happens when the client sees the
second bit of reserved field set ? 

The protocol docs merely say

  "S: 16 bits of zero (bits 1-15 reserved for future use; bit 0 in use to
   signal fixed newstyle (see below))"

But don't mention what clients should do upon seeing unknown bits
set in the server's message ?

If clients ignore unknown bits, then we have the same problem where a
client can ignore the TLS bit and start sending option name in the
clear before the server rejects the session.

If clients abort (close connection) on seeing unknown bits, then we
are good.



A 3rd alternative is to actually use a separate magic number, which
should guarantee the client would immediately close upon seeing a
magic number which indicated TLS is required.

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 :|

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBD
  2014-10-21  9:35                         ` Daniel P. Berrange
@ 2014-10-21 18:02                           ` Wouter Verhelst
  0 siblings, 0 replies; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-21 18:02 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: Florian Weimer, qemu-devel, Hani Benhabiles, libvir-list,
	mprivozn, nbd-general, Markus Armbruster, Richard W.M. Jones,
	Stefan Hajnoczi, nick, Paolo Bonzini, Max Reitz

On Tue, Oct 21, 2014 at 10:35:06AM +0100, Daniel P. Berrange wrote:
> On Tue, Oct 21, 2014 at 12:10:39AM +0200, Wouter Verhelst wrote:
> > On Mon, Oct 20, 2014 at 01:56:43PM +0200, Florian Weimer wrote:
> > > I cannot comment on whether the proposed STARTTLS command is at the correct
> > > stage of the NBD protocol.  If there is a protocol description for NBD, I
> > > can have a look.
> > 
> > To make this discussion in that regard a bit easier, I've committed the
> > proposed spec to a separate branch:
> > 
> > https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt
> 
> So, if I'm understanding correctly the new style "fixed" handshake
> currently works like this:
> 
> Server starts transmission with version info
> 
>   - S: "NBDMAGIC"
>   - S: 0x49484156454F5054
>   - S: 0x1
> 
> And the client acknowledges with a 32 bits of and first bit set
> 
>   - C: 0x1
> 
> 
> Now the client has to request one or more options, of which the
> NBD_OPT_EXPORT_NAME (0x1) option is mandatory, so it will be the
> first thing the client sends next.

No, on the contrary. NBD_OPT_EXPORT_NAME _must_ be the very last thing the
client sends if the client wants to actually select an export to be
used. This is because for historical reasons, NBD_OPT_EXPORT_NAME can't
get an NBD_REP_ACK, but must immediately introduce the next phase of the
protocol (where data is flowing across the channel).

It should maybe be renamed to something like "NBD_OPT_SELECT_AND_GO" or
some such, but I'm not sure that makes much sense.

> So it would send
> 
>   - C: 0x49484156454F5054
>   - C: 0x1
>   - C: 0xa  (length of name)
>   - C: "volumename"
> 
> 
> 
> You're proposing to add a new option NBD_OPT_STARTTLS (0x5). For this to
> work we would need to update the language to say that the NBD_OPT_STARTTLS
> must be the first option that the client sends to the server, because we
> want the option name to transmit in ciphertext.

Yes, my proposed protocol allows that.

> So the new protocol startup would be
> 
> Server starts transmission with version info
> 
>   - S: "NBDMAGIC"
>   - S: 0x49484156454F5054
>   - S: 0x1
> 
> And the client acknowledges with a 32 bits of zero
> 
>   - C: 0x1
> 
> Client requests TLS
> 
>   - C: 0x49484156454F5054
>   - C: 0x5
> 
> Server acknowledges TLS:
> 
>   - S: 0x3e889045565a9
>   - S: 0x5
>   - S: 0x1 (REP_ACK)
>   - S: 0x0
> 
> Now the TLS handshake is performed by client + server
> 
> 
> 
> The client can now set other options using the secure
> channel, of which the NBD_OPT_EXPORT_NAME (0x1) option
> is mandatory, so it will be the first thing the client

(last thing)

> sends next. So they would send
> 
>   - C: "0x49484156454F5054"
>   - C: 0x1
>   - C: 0xa  (length of name)
>   - C: "volumename"
> 
> ....etc...
> 
> 
> This is secure when both client and server want to use TLS.
> 
> This is secure when the client wants TLS and the server does
> not want TLS, because the server will reject TLS and the client
> will then close the connection.
> 
> My concern is the case where the client does not want TLS but
> the server does want TLS. In this case the client will immediately
> send the NBD_OPT_EXPORT_NAME in clear text over the wire, not giving
> the server any chance to reject the use of a clear text channel.

In that case (if the client doesn't want TLS and also doesn't want to
negotiate anything else apart from a name), the server should disconnect
(anything else will fail to communicate with older clients).

> This problem is inherant to the approach of using the NBD protocol
> options to negotiate TLS.

True.

> One way I see to solve that insecurity, would be for the server
> to make use of one of the extra reserved bits in the very first
> message it sends. ie we need to negotiate TLS immediately after the
> version number / magic acknowledgment, before we actually start
> the main body of the protocol
> 
> ie, with the new style fixed handshake the server should send
> 
> Server starts transmission with version info
> 
>   - S: "NBDMAGIC"
>   - S: 0x49484156454F5054
>   - S: 0x2 

That would be 0x4, because 0x2 is already used for something else (yes,
yes, details).

> And the client acknowledges with a 32bits and first two bits set
> 
>   - C: 0x2
> 
> The questionmark here is what happens when the client sees the
> second bit of reserved field set ? 
> 
> The protocol docs merely say
> 
>   "S: 16 bits of zero (bits 1-15 reserved for future use; bit 0 in use to
>    signal fixed newstyle (see below))"
> 
> But don't mention what clients should do upon seeing unknown bits
> set in the server's message ?

If the client doesn't send the bit you proposed, then the server would
know it doesn't understand the TLS variant of the protocol and would
then either fall back to unencrypted or drop the connection (depending
on policy).

I'm not sure using a separate bit for this is necessary, though. The
server is supposed to send NBD_REP_ERR_UNSUP for messages it doesn't
understand, so a client that talks to a server which doesn't support the
STARTTLS message should be able to communicate just fine (if client
policy allows unencrypted communication, yada yada).

> If clients ignore unknown bits, then we have the same problem where a
> client can ignore the TLS bit and start sending option name in the
> clear before the server rejects the session.
> 
> If clients abort (close connection) on seeing unknown bits, then we
> are good.
> 
> 
> 
> A 3rd alternative is to actually use a separate magic number, which
> should guarantee the client would immediately close upon seeing a
> magic number which indicated TLS is required.

Yes, but that introduces a backwards incompatible change for no good
reason (the other two options are backwards-compatible), so is not an
option for me.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] spec, RFC: TLS support for NBDµ
  2014-10-21  8:17                       ` Markus Armbruster
@ 2014-10-21 18:30                         ` Wouter Verhelst
  2014-10-25 10:43                           ` [Qemu-devel] [Nbd] " Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-21 18:30 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Florian Weimer, Stefan Hajnoczi, libvir-list, mprivozn,
	nbd-general, Richard W.M. Jones, qemu-devel, Hani Benhabiles,
	nick, Paolo Bonzini, Max Reitz

Hi Markus,

On Tue, Oct 21, 2014 at 10:17:17AM +0200, Markus Armbruster wrote:
> Wouter Verhelst <w@uter.be> writes:
> > On Mon, Oct 20, 2014 at 01:51:43PM +0200, Markus Armbruster wrote:
[...]
> >> 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?
> >
> > As I've said before in this discussion, encryption downgrade attacks are
> > not specific to STARTTLS; as soon as you have have an "encrypted" and an
> > "unencrypted" variant of a protocol, that becomes a problem. After all,
> > if an attacker can modify the communication so that STARTTLS is filtered
> > out of the communication, they can most likely also redirect all traffic
> > to a decrypting/encrypting proxy.
> >
> > The only way to fix that is through userspace; make "opportunistic" TLS
> > (i.e., use it if available, but move on if not) difficult to achieve.
> >
> >> See also https://www.agwa.name/blog/post/starttls_considered_harmful
> >
> > A random blog post by an author who is speaking about STARTTLS in
> > general terms is not a good technical argument for why STARTTLS is a bad
> > idea in *this* specific case.
> 
> Misunderstanding.  I didn't mean to claim "STARTTLS is bad".  If I
> wanted to say that, I would've said it directly.  I was merely asking
> how you plan to guard against downgrade attacks.  I gather your advice
> is to make the client (QEMU) insist on TLS, and check the server's
> certificate.  Correct?

My advice is to give both client and server the ability to have TLS
switched on or off, and possibly (but not necessarily so, and certainly
not by default) also the _ability_ to negotiate TLS if the other side
supports it, while not aborting if it doesn't.

> > If I was defining a new protocol from scratch, I might dump the whole
> > thing in TLS to begin with. But that's just not the case, so I have to
> > deal with what exists already.
> 
> Yes.  You can still start over on a separate port.  However:

No, I can't, at least not if I want to continue to use an assigned port
without breaking backwards compatibility. When 10809 was assigned to
NBD, IANA made it perfectly clear that I wouldn't get a new port number
for a "secure" variant.

> > In addition, with the current state of affairs, it is *not possible* to
> > swap to an NBD device if you need to pipe its data through a separate
> > socket than the one you're handing to the kernel. The result of that is
> > that you can't do TLS on a device you want to swap to. This means we
> > need to continue to support a protocol that can do TLS for some exports,
> > and plain (unencrypted) traffic for other exports, *in the same running
> > nbd-server instance*.
> 
> I don't understand enough about NBD to challenge this argument.  The sad
> consequence is more complexity and a larger attack surface.
> 
> Out of curiosity: is this "merely" an implementation restriction, or is
> it more fundamental?

Well, to some extent, every software issue is merely an implementation
restriction...

The problem is that in order to clear memory when your swap device is on
the other side of a network connection, you need to write to said swap
device, which causes TCP traffic, which means the kernel needs to read
TCP ACK packets (or in the case of NBD, the reply packet to the
NBD_CMD_WRITE request that you sent out) to ensure that the traffic has
in fact reached its destination. Unfortunately, you can't impose
ordering on incoming network traffic, so that means you may need to read
through a whole lot of youtube MPEG traffic or some such in order to
find your one TCP ACK that tells you your swapout has been processed
correctly and you can now clear the memory which you wrote out to swap.

It's fairly obvious how that could lead to a deadlock if you don't know
that this one data stream is not related to swapspace while this other
one here is.

That's why a PF_MEMALLOC kernel-space socket option was created; sockets
marked with that option are related to a swapdevice, so when the kernel
is low on memory, it will throw away all incoming network traffic
_except_ for packets destined for a socket marked with that option, in
the hope that this will allow us to clear up some memory.

Since it's a kernel-space only thing, that means you can't mark sockets
as such from userspace; so if the NBD traffic is wrapped in some other
traffic from which it needs to be unwrapped in userspace, then the
userspace component would basically be a proxy with two sockets -- one
towards the server, one towards the kernel. The socket towards the
kernel would have the PF_MEMALLOC socket option set, but the one towards
the server would not, and *that* is the one which actually has data
flowing in over the network. There's your deadlock again.

Since, with the current state of affairs, the NBD handshake is done in
user space with the actual data pushing stuff later being done in kernel
space, that means you do actually need to unwrap the TLS traffic in
userspace.

I can't think of many ways that would allow us to get around that issue:
- Figure out a way to move negotiated keys from user space to kernel
  space, and handle the TLS in kernel space rather than user space. I'm
  not even sure if you _can_ do TLS in kernel space (let alone whether
  it's a good idea).
- Move the handshake to kernel space from user space. I'm not a fan of
  that idea. It would also still require much of the TLS in kernel
  space.
- Patch the kernel so that userspace can mark a particular socket with
  PF_MEMALLOC. And mlock() the whole TLS stack into memory (including
  keys etc).

All three of those require "patch the kernel", so at any rate going
there would require "wait until the kernel supports it before we can do
TLS", which I think is not a good option.

> > I did add the NBD_REP_ERR_TLS_REQD message for a server which does not
> > export anything unencrypted, so that it can (after the initial few
> > exchanges) reply to anything except for STARTTLS with "lalala, I'm not
> > listening until you encrypt things". However, unless it's fine to ditch
> > support for swapping to an NBD device (not an option from where I'm
> > standing), dropping support for unencrypted communication is not an
> > option.
> 
> That's a good idea.  It doesn't make things less complex, though.

True.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [Nbd]  spec, RFC: TLS support for NBDµ
  2014-10-21 18:30                         ` Wouter Verhelst
@ 2014-10-25 10:43                           ` Wouter Verhelst
  2014-10-30 10:40                             ` Stefan Hajnoczi
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-25 10:43 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Florian Weimer, libvir-list, mprivozn, nbd-general,
	Richard W.M. Jones, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	Max Reitz

Hi all,

I haven't seen a reply to this anymore. Do people still have comments?
I'm planning on doing a release of nbd later this weekend, and would
like to include this (not the TLS implementation yet, but at least the
spec)

Thanks,

On Tue, Oct 21, 2014 at 08:30:05PM +0200, Wouter Verhelst wrote:
> Hi Markus,
> 
> On Tue, Oct 21, 2014 at 10:17:17AM +0200, Markus Armbruster wrote:
> > Wouter Verhelst <w@uter.be> writes:
> > > On Mon, Oct 20, 2014 at 01:51:43PM +0200, Markus Armbruster wrote:
> [...]
> > >> 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?
> > >
> > > As I've said before in this discussion, encryption downgrade attacks are
> > > not specific to STARTTLS; as soon as you have have an "encrypted" and an
> > > "unencrypted" variant of a protocol, that becomes a problem. After all,
> > > if an attacker can modify the communication so that STARTTLS is filtered
> > > out of the communication, they can most likely also redirect all traffic
> > > to a decrypting/encrypting proxy.
> > >
> > > The only way to fix that is through userspace; make "opportunistic" TLS
> > > (i.e., use it if available, but move on if not) difficult to achieve.
> > >
> > >> See also https://www.agwa.name/blog/post/starttls_considered_harmful
> > >
> > > A random blog post by an author who is speaking about STARTTLS in
> > > general terms is not a good technical argument for why STARTTLS is a bad
> > > idea in *this* specific case.
> > 
> > Misunderstanding.  I didn't mean to claim "STARTTLS is bad".  If I
> > wanted to say that, I would've said it directly.  I was merely asking
> > how you plan to guard against downgrade attacks.  I gather your advice
> > is to make the client (QEMU) insist on TLS, and check the server's
> > certificate.  Correct?
> 
> My advice is to give both client and server the ability to have TLS
> switched on or off, and possibly (but not necessarily so, and certainly
> not by default) also the _ability_ to negotiate TLS if the other side
> supports it, while not aborting if it doesn't.
> 
> > > If I was defining a new protocol from scratch, I might dump the whole
> > > thing in TLS to begin with. But that's just not the case, so I have to
> > > deal with what exists already.
> > 
> > Yes.  You can still start over on a separate port.  However:
> 
> No, I can't, at least not if I want to continue to use an assigned port
> without breaking backwards compatibility. When 10809 was assigned to
> NBD, IANA made it perfectly clear that I wouldn't get a new port number
> for a "secure" variant.
> 
> > > In addition, with the current state of affairs, it is *not possible* to
> > > swap to an NBD device if you need to pipe its data through a separate
> > > socket than the one you're handing to the kernel. The result of that is
> > > that you can't do TLS on a device you want to swap to. This means we
> > > need to continue to support a protocol that can do TLS for some exports,
> > > and plain (unencrypted) traffic for other exports, *in the same running
> > > nbd-server instance*.
> > 
> > I don't understand enough about NBD to challenge this argument.  The sad
> > consequence is more complexity and a larger attack surface.
> > 
> > Out of curiosity: is this "merely" an implementation restriction, or is
> > it more fundamental?
> 
> Well, to some extent, every software issue is merely an implementation
> restriction...
> 
> The problem is that in order to clear memory when your swap device is on
> the other side of a network connection, you need to write to said swap
> device, which causes TCP traffic, which means the kernel needs to read
> TCP ACK packets (or in the case of NBD, the reply packet to the
> NBD_CMD_WRITE request that you sent out) to ensure that the traffic has
> in fact reached its destination. Unfortunately, you can't impose
> ordering on incoming network traffic, so that means you may need to read
> through a whole lot of youtube MPEG traffic or some such in order to
> find your one TCP ACK that tells you your swapout has been processed
> correctly and you can now clear the memory which you wrote out to swap.
> 
> It's fairly obvious how that could lead to a deadlock if you don't know
> that this one data stream is not related to swapspace while this other
> one here is.
> 
> That's why a PF_MEMALLOC kernel-space socket option was created; sockets
> marked with that option are related to a swapdevice, so when the kernel
> is low on memory, it will throw away all incoming network traffic
> _except_ for packets destined for a socket marked with that option, in
> the hope that this will allow us to clear up some memory.
> 
> Since it's a kernel-space only thing, that means you can't mark sockets
> as such from userspace; so if the NBD traffic is wrapped in some other
> traffic from which it needs to be unwrapped in userspace, then the
> userspace component would basically be a proxy with two sockets -- one
> towards the server, one towards the kernel. The socket towards the
> kernel would have the PF_MEMALLOC socket option set, but the one towards
> the server would not, and *that* is the one which actually has data
> flowing in over the network. There's your deadlock again.
> 
> Since, with the current state of affairs, the NBD handshake is done in
> user space with the actual data pushing stuff later being done in kernel
> space, that means you do actually need to unwrap the TLS traffic in
> userspace.
> 
> I can't think of many ways that would allow us to get around that issue:
> - Figure out a way to move negotiated keys from user space to kernel
>   space, and handle the TLS in kernel space rather than user space. I'm
>   not even sure if you _can_ do TLS in kernel space (let alone whether
>   it's a good idea).
> - Move the handshake to kernel space from user space. I'm not a fan of
>   that idea. It would also still require much of the TLS in kernel
>   space.
> - Patch the kernel so that userspace can mark a particular socket with
>   PF_MEMALLOC. And mlock() the whole TLS stack into memory (including
>   keys etc).
> 
> All three of those require "patch the kernel", so at any rate going
> there would require "wait until the kernel supports it before we can do
> TLS", which I think is not a good option.
> 
> > > I did add the NBD_REP_ERR_TLS_REQD message for a server which does not
> > > export anything unencrypted, so that it can (after the initial few
> > > exchanges) reply to anything except for STARTTLS with "lalala, I'm not
> > > listening until you encrypt things". However, unless it's fine to ditch
> > > support for swapping to an NBD device (not an option from where I'm
> > > standing), dropping support for unencrypted communication is not an
> > > option.
> > 
> > That's a good idea.  It doesn't make things less complex, though.
> 
> True.
> 
> -- 
> It is easy to love a country that is famous for chocolate and beer
> 
>   -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26
> 
> ------------------------------------------------------------------------------
> Comprehensive Server Monitoring with Site24x7.
> Monitor 10 servers for $9/Month.
> Get alerted through email, SMS, voice calls or mobile push notifications.
> Take corrective actions from your mobile device.
> http://p.sf.net/sfu/Zoho
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [Nbd]  spec, RFC: TLS support for NBDµ
  2014-10-25 10:43                           ` [Qemu-devel] [Nbd] " Wouter Verhelst
@ 2014-10-30 10:40                             ` Stefan Hajnoczi
  2014-10-31 18:15                               ` Wouter Verhelst
  0 siblings, 1 reply; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-10-30 10:40 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Florian Weimer, qemu-devel, libvir-list, mprivozn, nbd-general,
	Markus Armbruster, Richard W.M. Jones, Paolo Bonzini, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On Sat, Oct 25, 2014 at 12:43:35PM +0200, Wouter Verhelst wrote:
> I haven't seen a reply to this anymore. Do people still have comments?
> I'm planning on doing a release of nbd later this weekend, and would
> like to include this (not the TLS implementation yet, but at least the
> spec)

Hi Wouter,
From https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt:

  * NBD_OPT_STARTTLS (5)

  The client wishes to initiate TLS. XXX Data.

Is there text missing for "XXX Data"?

Also, I suggest at least developing a prototype before releasing the
specification changes.  Issues that were unknown ahead of time might be
discovered during development.  Why the rush to release specification
changes?

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [Nbd]  spec, RFC: TLS support for NBDµ
  2014-10-30 10:40                             ` Stefan Hajnoczi
@ 2014-10-31 18:15                               ` Wouter Verhelst
  2014-11-03 14:30                                 ` Stefan Hajnoczi
  0 siblings, 1 reply; 44+ messages in thread
From: Wouter Verhelst @ 2014-10-31 18:15 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Florian Weimer, qemu-devel, libvir-list, mprivozn, nbd-general,
	Markus Armbruster, Richard W.M. Jones, Paolo Bonzini, Max Reitz

Hi,

On Thu, Oct 30, 2014 at 10:40:56AM +0000, Stefan Hajnoczi wrote:
> On Sat, Oct 25, 2014 at 12:43:35PM +0200, Wouter Verhelst wrote:
> > I haven't seen a reply to this anymore. Do people still have comments?
> > I'm planning on doing a release of nbd later this weekend, and would
> > like to include this (not the TLS implementation yet, but at least the
> > spec)
> 
> Hi Wouter,
> From https://github.com/yoe/nbd/blob/tlsspec/doc/proto.txt:
> 
>   * NBD_OPT_STARTTLS (5)
> 
>   The client wishes to initiate TLS. XXX Data.
> 
> Is there text missing for "XXX Data"?

Ah, ehm, oops. Yes :-)

That was meant to be a reminder that I hadn't given that any thought
yet. The idea was that maybe we could use the "data" field in the
STARTTLS message to send something to initiate the TLS communication. If
the server rejects TLS, then that data is lost, but otherwise it might
be useful.

OTOH, it could be too complicated to implement.

> Also, I suggest at least developing a prototype before releasing the
> specification changes.  Issues that were unknown ahead of time might be
> discovered during development.

Yeah, that's fair enough.

> Why the rush to release specification changes?

Not really a rush, I just thought it might be useful. But I suppose
you're right.

Note that I'm not likely to be implementing this "soon". I haven't got
much time right now, and it would be my first time to implement
something which uses TLS; so I would need to do some research in that
area first.

-- 
It is easy to love a country that is famous for chocolate and beer

  -- Barack Obama, speaking in Brussels, Belgium, 2014-03-26

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [Qemu-devel] [Nbd]  spec, RFC: TLS support for NBDµ
  2014-10-31 18:15                               ` Wouter Verhelst
@ 2014-11-03 14:30                                 ` Stefan Hajnoczi
  0 siblings, 0 replies; 44+ messages in thread
From: Stefan Hajnoczi @ 2014-11-03 14:30 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Florian Weimer, qemu-devel, libvir-list, mprivozn, nbd-general,
	Markus Armbruster, Richard W.M. Jones, Paolo Bonzini, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 960 bytes --]

On Fri, Oct 31, 2014 at 07:15:08PM +0100, Wouter Verhelst wrote:
> On Thu, Oct 30, 2014 at 10:40:56AM +0000, Stefan Hajnoczi wrote:
> > Also, I suggest at least developing a prototype before releasing the
> > specification changes.  Issues that were unknown ahead of time might be
> > discovered during development.
> 
> Yeah, that's fair enough.
> 
> > Why the rush to release specification changes?
> 
> Not really a rush, I just thought it might be useful. But I suppose
> you're right.
> 
> Note that I'm not likely to be implementing this "soon". I haven't got
> much time right now, and it would be my first time to implement
> something which uses TLS; so I would need to do some research in that
> area first.

Hi Wouter,
That's fine, thanks for putting together the draft spec.

If someone I work with starts implementing NBD TLS we'll follow your
spec and continue developing/discussing on the mailing list.

Thanks,
Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2014-11-03 14:31 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-03 16:44 [Qemu-devel] NBD TLS support in QEMU Stefan Hajnoczi
2014-09-04 14:19 ` Benoît Canet
2014-09-04 14:34   ` [Qemu-devel] [libvirt] " Daniel P. Berrange
2014-09-04 15:04     ` Benoît Canet
2014-09-04 15:45       ` Stefan Hajnoczi
2014-09-04 15:54     ` John Snow
2014-09-04 22:07   ` [Qemu-devel] " Wouter Verhelst
2014-09-04 22:54     ` Benoît Canet
2014-09-05  8:42       ` Wouter Verhelst
2014-09-05 12:15       ` Stefan Hajnoczi
2014-09-04 22:02 ` Wouter Verhelst
2014-09-05  8:13   ` Daniel P. Berrange
2014-09-05  8:34     ` Wouter Verhelst
2014-09-05 12:21   ` Stefan Hajnoczi
2014-09-05  6:23 ` [Qemu-devel] [libvirt] " Michal Privoznik
2014-09-05  8:10   ` Daniel P. Berrange
2014-09-05  8:46 ` [Qemu-devel] " Hani Benhabiles
2014-09-05 12:31   ` Stefan Hajnoczi
2014-09-05 13:26   ` Wouter Verhelst
2014-10-01 20:23     ` Wouter Verhelst
2014-10-02 11:00       ` Paolo Bonzini
2014-10-02 13:50         ` Wouter Verhelst
2014-10-08 18:16           ` Wouter Verhelst
2014-10-09 12:42             ` Paolo Bonzini
2014-10-02 11:05       ` Daniel P. Berrange
2014-10-02 11:28         ` Paolo Bonzini
2014-10-17 22:03           ` [Qemu-devel] spec, RFC: TLS support for NBD Wouter Verhelst
2014-10-18  6:33             ` Richard W.M. Jones
2014-10-20  7:58               ` Daniel P. Berrange
2014-10-20  9:56                 ` Stefan Hajnoczi
2014-10-20 11:51                   ` Markus Armbruster
2014-10-20 11:56                     ` Florian Weimer
2014-10-20 12:48                       ` Richard W.M. Jones
2014-10-20 22:10                       ` Wouter Verhelst
2014-10-21  9:35                         ` Daniel P. Berrange
2014-10-21 18:02                           ` Wouter Verhelst
2014-10-20 12:08                     ` Daniel P. Berrange
2014-10-20 21:53                     ` [Qemu-devel] spec, RFC: TLS support for NBDµ Wouter Verhelst
2014-10-21  8:17                       ` Markus Armbruster
2014-10-21 18:30                         ` Wouter Verhelst
2014-10-25 10:43                           ` [Qemu-devel] [Nbd] " Wouter Verhelst
2014-10-30 10:40                             ` Stefan Hajnoczi
2014-10-31 18:15                               ` Wouter Verhelst
2014-11-03 14:30                                 ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.