tailcfg, wgengine/filter: remove most FilterRule.SrcBits code

The control plane hasn't sent it to clients in ages.

Updates tailscale/corp#20965

Change-Id: I1d71a4b6dd3f75010a05c544ee39827837c30772
Signed-off-by: Brad Fitzpatrick <[email protected]>

tailcfg/tailcfg.go

@@ -1333,7 +1333,7 @@ var PortRangeAny = PortRange{0, 65535}
 type NetPortRange struct {
 	_     structs.Incomparable
 	IP    string // IP, CIDR, Range, or "*" (same formats as FilterRule.SrcIPs)
-	Bits  *int   // deprecated; the old way to turn IP into a CIDR
+	Bits  *int   // deprecated; the 2020 way to turn IP into a CIDR. See FilterRule.SrcBits.
 	Ports PortRange
 }
 
@@ -1470,7 +1470,7 @@ func (c PeerCapMap) HasCapability(cap PeerCapability) bool {
 // FilterRule represents one rule in a packet filter.
 //
 // A rule is logically a set of source CIDRs to match (described by
-// SrcIPs and SrcBits), and a set of destination targets that are then
+// SrcIPs), and a set of destination targets that are then
 // allowed if a source IP is matches of those CIDRs.
 type FilterRule struct {
 	// SrcIPs are the source IPs/networks to match.
@@ -1482,7 +1482,7 @@ type FilterRule struct {
 	//     * a range of two IPs, inclusive, separated by hyphen ("2eff::1-2eff::0800")
 	SrcIPs []string
 
-	// SrcBits is deprecated; it's the old way to specify a CIDR
+	// SrcBits is deprecated; it was the old way to specify a CIDR
 	// prior to CapabilityVersion 7. Its values correspond to the
 	// SrcIPs above.
 	//
@@ -1493,10 +1493,14 @@ type FilterRule struct {
 	// position is 32, as if the SrcIPs above were a /32 mask. For
 	// a "*" SrcIPs value, the corresponding SrcBits value is
 	// ignored.
+	//
+	// This is still present in this file because the Tailscale control plane
+	// code still uses this type, for 118 clients that are still connected as of
+	// 2024-06-18, 3.5 years after the last release that used this type.
 	SrcBits []int `json:",omitempty"`
 
 	// DstPorts are the port ranges to allow once a source IP
-	// matches (is in the CIDR described by SrcIPs & SrcBits).
+	// matches (is in the CIDR described by SrcIPs).
 	//
 	// CapGrant and DstPorts are mutually exclusive: at most one can be non-nil.
 	DstPorts []NetPortRange `json:",omitempty"`
@@ -1527,11 +1531,9 @@ type FilterRule struct {
 
 var FilterAllowAll = []FilterRule{
 	{
-		SrcIPs:  []string{"*"},
-		SrcBits: nil,
+		SrcIPs: []string{"*"},
 		DstPorts: []NetPortRange{{
 			IP:    "*",
-			Bits:  nil,
 			Ports: PortRange{0, 65535},
 		}},
 	},

util/deephash/deephash_test.go

@@ -483,8 +483,8 @@ func TestGetTypeHasher(t *testing.T) {
 		{
 			name:  "packet_filter",
 			val:   filterRules,
-			out:   "\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00*\v\x00\x00\x00\x00\x00\x00\x0010.1.3.4/32\v\x00\x00\x00\x00\x00\x00\x0010.0.0.0/24\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00\x00\x00\x00\x001.2.3.4/32\x01 \x00\x00\x00\x00\x00\x00\x00\x01\x00\x02\x00\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04!\x01\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00foo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00",
-			out32: "\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00*\v\x00\x00\x00\x00\x00\x00\x0010.1.3.4/32\v\x00\x00\x00\x00\x00\x00\x0010.0.0.0/24\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x03\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00\x00\x00\x00\x001.2.3.4/32\x01 \x00\x00\x00\x01\x00\x02\x00\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x02\x00\x00\x00\x03\x00\x00\x00\x04\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04!\x01\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00foo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00",
+			out:   "\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00*\v\x00\x00\x00\x00\x00\x00\x0010.1.3.4/32\v\x00\x00\x00\x00\x00\x00\x0010.0.0.0/24\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00\x00\x00\x00\x001.2.3.4/32\x01 \x00\x00\x00\x00\x00\x00\x00\x01\x00\x02\x00\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04!\x01\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00foo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00",
+			out32: "\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x03\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00*\v\x00\x00\x00\x00\x00\x00\x0010.1.3.4/32\v\x00\x00\x00\x00\x00\x00\x0010.0.0.0/24\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00\x00\x00\x00\x001.2.3.4/32\x01 \x00\x00\x00\x00\x00\x00\x00\x01\x00\x02\x00\x01\x04\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\x01\x02\x03\x04!\x01\x01\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00foo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\v\x00\x00\x00\x00\x00\x00\x00foooooooooo\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\f\x00\x00\x00\x00\x00\x00\x00baaaaaarrrrr\x00\x01\x00\x02\x00\x00\x00",
 		},
 		{
 			name: "netip.Addr",
@@ -771,8 +771,7 @@ func BenchmarkHash(b *testing.B) {
 // packet filters as sent to clients.
 var filterRules = []tailcfg.FilterRule{
 	{
-		SrcIPs:  []string{"*", "10.1.3.4/32", "10.0.0.0/24"},
-		SrcBits: []int{1, 2, 3},
+		SrcIPs: []string{"*", "10.1.3.4/32", "10.0.0.0/24"},
 		DstPorts: []tailcfg.NetPortRange{{
 			IP:    "1.2.3.4/32",
 			Bits:  ptr.To(32),

wgengine/filter/filter_test.go

@@ -251,41 +251,30 @@ func TestNoAllocs(t *testing.T) {
 func TestParseIPSet(t *testing.T) {
 	tests := []struct {
 		host    string
-		bits    int
 		want    []netip.Prefix
 		wantErr string
 	}{
-		{"8.8.8.8", 24, pfx("8.8.8.8/24"), ""},
-		{"2601:1234::", 64, pfx("2601:1234::/64"), ""},
-		{"8.8.8.8", 33, nil, `invalid CIDR size 33 for IP "8.8.8.8"`},
-		{"8.8.8.8", -1, pfx("8.8.8.8/32"), ""},
-		{"8.8.8.8", 32, pfx("8.8.8.8/32"), ""},
-		{"8.8.8.8/24", -1, nil, "8.8.8.8/24 contains non-network bits set"},
-		{"8.8.8.0/24", 18, pfx("8.8.8.0/24"), ""}, // the 18 is ignored
-		{"1.0.0.0-1.255.255.255", 5, pfx("1.0.0.0/8"), ""},
-		{"1.0.0.0-2.1.2.3", 5, pfx("1.0.0.0/8", "2.0.0.0/16", "2.1.0.0/23", "2.1.2.0/30"), ""},
-		{"1.0.0.2-1.0.0.1", -1, nil, "invalid IP range \"1.0.0.2-1.0.0.1\""},
-		{"2601:1234::", 129, nil, `invalid CIDR size 129 for IP "2601:1234::"`},
-		{"0.0.0.0", 24, pfx("0.0.0.0/24"), ""},
-		{"::", 64, pfx("::/64"), ""},
-		{"*", 24, pfx("0.0.0.0/0", "::/0"), ""},
+		{"8.8.8.8", pfx("8.8.8.8/32"), ""},
+		{"1::2", pfx("1::2/128"), ""},
+		{"8.8.8.0/24", pfx("8.8.8.0/24"), ""},
+		{"8.8.8.8/24", nil, "8.8.8.8/24 contains non-network bits set"},
+		{"1.0.0.0-1.255.255.255", pfx("1.0.0.0/8"), ""},
+		{"1.0.0.0-2.1.2.3", pfx("1.0.0.0/8", "2.0.0.0/16", "2.1.0.0/23", "2.1.2.0/30"), ""},
+		{"1.0.0.2-1.0.0.1", nil, "invalid IP range \"1.0.0.2-1.0.0.1\""},
+		{"*", pfx("0.0.0.0/0", "::/0"), ""},
 	}
 	for _, tt := range tests {
-		var bits *int
-		if tt.bits != -1 {
-			bits = &tt.bits
-		}
-		got, err := parseIPSet(tt.host, bits)
+		got, err := parseIPSet(tt.host)
 		if err != nil {
 			if err.Error() == tt.wantErr {
 				continue
 			}
-			t.Errorf("parseIPSet(%q, %v) error: %v; want error %q", tt.host, tt.bits, err, tt.wantErr)
+			t.Errorf("parseIPSet(%q) error: %v; want error %q", tt.host, err, tt.wantErr)
 		}
 		compareIP := cmp.Comparer(func(a, b netip.Addr) bool { return a == b })
 		compareIPPrefix := cmp.Comparer(func(a, b netip.Prefix) bool { return a == b })
 		if diff := cmp.Diff(got, tt.want, compareIP, compareIPPrefix); diff != "" {
-			t.Errorf("parseIPSet(%q, %v) = %s; want %s", tt.host, tt.bits, got, tt.want)
+			t.Errorf("parseIPSet(%q) = %s; want %s", tt.host, got, tt.want)
 			continue
 		}
 	}

wgengine/filter/tailcfg.go

@@ -33,6 +33,9 @@ func MatchesFromFilterRules(pf []tailcfg.FilterRule) ([]Match, error) {
 	var erracc error
 
 	for _, r := range pf {
+		if len(r.SrcBits) > 0 {
+			return nil, fmt.Errorf("unexpected SrcBits; control plane should not send this to this client version")
+		}
 		// Profiling determined that this function was spending a lot
 		// of time in runtime.growslice. As such, we attempt to
 		// pre-allocate some slices. Multipliers were chosen arbitrarily.
@@ -54,12 +57,8 @@ func MatchesFromFilterRules(pf []tailcfg.FilterRule) ([]Match, error) {
 			m.IPProto = views.SliceOf(filtered)
 		}
 
-		for i, s := range r.SrcIPs {
-			var bits *int
-			if len(r.SrcBits) > i {
-				bits = &r.SrcBits[i]
-			}
-			nets, err := parseIPSet(s, bits)
+		for _, s := range r.SrcIPs {
+			nets, err := parseIPSet(s)
 			if err != nil && erracc == nil {
 				erracc = err
 				continue
@@ -69,7 +68,10 @@ func MatchesFromFilterRules(pf []tailcfg.FilterRule) ([]Match, error) {
 		m.SrcsContains = ipset.NewContainsIPFunc(views.SliceOf(m.Srcs))
 
 		for _, d := range r.DstPorts {
-			nets, err := parseIPSet(d.IP, d.Bits)
+			if d.Bits != nil {
+				return nil, fmt.Errorf("unexpected DstBits; control plane should not send this to this client version")
+			}
+			nets, err := parseIPSet(d.IP)
 			if err != nil && erracc == nil {
 				erracc = err
 				continue
@@ -119,14 +121,11 @@ var (
 //   - a CIDR (e.g. "192.168.0.0/16")
 //   - a range of two IPs, inclusive, separated by hyphen ("2eff::1-2eff::0800")
 //
-// bits, if non-nil, is the legacy SrcBits CIDR length to make a IP
-// address (without a slash) treated as a CIDR of *bits length.
-//
 // TODO(bradfitz): make this return an IPSet and plumb that all
 // around, and ultimately use a new version of IPSet.ContainsFunc like
 // Contains16Func that works in [16]byte address, so we we can match
 // at runtime without allocating?
-func parseIPSet(arg string, bits *int) ([]netip.Prefix, error) {
+func parseIPSet(arg string) ([]netip.Prefix, error) {
 	if arg == "*" {
 		// User explicitly requested wildcard.
 		return []netip.Prefix{
@@ -155,7 +154,7 @@ func parseIPSet(arg string, bits *int) ([]netip.Prefix, error) {
 			return nil, err
 		}
 		r := netipx.IPRangeFrom(ip1, ip2)
-		if !r.Valid() {
+		if !r.IsValid() {
 			return nil, fmt.Errorf("invalid IP range %q", arg)
 		}
 		return r.Prefixes(), nil
@@ -164,12 +163,5 @@ func parseIPSet(arg string, bits *int) ([]netip.Prefix, error) {
 	if err != nil {
 		return nil, fmt.Errorf("invalid IP address %q", arg)
 	}
-	bits8 := uint8(ip.BitLen())
-	if bits != nil {
-		if *bits < 0 || *bits > int(bits8) {
-			return nil, fmt.Errorf("invalid CIDR size %d for IP %q", *bits, arg)
-		}
-		bits8 = uint8(*bits)
-	}
-	return []netip.Prefix{netip.PrefixFrom(ip, int(bits8))}, nil
+	return []netip.Prefix{netip.PrefixFrom(ip, ip.BitLen())}, nil
 }