Re: [Nbd] [PATCH] doc: Document maximum message length during option haggling

[Alex Bligh <alex@...872...>]

Eric,

I'm not convinced this fixes anything.

You're now saying you MAY treat anything over 16k as a
denial of service attack. But in the existing text, you
MAY do that anyway - you don't have to treat all lengths
alike for all commands.

Alex

On 11 May 2016, at 16:52, Eric Blake <eblake@...696...> wrote:

> In the transmission phase, large lengths make sense for
> NBD_CMD_READ/WRITE, both because the client really does
> want to access large blocks of data, and because requests
> can be serviced out of order so that a large request need
> not hold up a subsequent small request.
> 
> But these arguments do not apply during option haggling.
> First, we are NOT handling arbitrary sizes at the client's
> whim, where economies of scale matter, but instead dealing
> with known sizes based on the specifications of the
> individual options and replies (our worst case is currently
> NBD_REP_SERVER, which can contain up to 8196 bytes based on
> our hard maximum of 4096 bytes per string).  Second, the
> option requests are synchronous, so one side cannot
> send another message until it has first processed all the
> data from the previous message.  Even if the request or
> reply is unknown, forcing the remote side to read to the
> end of a huge unknown message before it can continue may be
> a waste of time compared to the remote side considering the
> sender to be compromised and just disconnecting.
> 
> Document a restriction that we intend all future defined
> option requests and replies to fit with 16k, and that either
> client or server may initiate a hard disconnect on a size
> larger than that.
> 
> Since this means that the upper two bytes of the length
> field should always be 0, we may want to repurpose those bytes
> in a future version of the protocol; for example, if we find
> ourselves needing flags along with option requests, we could
> break the 32-bit length field into a 16-bit flags and 16-bit
> length (similar to how we broke the 32-bit request type into
> 16-bit flag and 16-bit request for the transmission phase).
> But doing so would cause any older recipient to treat non-zero
> flags as a rather large length, so document that such a change
> in the protocol should also be accompanied by a new global
> flag and client response to make it clear that both sides agree
> on the new interpretation of those two bytes.  Also note that
> we could introduce flags by breaking up the 32-bit option type
> field rather than the length field (although then we'd have to
> be careful about whether an NBD_REP message could have different
> flags than those requested by the client).
> 
> Signed-off-by: Eric Blake <eblake@...696...>
> ---
> doc/proto.md | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 2956746..0daed51 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -137,9 +137,18 @@ C: 32 bits, option
> C: 32 bits, length of option data (unsigned)  
> C: any data needed for the chosen option, of length as specified above.  
> 
> -The presence of the option length in every option allows the server
> -to skip any options presented by the client that it does not
> -understand.
> +The presence of the option length in every option allows the server to
> +skip any options presented by the client that it does not understand.
> +Although *length* is a 32-bit field, none of the options defined in
> +this standard include more than one string (which is capped at 4096
> +bytes); and extensions should likewise limit the amount of data the
> +client can send for a single valid option.  Therefore, the server MAY
> +treat any option request with *length* larger than 16,384 as a denial
> +of service attack, and initiate a hard disconnect.  If needed, a
> +future version of the standard might introduce a global flag and
> +client response flag that would allow the client and server to replace
> +the 32-bit length field with a pair of 16-bit flags and 16-bit length
> +fields.
> 
> If the value of the option field is `NBD_OPT_EXPORT_NAME` and the server
> is willing to allow the export, the server replies with information
> @@ -192,6 +201,19 @@ 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`)  
> 
> +Although *length* is a 32-bit field, none of the option replies
> +defined in this standard include more than two strings (which are
> +capped at 4096 bytes each); and extensions should likewise limit the
> +amount of data the server can send for a single option reply.
> +Therefore, the client MAY treat any option reply with *length* larger
> +than 16,384 as a corrupt server, and initiate a hard disconnect (this
> +limit applies only to a single option reply, and not to the cumulative
> +length of a series of replies to a single option request).  If needed,
> +a future version of the standard might introduce a global flag and
> +client response flag that would allow the client and server to replace
> +the 32-bit length field with a pair of 16-bit flags and 16-bit length
> +fields.
> +
> 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.
> `NBD_OPT_LIST` have multiple replies, and the final reply is
> -- 
> 2.5.5
> 
> 
> ------------------------------------------------------------------------------
> Mobile security can be enabling, not merely restricting. Employees who
> bring their own devices (BYOD) to work are irked by the imposition of MDM
> restrictions. Mobile Device Manager Plus allows you to control only the
> apps on BYO-devices by containerizing them, leaving personal data untouched!
> https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
Alex Bligh