On 11 May 2016, at 21:51, Eric Blake <eblake@...696...> wrote: > On 05/11/2016 02:20 PM, Alex Bligh wrote: >> >> On 11 May 2016, at 20:08, Eric Blake <eblake@...696...> 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
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail