Some comments on draft-ietf-tsvwg-sctp-strrst-05.txt
Gorry Fairhurst <gorry@erg.abdn.ac.uk> Wed, 08 September 2010 09:05 UTC
Return-Path: <gorry@erg.abdn.ac.uk>
X-Original-To: tsvwg@core3.amsl.com
Delivered-To: tsvwg@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 43D983A67F7 for <tsvwg@core3.amsl.com>; Wed, 8 Sep 2010 02:05:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -101.162
X-Spam-Level:
X-Spam-Status: No, score=-101.162 tagged_above=-999 required=5 tests=[AWL=-1.333, BAYES_40=-0.185, SARE_SUB_6CONS_WORD=0.356, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Amo1abBao9Vp for <tsvwg@core3.amsl.com>; Wed, 8 Sep 2010 02:05:40 -0700 (PDT)
Received: from erg.abdn.ac.uk (dee.erg.abdn.ac.uk [IPv6:2001:630:241:204:203:baff:fe9a:8c9b]) by core3.amsl.com (Postfix) with ESMTP id 5CDFD3A6782 for <tsvwg@ietf.org>; Wed, 8 Sep 2010 02:05:38 -0700 (PDT)
Received: from Gorry-Fairhursts-MacBook-Pro.local (ra-gorry.erg.abdn.ac.uk [139.133.204.42]) (authenticated bits=0) by erg.abdn.ac.uk (8.13.4/8.13.4) with ESMTP id o88961pN012675 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT) for <tsvwg@ietf.org>; Wed, 8 Sep 2010 10:06:01 +0100 (BST)
Message-ID: <4C8751F9.5000708@erg.abdn.ac.uk>
Date: Wed, 08 Sep 2010 10:06:01 +0100
From: Gorry Fairhurst <gorry@erg.abdn.ac.uk>
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.8) Gecko/20100802 Thunderbird/3.1.2
MIME-Version: 1.0
To: tsvwg@ietf.org
Subject: Some comments on draft-ietf-tsvwg-sctp-strrst-05.txt
Content-Type: text/plain; charset="windows-1252"; format="flowed"
Content-Transfer-Encoding: 8bit
X-ERG-MailScanner: Found to be clean
X-ERG-MailScanner-From: gorry@erg.abdn.ac.uk
X-BeenThere: tsvwg@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Transport Area Working Group <tsvwg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/tsvwg>, <mailto:tsvwg-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/tsvwg>
List-Post: <mailto:tsvwg@ietf.org>
List-Help: <mailto:tsvwg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tsvwg>, <mailto:tsvwg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 08 Sep 2010 09:05:41 -0000
I've read draft-ietf-tsvwg-sctp-strrst-05.txt, since this document has been suggested as nearing completion. I have some comments: In this version of the draft, I was unable to check the details of the specification, because I didn't find the current language sufficeintly tight. This is therefore a non-chair review, intended to help preparation of the next revision of this document. I'd expect the WG to comment on the protocol details. I also plan to review the next version. Gorry (not wearing a chair hat this time!) ---- The document speaks of/maximum segment lifetime/ - is this the IP-level MSL or something more specific? (MSL is a rather imprecise concept in the current IETF). --- Introduction needs to be clearer - please check English and that this is clear about what the document describes and how this may be used. --- Section 3 (NiT) - This section should assume IANA makes the assignment and therefore simply say "the chunk type assigned by IANA for a STREAM RESET" or "the value XXTBDXX" - with a note to IANA to replace the string with the assigned type. --- sect 3, point 8 - is the error value unambiguous? can you say what the code is, or refer to the section number in an RFC that defines appropriate values? --- Section 4.1 Stream Reset Request Sequence Number: 4 bytes (unsigned integer) /It is increased by 1./ - when? Is this once for each request? - please specify. Stream Number N: 2 bytes (unsigned integer) / This optional field, if included, is used to indicates specific streams that are to be reset./ - Can this field be present several times in the same request? - How does the receiver know this? --- Section 4.2 - Same two questions as in section 4.1 above. --- Section 4.2 Stream Reset Request Sequence Number: 4 bytes (unsigned integer) /It is increased by 1./ - when? Is this once for each request? --- Section 4.5 /This field holds the length of the parameter, a fixed value of 12 MUST be found in this field./ Suggest: /This field holds the length of the parameter, this MUST be a fixed value of 12./ When is Stream Reset Request Sequence Number increased (as before). /Number of new streams: 2 bytes (unsigned integer) This value holds the number of additional outgoing streams the sender would like added to the association./ - /would like added/? or /requests to be added/? / This parameter can appear in a STREAM RESET chunk. This parameter/ ^^^ - is this /MAY/? --- Section 5.1.1 para 2 If the chunk type of the STREAM_RESET chunk does NOT appear in the ^^^ - This does not seem to be a correct use of a keyword, could you please check? /with an appropriate error indicating the peer SCTP stack does not support the stream reset extension./ - What is the appropriate error code? --- Section 5.1.2 /The following steps MUST be followed:/ - This does not seem an appropriate use of MUST. /it is unknown as to if/ - it is not known whether /will accept or deny it/ - is the word /deny/ or /reject/ or /not accept/ ? /A3: If this Outgoing SSN Reset Request Parameter is sent in response to an Incoming SSN Request Parameter, the Stream Numbers between the Outgoing and the Incoming parameters MUST match. / - I don't understand the MUST. - What happens if they do not match? I suspect the wording needs to be changed to say, if this xxx is received, the receiver MUST , otherwise it MUST do yyy. A.5 /no Stream Numbers MUST be put into the Outgoing SSN Reset Request Parameter/ - This does not seem a requirement,please use lower case /must/. /If the sender wants only some outgoing streams to be reset these Stream Numbers MUST be filled in the Outgoing SSN Reset Request Parameter./ - perhaps should be something like: /If the sender requests only some outgoing streams to be reset the set of Stream are identified in the Outgoing SSN Reset Request Parameter./ / It MAY be put together with an Incoming SSN Reset Request Parameter or an Stream Reset Response Parameter and MUST NOT be put together with any other parameter./ - I am not sure exactly what this means. - Please check section 5.1.3, and 5.1.4, for similar cases. ----- / There SHOULD NOT be sent more than one SSN/TSN Reset Request within a maximum segment lifetime./ - where is the maximum segment lifetime defined, if this is the IP MSL, then I suggest an explicit time should be declared. This could be the MSL value, is this seems appropriate. ---- 5.1.5 / D2: The result of the processing of the incoming request is filled/ - I'm not sure what this means, is it the resulting value, or return code or what? --- / D3: If the incoming request is a SSN/TSN reset requests, / ^ - delete /s/ --- /is filled with the next TSN the sender of this Stream Reset Response Parameter will assign/ - perhaps clearer as /the next TSN value to be assigned by the sender of this Stream Reset Response Parameter/ --- /the Sender's next TSN field is not filled./ - SHOULD be zero? - or unchanged or what? --- D4 - This procedure seems under-specified. /for this is the highest TSN it has seen plus some delta. / - I get the idea, but this is not much help to an implementor, can we use some SHOULD and MUSTs to guide this? - How big is delta - or how do you choose? --- 5.1.6 /it is able to send to/ suggest: /to which it is able to send/ --- / Note that new streams are added adjacent to the previous steams with no gaps/ - no gaps? - in what space, please clarify - does this mean streams are numbered consecutively? --- / Any new stream MUST number its first message to be stream sequence 0./ -/Any/A/ - I'm not sure what this sentence is about, is this the SCTP sender action for a new stream, what is being numbered? --- 5.2.1 / Upon reception of a Stream Reset Chunk each parameter within it should be processed. If some parameters have to be sent back, they MUST all be put into one STREAM RESET chunk./ - The language here needs to be a protocol spec, not a narrative. Please use some SHOULD and MUSTs to specify this. --- /the same STREAM RESET chunk has to be sent back in response as earlier./ - The language here needs to be a protocol spec, not a narrative. Please use some SHOULD and MUSTs to specify this. --- / formed. If for whatever reason the endpoint does NOT wish to process/ ^^^ - I think this should be lower case, it does not seem like a protocol action. --- 5.2.2. Receiver Side Procedures for the Outgoing SSN Reset Request Parameter In the case that the endpoint is willing to perform a stream reset the following steps SHOULD be followed: ^^^^^^^ - This SHOULD conflicts with /MUST/ later in the same section. Either the actions are enquired or recommended, please refine use of RFC 2119 keywords in this section. - This also applies 5.2.3, and 5.2.4! - 5.2.6 has the opposite problem, it requires a recommended procedure. - It may be best to omit the keyword in the introduction and ensure each protocol action is clearly called-out as recommended, optional, or required using MUST, SHOULD or MAY. --- E1: /NOT/not/ --- E2: /If the Stream Reset Timer is running for the Stream Reset Request Sequence Number indicated in the Stream Reset Response Sequence Number field, mark the Stream Reset Request Sequence Number as acknowledged. If all Stream Reset Request Sequence Numbers the Stream Reset Timer is running for are acknowledged, stop the Stream Reset Timer./ - This and following sections need to be more concrete and use RFC 2119 language. --- 5.2.5 - what is the action if not successful? --- 5.3 /Various Examples of the Stream Reset procedures/ - omit Various? ---- - please include one sentence explaining the diagram, saying what E-Z is, and saying that time progresses in a downwards direction. --- 6 - I'd have changed /SHOULD/should/ - it is not a specific protocol requirement. You may even prefer to say /needs to be extended/ ? Overall: --- - I have a question on section 6: - What is the intended status of this API change? /The suggested value of this field for IANA is/ ---- 7 Please further consider the risks and countermeasures. It appears to me there is no ambiguity at the application in the re-use of a stream, can you add text to explain why this does not result in applications problems by receiving the numbered data twice I see the point on the abort… but if I understand, this method can also be used to create extra state for "rogue" flows, and therefore this is a vulnerability that may need to be countered, and is different to aborting a connection. Reseting a stream "breaks" the connection, but can it leave state? How do the end points detect this? etc. - I'm not suggesting these are show-stoppers, but they could be points that may require a robust implementation to take precautions. Does the "maximum segment lifetime" mitigate this? ---- 8.1,8.2 - /The value MUST be from/IANA shoal assign this value from/ --- References - Normative reference needed to Chunk Types ID. - The current text will require a NORMATIVE reference to the API, since this is a standards-track document that updates this. This could be relaxed if the API were explicitly stated as an "informative" section or an informative annexe. Either way though, this looks like it may need APPs area review, in the same way that the API will need this - so there is a logical dependency at least for getting the reviews completed. --- The document, I suggest could benefit from a little explanation of WHY this is OK for SCTP and would be difficult for TCP. This understanding may be clear to the SCTP community, but I do not think it is necessarily clear to a TSV reviewer or the IETF in general. I discussed this with Randal informally - things like "how do you reset a stream sequence number during and avoid problems with older mis-sequenced in-flight segments?" (i.e. explain how this works in SCTP); "how do you avoid blind replay attacks (i.e. again explain how SCTP has a way to protect this); ---
- Some comments on draft-ietf-tsvwg-sctp-strrst-05.… Gorry Fairhurst
- Re: Some comments on draft-ietf-tsvwg-sctp-strrst… Michael Tüxen
- Re: Some comments on draft-ietf-tsvwg-sctp-strrst… Gorry Fairhurst
- Re: Some comments on draft-ietf-tsvwg-sctp-strrst… Michael Tüxen