Update packet to RFC 9260 (after reverts)

This reverts commit 096446a.

Author: philipch07

chunk_test.go

@@ -104,13 +104,29 @@ func TestChromeChunk2InitAck(t *testing.T) {
 		0x00, 0x06, 0x00, 0x01, 0x00, 0x00, 0x80, 0x03, 0x00, 0x06, 0x80, 0xc1, 0x00, 0x00, 0xca, 0x0c, 0x21, 0x11,
 		0xce, 0xf4, 0xfc, 0xb3, 0x66, 0x99, 0x4f, 0xdb, 0x4f, 0x95, 0x6b, 0x6f, 0x3b, 0xb1, 0xdb, 0x5a,
 	}
+
 	err := pkt.unmarshal(true, rawPkt)
 	assert.NoError(t, err)
 
-	rawPkt2, err := pkt.marshal(true)
+	// Strict marshal (CRC32c): should reproduce the original bit-for-bit.
+	rawStrict, err := pkt.marshal(false)
 	assert.NoError(t, err)
+	assert.Equal(t, rawPkt, rawStrict)
 
-	assert.Equal(t, rawPkt, rawPkt2)
+	// ZCA marshal (zero checksum allowed, and this packet has INIT-ACK, not restricted)
+	rawZero, err := pkt.marshal(true)
+	assert.NoError(t, err)
+
+	// Expect the same bytes as original, except checksum field zeroed.
+	expZero := make([]byte, len(rawPkt))
+	copy(expZero, rawPkt)
+	expZero[8], expZero[9], expZero[10], expZero[11] = 0, 0, 0, 0
+	assert.Equal(t, expZero, rawZero)
+
+	// Receiver behavior: zero-checksum packet is rejected in strict mode, accepted in ZCA mode.
+	pkt2 := &packet{}
+	assert.Error(t, pkt2.unmarshal(false, rawZero))
+	assert.NoError(t, pkt2.unmarshal(true, rawZero))
 }
 
 func TestInitMarshalUnmarshal(t *testing.T) {

packet.go

@@ -18,7 +18,7 @@ var castagnoliTable = crc32.MakeTable(crc32.Castagnoli) // nolint:gochecknogloba
 var fourZeroes [4]byte // nolint:gochecknoglobals
 
 /*
-Packet represents an SCTP packet, defined in https://tools.ietf.org/html/rfc4960#section-3
+Packet represents an SCTP packet, defined in https://tools.ietf.org/html/rfc9260#section-3
 An SCTP packet is composed of a common header and chunks.  A chunk
 contains either control information or user data.
 
@@ -39,7 +39,7 @@ contains either control information or user data.
 	 0                   1                   2                   3
 	 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 	+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-	|     Source Value Number      |     Destination Value Number   |
+	|     Source Port Number       |     Destination Port Number    |
 	+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 	|                      Verification Tag                         |
 	+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
@@ -65,27 +65,39 @@ var (
 	ErrChecksumMismatch            = errors.New("checksum mismatch theirs")
 )
 
+// unmarshal parses an SCTP packet and verifies its checksum.
+//
+// RFC 9260 section 6.8: every packet MUST be protected with CRC32c; receivers verify it.
+// RFC 9653 adds an extension (ZCA) that allows a receiver to accept a packet
+// whose checksum is incorrect but equal to zero, when an alternate error
+// detection method is active for the path (ex: SCTP-over-DTLS) and ZCA has
+// been negotiated.
+//
+// doChecksum indicates whether ZCA is active for the *receiving path*:
+//   - doChecksum == true  : accept a packet if the only checksum issue is that
+//     it is exactly zero (ZCA + path constraints satisfied).
+//   - doChecksum == false : strict 9260 verification; any checksum mismatch fails.
+//
+// Higher layers MUST only pass doChecksum=true if the peer advertised ZCA
+// in INIT/INIT-ACK and the current path meets the method’s constraints
+// (ex: DTLS). This function does not relax sender-side rules: packets that
+// contain INIT or COOKIE ECHO MUST still carry a correct CRC32c (enforced in marshal()).
+//
+// See RFC 9653 section 5.1 and section 5.3.
 func (p *packet) unmarshal(doChecksum bool, raw []byte) error { //nolint:cyclop
 	if len(raw) < packetHeaderSize {
 		return fmt.Errorf("%w: raw only %d bytes, %d is the minimum length", ErrPacketRawTooSmall, len(raw), packetHeaderSize)
 	}
 
 	offset := packetHeaderSize
 
-	// Check if doing CRC32c is required.
-	// Without having SCTP AUTH implemented, this depends only on the type
-	// og the first chunk.
-	if offset+chunkHeaderSize <= len(raw) {
-		switch chunkType(raw[offset]) {
-		case ctInit, ctCookieEcho:
-			doChecksum = true
-		default:
-		}
-	}
+	// always compute CRC32c (RFC 9260 section 6.8).
 	theirChecksum := binary.LittleEndian.Uint32(raw[8:])
-	if theirChecksum != 0 || doChecksum {
-		ourChecksum := generatePacketChecksum(raw)
-		if theirChecksum != ourChecksum {
+	ourChecksum := generatePacketChecksum(raw)
+	if theirChecksum != ourChecksum {
+		// RFC 9653: if (a) checksum is zero and (b) caller indicates ZCA is active
+		// and method constraints are met, accept; otherwise error.
+		if !doChecksum || theirChecksum != 0 {
 			return fmt.Errorf("%w: %d ours: %d", ErrChecksumMismatch, theirChecksum, ourChecksum)
 		}
 	}
@@ -148,6 +160,16 @@ func (p *packet) unmarshal(doChecksum bool, raw []byte) error { //nolint:cyclop
 	return nil
 }
 
+// marshal builds an SCTP packet.
+//
+// If doChecksum == true, a zero checksum is written (RFC 9653) unless
+// the packet contains a restricted chunk (INIT or COOKIE ECHO), in which case
+// a correct CRC32c is always written (RFC 9653 section 5.2). If doChecksum == false,
+// a correct CRC32c is always written (RFC 9260 section 6.8).
+//
+// The caller sets doChecksum=true only when the peer has advertised ZCA and
+// the current path satisfies the alternate method’s constraints (e.g., DTLS).
+// For OOTB responses and other control paths, always marshal with doChecksum=false.
 func (p *packet) marshal(doChecksum bool) ([]byte, error) {
 	raw := make([]byte, packetHeaderSize)
 
@@ -171,19 +193,27 @@ func (p *packet) marshal(doChecksum bool) ([]byte, error) {
 		}
 	}
 
-	if doChecksum {
-		// golang CRC32C uses reflected input and reflected output, the
-		// net result of this is to have the bytes flipped compared to
-		// the non reflected variant that the spec expects.
-		//
-		// Use LittleEndian.PutUint32 to avoid flipping the bytes in to
-		// the spec compliant checksum order
+	if doChecksum && !hasRestrictedChunk(p.chunks) {
+		binary.LittleEndian.PutUint32(raw[8:], 0)
+	} else {
 		binary.LittleEndian.PutUint32(raw[8:], generatePacketChecksum(raw))
 	}
 
 	return raw, nil
 }
 
+// restrictedChunks per RFC 9653 section 5.2: INIT and COOKIE ECHO.
+func hasRestrictedChunk(chs []chunk) bool {
+	for _, c := range chs {
+		switch c.(type) {
+		case *chunkInit, *chunkCookieEcho:
+			return true
+		}
+	}
+
+	return false
+}
+
 func generatePacketChecksum(raw []byte) (sum uint32) {
 	// Fastest way to do a crc32 without allocating.
 	sum = crc32.Update(sum, castagnoliTable, raw[0:8])
@@ -215,11 +245,13 @@ func (p *packet) String() string {
 // TryMarshalUnmarshal attempts to marshal and unmarshal a message. Added for fuzzing.
 func TryMarshalUnmarshal(msg []byte) int {
 	p := &packet{}
+	// Strict mode first (RFC 9260): require valid CRC32c.
 	err := p.unmarshal(false, msg)
 	if err != nil {
 		return 0
 	}
 
+	// Strict send (emit CRC32c).
 	_, err = p.marshal(false)
 	if err != nil {
 		return 0

packet_test.go

@@ -4,6 +4,7 @@
 package sctp
 
 import (
+	"encoding/binary"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -46,11 +47,21 @@ func TestPacketMarshal(t *testing.T) {
 	headerOnly := []byte{0x13, 0x88, 0x13, 0x88, 0x00, 0x00, 0x00, 0x00, 0x06, 0xa9, 0x00, 0xe1}
 	assert.NoError(t, pkt.unmarshal(true, headerOnly), "Unmarshal failed for SCTP packet with no chunks")
 
-	headerOnlyMarshaled, err := pkt.marshal(true)
+	// Marshal in strict mode (RFC 9260) so we emit the real CRC32c.
+	headerOnlyMarshaled, err := pkt.marshal(false)
 	if assert.NoError(t, err, "Marshal failed for SCTP packet with no chunks") {
-		assert.Equal(t, headerOnly, headerOnlyMarshaled,
-			"Unmarshal/Marshaled header only packet did not match \nheaderOnly: % 02x \nheaderOnlyMarshaled % 02x",
-			headerOnly, headerOnlyMarshaled)
+		assert.Equal(t, packetHeaderSize, len(headerOnlyMarshaled))
+
+		// First 8 bytes (ports + vtag) must match the original header.
+		assert.Equal(t, headerOnly[:8], headerOnlyMarshaled[:8])
+
+		// The checksum we wrote must equal CRC32c over the header with the checksum field zeroed.
+		cpy := make([]byte, len(headerOnlyMarshaled))
+		copy(cpy, headerOnlyMarshaled)
+		binary.LittleEndian.PutUint32(cpy[8:], 0)
+		want := generatePacketChecksum(cpy)
+		got := binary.LittleEndian.Uint32(headerOnlyMarshaled[8:])
+		assert.Equal(t, want, got, "checksum must be correct CRC32c")
 	}
 }
 
@@ -61,3 +72,81 @@ func BenchmarkPacketGenerateChecksum(b *testing.B) {
 		_ = generatePacketChecksum(data[:])
 	}
 }
+
+func FuzzPacket_StrictRoundTrip(f *testing.F) {
+	// Seed with a tiny valid strict packet.
+	if raw, err := (&packet{
+		sourcePort:      5000,
+		destinationPort: 5000,
+		verificationTag: 0x01020304,
+		chunks:          []chunk{&chunkCookieAck{}},
+	}).marshal(false); err == nil {
+		f.Add(raw)
+	}
+
+	f.Fuzz(func(t *testing.T, data []byte) {
+		var p packet
+		if err := p.unmarshal(false, data); err != nil {
+			return // skip invalid packets
+		}
+
+		out, err := p.marshal(false)
+		assert.NoError(t, err, "marshal(strict) after strict unmarshal")
+
+		if err != nil {
+			return
+		}
+
+		var q packet
+		assert.NoError(t, q.unmarshal(false, out), "strict unmarshal after marshal")
+	})
+}
+
+func FuzzPacket_ZeroChecksumAcceptance(f *testing.F) {
+	// Seed with a simple non-restricted strict packet.
+	if raw, err := (&packet{
+		sourcePort:      9,
+		destinationPort: 9,
+		verificationTag: 1,
+		chunks:          []chunk{&chunkCookieAck{}},
+	}).marshal(false); err == nil {
+		f.Add(raw)
+	}
+
+	f.Fuzz(func(t *testing.T, data []byte) {
+		var pak packet
+		if err := pak.unmarshal(false, data); err != nil {
+			return // only start from valid strict inputs
+		}
+
+		// Force checksum to zero.
+		mut := make([]byte, len(data))
+		copy(mut, data)
+		binary.LittleEndian.PutUint32(mut[8:], 0)
+
+		// Strict must reject zero checksum.
+		assert.Error(t, pak.unmarshal(false, mut), "strict receiver should reject zero checksum")
+
+		// ZCA must accept zero checksum.
+		assert.NoError(t, pak.unmarshal(true, mut), "ZCA receiver should accept zero checksum")
+	})
+}
+
+func FuzzPacket_TryMarshalUnmarshal_NoPanic(f *testing.F) {
+	f.Add([]byte{0, 0, 0, 0})
+
+	if raw, err := (&packet{
+		sourcePort:      7,
+		destinationPort: 7,
+		verificationTag: 0xdeadbeef,
+		chunks:          []chunk{&chunkCookieAck{}},
+	}).marshal(false); err == nil {
+		f.Add(raw)
+	}
+
+	f.Fuzz(func(t *testing.T, b []byte) {
+		assert.NotPanics(t, func() {
+			_ = TryMarshalUnmarshal(b)
+		})
+	})
+}