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 <[email protected]> > --- > 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 <[email protected]> > > 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
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Find and fix application performance issues faster with Applications Manager Applications Manager provides deep performance insights into multiple tiers of your business applications. It resolves application problems quickly and reduces your MTTR. Get your free trial! https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
