[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] structured-reply: Tweak some protocol requirements



On Wed, Oct 18, 2017 at 12:17:58PM -0500, Eric Blake wrote:
> On 10/18/2017 11:19 AM, Wouter Verhelst wrote:
> > On Mon, Oct 16, 2017 at 10:25:24AM -0500, Eric Blake wrote:
> >> Several tweaks noticed while implementing structured reply for qemu:
> >> - Document what a server may do if the client tries to negotiate
> >> structured replies more than once.
> >> - Reformat the paragraph on NBD_CMD_FLAG_DF.
> >> - Mention what a client should do if a server sends an unexpected
> >> structured reply
> >>
> >> Signed-off-by: Eric Blake <eblake@redhat.com>
> >> ---
> 
> >> +++ b/doc/proto.md
> >> @@ -1093,6 +1093,8 @@ of the newstyle negotiation.
> >>        server MUST use structured replies to the `NBD_CMD_READ`
> >>        transmission request.  Other extensions that require structured
> >>        replies may now be negotiated.
> >> +    - `NBD_REP_ERR_INVALID`: Structured replies have already been
> >> +      previously negotiated.
> > 
> > I'm not sure that's really a problem.
> > 
> > If a client wants to do structured replies, the server should ensure
> > that the bit "structured replies are okay" is set. If it's already set,
> > it's already set and we're fine.
> > 
> > The TLS situation is somewhat different; if a client wants to do TLS,
> > the server should fire up the TLS library and initiate the encryption.
> > Once that's already done, you can't do it again. There is no useful
> > response to that.
> > 
> > Having said that though, I don't feel too strongly about it.
> 
> Any way to word it that they server MAY fail with ERR_INVALID on
> duplicate response (but not MUST), and therefore the client SHOULD NOT
> send duplicates (but not MUST NOT)?

Probably best to do something along those lines, yes, although we should
perhaps say it in more generic terms that encompass any message the
client shouldn't have to send more than once.

something like...

"Some messages the client sends instruct the server to change some of
 its internal state. The client SHOULD NOT send such messages more than
 once. If it does, the server MAY fail the message with
 NBD_REP_ERR_INVALID."

?

not sure where exactly to put it though.

> Another consideration - I know at least nbdkit enforces a cap on the
> maximum number of NBD_OPT that a client may send before it must move
> into transmission phase, on the grounds that otherwise, a client can
> cause a denial of service by tying up server state with an infinite
> stream of useless NBD_OPT requests preventing the server from handling
> other well-behaved clients.  Should that be documented in the standard
> somehow (that a server MAY disconnect if NBD_OPT_GO is not received in a
> timely manner, but conversely setting appropriate minimum bounds that it
> MUST accept so that clients need not worry about getting enough
> handshake done)?

I thought we had wording to that effect already?

<checks>

the section "Termination of the session during option haggling" already
has a bit saying "except that if a server believes a client's behaviour
constitutes a denial of service, it MAY initiate a hard disconnect",
which if I recall correctly was added precisely to document the nbdkit
behaviour in this regard.

I suppose we can reword that if you think we can do it better, but let's
not re-document the same behaviour three times? ;-)

[...]

-- 
Could you people please use IRC like normal people?!?

  -- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
     Hacklab


Reply to: