On 11 May 2016, at 21:51, Eric Blake <[email protected]> wrote: > On 05/11/2016 02:20 PM, Alex Bligh wrote: >> >> On 11 May 2016, at 20:08, Eric Blake <[email protected]> wrote: >> >>> Calling out a MUCH smaller limit of 16k during handshake phase in >>> contrast to transmission phase, and acknowledging that we have bytes in >>> transmission phase that will always be 0 without some other extension to >>> state otherwise, seems worthwhile. >> >> OK, point taken. I'm convinced in principle of this one. >> >> However, I think 16k is too small if the string limit stays at 4k. That's >> four whole strings. If we are going to support 4k strings, we should >> support 4k strings. I thing it would be pretty hard to effectively >> DoS any server with (e.g.) 1MB. I know the current limit (NBD_REP_SERVER) >> is 8192 bytes plus loose change, but I don't think we should be in a >> position where we need to change this every time a new option comes up. > > At what point do we think a limit is reasonable that no option or option > reply would ever need to exceed a given size?
I went for 1MB > Would 0xffff be > reasonable as the upper limit (15 max-sized strings plus other data > including lengths of those strings)? That way, we still manage to > effectively reserve ourselves 2 bytes in case we ever finding ourselves > wanting to use them for other purposes. Also, you can pack more than 15 > strings, as long as at least one of them is not the maximum length. I don't think reserving bytes is a useful consideration. You're practically guaranteed to run into an issue where some client (or server, as the case may be) reads the 32 bit quantity as a 32 bit quantity. As we're talking about the option haggling phase, adding a few bytes here and there is not the end of the world (nbd survived with 168 bytes of zeroes for many years!) > And remember that the limit is only on single messages; it's still > fairly easy to break things up into multiple messages: if you need to > send an array of strings to the server, do it via back-to-back NBD_OPT_ > calls (one call per array entry) rather than a single NBD_OPT call (with > the entire array). And we already do just that for replies > (NBD_REP_SERVER and NBD_REP_INFO are both arrays of information broken > up into smaller replies). Sure, but as I said, why limit it to an artificially low limit and box yourself in? Best make it the highest we might ever realistically need. >> Also the bit about potentially repurposing bits shouldn't be somewhere >> where it hits the commit log, so I'd drop the last paragraph. > > I thought it made a stronger case for WHY we are explicitly choosing a > value less than 0x10000 as the cutoff, because it leaves us room for > future expansion As per above, I think this is actually a bad reason. >>> +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. >> >> This is the problem above. Let's say someone introduces NBD_OPT_FOO >> which contains 5 strings. > > Even 4 strings is too much for a 16k message - you also have to reserve > space for the lengths of those strings (so a string at 4096 bytes > actually occupies 4098 bytes of the message if you use a minimum 16-bit > string length field; Exactly, that's why I suggested (handwavingly) 1MB. > and with NBD_OPT_GO, we just introduced a 32-bit > string length field even after I pointed out that 16 bits would have > been sufficient, on the grounds that it more closely resembles > NBD_OPT_EXPORT_NAME). Again, I'm more worried about consistency than cheeseparing bits in the negotiation stage. >>> 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, >> >> I think simply as a DoS attack > > In my mind, a DoS is an attack where someone can tie up a long-running > process to the exclusion of others. Surely 'or exhaust memory'? > A client can DoS a server (make the > server do so much work it can't accept other clients), but how does a > server DoS a client (the client isn't trying to accept other > connections)? I considered using the same wording in both places, until > I realized that DoS doesn't make as much sense from server back to > client. But if you're okay with the term in both directions, then I > don't mind using it for consistency. I think the server sending a near-infinite length reply to the client is a DoS. -- Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail
------------------------------------------------------------------------------ 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
