diff --git a/association.go b/association.go index 47c0d94e..730ff6f7 100644 --- a/association.go +++ b/association.go @@ -131,6 +131,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 @@ -298,11 +300,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(), @@ -1112,7 +1120,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 @@ -1159,6 +1170,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 } @@ -1299,6 +1312,8 @@ 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 @@ -1306,6 +1321,7 @@ func (a *Association) handleCookieEcho(c *chunkCookieEcho) []*packet { a.storedCookieEcho = nil a.setState(established) + // Note: This is a future place where the user could be notified (COMMUNICATION UP) a.handshakeCompletedCh <- nil } @@ -1334,6 +1350,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 } @@ -1347,9 +1364,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 } @@ -2404,13 +2421,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) @@ -2428,6 +2450,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) diff --git a/association_test.go b/association_test.go index a12503d7..5d3822a6 100644 --- a/association_test.go +++ b/association_test.go @@ -411,8 +411,8 @@ 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) @@ -420,8 +420,8 @@ func TestAssocReliable(t *testing.T) { 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) diff --git a/rtx_timer.go b/rtx_timer.go index 823186c3..ff8b0709 100644 --- a/rtx_timer.go +++ b/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 + rtoInitial float64 = 1.0 * 1000 // msec + + // RTO.Min + rtoMin float64 = 1.0 * 1000 // msec + + // RTO.Max + defaultRTOMax float64 = 60.0 * 1000 // msec + + // 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.