Re: [Qemu-devel] [Nbd] [PATCHv8] Improve documentation for TLS

[Wouter Verhelst <w@uter.be>]

On Mon, Apr 11, 2016 at 09:34:44PM +0100, Alex Bligh wrote:
> Eric,
> 
> On 11 Apr 2016, at 21:14, Eric Blake <eblake@redhat.com> wrote:
> > Current qemu NBD server implementation does NOT send a reply to
> > NBD_OPT_ABORT, but immediately closes the connection. I don't know if
> > that is a bug in qemu (especially given the discussion on NBD_CMD_DISC),
> > but it is an independent issue from TLS documentation, so may be better
> > discussed on that thread.
> 
> Ha, neither does mine, despite my reading of the protocol being
> that it should.
> 
> Reference nbd-server.c doesn't either.

Indeed. That may have been a bad idea.

> > Likewise, current qemu NBD client implementation does NOT send
> > NBD_OPT_ABORT at all, so it's hard to say whether waiting around for a
> > reply is worthwhile.
> 
> :-)
> 
> nbd-client.c only appears to send it after asking for a list,
> and not in any error conditions.

Well, it was added together with NBD_OPT_LIST, and with fixed newstyle (it was
my test case for implementing fixed newstyle ;-)

> >> Obviously NBD_OPT_ABORT and aborting the connection needs
> >> more clearing up, but I'm loathe to do it in the TLS patch.
> >> 
> >> In order not to make things worse, how about:
> >> 
> >>> There is no requirement for the client or server to complete a
> >>> negotiation if it does not wish to do so. Either end may simply
> >>> close the TCP connection (though see below re prior use
> > 
> > Not sure if the use of "re" is ideal (are you abbreviating for "regarding")?
> 
> OK will fix that if Wouter likes the words.
> 
> >>> of NBD_OPT_ABORT). Under certain circumstances either
> >>> the client or the server may be required by this document to close
> >>> the TCP connection. In each case, this is referred to as 'terminate
> >>> the session'.
> >>> 
> >>> If the client wishes to terminate the session in the negotiation
> >>> phase, and is not doing so because it is required to do so
> >>> by this document, it SHOULD send NBD_OPT_ABORT first if the protocol
> >>> permits. There are instances where this is impossible, such as after
> >>> an NBD_OPT_EXPORTNAME has been issued, or on an unsuccessful
> >>> negotiation of TLS.  For instance, if the client does not find an
> >>> export it is looking for, it may simply send an NBD_OPT_ABORT
> >>> and close the TCP connection.
> > 
> > Otherwise, this seems reasonable, other than the fact that qemu needs
> > patches to actually start sending NBD_OPT_ABORT where possible.
> 
> I'd suggest waiting for a definitive answer on whether it's meant
> to have a reply.

Clearly from reading the code it wasn't meant to, at the time.

That doesn't mean OPT_ABORT not having a reply is necessarily a good
idea. Since it's only used by reference nbd-client in just one use case
at this point, I don't think it's particularly bad to change the
definition to say that the server SHOULD send a reply (NBD_REP_ACK),
upon which the server drops the connection.

The client should probably wait for that too, and not close its socket
until either it gets a zero read (indicating that the server closed it
already) or it gets an NBD_REP_ACK from the NBD_OPT_ABORT message.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12