Add some RFC based comments

association.go

@@ -137,6 +137,8 @@ func getAssociationStateString(a uint32) string {
 //
 // Note: No "CLOSED" state is illustrated since if a
 // association is "CLOSED" its TCB SHOULD be removed.
+// Note: By nature of an Association being constructed with one net.Conn,
+// it is not a multi-home supporting implementation of SCTP.
 type Association struct {
 	bytesReceived uint64
 	bytesSent     uint64
@@ -304,11 +306,17 @@ func createAssociation(config Config) *Association {
 
 	tsn := globalMathRandomGenerator.Uint32()
 	a := &Association{
-		netConn:                 config.NetConn,
-		maxReceiveBufferSize:    maxReceiveBufferSize,
-		maxMessageSize:          maxMessageSize,
+		netConn:              config.NetConn,
+		maxReceiveBufferSize: maxReceiveBufferSize,
+		maxMessageSize:       maxMessageSize,
+
+		// These two max values have us not need to follow
+		// 5.1.1 where this peer may be incapable of supporting
+		// the requested amount of outbound streams from the other
+		// peer.
 		myMaxNumOutboundStreams: math.MaxUint16,
 		myMaxNumInboundStreams:  math.MaxUint16,
+
 		payloadQueue:            newPayloadQueue(),
 		inflightQueue:           newPayloadQueue(),
 		pendingQueue:            newPendingQueue(),
@@ -1123,7 +1131,10 @@ func (a *Association) handleInit(p *packet, i *chunkInit) ([]*packet, error) {
 		return nil, fmt.Errorf("%w: %s", ErrHandleInitState, getAssociationStateString(state))
 	}
 
-	// Should we be setting any of these permanently until we've ACKed further?
+	// NOTE: Setting these prior to a reception of a COOKIE ECHO chunk containing
+	// our cookie is not compliant with https://www.rfc-editor.org/rfc/rfc9260#section-5.1-2.2.3.
+	// It makes us more vulnerable to resource attacks, albeit minimally so.
+	//  https://www.rfc-editor.org/rfc/rfc9260#sec_handle_stream_parameters
 	a.myMaxNumInboundStreams = min16(i.numInboundStreams, a.myMaxNumInboundStreams)
 	a.myMaxNumOutboundStreams = min16(i.numOutboundStreams, a.myMaxNumOutboundStreams)
 	a.peerVerificationTag = i.initiateTag
@@ -1169,6 +1180,8 @@ func (a *Association) handleInit(p *packet, i *chunkInit) ([]*packet, error) {
 
 	if a.myCookie == nil {
 		var err error
+		// NOTE: This generation process is not compliant with
+		// 5.1.3.  Generating State Cookie (https://www.rfc-editor.org/rfc/rfc4960#section-5.1.3)
 		if a.myCookie, err = newRandomStateCookie(); err != nil {
 			return nil, err
 		}
@@ -1308,13 +1321,16 @@ func (a *Association) handleCookieEcho(c *chunkCookieEcho) []*packet {
 			return nil
 		}
 
+		// RFC wise, these do not seem to belong here, but removing them
+		// causes TestCookieEchoRetransmission to break
 		a.t1Init.stop()
 		a.storedInit = nil
 
 		a.t1Cookie.stop()
 		a.storedCookieEcho = nil
 
 		a.setState(established)
+		// Note: This is a future place where the user could be notified (COMMUNICATION UP)
 		a.handshakeCompletedCh <- nil
 	}
 
@@ -1343,6 +1359,7 @@ func (a *Association) handleCookieAck() {
 	a.storedCookieEcho = nil
 
 	a.setState(established)
+	// Note: This is a future place where the user could be notified (COMMUNICATION UP)
 	a.handshakeCompletedCh <- nil
 }
 
@@ -1356,9 +1373,9 @@ func (a *Association) handleData(d *chunkPayloadData) []*packet {
 	if canPush {
 		s := a.getOrCreateStream(d.streamIdentifier, true, PayloadTypeUnknown)
 		if s == nil {
-			// silentely discard the data. (sender will retry on T3-rtx timeout)
+			// silently discard the data. (sender will retry on T3-rtx timeout)
 			// see pion/sctp#30
-			a.log.Debugf("discard %d", d.streamSequenceNumber)
+			a.log.Debugf("[%s] discard %d", a.name, d.streamSequenceNumber)
 			return nil
 		}
 
@@ -2415,13 +2432,18 @@ func (a *Association) handleChunk(p *packet, c chunk) error {
 	var err error
 
 	if _, err = c.check(); err != nil {
-		a.log.Errorf("[ %s ] failed validating chunk: %s ", a.name, err)
+		a.log.Errorf("[%s] failed validating chunk: %s ", a.name, err)
 		return nil
 	}
 
 	isAbort := false
 
 	switch c := c.(type) {
+	// Note: We do not do the following for chunkInit, chunkInitAck, and chunkCookieEcho:
+	// If an endpoint receives an INIT, INIT ACK, or COOKIE ECHO chunk but decides not to establish the
+	// new association due to missing mandatory parameters in the received INIT or INIT ACK chunk, invalid
+	// parameter values, or lack of local resources, it SHOULD respond with an ABORT chunk.
+
 	case *chunkInit:
 		packets, err = a.handleInit(p, c)
 
@@ -2439,6 +2461,7 @@ func (a *Association) handleChunk(p *packet, c chunk) error {
 		}
 		a.log.Debugf("[%s] Error chunk, with following errors: %s", a.name, errStr)
 
+	// Note: chunkHeartbeatAck not handled?
 	case *chunkHeartbeat:
 		packets = a.handleHeartbeat(c)
 

association_test.go

@@ -411,17 +411,17 @@ func establishSessionPair(br *test.Bridge, a0, a1 *Association, si uint16) (*Str
 }
 
 func TestAssocReliable(t *testing.T) {
-	// sbuf - small enogh not to be fragmented
-	//        large enobh not to be bundled
+	// sbuf - small enough not to be fragmented
+	//        large enough not to be bundled
 	sbuf := make([]byte, 1000)
 	for i := 0; i < len(sbuf); i++ {
 		sbuf[i] = byte(i & 0xff)
 	}
 	rand.Seed(time.Now().UnixNano())
 	rand.Shuffle(len(sbuf), func(i, j int) { sbuf[i], sbuf[j] = sbuf[j], sbuf[i] })
 
-	// sbufL - large enogh to be fragmented into two chunks and each chunks are
-	//        large enobh not to be bundled
+	// sbufL - large enough to be fragmented into two chunks and each chunks are
+	//        large enough not to be bundled
 	sbufL := make([]byte, 2000)
 	for i := 0; i < len(sbufL); i++ {
 		sbufL[i] = byte(i & 0xff)
@@ -821,8 +821,8 @@ func TestAssocReliable(t *testing.T) {
 
 func TestAssocUnreliable(t *testing.T) {
 	// sbuf1, sbuf2:
-	//    large enogh to be fragmented into two chunks and each chunks are
-	//    large enobh not to be bundled
+	//    large enough to be fragmented into two chunks and each chunks are
+	//    large enough not to be bundled
 	sbuf1 := make([]byte, 2000)
 	sbuf2 := make([]byte, 2000)
 	for i := 0; i < len(sbuf1); i++ {
@@ -836,8 +836,8 @@ func TestAssocUnreliable(t *testing.T) {
 	rand.Seed(time.Now().UnixNano())
 	rand.Shuffle(len(sbuf2), func(i, j int) { sbuf2[i], sbuf2[j] = sbuf2[j], sbuf2[i] })
 
-	// sbuf - small enogh not to be fragmented
-	//        large enobh not to be bundled
+	// sbuf - small enough not to be fragmented
+	//        large enough not to be bundled
 	sbuf := make([]byte, 1000)
 	for i := 0; i < len(sbuf); i++ {
 		sbuf[i] = byte(i & 0xff)
@@ -1752,7 +1752,7 @@ func TestAssocT3RtxTimer(t *testing.T) {
 }
 
 func TestAssocCongestionControl(t *testing.T) {
-	// sbuf - large enobh not to be bundled
+	// sbuf - large enough not to be bundled
 	sbuf := make([]byte, 1000)
 	for i := 0; i < len(sbuf); i++ {
 		sbuf[i] = byte(i & 0xcc)

rtx_timer.go

@@ -10,14 +10,28 @@ import (
 )
 
 const (
-	rtoInitial     float64 = 1.0 * 1000  // msec
-	rtoMin         float64 = 1.0 * 1000  // msec
-	defaultRTOMax  float64 = 60.0 * 1000 // msec
-	rtoAlpha       float64 = 0.125
-	rtoBeta        float64 = 0.25
-	maxInitRetrans uint    = 8
-	pathMaxRetrans uint    = 5
-	noMaxRetrans   uint    = 0
+	// RTO.Initial in msec
+	rtoInitial float64 = 1.0 * 1000
+
+	// RTO.Min in msec
+	rtoMin float64 = 1.0 * 1000
+
+	// RTO.Max in msec
+	defaultRTOMax float64 = 60.0 * 1000
+
+	// RTO.Alpha
+	rtoAlpha float64 = 0.125
+
+	// RTO.Beta
+	rtoBeta float64 = 0.25
+
+	// Max.Init.Retransmits:
+	maxInitRetrans uint = 8
+
+	// Path.Max.Retrans
+	pathMaxRetrans uint = 5
+
+	noMaxRetrans uint = 0
 )
 
 // rtoManager manages Rtx timeout values.