diff --git a/handler/tun/datagram_frame_test.go b/handler/tun/datagram_frame_test.go new file mode 100644 index 0000000..7870679 --- /dev/null +++ b/handler/tun/datagram_frame_test.go @@ -0,0 +1,136 @@ +package tun + +import ( + "context" + "net" + "testing" + "time" + + xlogger "github.com/go-gost/x/logger" +) + +// recordDevice remembers what the hub handed to the device, so a test can +// assert a frame never reached it. +type recordDevice struct { + nopDevice + wrote [][]byte +} + +func (d *recordDevice) Write(p []byte) (int, error) { + d.wrote = append(d.wrote, append([]byte(nil), p...)) + return len(p), nil +} + +// A frame that is not a parseable IPv4 or IPv6 packet is one the kernel will +// refuse with EINVAL — measured, not assumed: on a real up tun device the +// version nibble is the only thing it checks, and anything other than 4 or 6 +// comes back EINVAL at any length. Handing one to the device is what killed +// the hub, because that error propagates out of fromSpoke and tears down the +// peer's whole stream. So the frame is dropped and counted instead, exactly +// as an oversized one already is. +func TestP2PNonIPDatagramKeepsStream(t *testing.T) { + garbage := func(n int, first byte) []byte { + p := make([]byte, n) + for i := range p { + p[i] = 0xFF + } + p[0] = first + return p + } + + tests := []struct { + name string + pkt []byte + }{ + {"version 5", garbage(60, 0x55)}, + {"version 7", garbage(60, 0x75)}, + {"all zeroes", garbage(60, 0x00)}, + {"empty", []byte{}}, + {"oversized and non-IP", garbage(20530, 0x55)}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + dev := &recordDevice{} + var warns []string + hub := newP2PHub(dev, newTestRouter(), nil, + func(s string) { warns = append(warns, s) }) + _, peerEnd := newDatagramPipe(4) + s := newPeerStream(context.Background(), "peer-a", peerEnd) + + if err := hub.fromSpoke(s, tc.pkt); err != nil { + t.Fatalf("fromSpoke: %v, want nil (drop, don't kill the stream)", err) + } + if len(dev.wrote) != 0 { + t.Fatalf("device got %d writes, want 0", len(dev.wrote)) + } + if len(warns) != 1 { + t.Fatalf("warnings = %d, want 1 (a dropped frame must stay visible)", len(warns)) + } + + // The guard must not have cost the peer its link: the next + // well-formed packet still has to reach the device. + good := udpPacket("10.0.0.1", "10.0.0.9", 1, 3) + if err := hub.fromSpoke(s, good); err != nil { + t.Fatalf("fromSpoke (valid packet after a bad one): %v", err) + } + if len(dev.wrote) != 1 { + t.Fatalf("device got %d writes, want 1", len(dev.wrote)) + } + }) + } +} + +// The spoke's downlink is the same hazard and already carries this guard — +// which is why the phone logs "unknown packet, discarded" all day and never +// dies, while the hub has no equivalent. This pins that asymmetry: if the +// spoke's check is ever dropped, the device must still receive nothing, and +// the run must end on the deadline rather than on an EINVAL from the kernel. +func TestTransportClientDropsNonIPFrames(t *testing.T) { + device, kernelSide := net.Pipe() + ccA, ccB := net.Pipe() + defer device.Close() + defer ccA.Close() + + h := newTestHandler(nil, nil) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + deviceGot := make(chan []byte, 1) + go func() { + var acc []byte + buf := make([]byte, 64) + kernelSide.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + for { + n, err := kernelSide.Read(buf[:]) + if n > 0 { + acc = append(acc, buf[:n]...) + } + if err != nil { + break + } + } + deviceGot <- acc + }() + + errCh := make(chan error, 1) + go func() { errCh <- h.transportClient(ctx, device, ccA, xlogger.Nop()) }() + + bad := make([]byte, 60) + for i := range bad { + bad[i] = 0x55 // version 5: not IPv4, not IPv6 + } + if _, err := ccB.Write(bad); err != nil { + t.Fatal(err) + } + // A well-formed packet behind it must still arrive: if the guard dropped + // the link instead of the frame, this never lands. + good := realIPv4Packet() + if _, err := ccB.Write(good); err != nil { + t.Fatal(err) + } + + got := <-deviceGot + if len(got) != len(good) { + t.Fatalf("device received %d bytes, want %d (only the well-formed packet)", len(got), len(good)) + } +} diff --git a/handler/tun/p2p.go b/handler/tun/p2p.go index 29f10f7..a6249e3 100644 --- a/handler/tun/p2p.go +++ b/handler/tun/p2p.go @@ -220,6 +220,13 @@ type p2pHub struct { overMu sync.Mutex overCount int overSince time.Time + + // nonIPMu guards the tally that countNonIP keeps. Same shape again: a peer + // whose link is losing frame boundaries sends these continuously, and one + // line per packet would bury the log it is meant to explain. + nonIPMu sync.Mutex + nonIPCount int + nonIPSince time.Time } func newP2PHub(device io.ReadWriter, router *peerRouter, ownNets []net.IPNet, warn func(string)) *p2pHub { @@ -375,6 +382,33 @@ func (h *p2pHub) countOversize(size int) { time.Since(since).Round(time.Millisecond).String()) } +// countNonIP records one frame that was not a parseable IP packet, reporting +// the first at once and the rest once per window — the same shape as the two +// tallies above, for the same reason: the condition is a symptom of a link +// losing its framing, so it arrives in bursts and has to stay countable +// without becoming the log. +func (h *p2pHub) countNonIP(size int) { + h.nonIPMu.Lock() + now := time.Now() + if h.nonIPSince.IsZero() { + h.nonIPSince = now + h.nonIPCount = 1 + h.nonIPMu.Unlock() + h.warnf("non-IP frame (%d bytes), discarded", size) + return + } + h.nonIPCount++ + if now.Sub(h.nonIPSince) < unroutedWindow { + h.nonIPMu.Unlock() + return + } + n, since := h.nonIPCount, h.nonIPSince + h.nonIPSince, h.nonIPCount = now, 0 + h.nonIPMu.Unlock() + h.warnf("non-IP frames discarded: %d in %s", n, + time.Since(since).Round(time.Millisecond).String()) +} + // fromSpoke handles one datagram from one peer: a keepalive is answered on the // peer it arrived from, a packet for another registered peer is delivered // straight to that peer's stream, and anything else is the device's. @@ -450,6 +484,21 @@ func (h *p2pHub) fromSpoke(s *peerStream, pkt []byte) error { h.countUnrouted(dst) return nil } + } else { + // Not a parseable IPv4 or IPv6 packet, which is the one thing the + // kernel is certain to refuse. Measured on a real up tun device: the + // version nibble is the *only* thing it inspects — anything but 4 or 6 + // comes back EINVAL at any length, while 1421, 1500 and 2000-byte + // frames with a valid header all go through untouched. So this branch, + // not size, is the one that killed the hub: the error left fromSpoke, + // unwound Handle, and took a 54-minute peer stream down with it. + // + // Dropping here is also the symmetric choice: the spoke already refuses + // the same frames before its device write (client.go), which is why a + // phone logs "unknown packet, discarded" all day and never dies while + // the hub had no equivalent. + h.countNonIP(len(pkt)) + return nil } h.wmu.Lock()