control/controlclient: support incremental packet filter updates [capver 81]

Updates #10299 Change-Id: I87e4235c668a1db7de7ef1abc743f0beecb86d3d Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2 years ago · fb829ea7f1
parent b8a2aedccd
commit fb829ea7f1
4 changed files with 156 additions and 4 deletions
--- a/control/controlclient/map.go
+++ b/control/controlclient/map.go
@ -16,6 +16,7 @@ import (
 	"sync"
 	"time"

+	xmaps "golang.org/x/exp/maps"
 	"tailscale.com/control/controlknobs"
 	"tailscale.com/envknob"
 	"tailscale.com/tailcfg"
@ -27,6 +28,7 @@ import (
 	"tailscale.com/types/views"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/cmpx"
+	"tailscale.com/util/mak"
 	"tailscale.com/wgengine/filter"
 )

@ -75,7 +77,8 @@ type mapSession struct {
 	lastDNSConfig          *tailcfg.DNSConfig
 	lastDERPMap            *tailcfg.DERPMap
 	lastUserProfile        map[tailcfg.UserID]tailcfg.UserProfile
-	lastPacketFilterRules  views.Slice[tailcfg.FilterRule]
+	lastPacketFilterRules  views.Slice[tailcfg.FilterRule] // concatenation of all namedPacketFilters
+	namedPacketFilters     map[string]views.Slice[tailcfg.FilterRule]
 	lastParsedPacketFilter []filter.Match
 	lastSSHPolicy          *tailcfg.SSHPolicy
 	collectServices        bool
@ -259,10 +262,39 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) {
 		ms.lastDERPMap = dm
 	}

+	var packetFilterChanged bool
+	// Older way, one big blob:
 	if pf := resp.PacketFilter; pf != nil {
+		packetFilterChanged = true
+		mak.Set(&ms.namedPacketFilters, "base", views.SliceOf(pf))
+	}
+	// Newer way, named chunks:
+	if m := resp.PacketFilters; m != nil {
+		packetFilterChanged = true
+		if v, ok := m["*"]; ok && v == nil {
+			ms.namedPacketFilters = nil
+		}
+		for k, v := range m {
+			if k == "*" {
+				continue
+			}
+			if v != nil {
+				mak.Set(&ms.namedPacketFilters, k, views.SliceOf(v))
+			} else {
+				delete(ms.namedPacketFilters, k)
+			}
+		}
+	}
+	if packetFilterChanged {
+		keys := xmaps.Keys(ms.namedPacketFilters)
+		sort.Strings(keys)
+		var concat []tailcfg.FilterRule
+		for _, v := range keys {
+			concat = ms.namedPacketFilters[v].AppendTo(concat)
+		}
+		ms.lastPacketFilterRules = views.SliceOf(concat)
 		var err error
-		ms.lastPacketFilterRules = views.SliceOf(pf)
-		ms.lastParsedPacketFilter, err = filter.MatchesFromFilterRules(pf)
+		ms.lastParsedPacketFilter, err = filter.MatchesFromFilterRules(concat)
 		if err != nil {
 			ms.logf("parsePacketFilter: %v", err)
 		}
--- a/control/controlclient/map_test.go
+++ b/control/controlclient/map_test.go
@ -547,6 +547,98 @@ func TestNetmapForResponse(t *testing.T) {
 			t.Errorf("Node mismatch in 2nd netmap; got: %s", j)
 		}
 	})
