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 <[email protected]> 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 <[email protected]> > --- > 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 > [email protected] > https://lists.sourceforge.net/lists/listinfo/nbd-general > -- Alex Bligh ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
