fix: report malformed notification records the reader does not keep

Merged
GO
15d ago
+701-19

Summary

Closes #41.

A notification record the reader does not keep was reset and skipped without the accumulator's verdict, so a malformed one was consumed silently and the next message came back with a nil error. The accumulator documents that a malformed notification is never consumed silently, and RFC 6458 §6.1 defines sn_length as the length of the whole record, header included.

The defect was on every receive path, not only the two the issue names:

  • Conn.RecvMsg and Conn.Read (Conn.recvNext): records Read consumes without a handler, unsubscribed SCTP_ASSOC_CHANGE records, and records that end before their 8-byte header is whole.
  • Endpoint.RecvMsg (Endpoint.recvNext): records that end before their header is whole.
  • Conn.ReadMsg (readMsgNotification): records it consumes without a handler, unsubscribed SCTP_ASSOC_CHANGE records and records past a full notification queue. In the branch for a record that cuts the message short, a kept record's framing or parsing error was dropped as well.

Each path now takes the verdict (recvState.dropRecord) before discarding the record. A record without the whole header or exactly its declared length, or longer than NotificationReassemblyLimit, fails the read that consumes its end with an error matching ErrShortNotification or ErrNotificationTooLong (a *net.OpError with Op "read"), and the message after it stays queued. After a malformed unkept AssocShutdownComplete, the error comes first and the next read returns io.EOF, as for a kept record. Well-formed records are delivered or consumed as before, and dropping a record still allocates nothing.

The documentation of Conn.RecvMsg (which Read and ReadMsg refer to) and Endpoint.RecvMsg, the README, ECOSYSTEM.md and STANDARDS.md say so. STANDARDS.md also records the 2026-10-16 check of RFC 6458: Informational, with no obsoleting or updating document in either the RFC Editor or the Datatracker, and 14 errata, none of which changes the §6.1 header.

Tests

  • TestConnNotificationFramingVerdict and TestEndpointNotificationFramingVerdict cross every reader, every reason a record is not kept (and kept records), six malformed shapes (header short, declared length below 8, ended short, ended long, declared length over the limit, total over the limit), three piece splits and 1-, 20- and 64-byte buffers. Each case checks the exact sentinel and the *net.OpError, a zero result, no handler call, that exactly the record was consumed and the accumulator reset, and that the next read returns the following message whole, with its metadata.
  • TestWellFormedNotificationDeliveryPolicy: well-formed records are still consumed unseen or delivered, on every reader.
  • TestNotificationFramingVerdictParity: the unkept and the kept raw path give the same sentinel for every shape.
  • TestReadUnkeptNotificationBeforeShutdown, TestReadMsgInterleavedNotificationVerdict, TestReadMsgCutNotificationVerdict and TestReadMsgQueueFullNotificationVerdict: order against the end of the association, a record between a message's pieces, a record that cuts a message short, and a full queue.
  • TestDropNotificationRecordZeroAllocs: no allocation for any shape.
  • FuzzReceiveNotificationVerdict: arbitrary records and splits through Read, raw RecvMsg and Endpoint.RecvMsg, checked against a framing oracle independent of the accumulator.

With main's receive code, 912 matrix cases fail (288 Conn.RecvMsg, 360 Conn.Read, 120 Conn.ReadMsg, 144 Endpoint.RecvMsg), as do 40 parity cases, every shutdown-order and interleaving case, the malformed cut and queue-full cases and 105 fuzz seeds; the issue's own two tests fail 48 of 48 and 2 of 2. The well-formed, kept and allocation cases pass on main and on this branch. Reverting only the Conn site fails the 648 Conn.RecvMsg and Conn.Read cases, the parity cases, the Read shutdown cases and the fuzz seeds, and leaves the ReadMsg, Endpoint and well-formed cases passing. Reverting each other site, ignoring the verdict, skipping the reset, returning before the record's end, or checking the end of the association first each fails a named test.

Validation

  • go build, go vet (linux, darwin and 386), gofmt, staticcheck, golangci-lint and gopls.
  • Docker Linux suite, 0 failures: sysctls on 3,354 pass, sysctls off 3,353 pass, -race 3,266 pass. The skips are the two-host tests, tests that need the other sysctl state, and measurements unsuitable under the race detector.
  • FuzzReceiveNotificationVerdict for 3 minutes, twice, in Docker, with no failure (845,837 executions in the second run).

A correct kernel does not deliver these records, so this is defensive validation and there is nothing to capture on the wire; well-formed notifications on real sockets are covered by the suites above.

Summary by cubic

Fixes silently consumed malformed notification records. A record that Read or ReadMsg consumes without a handler, an unsubscribed SCTP_ASSOC_CHANGE, or one ending before its 8-byte header was whole was reset and skipped without the accumulator's verdict, so the next message came back with a nil error. Every receive path now takes the verdict before discarding: a record without the whole header, not exactly its declared sn_length (RFC 6458 §6.1), or longer than NotificationReassemblyLimit fails the read that consumes its end with an error matching ErrShortNotification or ErrNotificationTooLong, and the message after it stays queued. Well-formed records are delivered or consumed as before, and dropping one still allocates nothing. Closes #41.

Tests

  • A matrix crosses every reader, each reason a record is not kept, six malformed shapes, three piece splits, and buffer sizes, checking the sentinel, zero result, no handler call, exact consumption, accumulator reset, and the following message.
  • Well-formed delivery, kept/unkept parity, shutdown and interleaving order, and no-allocation checks, plus a fuzz target checked against an independent framing oracle.