diff --git a/net/tstun/wrap.go b/net/tstun/wrap.go index 7fef73b2e..ec7bc94ea 100644 --- a/net/tstun/wrap.go +++ b/net/tstun/wrap.go @@ -1101,7 +1101,10 @@ func invertGSOChecksum(pkt []byte, gso netstack_GSO) { pkt[at+1] = ^pkt[at+1] } -// injectedRead handles injected reads, which bypass filters. +// injectedRead handles injected reads. Injected packets bypass the outbound +// filter rules, but UDP/SCTP flow state is still recorded via +// [filter.Filter.UpdateOutboundFlowState] so inbound replies are admitted by +// [filter.Filter.RunIn]. func (t *Wrapper) injectedRead(res tunInjectedRead, outBuffs [][]byte, sizes []int, offset int) (n int, err error) { var gso netstack_GSO @@ -1128,6 +1131,28 @@ func (t *Wrapper) injectedRead(res tunInjectedRead, outBuffs [][]byte, sizes []i defer parsedPacketPool.Put(p) p.Decode(pkt) + // Record reverse-flow connection-tracking state for this outbound packet so + // that inbound replies are admitted by the filter. Injected packets bypass + // the regular RunOut path that records this state for UDP/SCTP flows; doing + // it here keeps userspace-networking and tsnet UDP replies from being + // dropped as "no matching rule". This must run before SNAT so the tracked + // tuple matches what RunIn sees after DNAT on the inbound side. Select + // between the normal and jailed filters the same way + // filterPacketOutboundToWireGuard does, so jailed peers (e.g. Mullvad exit + // nodes) record state on the filter that will run on the reply. See #14229 + // and #20064. + if !t.disableFilter { + var filt *filter.Filter + if pc.outboundPacketIsJailed(p) { + filt = t.jailedFilter.Load() + } else { + filt = t.filter.Load() + } + if filt != nil { + filt.UpdateOutboundFlowState(p) + } + } + invertGSOChecksum(pkt, gso) pc.snat(p) invertGSOChecksum(pkt, gso) @@ -1500,7 +1525,8 @@ func (t *Wrapper) injectOutboundPong(pp *packet.Parsed, req packet.TSMPPingReque // InjectOutbound makes the Wrapper device behave as if a packet // with the given contents was sent to the network. // It does not block, but takes ownership of the packet. -// The injected packet will not pass through outbound filters. +// The injected packet will not pass through outbound filter rules, +// but UDP/SCTP flow state is recorded so inbound replies are admitted. // Injecting an empty packet is a no-op. func (t *Wrapper) InjectOutbound(pkt []byte) error { if len(pkt) > MaxPacketSize { diff --git a/net/tstun/wrap_test.go b/net/tstun/wrap_test.go index 57b300513..644bb9f53 100644 --- a/net/tstun/wrap_test.go +++ b/net/tstun/wrap_test.go @@ -458,6 +458,71 @@ func TestFilter(t *testing.T) { assertMetricPackets(t, "outACL", 0, metricOutboundDroppedPacketsACL) } +// TestInjectOutboundRecordsUDPFlowState verifies that an injected outbound UDP +// packet (as produced by netstack on userspace-networking / tsnet / SOCKS5 +// callers) records reverse-flow state so that the matching inbound reply is +// admitted by the inbound filter, even when no explicit ACL rule covers the +// reply. See tailscale/tailscale#14229 and tailscale/tailscale#20064. +func TestInjectOutboundRecordsUDPFlowState(t *testing.T) { + bus := eventbustest.NewBus(t) + chtun, tun := newChannelTUN(t.Logf, bus, true) // secure: install filter + defer tun.Close() + + // 53 isn't in setfilter's allowed inbound port range (89-90), so a reply + // from 5.6.7.8:53 → 1.2.3.4: is only admissible via reverse-flow + // state recorded by the prior outbound packet. + const localPort, peerPort = 33333, 53 + const localIP, peerIP = "1.2.3.4", "5.6.7.8" + + // Inject a UDP packet outbound. Run in a goroutine since + // InjectOutbound blocks on the unbuffered vectorOutbound channel + // until Read drains it. + go func() { + if err := tun.InjectOutbound(udp4(localIP, peerIP, localPort, peerPort)); err != nil { + t.Errorf("InjectOutbound: %v", err) + } + }() + + // Drain the injected packet via Read. This drives injectedRead, which + // is what records the reverse-flow tuple in filter state. + var buf [MaxPacketSize]byte + sizes := make([]int, 1) + if n, err := tun.Read([][]byte{buf[:]}, sizes, 0); err != nil { + t.Fatalf("Read: %v", err) + } else if n != 1 { + t.Fatalf("Read returned %d packets, want 1", n) + } + + // Now simulate the inbound UDP reply. Without flow-state tracking on the + // injected outbound path, the inbound filter has no matching rule and + // drops the reply silently. With tracking, it should be delivered. + replyPkt := udp4(peerIP, localIP, peerPort, localPort) + + // tun.Write blocks writing to chtun.Inbound when the filter accepts the + // packet, so drain Inbound concurrently and confirm delivery there. + delivered := make(chan []byte, 1) + go func() { + select { + case got := <-chtun.Inbound: + delivered <- got + case <-tun.closed: + } + }() + + if _, err := tun.Write([][]byte{replyPkt}, 0); err != nil { + t.Fatalf("Write: %v", err) + } + + select { + case got := <-delivered: + if !bytes.Equal(got, replyPkt) { + t.Errorf("delivered packet mismatch") + } + case <-time.After(5 * time.Second): + t.Fatal("inbound UDP reply was dropped by filter; injected outbound did not record flow state") + } +} + func assertMetricPackets(t *testing.T, metricName string, want, got int64) { t.Helper() if want != got { diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index 3c9177937..ce1162fc6 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -63,6 +63,7 @@ import ( "tailscale.com/types/views" "tailscale.com/util/mak" "tailscale.com/util/must" + "tailscale.com/wgengine/filter" ) // pingTimeout returns a per-ping budget for use within the larger test ctx: @@ -3169,6 +3170,146 @@ func TestDialUDP(t *testing.T) { }) } +// TestDialUDPInjectedReadRecordsFlowState reproduces tailscale/tailscale#14229 +// and #20064: a tsnet/netstack client dialing UDP must record reverse-flow +// state in its inbound filter for the outbound packet it injects via +// [netstack.Impl] → [tstun.Wrapper.InjectOutboundPacketBuffer]. If it doesn't, +// the inbound reply is silently dropped by the inbound packet filter when no +// ACL rule explicitly admits it. +// +// [TestDialUDP] doesn't catch this because [testcontrol.Server] serves +// [tailcfg.FilterAllowAll] by default, so the reply is always admitted by +// rule and the flow-state path is never exercised. Each subtest below sets +// up s1 and s2 so that the inbound reply is admissible only via the +// reverse-flow state recorded when s2 dialed. +func TestDialUDPInjectedReadRecordsFlowState(t *testing.T) { + // RestrictedACL replaces the default allow-all PacketFilter with a + // one-way rule that permits s2 → s1 only. The reply path s1 → s2 matches + // no rule, so it can only be admitted by reverse-flow state on s2's + // main filter. + t.Run("RestrictedACL", func(t *testing.T) { + lt := setupTwoClientTest(t, false) // netstack on both sides. + rule := []tailcfg.FilterRule{{ + SrcIPs: []string{lt.s2ip4.String(), lt.s2ip6.String()}, + DstPorts: []tailcfg.NetPortRange{ + {IP: lt.s1ip4.String(), Ports: tailcfg.PortRange{First: 0, Last: 65535}}, + {IP: lt.s1ip6.String(), Ports: tailcfg.PortRange{First: 0, Last: 65535}}, + }, + IPProto: []int{int(ipproto.TCP), int(ipproto.UDP)}, + }} + + for _, s := range []*Server{lt.s1, lt.s2} { + if !lt.control.AddRawMapResponse(s.lb.NodeKey(), &tailcfg.MapResponse{ + PacketFilter: rule, + }) { + t.Fatalf("AddRawMapResponse(%s) failed", s.Hostname) + } + } + + // PacketFilter-only changes don't necessarily fire peer/netmap + // notifications, so poll the wgengine filter directly. + if err := tstest.WaitFor(30*time.Second, func() error { + f := lt.s2.lb.GetFilterForTest() + if f == nil { + return errors.New("no filter yet") + } + if got := f.Check(lt.s1ip4, lt.s2ip4, 1234, ipproto.UDP); got != filter.Drop { + return fmt.Errorf("inbound s1 → s2:1234 UDP: got %v, want Drop", got) + } + return nil + }); err != nil { + t.Fatalf("waiting for restrictive filter on s2: %v", err) + } + + runDialUDPEcho(t, lt) + }) + + // JailedPeer marks s1 as jailed from s2's perspective. tstun.Wrapper + // then routes s2's outbound to s1 and inbound from s1 through a + // separate "jailed" filter (a shields-up filter with no rules; also + // used for Mullvad exit nodes). The reply path can only be admitted + // by reverse-flow state on the *jailed* filter, so injectedRead must + // select the right filter for the outbound packet. + t.Run("JailedPeer", func(t *testing.T) { + lt := setupTwoClientTest(t, false) // netstack on both sides. + s1Key := lt.s1.lb.NodeKey() + lt.control.SetJailed(lt.s2.lb.NodeKey(), s1Key, true) + + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + if err := waitFor(t, ctx, lt.s2, func(nm *netmap.NetworkMap) bool { + for _, p := range nm.Peers { + if p.Key() == s1Key && p.IsJailed() { + return true + } + } + return false + }); err != nil { + t.Fatalf("waiting for s1 to appear jailed in s2's netmap: %v", err) + } + + runDialUDPEcho(t, lt) + }) +} + +// runDialUDPEcho runs an s2.Dial("udp", s1-listener)/Write/Read round trip +// against listeners on lt.s1's IPv4 and IPv6 addresses as t.Run subtests, +// asserting that the echoed reply makes it back to s2. The caller is +// responsible for configuring lt so that the inbound reply on s2 is only +// admissible via reverse-flow state recorded by the outbound dial. +func runDialUDPEcho(t *testing.T, lt *listenTest) { + t.Helper() + test := func(t *testing.T, listenIP netip.Addr) { + pc, err := lt.s1.ListenPacket("udp", netip.AddrPortFrom(listenIP, 0).String()) + if err != nil { + t.Fatalf("ListenPacket: %v", err) + } + defer pc.Close() + + echoErr := make(chan error, 1) + go func() { + buf := make([]byte, 1500) + n, addr, err := pc.ReadFrom(buf) + if err != nil { + echoErr <- err + return + } + if _, err := pc.WriteTo(buf[:n], addr); err != nil { + echoErr <- err + } + }() + + conn, err := lt.s2.Dial(t.Context(), "udp", pc.LocalAddr().String()) + if err != nil { + t.Fatalf("Dial: %v", err) + } + defer conn.Close() + + want := "hello udp" + if _, err := conn.Write([]byte(want)); err != nil { + t.Fatalf("Write: %v", err) + } + + conn.SetReadDeadline(time.Now().Add(5 * time.Second)) + got := make([]byte, 1024) + n, err := conn.Read(got) + if err != nil { + select { + case e := <-echoErr: + t.Fatalf("echo error: %v; read error: %v", e, err) + default: + t.Fatalf("UDP reply dropped — injectedRead didn't record flow state on the filter that runs on the reply (#14229, #20064): %v", err) + } + } + if string(got[:n]) != want { + t.Errorf("got %q, want %q", got[:n], want) + } + } + + t.Run("IPv4", func(t *testing.T) { test(t, lt.s1ip4) }) + t.Run("IPv6", func(t *testing.T) { test(t, lt.s1ip6) }) +} + // buildDNSQuery builds a UDP/IP packet containing a DNS query for name to the // Tailscale service IP (100.100.100.100 for IPv4, fd7a:115c:a1e0::53 for IPv6). func buildDNSQuery(name string, srcIP netip.Addr) []byte { diff --git a/wgengine/filter/filter.go b/wgengine/filter/filter.go index b2be836c7..7fdf4d024 100644 --- a/wgengine/filter/filter.go +++ b/wgengine/filter/filter.go @@ -621,8 +621,25 @@ func (f *Filter) runIn6(q *packet.Parsed) (r Response, why string) { return noVerdict, "no rules matched" } -// runIn runs the output-specific part of the filter logic. +// runOut runs the output-specific part of the filter logic. func (f *Filter) runOut(q *packet.Parsed) (r Response, why string) { + f.UpdateOutboundFlowState(q) + return Accept, "ok out" +} + +// UpdateOutboundFlowState records reverse-flow connection-tracking state for +// the given outbound packet so that subsequent inbound replies on the same +// flow are admitted by [Filter.RunIn] without an explicit allow rule. +// +// Only UDP and SCTP packets are tracked; for other protocols this is a no-op. +// +// It is intended for callers that synthesize outbound packets and bypass +// [Filter.RunOut] (for example netstack's [InjectOutbound] path used by +// userspace networking, tsnet and the SOCKS5/HTTP proxies), so that reply +// packets matching an outbound UDP flow are not silently dropped as "no +// matching rule" by [Filter.RunIn]. See tailscale/tailscale#14229 and +// tailscale/tailscale#20064. +func (f *Filter) UpdateOutboundFlowState(q *packet.Parsed) { switch q.IPProto { case ipproto.UDP, ipproto.SCTP: tuple := flowtrack.MakeTuple(q.IPProto, q.Dst, q.Src) // src/dst reversed @@ -630,7 +647,6 @@ func (f *Filter) runOut(q *packet.Parsed) (r Response, why string) { f.state.lru.Add(tuple, struct{}{}) f.state.mu.Unlock() } - return Accept, "ok out" } // direction is whether a packet was flowing into this machine, or