On 04/17/2016 11:31 AM, Alex Bligh wrote:
> Currently the protocol permits options to be sent even when there
> are outstanding replies, and for the server to process those
> options out of order. This causes a number of issues.
>
> If option replies remain outstanding when NBD_OPT_STARTTLS is issued
> then those replies may arrive unencrypted when NBD_OPT_STARTTLS
> is processed. Similarly options sent after NBD_OPT_STARTTLS is
> issued (but not replied to) may be sent unencrypted but are
> expected to be encrypted. This is difficult to track server side.
> This needs fixing in any case, and it's better to fix it for
> all options.
>
> Few (if any) known clients issue options without waiting for replies,
> and few (if any) servers do anything other than process replies.
> So the ability to send options without waiting for replies gives
> few benefits. It also complicates the specification, and (as
> it is so little tested) is a ripe field for subtle bugs where the
> protocol authors assume that a reply to a given option has been
> received before the client sends another option.
>
> Without this serialisation, it is not possible to effectively
> enforce things like "The client MUST NOT send NBD_OPT_X if
> the server has previously replied NBD_REP_Y" because the
> client can send NBD_OPT_X before receiving the reply.
>
> Signed-off-by: Alex Bligh <alex@...872...>
> ---
> doc/proto.md | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> This one is my preferred fix.
Also my preferred fix; my recent patch proposal for
NBD_REP_ERR_BLOCK_SIZE_REQD is yet another case where strict lockstep
between client requests and server responses makes life easier to reason
about on whether a server will be using the error message in response to
NBD_OPT_INFO, based on whether it has already seen NBD_OPT_BLOCK_SIZE.
Reviewed-by: Eric Blake <eblake@...696...>
>
> diff --git a/doc/proto.md b/doc/proto.md
> index 24ab813..07f18b0 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -194,12 +194,10 @@ S: 32 bits, length of the reply. This MAY be zero for some replies, in
> S: any data as required by the reply (e.g., an export name in the case
> of `NBD_REP_SERVER`)
>
> -As there is no unique number for client requests, clients who want to
> -differentiate between answers to two instances of the same option during
> -any negotiation must make sure they've seen the answer to an outstanding
> -request before sending the next one of the same type. The server MAY
> -send replies in the order that the requests were received, but is not
> -required to.
> +The client MUST NOT send any option until it has received a final
> +reply to any option it has sent (note that some options e.g.
s/to any/to any earlier/ ?
> +`NBD_OPT_LIST` have multiple replies, and the final reply is
> +the last of those).
And if we make this change, there are other simplifications we should make:
diff --git i/doc/proto.md w/doc/proto.md
index 41e33cd..0bff188 100644
--- i/doc/proto.md
+++ w/doc/proto.md
@@ -179,8 +179,6 @@ To fix these two issues, the following changes were
implemented:
field too, though its side of the protocol does not change incompatibly.
- The client MAY now send other options to the server as appropriate, in
the generic format for sending an option as described above.
-- The server MUST NOT send a response to `NBD_OPT_EXPORT_NAME` until all
- other pending option requests have had their final reply.
- The server will reply to any option apart from `NBD_OPT_EXPORT_NAME`
with reply packets in the following format:
@@ -258,7 +256,7 @@ error inbound options until the client gets the hint
that it is
unwelcome, except that if a server believes a client's behaviour
constitutes a denial of service, it MAY initiate a hard disconnect.
If the server is in the process of being shut down it MAY
-error inflight options and SHOULD error further options received
+error any inflight option and SHOULD error further options received
(other than an `NBD_OPT_ABORT`) with `NBD_REP_ERR_SHUTDOWN`.
If the client receives `NBD_REP_ERR_SHUTDOWN` it MUST initiate
plus simplifications to NBD_OPT_GO:
diff --git i/doc/proto.md w/doc/proto.md
index 8ef339c..c85e0aa 100644
--- i/doc/proto.md
+++ w/doc/proto.md
@@ -949,11 +949,7 @@ of the newstyle negotiation.
with `NBD_REP_ACK`), the client and the server both immediately
enter the transmission phase. The server MUST NOT send any zero
padding bytes after the `NBD_REP_ACK` data, whether or not the
- client negotiated the `NBD_FLAG_C_NO_ZEROES` flag. The server MUST
- NOT send the final `NBD_REP_ACK` reply until all other pending
- option replies have been sent by the server, and a client MUST NOT
- send any further option requests after `NBD_OPT_GO` unless it
- first receives an error reply.
+ client negotiated the `NBD_FLAG_C_NO_ZEROES` flag.
- `NBD_OPT_GO` (7)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature