Subject: Re: [tsvwg] Reminder: WGLC for SCTP.bis at midnight UTC on 14th April 2021

From: Magnus Westerlund <[email-masked]>
Date: Fri, 7 May 2021 16:16:45 +0000

Hi,

Sorry for the delay in finishing this review. However, I have now completed it. I don’t think I have reviewed SCTP completely since maybe 2006. Thus, this review reflects the review of just about everything that I have reacted to. Some of these comments the answer will simply be, we can’t do anything without affecting backwards compatibility. However, I think there are some clarifications and improvements that can be made to make a first time reader make less mistakes. 

I did review the HTML version: https://www.ietf.org/archive/id/draft-ietf-tsvwg-rfc4960-bis-11.html

High level comments.

First of all I think the concept behind assignment of TSN should be improved in Section 2. I struggled with several aspects that made understanding things harder. I think the TSN requires strict increase by one for each data chunk transmitted, independently if they are bundled or not and in order of inclusion in the packet. I would also include a statement that makes it clears that retransmissions are of Data chunks not of SCTP packets. Thus, when retransmitting a data chunk the same TSN is used. 

Multipath and the destination dependency only: There are several transport functions that appear to be significantly impacted by which source – destination pair that is used, but here are only tracked on destination basis. These include congestion window, path MTU, and path liveness. 

Section 2.3:

Path: The route taken by the SCTP packets sent by one SCTP endpoint to a specific destination transport address of its peer SCTP endpoint. Sending to different destination transport addresses does not necessarily guarantee getting separate paths.

I am a bit surprised that there are no mention of the potential impact on path by sending from another source IP/interface. 

Missing Term: State Cookie. State Cookie is used extensively in Section 3, but not really explained until one read through Section 5. So I think it would be good to have a high level explanation of this term. 

Section 2.5.5:

When chunk bundling of retransmissions, are there any refragmentation done? 

This is later answered in Section 6.10 but could maybe be included as a fundamental principle that simplifies but limits the protocol. I would in fact dare to state that this principle can in fact stall out an SCTP association completely. Because if one have committed to a particular fragmentation, and the underlying path change PMTU then this user message may never be delivered if IP fragmentation doesn’t work on the path. 

Section 2.3, and 2.5.6: Endpoint looking up association state based on VTAG rather than just on transport address pair. The SCTP association is defined to be on port. However, the SCTPNAT work has shown that this is limiting. Has there been discussion on making this change in rfc4960bis rather than having the SCTPNAT apply that update?

Section 3.3.1

"When a user message is fragmented into multiple chunks, the TSNs are used by the receiver to reassemble the message. This means that the TSNs for each fragment of a fragmented user message MUST be strictly sequential."

Shouldn't the HOL issue if one uses large user message that blocks also other streams and an informational reference to the I-DATA chunk in RFC 8260 be added here? 

Section 3.3.1:

Stream Sequence Number n: 16 bits (unsigned integer)

This value represents the Stream Sequence Number of the following user data within the stream S. Valid range is 0 to 65535.

For clarity here. I would recommend changing "user data" to "user message". 

Section 3.3.4

Cumulative TSN Ack: 32 bits (unsigned integer)

This parameter contains the TSN of the last DATA chunk received in sequence before a gap. In the case where no DATA chunk has been received, this value is set to the peer's Initial TSN minus one.

I initially interpret this definition to always point to the highest TSN that was received in sequence, i.e. no losses from initial TSN until this. I think maybe one should make it clear that this is indicating all the TSN received, including after retransmissions. 

Section 3.3.5: 

When a HEARTBEAT chunk is being used for path verification purposes, it MUST hold a 64-bit random nonce.

Shouldn't it say a least 64-bit in length random nonce? Are there more text that defines requirement on the nonce, I assume this is a cryptographically random nonce, i.e. something that fulfills the requirements in RFC 4086 . 

Heartbeat Info field: I would think that there would be a benefit to be clearer here on a couple of things:

*         That it is variable size and sender implementation specific. There are no discussion about sizes. The TLV field supports the whole TLV to be 2^16-1 bytes. But, I assume that there actually should be a recommendation that the complete heartbeat should not be larger than one SCTP packet, as it otherwise have to rely on IP fragmentation.

*         That it is recommended that the sender actually encrypts and integrity protect the Info with a private key. This for two reasons, the first to prevent information leakage to third parties on-path. Second to prevent any type of manipulation of this information. And by encrypting it the 64-bit random nonce can be replaced by any mechanism that ensures each HB info to be unique, i.e. a global sequence number would be sufficient as no other party can read or manipulate it. 

Section 3.3.8:

Cumulative TSN Ack: 32 bits (unsigned integer)

This parameter contains the TSN of the last chunk received in sequence before any gaps.

Once more the "in sequence before any gaps" which I think needs a proper definition that makes it actually represent what it should.

"Since SHUTDOWN does not contain Gap Ack Blocks, the receiver of the SHUTDOWN MUST NOT interpret the lack of a Gap Ack Block as a renege. (See  <https://www.ietf.org/archive/id/draft-ietf-tsvwg-rfc4960-bis-11.html#sec_acknowledgements_of_reception_of_data_chunks> Section 6.2 for information on reneging.)"

Section 9.2 says: 

To indicate any gaps in TSN, the endpoint MAY also bundle a SACK with the SHUTDOWN chunk in the same SCTP packet.

Considering the comment here regarding gap-acks, shouldn't it be explicit to state that one can use SACK chunks together with the SHUTDOWN to indicate beyond the Cumulative TSN Ack value?

Section 3.3.9

I don't find anything in the document about what one does if one receives a SHUTDOWN ACK without having sent a SHUTDOWN. I assume it should be dropped, but I don't find anything on this. I find a lot said about SHUTDOWN ACK handling for OOB, and in cases when the SHUTDOWN process are beyond having sent the SHUTDOWN ACK once. 

Section 3.3.10

What do you do with unknown cause codes? Are there recognition required codes, vs errors that are mostly informative but enables the association to continue? Anyway some text about what to do if one don’t know the cause code is needed here. 

Section 3.3.10.13:

“An implementation MAY provide additional information specifying what kind of protocol violation has been detected.”

In what format is this information. Is it UFT-8 text, or binary or what?

Section 4:

In the SHUTDOWN-SENT state, the endpoint MUST acknowledge any received DATA chunks without delay.

Is this formulation causing a potential for creating abnormal traffic patterns from an SCTP endpoint. If the attacker initiates an association, then sends some DATA packets or user protocol messages that will result in the peer shutting down the association, but before that creates a situation where a large number of DATA chunks appears to be outstanding, will that create a situation where the endpoint will ACK every incoming packet? 

Section 5.1, B) : 

The Init ACK response is explicit about what to set the destination IP address. However, it is not explicit about the need to send the packet back with the source and destination ports flipped from the INIT. 

Section 5.1.3:

An implementation SHOULD make the cookie as small as possible to ensure interoperability.

I find this statement strange. I can understand performance issues etc with to large cookies, however interoperability. And if there exists issues, should there be any size requirements here? 

Section 6

When converting user messages into DATA chunks, an endpoint MUST fragment user messages into multiple DATA chunks.

If read as written, this formulation implies that even a User messages that can be sent in a single data chunk fitting within a single SCTP packet MUST be fragmented. I don't think that is intended and the purpose of the MUST is a bit strange. Is this intended to say that user messages that cannot be sent within a single DATA chunk, i.e.  that are larger than AMDCS MUST be fragmented into multiple data chunks, so that each DATA chunk SHOULD be no larger than AMDCS. 

Or if the intention is to say that each user message MUST be sent as one or more DATA chunks then some reformulation is needed. This later comment is related to the plural of the "user messages". 

Section 6.1:

When the receiver has no buffer space, a probe being sent is called a zero window probe. A zero window probe SHOULD only be sent when all outstanding DATA chunks have been cumulatively acknowledged and no DATA chunks are in flight. Zero window probing MUST be supported.

I struggled with this paragraph. My understanding after having read the whole section is that a zero window probe is a packet that is sent when there is less RWND than one PMDCS available, and no outstanding packets needing retransmission. And in that case the sender can send a zero window probe. That probe can be up to PMDCS worth of new data to probe if the window has been increased. I think this section could benefit from some very high level description and how the zero window probe relate to this. 

