Tcp rst handling (#40)

* claude suggests a way forward

* add timing to capture printer

* add pcap.Flags

* fix ICMP CRC calculation and add test

* bugfix: still send data on half-close state(close-wait)

* fix pcap test
This commit is contained in:
Pat Whittingslow
2026-02-23 13:50:13 +01:00
committed by GitHub
parent 08423d0dba
commit 7d323aae19
10 changed files with 386 additions and 46 deletions
+16 -14
View File
@@ -183,8 +183,9 @@ func (tcb *ControlBlock) PendingSegment(payloadLen int) (_ Segment, ok bool) {
}
pending := tcb.pending[0]
established := tcb._state == StateEstablished
if !established && tcb._state != StateCloseWait {
payloadLen = 0 // Can't send data if not established.
canSendData := established || tcb._state == StateCloseWait
if !canSendData {
payloadLen = 0 // Can't send data if not established or close-wait.
}
if pending == 0 && payloadLen == 0 {
return Segment{}, false // No pending segment.
@@ -206,7 +207,7 @@ func (tcb *ControlBlock) PendingSegment(payloadLen int) (_ Segment, ok bool) {
pending |= FlagPSH // By default ensure all data flushed to destination application immediately on receive.
}
if established {
if canSendData {
pending |= FlagACK // ACK is always set in established state. Not in RFC9293 but somehow expected?
} else {
payloadLen = 0 // Can't send data if not established.
@@ -420,7 +421,7 @@ func (tcb *ControlBlock) validateIncomingSegment(seg Segment) (err error) {
case checkSEQ && !seg.Last().InWindow(tcb.rcv.NXT, tcb.rcv.WND) && !zeroWindowOK:
err = errLastNotInWindow
case checkSEQ && seg.SEQ != tcb.rcv.NXT:
case checkSEQ && !flags.HasAny(FlagRST) && seg.SEQ != tcb.rcv.NXT:
// This part diverts from TCB as described in RFC 9293. We want to support
// only sequential segments to keep implementation simple and maintainable. See SHLD-31.
err = errRequireSequential
@@ -485,22 +486,23 @@ func (tcb *ControlBlock) resetRcv(localWND Size, remoteISS Value) {
func (tcb *ControlBlock) handleRST(seq Value) error {
tcb.debug("rcv:RST", slog.String("state", tcb._state.String()))
if seq != tcb.rcv.NXT {
// See RFC9293: If the RST bit is set and the sequence number does not exactly match the next expected sequence value, yet is within the current receive window, TCP endpoints MUST send an acknowledgment (challenge ACK).
tcb.challengeAck = true
tcb.pending[0] |= FlagACK
return errDropSegment
}
if tcb._state.IsPreestablished() {
// RFC 9293 §3.5.3: non-synchronized states accept RST if SEQ is in window.
// No challenge ACK for non-synchronized states. Return to LISTEN.
tcb.pending[0] = 0
tcb._state = StateListen
tcb.resetSnd(tcb.snd.ISS+tcb.rstJump(), tcb.snd.WND)
tcb.resetRcv(tcb.rcv.WND, 3_14159_2653^tcb.rcv.IRS)
} else {
tcb.Abort() // Enter closed state and return.
return net.ErrClosed
return errDropSegment
}
return errDropSegment
// Synchronized states: exact match required, challenge ACK for in-window non-exact.
if seq != tcb.rcv.NXT {
tcb.challengeAck = true
tcb.pending[0] |= FlagACK
return errDropSegment
}
tcb.Abort()
return net.ErrClosed
}
func (tcb *ControlBlock) rstJump() Value {
+152
View File
@@ -468,6 +468,158 @@ func TestWindowUpdateSWSAvoidance(t *testing.T) {
}
}
// TestWriteAfterRemoteFIN verifies that when a remote peer sends FIN (entering
// CLOSE_WAIT on our side), we can still write and send data before closing.
// This is a regression test for a panic in sentlist.AddPacket caused by
// PendingSegment returning DATALEN=0 while Handler.Send calls MakePacket with
// available > 0, creating degenerate zero-data packets in the sent queue.
//
// The sequence that triggers the panic:
// 1. Connection established
// 2. Remote sends FIN,ACK → local enters CLOSE_WAIT
// 3. Application writes data to TX buffer
// 4. Handler.Send() is called: PendingSegment sets PSH because payloadLen>0,
// then zeroes payloadLen because !established → DATALEN=0 but ok=true
// 5. MakePacket called with zero-length buffer → creates {off:0,end:0} entry
// 6. Handler.Send() called again → same thing → AddPacket panics because
// off=0 but lastPkt.end=0 != bufsize
func TestWriteAfterRemoteFIN(t *testing.T) {
const mtu = 1500
const maxpackets = 3
rng := rand.New(rand.NewSource(11))
client, server := newHandler(t, mtu, maxpackets), newHandler(t, mtu, maxpackets)
setupClientServer(t, rng, client, server)
var rawbuf [mtu]byte
establish(t, client, server, rawbuf[:])
if server.State() != StateEstablished {
t.Fatal("server not established:", server.State())
}
// Client initiates close (sends FIN).
err := client.Close()
if err != nil {
t.Fatal("client close:", err)
}
clear(rawbuf[:])
n, err := client.Send(rawbuf[:])
if err != nil {
t.Fatal("client sending FIN:", err)
}
if n < sizeHeaderTCP {
t.Fatal("expected FIN packet")
}
if client.State() != StateFinWait1 {
t.Fatal("client not in FIN_WAIT_1:", client.State())
}
// Server receives FIN → enters CLOSE_WAIT.
err = server.Recv(rawbuf[:n])
if err != nil {
t.Fatal("server receiving FIN:", err)
}
if server.State() != StateCloseWait {
t.Fatal("server not in CLOSE_WAIT:", server.State())
}
// Application writes data (like an HTTP 404 response).
responseData := []byte("HTTP/1.1 404 Not Found\r\n\r\n")
nw, err := server.Write(responseData)
if err != nil {
t.Fatal("server write:", err)
}
if nw != len(responseData) {
t.Fatal("short write:", nw)
}
// Server sends response — this should include the data, not panic.
// The bug causes a panic on the second Send() call because the first
// creates a degenerate zero-data packet in the sentlist.
clear(rawbuf[:])
n, err = server.Send(rawbuf[:])
if err != nil {
t.Fatal("server send 1:", err)
}
clear(rawbuf[:])
n, err = server.Send(rawbuf[:])
if err != nil {
t.Fatal("server send 2:", err)
}
}
// TestRSTinSynReceived verifies that a RST received during the SYN-RECEIVED
// state correctly reverts the connection to LISTEN per RFC 9293 §3.5.3.
// This is a regression test for a bug where RST segments in non-synchronized
// states were blocked by errRequireSequential, causing connection pool leaks.
func TestRSTinSynReceived(t *testing.T) {
const mtu = 1500
const maxpackets = 3
rng := rand.New(rand.NewSource(2))
client, server := newHandler(t, mtu, maxpackets), newHandler(t, mtu, maxpackets)
setupClientServer(t, rng, client, server)
var rawbuf [mtu]byte
// Client sends SYN.
clear(rawbuf[:])
n, err := client.Send(rawbuf[:])
if err != nil {
t.Fatal("client sending SYN:", err)
}
if client.State() != StateSynSent {
t.Fatal("client not in SynSent:", client.State())
}
// Server receives SYN → transitions to SYN-RECEIVED.
err = server.Recv(rawbuf[:n])
if err != nil {
t.Fatal("server receiving SYN:", err)
}
if server.State() != StateSynRcvd {
t.Fatal("server not in SynRcvd:", server.State())
}
// Server sends SYN,ACK.
clear(rawbuf[:])
n, err = server.Send(rawbuf[:])
if err != nil {
t.Fatal("server sending SYN,ACK:", err)
}
if n < sizeHeaderTCP {
t.Fatal("expected SYN,ACK packet")
}
synackFrm, _ := NewFrame(rawbuf[:n])
synackSeg := synackFrm.Segment(0)
// Construct RST packet from client perspective (as if the remote peer
// rejected the connection). SEQ = ACK from SYN,ACK, no ACK flag, no payload.
clear(rawbuf[:])
rstFrm, err := NewFrame(rawbuf[:])
if err != nil {
t.Fatal("new frame:", err)
}
rstSeg := Segment{
SEQ: synackSeg.ACK, // SEQ = server's ACK value = in window.
Flags: FlagRST,
}
rstFrm.SetSourcePort(client.localPort)
rstFrm.SetDestinationPort(server.localPort)
rstFrm.SetSegment(rstSeg, 5)
rstFrm.SetUrgentPtr(0)
// Server receives RST → should revert to LISTEN per RFC 9293 §3.5.3.
err = server.Recv(rawbuf[:sizeHeaderTCP])
if !IsDroppedErr(err) {
t.Fatal("expected drop segment error from RST recv, got:", err)
}
if server.State() != StateListen {
t.Fatalf("expected server LISTEN after RST in SYN-RECEIVED, got %s", server.State())
}
if server.scb.HasPending() {
t.Fatal("server should have no pending segments after RST")
}
}
// TestBufferNotClearedOnPassiveClose tests that data remains readable after
// the TCP connection is closed by the remote peer. This is a regression test
// for a bug where the receive buffer was cleared when the connection transitioned
+5 -3
View File
@@ -264,8 +264,9 @@ func (listener *Listener) maintainConns() {
continue
}
state := conn.State()
if state > StateEstablished || state.IsClosed() {
// Something went wrong in handshake or pool aborted/closed the connection.
if state > StateEstablished || state.IsClosed() || state == StateListen {
// Something went wrong in handshake, pool aborted/closed the connection,
// or RST reverted the connection to LISTEN (RFC 9293 §3.5.3).
listener.returnIncoming(i)
}
}
@@ -289,7 +290,8 @@ func getConn(conns []handler, remotePort uint16, remoteAddr []byte) int {
func (listener *Listener) maintainConn(conns []handler, idx int, err error) error {
if err == net.ErrClosed {
listener.returnAccepted(idx)
listener.poolReturn(conns[idx].conn)
conns[idx] = handler{}
return nil // avoid closing listener entirely.
}
return err