wgengine: don't lose filter state on filter reconfig.

We were abandoning the UDP port LRU every time we got a new packet
filter from tailcontrol, which caused return packets to suddenly stop
arriving.

ipn/local.go

@@ -320,7 +320,7 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
 		} else {
 			b.logf("netmap packet filter: (suppressed)\n")
 		}
-		b.e.SetFilter(filter.New(netMap.PacketFilter))
+		b.e.SetFilter(filter.New(netMap.PacketFilter, b.e.GetFilter()))
 	}
 }
 

wgengine/filter/filter.go

@@ -15,11 +15,14 @@
 	"tailscale.com/wgengine/packet"
 )
 
+type filterState struct {
+	mu  sync.Mutex
+	lru *lru.Cache
+}
+
 type Filter struct {
 	matches Matches
-
-	udpMu  sync.Mutex
-	udplru *lru.Cache
+	state   *filterState
 }
 
 type Response int
@@ -66,17 +69,25 @@ type tuple struct {
 }
 
 func NewAllowAll() *Filter {
-	return New(MatchAllowAll)
+	return New(MatchAllowAll, nil)
 }
 
 func NewAllowNone() *Filter {
-	return New(nil)
+	return New(nil, nil)
 }
 
-func New(matches Matches) *Filter {
+func New(matches Matches, shareStateWith *Filter) *Filter {
+	var state *filterState
+	if shareStateWith != nil {
+		state = shareStateWith.state
+	} else {
+		state = &filterState{
+			lru: lru.New(LRU_MAX),
+		}
+	}
 	f := &Filter{
 		matches: matches,
-		udplru:  lru.New(LRU_MAX),
+		state:   state,
 	}
 	return f
 }
@@ -144,6 +155,10 @@ func (f *Filter) runIn(q *packet.QDecode) (r Response, why string) {
 	switch q.IPProto {
 	case packet.ICMP:
 		// If any port is open to an IP, allow ICMP to it.
+		// TODO(apenwarr): allow ICMP packets on existing sessions.
+		//   Right now ICMP Echo Response doesn't always work, and
+		//   probably important ICMP responses on TCP sessions
+		//   also get blocked.
 		if matchIPWithoutPorts(f.matches, q) {
 			return Accept, "icmp ok"
 		}
@@ -165,9 +180,9 @@ func (f *Filter) runIn(q *packet.QDecode) (r Response, why string) {
 	case packet.UDP:
 		t := tuple{q.SrcIP, q.DstIP, q.SrcPort, q.DstPort}
 
-		f.udpMu.Lock()
-		_, ok := f.udplru.Get(t)
-		f.udpMu.Unlock()
+		f.state.mu.Lock()
+		_, ok := f.state.lru.Get(t)
+		f.state.mu.Unlock()
 
 		if ok {
 			return Accept, "udp cached"
@@ -182,12 +197,13 @@ func (f *Filter) runIn(q *packet.QDecode) (r Response, why string) {
 }
 
 func (f *Filter) runOut(q *packet.QDecode) (r Response, why string) {
+	// TODO(apenwarr): create sessions on ICMP Echo Request too.
 	if q.IPProto == packet.UDP {
 		t := tuple{q.DstIP, q.SrcIP, q.DstPort, q.SrcPort}
 
-		f.udpMu.Lock()
-		f.udplru.Add(t, t)
-		f.udpMu.Unlock()
+		f.state.mu.Lock()
+		f.state.lru.Add(t, t)
+		f.state.mu.Unlock()
 	}
 	return Accept, "ok out"
 }

wgengine/filter/filter_test.go

@@ -39,7 +39,7 @@ func TestFilter(t *testing.T) {
 		{SrcIPs: []IP{0}, DstPorts: ippr(0, 443, 443)},
 		{SrcIPs: []IP{0x99010101, 0x99010102, 0x99030303}, DstPorts: ippr(0x01020304, 999, 999)},
 	}
-	acl := New(mm)
+	acl := New(mm, nil)
 
 	for _, ent := range []Matches{Matches{mm[0]}, mm} {
 		b, err := json.Marshal(ent)

wgengine/userspace.go

@@ -36,6 +36,7 @@ type userspaceEngine struct {
 	router    Router
 	magicConn *magicsock.Conn
 	linkMon   *monitor.Mon
+	filt      *filter.Filter
 
 	wgLock       sync.Mutex // serializes all wgdev operations
 	lastReconfig string
@@ -380,7 +381,13 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, dnsDomains []string) error
 	return nil
 }
 
+func (e *userspaceEngine) GetFilter() *filter.Filter {
+	return e.filt
+}
+
 func (e *userspaceEngine) SetFilter(filt *filter.Filter) {
+	e.filt = filt
+
 	var filtin, filtout func(b []byte) device.FilterResult
 	if filt == nil {
 		e.logf("wgengine: nil filter provided; no access restrictions.\n")

wgengine/watchdog.go

@@ -63,6 +63,11 @@ func (e *watchdogEngine) watchdog(name string, fn func()) {
 func (e *watchdogEngine) Reconfig(cfg *wgcfg.Config, dnsDomains []string) error {
 	return e.watchdogErr("Reconfig", func() error { return e.wrap.Reconfig(cfg, dnsDomains) })
 }
+func (e *watchdogEngine) GetFilter() *filter.Filter {
+	var x *filter.Filter
+	e.watchdog("GetFilter", func() { x = e.wrap.GetFilter() })
+	return x
+}
 func (e *watchdogEngine) SetFilter(filt *filter.Filter) {
 	e.watchdog("SetFilter", func() { e.wrap.SetFilter(filt) })
 }

wgengine/wgengine.go

@@ -101,6 +101,9 @@ type Engine interface {
 	// sends an updated network map.
 	Reconfig(cfg *wgcfg.Config, dnsDomains []string) error
 
+	// GetFilter returns the current packet filter, if any
+	GetFilter() *filter.Filter
+
 	// SetFilter updates the packet filter.
 	SetFilter(*filter.Filter)