Section 6.1: 

The data sender SHOULD NOT use a TSN that is more than 2**31 - 1 above the beginning TSN of the current send window.

Is this another way of sending that a sender SHOULD NOT attempt to have more than 2**31-1 packets outstanding counted between cumulative acknowledged and highest TSN sent mod 2**32? 

Section 6.2: 

If the new incoming DATA chunk holds a TSN value less than the largest TSN received so far, then the receiver SHOULD drop the largest TSN held for reordering and accept the new incoming DATA chunk.

I find this sentence confusing. There is no qualification of this sentence that the largest TSN actually is containing data beyond available receiver window, i.e. being a zero window probe. If a sender is behaving and keeping within window earlier advertised. Then providing a rwnd to the sender of 0 is basically saying to it stop sending new data, and even if there was some window left from earlier I withdraw that. But, this appears to indicate that the receiver should throw away data that has been received within limits that was available when the sender transmitted. Is this how it is intended, or is this primarily to deal with received zero window probes needing to be thrown away when retransmissions occur? 

Section 3.3.1  and 6.1: I think the assignment of TSN to Data Chunks are underspecified. It is not clear that each Data chunk needs a TSN increased for each DATA chunk that is bundled. Also, that they need to be strictly sequential is also not mandated anywhere what I can find. Section 6.9 and 6.10 implies things but it is not generally specified. 

Section 6.2.1 c)

Any time a DATA chunk is marked for retransmission, either via T3-rtx timer expiration ( <https://www.ietf.org/archive/id/draft-ietf-tsvwg-rfc4960-bis-11.html#sec_handle_t3_rtx_expiration> Section 6.3.3) or via Fast Retransmit ( <https://www.ietf.org/archive/id/draft-ietf-tsvwg-rfc4960-bis-11.html#sec_fast_retransmit_on_gap_reports> Section 7.2.4), add the data size of those chunks to the rwnd.

So is this C) option to re add the data that was subtracted in B) by the scheduling of the retransmission? I would assume that retransmission are done using already committed RWND space? It is not clear that this is intended to negate execution of B). 

Section 6.3.1:

Shouldn't there be a reference here to Section 16 as there are so many parameters used. 

Section 6.5:

The Stream Sequence Number in all the streams MUST start from 0 when the association is established. Also, when the Stream Sequence Number reaches the value 65535 the next Stream Sequence Number MUST be set to 0.

This is not stating that all new User Messages transmitted on a particular streamID with ordered delivery increases the Stream Sequence Number with 1. Nor is it explicit that unordered MUST NOT increase the sequence number, which should follow if the ordered are required to be increased by only 1. 

Section 7.1:

The sender keeps a separate congestion control parameter set for each of the destination addresses it can send to (not each source-destination pair but for each destination).

I am quite surprised by this. Because the network path may be significantly different depending on source address. So what is the motivation behind keeping SCTP like this? Partial path sharing may occur in both cases, but assuming that they are the same independent on the source address appears strange.

Section 7.3:

An endpoint SHOULD apply these techniques, and SHOULD do so on a per-destination-address basis.

How can this not be done based on source and destination pairs. If the first hop(s) from the source are the MTU limiting part, then it source address will be the most important factor in the PMTU value that is possible to use. 

Section 11.1.14:

“This primitive reads a user message, which has never been sent, into the buffer specified by ULP.”

I am uncertain what this does. Is it removing a message from the transmission buffer prior to having been sent? 

Also, I don't see how the message can have a stream sequence number associated with it. As if it wasn't sent, this was not committed, and the next in sequence user message on this stream will use the stream sequence number of this removed message. 

Section 12.2.3: Shouldn't RFC 6083 be noted as an alternative solution for confidentiality here?

Section 16: Looking on a number of parameters here I find that they appear to be far from the values I would expect to provide good performance on today's Internet. Shouldn't these values have been updated a bit. 

For example RTO.min of 1 s looks ridiculous. Isn't the limit factors here related to clock resolution and the smallest RTT are very short today, while still one need to take into account Internet usage. Therefore a  RTO.Initial of 1 seconds is not particularly problematic. So aren't there more modern recommendations to be made here? 

 Cheers

Magnus Westerlund