go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Reply to invalid ACKs even when accept queue is full 

Before checking if there is space in the accept queue, the listener
should verify that the cookie is valid. If it is not, instead of
silently dropping the packet, reply with an RST.

Fixes #6683

PiperOrigin-RevId: 400807346
This commit is contained in:
Arthur Sfez 2021-10-04 13:55:19 -07:00 committed by gVisor bot
parent 429821b0a9
commit 6c1237da03
2 changed files with 22 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
pkg/tcpip/transport/tcp/accept.go
View File

@ -583,23 +583,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
return nil
case s.flags.Contains(header.TCPFlagAck):
// Keep hold of acceptMu until the new endpoint is in the accept queue (or
// if there is an error), to guarantee that we will keep our spot in the
// queue even if another handshake from the syn queue completes.
e.acceptMu.Lock()
if e.acceptQueue.isFull() {
// Silently drop the ack as the application can't accept
// the connection at this point. The ack will be
// retransmitted by the sender anyway and we can
// complete the connection at the time of retransmit if
// the backlog has space.
e.acceptMu.Unlock()
e.stack.Stats().TCP.ListenOverflowAckDrop.Increment()
e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment()
e.stack.Stats().DroppedPackets.Increment()
return nil
}
iss := s.ackNumber - 1
irs := s.sequenceNumber - 1
@ -615,7 +598,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
// Validate the cookie.
data, ok := ctx.isCookieValid(s.id, iss, irs)
if !ok || int(data) >= len(mssTable) {
e.acceptMu.Unlock()
e.stack.Stats().TCP.ListenOverflowInvalidSynCookieRcvd.Increment()
e.stack.Stats().DroppedPackets.Increment()
@ -636,6 +618,24 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
// ACK was received from the sender.
return replyWithReset(e.stack, s, e.sendTOS, e.ttl)
}
// Keep hold of acceptMu until the new endpoint is in the accept queue (or
// if there is an error), to guarantee that we will keep our spot in the
// queue even if another handshake from the syn queue completes.
e.acceptMu.Lock()
if e.acceptQueue.isFull() {
// Silently drop the ack as the application can't accept
// the connection at this point. The ack will be
// retransmitted by the sender anyway and we can
// complete the connection at the time of retransmit if
// the backlog has space.
e.acceptMu.Unlock()
e.stack.Stats().TCP.ListenOverflowAckDrop.Increment()
e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment()
e.stack.Stats().DroppedPackets.Increment()
return nil
}
e.stack.Stats().TCP.ListenOverflowSynCookieRcvd.Increment()
// Create newly accepted endpoint and deliver it.
rcvdSynOptions := header.TCPSynOptions{

15
test/packetimpact/tests/tcp_listen_backlog_test.go
View File

@ -150,17 +150,10 @@ func TestTCPListenBacklog(t *testing.T) {
conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{})
defer conn.Close(t)
conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
if !dut.Uname.IsLinux() {
// TODO(https://gvisor.dev/issues/6683): Expect a RST.
if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err == nil {
t.Errorf("expected no TCP frame, got %s", got)
}
} else {
if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err != nil {
t.Errorf("expected TCP frame: %s", err)
} else if got, want := *got.Flags, header.TCPFlagRst; got != want {
t.Errorf("got %s, want %s", got, want)
}
if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err != nil {
t.Errorf("expected TCP frame: %s", err)
} else if got, want := *got.Flags, header.TCPFlagRst; got != want {
t.Errorf("got %s, want %s", got, want)
}
}()