+	t.Run("named_packetfilter", func(t *testing.T) {
+		pfA := []tailcfg.FilterRule{
+			{
+				SrcIPs: []string{"10.0.0.1"},
+				DstPorts: []tailcfg.NetPortRange{
+					{IP: "10.2.3.4", Ports: tailcfg.PortRange{First: 22, Last: 22}},
+				},
+			},
+		}
+		pfB := []tailcfg.FilterRule{
+			{
+				SrcIPs: []string{"10.0.0.2"},
+				DstPorts: []tailcfg.NetPortRange{
+					{IP: "10.2.3.4", Ports: tailcfg.PortRange{First: 22, Last: 22}},
+				},
+			},
+		}
+		ms := newTestMapSession(t, nil)
+
+		// Mix of old & new style (PacketFilter and PacketFilters).
+		nm1 := ms.netmapForResponse(&tailcfg.MapResponse{
+			Node:         new(tailcfg.Node),
+			PacketFilter: pfA,
+			PacketFilters: map[string][]tailcfg.FilterRule{
+				"pf-b": pfB,
+			},
+		})
+		if got, want := len(nm1.PacketFilter), 2; got != want {
+			t.Fatalf("PacketFilter length = %v; want %v", got, want)
+		}
+		if got, want := first(nm1.PacketFilter[0].Srcs).String(), "10.0.0.1/32"; got != want {
+			t.Fatalf("PacketFilter[0].Srcs = %v; want %v", got, want)
+		}
+		if got, want := first(nm1.PacketFilter[1].Srcs).String(), "10.0.0.2/32"; got != want {
+			t.Fatalf("PacketFilter[0].Srcs = %v; want %v", got, want)
+		}
+
+		// No-op change. Remember the old stuff.
+		nm2 := ms.netmapForResponse(&tailcfg.MapResponse{
+			Node:          new(tailcfg.Node),
+			PacketFilter:  nil,
+			PacketFilters: nil,
+		})
+		if got, want := len(nm2.PacketFilter), 2; got != want {
+			t.Fatalf("PacketFilter length = %v; want %v", got, want)
+		}
+		if !reflect.DeepEqual(nm1.PacketFilter, nm2.PacketFilter) {
+			t.Error("packet filters differ")
+		}
+
+		// New style only, with clear.
+		nm3 := ms.netmapForResponse(&tailcfg.MapResponse{
+			Node:         new(tailcfg.Node),
+			PacketFilter: nil,
+			PacketFilters: map[string][]tailcfg.FilterRule{
+				"*":    nil,
+				"pf-b": pfB,
+			},
+		})
+		if got, want := len(nm3.PacketFilter), 1; got != want {
+			t.Fatalf("PacketFilter length = %v; want %v", got, want)
+		}
+		if got, want := first(nm3.PacketFilter[0].Srcs).String(), "10.0.0.2/32"; got != want {
+			t.Fatalf("PacketFilter[0].Srcs = %v; want %v", got, want)
+		}
+
+		// New style only, adding pfA back, not as the legacy "base" layer:.
+		nm4 := ms.netmapForResponse(&tailcfg.MapResponse{
+			Node:         new(tailcfg.Node),
+			PacketFilter: nil,
+			PacketFilters: map[string][]tailcfg.FilterRule{
+				"pf-a": pfA,
+			},
+		})
+		if got, want := len(nm4.PacketFilter), 2; got != want {
+			t.Fatalf("PacketFilter length = %v; want %v", got, want)
+		}
+		if got, want := first(nm4.PacketFilter[0].Srcs).String(), "10.0.0.1/32"; got != want {
+			t.Fatalf("PacketFilter[0].Srcs = %v; want %v", got, want)
+		}
+		if got, want := first(nm4.PacketFilter[1].Srcs).String(), "10.0.0.2/32"; got != want {
+			t.Fatalf("PacketFilter[0].Srcs = %v; want %v", got, want)
+		}
+	})
+}
+
+func first[T any](s []T) T {
+	if len(s) == 0 {
+		var zero T
+		return zero
+	}
+	return s[0]
 }

 func TestDeltaDERPMap(t *testing.T) {
--- a/tailcfg/tailcfg.go
+++ b/tailcfg/tailcfg.go
@ -121,7 +121,8 @@ type CapabilityVersion int
 //   - 78: 2023-10-05: can handle c2n Wake-on-LAN sending
 //   - 79: 2023-10-05: Client understands UrgentSecurityUpdate in ClientVersion
 //   - 80: 2023-11-16: can handle c2n GET /tls-cert-status
-const CurrentCapabilityVersion CapabilityVersion = 80
+//   - 81: 2023-11-17: MapResponse.PacketFilters (incremental packet filter updates)
+const CurrentCapabilityVersion CapabilityVersion = 81

 type StableID string

@ -1797,8 +1798,34 @@ type MapResponse struct {
 	// Note that this package's type, due its use of a slice and omitempty, is
 	// unable to marshal a zero-length non-nil slice. The control server needs
 	// to marshal this type using a separate type. See MapResponse docs.
+	//
+	// See PacketFilters for the newer way to send PacketFilter updates.
 	PacketFilter []FilterRule `json:",omitempty"`

+	// PacketFilters encodes incremental packet filter updates to the client
+	// without having to send the entire packet filter on any changes as
+	// required by the older PacketFilter (singular) field above. The map keys
+	// are server-assigned arbitrary strings. The map values are the new rules
+	// for that key, or nil to delete it. The client then concatenates all the
+	// rules together to generate the final packet filter. Because the
+	// FilterRules can only match or not match, the ordering of filter rules
+	// doesn't matter. (That said, the client generates the file merged packet
+	// filter rules by concananting all the packet filter rules sorted by the
+	// map key name. But it does so for stability and testability, not
+	// correctness. If something needs to rely on that property, something has
+	// gone wrong.)
+	//
+	// If the server sends a non-nil PacketFilter (above), that is equivalent to
+	// a named packet filter with the key "base". It is valid for the server to
+	// send both PacketFilter and PacketFilters in the same MapResponse or
+	// alternate between them within a session. The PacketFilter is applied
+	// first (if set) and then the PacketFilters.
+	//
+	// As a special case, the map key "*" with a value of nil means to clear all
+	// prior named packet filters (including any implicit "base") before
+	// processing the other map entries.
+	PacketFilters map[string][]FilterRule `json:",omitempty"`
+
 	// UserProfiles are the user profiles of nodes in the network.
 	// As as of 1.1.541 (mapver 5), this contains new or updated
 	// user profiles only.
--- a/types/netmap/nodemut.go
+++ b/types/netmap/nodemut.go
@ -161,6 +161,7 @@ func mapResponseContainsNonPatchFields(res *tailcfg.MapResponse) bool {
 		res.Domain != "" ||
 		res.CollectServices != "" ||
 		res.PacketFilter != nil ||
+		res.PacketFilters != nil ||
 		res.UserProfiles != nil ||
 		res.Health != nil ||
 		res.SSHPolicy != nil ||