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);

---