Browse Source

tailcfg,control/controlclient: treat nil AllowedIPs as Addresses [capver 112]

Updates #14635

Change-Id: I21e2bd1ec4eb384eb7a3fc8379f0788a684893f3
Signed-off-by: Brad Fitzpatrick <[email protected]>
Brad Fitzpatrick 1 year ago
parent
commit
7ecb69e32e
3 changed files with 50 additions and 4 deletions
  1. 4 0
      control/controlclient/map.go
  2. 32 0
      control/controlclient/map_test.go
  3. 14 4
      tailcfg/tailcfg.go

+ 4 - 0
control/controlclient/map.go

@@ -241,6 +241,10 @@ func upgradeNode(n *tailcfg.Node) {
 		}
 		}
 		n.LegacyDERPString = ""
 		n.LegacyDERPString = ""
 	}
 	}
+
+	if n.AllowedIPs == nil {
+		n.AllowedIPs = slices.Clone(n.Addresses)
+	}
 }
 }
 
 
 func (ms *mapSession) tryHandleIncrementally(res *tailcfg.MapResponse) bool {
 func (ms *mapSession) tryHandleIncrementally(res *tailcfg.MapResponse) bool {

+ 32 - 0
control/controlclient/map_test.go

@@ -1007,10 +1007,16 @@ func TestPatchifyPeersChanged(t *testing.T) {
 }
 }
 
 
 func TestUpgradeNode(t *testing.T) {
 func TestUpgradeNode(t *testing.T) {
+	a1 := netip.MustParsePrefix("0.0.0.1/32")
+	a2 := netip.MustParsePrefix("0.0.0.2/32")
+	a3 := netip.MustParsePrefix("0.0.0.3/32")
+	a4 := netip.MustParsePrefix("0.0.0.4/32")
+
 	tests := []struct {
 	tests := []struct {
 		name string
 		name string
 		in   *tailcfg.Node
 		in   *tailcfg.Node
 		want *tailcfg.Node
 		want *tailcfg.Node
+		also func(t *testing.T, got *tailcfg.Node) // optional
 	}{
 	}{
 		{
 		{
 			name: "nil",
 			name: "nil",
@@ -1037,6 +1043,29 @@ func TestUpgradeNode(t *testing.T) {
 			in:   &tailcfg.Node{HomeDERP: 2},
 			in:   &tailcfg.Node{HomeDERP: 2},
 			want: &tailcfg.Node{HomeDERP: 2},
 			want: &tailcfg.Node{HomeDERP: 2},
 		},
 		},
+		{
+			name: "implicit-allowed-ips-all-set",
+			in:   &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}, AllowedIPs: []netip.Prefix{a3, a4}},
+			want: &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}, AllowedIPs: []netip.Prefix{a3, a4}},
+		},
+		{
+			name: "implicit-allowed-ips-only-address-set",
+			in:   &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}},
+			want: &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}, AllowedIPs: []netip.Prefix{a1, a2}},
+			also: func(t *testing.T, got *tailcfg.Node) {
+				if t.Failed() {
+					return
+				}
+				if &got.Addresses[0] == &got.AllowedIPs[0] {
+					t.Error("Addresses and AllowIPs alias the same memory")
+				}
+			},
+		},
+		{
+			name: "implicit-allowed-ips-set-empty-slice",
+			in:   &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}, AllowedIPs: []netip.Prefix{}},
+			want: &tailcfg.Node{Addresses: []netip.Prefix{a1, a2}, AllowedIPs: []netip.Prefix{}},
+		},
 	}
 	}
 	for _, tt := range tests {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
@@ -1048,6 +1077,9 @@ func TestUpgradeNode(t *testing.T) {
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Errorf("wrong result (-want +got):\n%s", diff)
 				t.Errorf("wrong result (-want +got):\n%s", diff)
 			}
 			}
+			if tt.also != nil {
+				tt.also(t, got)
+			}
 		})
 		})
 	}
 	}
 
 

+ 14 - 4
tailcfg/tailcfg.go

@@ -154,7 +154,8 @@ type CapabilityVersion int
 //   - 109: 2024-11-18: Client supports filtertype.Match.SrcCaps (issue #12542)
 //   - 109: 2024-11-18: Client supports filtertype.Match.SrcCaps (issue #12542)
 //   - 110: 2024-12-12: removed never-before-used Tailscale SSH public key support (#14373)
 //   - 110: 2024-12-12: removed never-before-used Tailscale SSH public key support (#14373)
 //   - 111: 2025-01-14: Client supports a peer having Node.HomeDERP (issue #14636)
 //   - 111: 2025-01-14: Client supports a peer having Node.HomeDERP (issue #14636)
-const CurrentCapabilityVersion CapabilityVersion = 111
+//   - 112: 2025-01-14: Client interprets AllowedIPs of nil as meaning same as Addresses
+const CurrentCapabilityVersion CapabilityVersion = 112
 
 
 // ID is an integer ID for a user, node, or login allocated by the
 // ID is an integer ID for a user, node, or login allocated by the
 // control plane.
 // control plane.
@@ -343,9 +344,18 @@ type Node struct {
 	KeySignature tkatype.MarshaledSignature `json:",omitempty"`
 	KeySignature tkatype.MarshaledSignature `json:",omitempty"`
 	Machine      key.MachinePublic
 	Machine      key.MachinePublic
 	DiscoKey     key.DiscoPublic
 	DiscoKey     key.DiscoPublic
-	Addresses    []netip.Prefix   // IP addresses of this Node directly
-	AllowedIPs   []netip.Prefix   // range of IP addresses to route to this node
-	Endpoints    []netip.AddrPort `json:",omitempty"` // IP+port (public via STUN, and local LANs)
+
+	// Addresses are the IP addresses of this Node directly.
+	Addresses []netip.Prefix
+
+	// AllowedIPs are the IP ranges to route to this node.
+	//
+	// As of CapabilityVersion 112, this may be nil (null or undefined) on the wire
+	// to mean the same as Addresses. Internally, it is always filled in with
+	// its possibly-implicit value.
+	AllowedIPs []netip.Prefix
+
+	Endpoints []netip.AddrPort `json:",omitempty"` // IP+port (public via STUN, and local LANs)
 
 
 	// LegacyDERPString is this node's home LegacyDERPString region ID integer, but shoved into an
 	// LegacyDERPString is this node's home LegacyDERPString region ID integer, but shoved into an
 	// IP:port string for legacy reasons. The IP address is always "127.3.3.40"
 	// IP:port string for legacy reasons. The IP address is always "127.3.3.40"