Re: [Nbd] Thoughts about the handshake protocol

[Wouter Verhelst <w@...112...>]

*sigh*

On Mon, May 30, 2011 at 01:45:27PM +0200, Goswin von Brederlow wrote:
> Hi,
> 
> revently it was mentioned that the new-style handshake has some problems
> and needs some fixing up for the future. So I wanted to collect some
> thoughts about this to maybe spark a redesign. Turned out to describe
> pretty much the complete handshake. Let me know what you think:
> 
> ----------------------------------------------------------------------
> Currently the handshake starts with:
> 
> S: "NBDMAGIC" (as in the old style handshake)
> S: 0x49484156454F5054 (note different magic number)
> S: 16 bits of zero (reserved for future use)
> C: 32 bits of zero (reserved for future use)
> 
> So the server identifies itself as NBD server. The client though does
> not identify itself but just starts sending payload.
> 
> Now what are those 16/32 bit of zero for? Some sort of version number or
> bitfield of options? The problem I have with them is that they allow no
> room for compromise. If the client is not at least as new as the server
> it will not understand all the bits the server sends. The only option
> the client then has it to give up.

No, that's not true. Why would the client have to give up? If it doesn't
understand what a particular bit in the reserved field means, it must
ignore it. If a negotiation happens between a client and a server
whereby the client does not acknowledge understanding of a particular
option that is announced in the server's reserved field, then it's up to
the server to either end the communication, or fall back to old
behaviour. That's the whole point of these reserved bits.

The only kind of 'required' future bits that I could imagine adding are
those that would pertain to authentication. Everything else we can
disable if the client would not support it. And since we're not
encrypting data between the client and the server (and I have no
intention of doing that, either), any authenticated TCP session could
theoretically be hijacked, which makes authentication useless; so I'm
not even sure authentication is ever going to happen.

> Next the option haggling is a good idea. But it lacks a way for the
> server to say when it doesn't understand an option and for the client to
> say there are no more option comming.

We've talked about that, earlier, and there is a solution already that
does not require breaking backwards compatibility.

Now while I don't oppose improving the handshake per se if it can be
done and there is a benefit in doing so, I do oppose breaking backwards
compatibility if it can be avoided.

> ----------------------------------------------------------------------
> So I would like to get a bit more flexibility into this:
> 
> S: "NBDMAGIC" (as in the old style handshake)
> S: 8 byte magic number
> S: 16 bits required features
> S: 48 bits offer optional features

Doing it that way *will* break backwards compatibility, because you're
changing the initial greeting you get from the server which is sent to
the client before the client can even respond. That's *bad*, period.

I've changed the protocol incompatibly once, and I *do* *not* want to do
that again. Yes, I made some mistakes, and it would've been better with
some more review, but unfortunately I didn't get any at the time, so now
we're stuck with it. And that's the end of it.

Also, there's no need to differ between 'optional' and 'required'
features. If there is a required feature, there are several ways in
which the client can inform the server that it understands this new
option (through setting a bit in its reserved field, or through sending
an option message). If that doesn't happen, the server can simply close
the connection.

It might be nice to add a message that the server can send which would
say something like "you haven't acknowledged an option that I require",
or to have a way for the server to say "I don't understand that option".
But beyond that, I don't see the benefit of changing the negotation any
further.

[...]
> A client MUST understand all required features set by the server. For
> now all bits would be reserved and 0 and any incompatible change would
> allocate one bit and set it. Hopefully there won't be any. Bit 15 of
> required features can be reserved to indicate another 8 byte of feature
> bits follow. The client acknowledges that it understands all the
> required features by echoing them back. Again bit 15 of the required
> features would indicate another 8 byte of feature bits follow the reply
> just like in the servers hello.
> 
> Optional features will be set by the server to indicate it supports a
> certain feature (e.g. FUA or FLUSH). The client responds with a bitfield
> indicating those features it wishes to use. Any unknown feature must be
> disabled (set to 0) and this must be safe to do so. Only bits set by the
> server might be returned set by the client. Only those features set by
> both server and client MAY be used.

There's no need for any of that, IMO.

Things I will change include:
- Supporting more options by making NBD_OPT_EXPORT_NAME 'special', as
  discussed previously.
- Setting a flag, which (if the client acknowledges it through its
  reserved field) will allow the server to drop the 124 bytes of zeroes
  (they're useless cruft that I forgot to remove)

Beyond that, I don't see why it would be necessary to change more
things. As long as the client and the server can communicate and
negotiate with one another properly (and they can), there's no need to
change.

-- 
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a