On 04/12/2016 01:31 PM, Alex Bligh wrote:
> Improve the documentation as per the mailing list discussion.
> Here's what we deciced (broadly).
s/deciced/decided/
>
> * One side MAY drop the connection if the other end violates a
> MUST condition.
>
> * The server MUST drop the connection in the 'no way out' situations
> during the negotiation phase (error on NBD_OPT_EXPORT_NAME, error
> in negotiating text).
negotiating text, or negotiating TLS?
>
> * The server SHOULD NOT otherwise drop the connection. It can wait
> and error the next command. Clearly there are situations where
> this is going to happen (e.g. server shutdown).
>
> * If the server does need to drop the connection, it SHOULD wait
> until there are no commands in-flight in transmission mode,
> it possible.
>
> * If he client is going to drop the the connection, then other
s/he/the/
s/the the/the/
> than in the event of a protocol violation or a 'no way out'
> situation (e.g. TLS negotiation fails), it MUST use NBD_CMD_DISC
> or NBD_OPT_ABORT
>
> * We should tidy up the semantics and descriptions of NBD_CMD_DISC
> and NBD_OPT_ABORT, viz replies or not to the latter, shutting
> down TLS properly etc.
>
> Other changes:
>
> * Added a reply to NBD_OPT_ABORT. No harm if the client closes
> the connection anyway.
>
> * Said the offset and length fields in NBD_CMD_DISC MUST be zero.
> Not doing so is a protocol violation and would only lead to ...
> the connection being closed, so this is a useful tidy up.
>
> * Introduced consistent terminology for disconnection throughout.
>
> This patch applies on top of:
> 0001-docs-proto.md-Clarify-SHOULD-MUST-MAY-etc v7
>
> Signed-off-by: Alex Bligh <alex@...872...>
> ---
> doc/proto.md | 143 ++++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 107 insertions(+), 36 deletions(-)
>
> +#### Termination of the session during option haggling
> +
> +There are three possible mechanisms to end option haggling:
> +
> +* Transmission mode can be entered (by the client sending
> + `NBD_OPT_EXPORT_NAME` or by the server responding to an
> + `NBD_OPT_GO` with `NBD_REP_ACK`). This is documented
s/ACK/SERVER/
(although I may bite the bullet and create a new NBD_REP_INFO if we want
the name to be optional, since NBD_OPT_[INFO/GO] is still experimental,
as part of my rework on block size information)
> + elsewhere.
> +
> +* The client can send (and the server can reply to) an
> + `NBD_OPT_ABORT`. This MUST be followed by the client
> + shutting down TLS (if it is running), and the client
> + dropping the connection. This is referred to as
> + 'initiating a soft disconnect'; soft disconnects can
> + only be initiated by the client.
> +
> +* The client or the server can disconnect the TCP session
> + without activity at the NBD protocol level. If TLS is
> + negotiated, the party intitiating the transaction SHOULD
s/intitiating/initiating
> + shutdown TLS first if it is running. This is referrred
s/referrred/referred/
> + to as 'initiating a hard disconnect'.
> +
> +This section concerns the second and third of these, together
> +called 'terminating the session', and under which circumstances
> +they are valid.
> +
> +If either the client or the server detects a violation of a
> +mandatory condition ('MUST' etc.) by the other party, it MAY
> +initiate a hard discconect.
s/discconect/disconnect/
> +
> +A client MAY use a soft disconnect to terminate the session
> +whenever it wishes, provided that there are no outstanding
> +replies to options.
Why the disclaimer on no outstanding replies?
> +
> +A party that is mandated by this document to terminate the
> +session MUST initiate a hard disconnect if it is not possible
> +to use a soft disconnect. Such circumstances include: where
> +that party is the server and it cannot return an error
> +(e.g. after an `NBD_OPT_EXPORT_NAME` it cannot satisfy),
> +and where that party is the client following a failed TLS
> +negotiation.
> +
> +A party MUST NOT initiate a hard disconnect save where set out
> +in this section. Therefore, unless a client's situation falls
> +within the provisions of the previous paragraph, it MUST NOT
> +use a hard disconnect, and hence its only option to terminate
> +the session is via a soft disconnect.
> +
> 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 regarding prior use
> -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.
> +terminate the session. In the client's case, if it wishes to
> +do so it MUST use soft disconnect. In the server's case it
> +MUST (save where set out above) simply error inbound options until
> +the client gets the hint that it is unwelcome.
so basically wait for either the client to give up and close first, or
for the client to do something that is provably in violation of a MUST
in the protocol so the server can close the connection. Can a malicious
client abuse this requirement to tie up a server as a denial of service?
>
> ### Transmission
>
> @@ -225,8 +263,9 @@ the simple reply, and the experimental structured reply chunk. The
> transmission phase consists of a series of transactions, where the
> client submits requests and the server sends corresponding replies
> with either a single simple reply or a series of one or more
> -structured reply chunks per request. The phase continues until either
> -side closes the connection.
> +structured reply chunks per request. The phase continues until
> +either side terminates transmission; this can be performed cleanly
> +only by the client.
>
> Note that without client negotiation, the server MUST use only simple
> replies, and that it is impossible to tell by reading the server
> @@ -309,6 +348,35 @@ S: (*length* bytes of data if the request is of type `NBD_CMD_READ`)
> This reply type MUST NOT be used except as documented by the
> experimental `STRUCTURED_REPLY` extension; see below.
>
> +#### Terminating the transmission phase
> +
> +There are two methods of terminating the transmission phase:
> +
> +* The client sends `NBD_CMD_DISC` whereupon the server MUST
> + close down the TLS session (if one is running) and then
> + close the TCP connection. This is referred to as 'initiating
> + a soft disconnect'. Soft disconnects can only be
> + initiated by the client.
> +
> +* The client or the server drops the TCP session (in which
> + case it SHOULD shut down the TLS session first). This is
> + referred to as 'initiating a hard disconnect'.
> +
> +Together these are refrred to as 'terminating transmission'.
s/refrred/referred/
> +
> +Either side MAY initiate a hard disconnect if it detects
> +a violation by the other party of a mandatory condition
> +within this document.
> +
> +On a server shutdown, the server SHOULD wait for inflight
> +requests to be serviced prior to initiating a hard disconnect.
Maybe a mention that the server MAY use error replies to speed up the
processing of those requests, even if the command would normally succeed
if termination weren't pending?
> +
> +The client MAY issue a soft disconnect at any time, but
> +MUST wait until there are no inflight requests first.
Why MUST and not SHOULD? Didn't Wouter have an example of a client that
batches up its entire request sequence, including NBD_CMD_DISC, and
sends that in bulk before waiting for any server replies? I thought the
goal was that the server MUST NOT react to NBD_CMD_DISC until all other
pending requests have been dealt with, but don't necessarily see the
reason why the client MUST NOT send NBD_CMD_DISC while requests are
inflight.
> +
> +The client and the server MUST NOT initiate any form
> +of disconnect other than in one of the above circumstances.
> +
> ## TLS support
>
> The NBD protocol supports Transport Layer Security (TLS) (see
> @@ -665,7 +733,7 @@ receiving the handshake flags from the server.
> set `NBD_FLAG_NO_ZEROES`. If set, the server MUST NOT send the 124
> bytes of zeroes at the end of the negotiation.
>
> -Clients MUST NOT set any other flags; the server MUST drop the
> +Clients MUST NOT set any other flags; the server MUST drop the TCP
> connection if the client sets an unknown flag, or a flag that does
> not match something advertised by the server.
>
> @@ -701,8 +769,8 @@ of the newstyle negotiation.
>
> - `NBD_OPT_ABORT` (2)
>
> - The client desires to abort the negotiation and close the
> - connection.
> + The client desires to abort the negotiation and terminate the
> + session. The server MUST reply with `NBD_REP_ACK`.
Maybe explicitly mention that the client MAY disconnect immediately
rather than waiting to receive the response?
But overall looking like a reasonable improvement.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature