Re: [secdir] secdir review of draft-masotta-tftpexts-windowsize-opt-10

[joel jaeggli <[e-mail]>]

Thanks!  I think these are reasonable suggestions and will confer with
the author.

joel

On 7/14/14 5:32 PM, Waltermire, David A. wrote:
> I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.
>
> Status: Almost ready - There are a few security and other related concerns with this draft summarized below.
>
> The draft defines a method for overriding the default step-wise acknowledgement (ACK) behavior of TFTP. This extension suppresses the standard step-wise ACK response until a negotiated window size of blocks have been sent. A single ACK is then sent indicating that all blocks in the window were transmitted. This results in a general reduction in the number of ACKs over the exchange, allowing for higher transfer rates than using block size negotiation alone (RFC2348).
>
> Security Concerns:
>
> The specific security considerations statement appears to be copied from RFC2347. It acknowledges that TFTP does not have any explicit security mechanism and that the extension does not add any additional security risk beyond the base specification. Instead of copying the text from the other RFCs, this text should be replaced with text that references the security considerations from RFC1350, RFC2347, and probably RFC5405.
>
> Additionally, it seems like this option requires a state machine, involving both the client and server, to track the exchange of blocks within a window to support retransmission of failed blocks. If I am understanding this correctly, it looks like a potential attack vector exists where a malicious client (or server) could cause abnormal consumption of system and network resources through the use of large window sizes across a number of sessions. This malicious actor would need to selectively cause retransmission of blocks that still conform with max retry, timeout, and delay constraints. In part, the second paragraph (and the following example) in the "Congestion and Error Control" section provide some error handling guidance that also addresses this security consideration. At minimum a discussion of this attack vector and a reference back to this explanation should be included in the security considerations section. It would also be valuable to include some discussion on reasonable limits for window sizes, retry, timeout, and delay parameters, or at least some guidelines around determining them based on the characteristics of the data link layer protocol used.
>
> Other concerns:
>
> In the abstract:
>
> - The abstract should mention that this memo extends RFC1350 by adding a new option extension for ... based on RFC2347.
>
> In section "Windowsize Option Specification":
>
> - The text in this section should be updated to make use of RFC2119 capitalized keywords.
> - A specification for the Read and Write requests is provided along with an example. The specification of the corresponding OACK should be provided with the same degree of rigor.
> - The draft describes the ACK behavior for data exchange in the definition of the windowsize "#blocks" field. The actual requirements should be defined using RFC2119 language in a new paragraph after the discussion of option negotiation. This new text should express the actual windowing behavior requirements for implementations of the protocol extension. It should also make explicit which block number to send in the ACK, which I infer to be the last block sent in the window.
>
> General nits:
>
> There are a number of grammatical and punctuation issues throughout the document. Some of these make it more difficult to understand the document. A quick editorial pass through the document should address this concern. I am happy to provide a few specifics/suggestions in this regard if the author would like.
>
> Also, the nits tool reported:
> - An issue with 3 pages being longer than the required 58 lines per page.
> - The abstract contains bracketed references. See previous comment.
> - Whitespace warnings
>
> Regards,
> Dave Waltermire
>
>