From fb829ea7f1126da933d85d846eed80e659d07a64 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 17 Nov 2023 09:20:08 -0800 Subject: [PATCH] control/controlclient: support incremental packet filter updates [capver 81] Updates #10299 Change-Id: I87e4235c668a1db7de7ef1abc743f0beecb86d3d Signed-off-by: Brad Fitzpatrick --- control/controlclient/map.go | 38 ++++++++++++- control/controlclient/map_test.go | 92 +++++++++++++++++++++++++++++++ tailcfg/tailcfg.go | 29 +++++++++- types/netmap/nodemut.go | 1 + 4 files changed, 156 insertions(+), 4 deletions(-) diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 8f0d4cac2..90bf83213 100644 --- 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) } diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 50d659a5a..bdbfaf6e4 100644 --- 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) { diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 320d0abbc..5d4e5598d 100644 --- 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. diff --git a/types/netmap/nodemut.go b/types/netmap/nodemut.go index 31734aa72..932e0c186 100644 --- 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 ||