tailcfg: add Node.UnsignedPeerAPIOnly to let server mark node as peerapi-only

capver 48

Change-Id: I20b2fa81d61ef8cc8a84e5f2afeefb68832bd904
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

health/health.go

@@ -72,6 +72,10 @@ const (
 	// the Windows network adapter's "category" (public, private, domain).
 	// If it's unhealthy, the Windows firewall rules won't match.
 	SysNetworkCategory = Subsystem("network-category")
+
+	// SysValidUnsignedNodes is a health check area for recording problems
+	// with the unsigned nodes that the coordination server sent.
+	SysValidUnsignedNodes = Subsystem("valid-unsigned-nodes")
 )
 
 type watchHandle byte
@@ -99,6 +103,9 @@ func RegisterWatcher(cb func(key Subsystem, err error)) (unregister func()) {
 	}
 }
 
+// SetValidUnsignedNodes sets the state of the map response validation.
+func SetValidUnsignedNodes(err error) { set(SysValidUnsignedNodes, err) }
+
 // SetRouterHealth sets the state of the wgengine/router.Router.
 func SetRouterHealth(err error) { set(SysRouter, err) }
 

ipn/ipnlocal/local.go

@@ -1303,6 +1303,14 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs ipn.P
 			localNetsB.AddPrefix(p)
 		}
 		packetFilter = netMap.PacketFilter
+
+		if packetFilterPermitsUnlockedNodes(netMap.Peers, packetFilter) {
+			err := errors.New("server sent invalid packet filter permitting traffic to unlocked nodes; rejecting all packets for safety")
+			health.SetValidUnsignedNodes(err)
+			packetFilter = nil
+		} else {
+			health.SetValidUnsignedNodes(nil)
+		}
 	}
 	if prefs.Valid() {
 		ar := prefs.AdvertiseRoutes()
@@ -1375,6 +1383,45 @@ func (b *LocalBackend) updateFilterLocked(netMap *netmap.NetworkMap, prefs ipn.P
 	}
 }
 
+// packetFilterPermitsUnlockedNodes reports any peer in peers with the
+// UnsignedPeerAPIOnly bool set true has any of its allowed IPs in the packet
+// filter.
+//
+// If this reports true, the packet filter is invalid (the server is either broken
+// or malicious) and should be ignored for safety.
+func packetFilterPermitsUnlockedNodes(peers []*tailcfg.Node, packetFilter []filter.Match) bool {
+	var b netipx.IPSetBuilder
+	var numUnlocked int
+	for _, p := range peers {
+		if !p.UnsignedPeerAPIOnly {
+			continue
+		}
+		numUnlocked++
+		for _, a := range p.AllowedIPs { // not only addresses!
+			b.AddPrefix(a)
+		}
+	}
+	if numUnlocked == 0 {
+		return false
+	}
+	s, err := b.IPSet()
+	if err != nil {
+		// Shouldn't happen, but if it does, fail closed.
+		return true
+	}
+	for _, m := range packetFilter {
+		for _, r := range m.Srcs {
+			if !s.OverlapsPrefix(r) {
+				continue
+			}
+			if len(m.Dsts) != 0 {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func (b *LocalBackend) setFilter(f *filter.Filter) {
 	b.filterAtomic.Store(f)
 	b.e.SetFilter(f)

ipn/ipnlocal/local_test.go

@@ -22,6 +22,7 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
 	"tailscale.com/wgengine"
+	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/wgcfg"
 )
 
@@ -638,3 +639,109 @@ func TestInternalAndExternalInterfaces(t *testing.T) {
 		})
 	}
 }
+
+func TestPacketFilterPermitsUnlockedNodes(t *testing.T) {
+	tests := []struct {
+		name   string
+		peers  []*tailcfg.Node
+		filter []filter.Match
+		want   bool
+	}{
+		{
+			name: "empty",
+			want: false,
+		},
+		{
+			name: "no-unsigned",
+			peers: []*tailcfg.Node{
+				{ID: 1},
+			},
+			want: false,
+		},
+		{
+			name: "unsigned-good",
+			peers: []*tailcfg.Node{
+				{ID: 1, UnsignedPeerAPIOnly: true},
+			},
+			want: false,
+		},
+		{
+			name: "unsigned-bad",
+			peers: []*tailcfg.Node{
+				{
+					ID:                  1,
+					UnsignedPeerAPIOnly: true,
+					AllowedIPs: []netip.Prefix{
+						netip.MustParsePrefix("100.64.0.0/32"),
+					},
+				},
+			},
+			filter: []filter.Match{
+				{
+					Srcs: []netip.Prefix{netip.MustParsePrefix("100.64.0.0/32")},
+					Dsts: []filter.NetPortRange{
+						{
+							Net: netip.MustParsePrefix("100.99.0.0/32"),
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "unsigned-bad-src-is-superset",
+			peers: []*tailcfg.Node{
+				{
+					ID:                  1,
+					UnsignedPeerAPIOnly: true,
+					AllowedIPs: []netip.Prefix{
+						netip.MustParsePrefix("100.64.0.0/32"),
+					},
+				},
+			},
+			filter: []filter.Match{
+				{
+					Srcs: []netip.Prefix{netip.MustParsePrefix("100.64.0.0/24")},
+					Dsts: []filter.NetPortRange{
+						{
+							Net: netip.MustParsePrefix("100.99.0.0/32"),
+						},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "unsigned-okay-because-no-dsts",
+			peers: []*tailcfg.Node{
+				{
+					ID:                  1,
+					UnsignedPeerAPIOnly: true,
+					AllowedIPs: []netip.Prefix{
+						netip.MustParsePrefix("100.64.0.0/32"),
+					},
+				},
+			},
+			filter: []filter.Match{
+				{
+					Srcs: []netip.Prefix{netip.MustParsePrefix("100.64.0.0/32")},
+					Caps: []filter.CapMatch{
+						{
+							Dst: netip.MustParsePrefix("100.99.0.0/32"),
+							Cap: "foo",
+						},
+					},
+				},
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := packetFilterPermitsUnlockedNodes(tt.peers, tt.filter); got != tt.want {
+				t.Errorf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+
+}

ipn/ipnlocal/network-lock.go

@@ -23,6 +23,7 @@ import (
 	"tailscale.com/types/key"
 	"tailscale.com/types/netmap"
 	"tailscale.com/types/tkatype"
+	"tailscale.com/util/mak"
 )
 
 // TODO(tom): RPC retry/backoff was broken and has been removed. Fix?
@@ -38,7 +39,7 @@ type tkaState struct {
 }
 
 // tkaFilterNetmapLocked checks the signatures on each node key, dropping
-// nodes from the netmap who's signature does not verify.
+// nodes from the netmap whose signature does not verify.
 //
 // b.mu must be held.
 func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
@@ -49,27 +50,33 @@ func (b *LocalBackend) tkaFilterNetmapLocked(nm *netmap.NetworkMap) {
 		return // TKA not enabled.
 	}
 
-	toDelete := make(map[int]struct{}, len(nm.Peers))
+	var toDelete map[int]bool // peer index => true
 	for i, p := range nm.Peers {
+		if p.UnsignedPeerAPIOnly {
+			// Not subject to TKA.
+			continue
+		}
 		if len(p.KeySignature) == 0 {
 			b.logf("Network lock is dropping peer %v(%v) due to missing signature", p.ID, p.StableID)
-			toDelete[i] = struct{}{}
+			mak.Set(&toDelete, i, true)
 		} else {
 			if err := b.tka.authority.NodeKeyAuthorized(p.Key, p.KeySignature); err != nil {
 				b.logf("Network lock is dropping peer %v(%v) due to failed signature check: %v", p.ID, p.StableID, err)
-				toDelete[i] = struct{}{}
+				mak.Set(&toDelete, i, true)
 			}
 		}
 	}
 
 	// nm.Peers is ordered, so deletion must be order-preserving.
+	if len(toDelete) > 0 {
 		peers := make([]*tailcfg.Node, 0, len(nm.Peers))
 		for i, p := range nm.Peers {
-		if _, delete := toDelete[i]; !delete {
+			if !toDelete[i] {
 				peers = append(peers, p)
 			}
 		}
 		nm.Peers = peers
+	}
 }
 
 // tkaSyncIfNeeded examines TKA info reported from the control plane,

tailcfg/tailcfg.go

@@ -84,7 +84,8 @@ type CapabilityVersion int
 //   - 45: 2022-09-26: c2n /debug/{goroutines,prefs,metrics}
 //   - 46: 2022-10-04: c2n /debug/component-logging
 //   - 47: 2022-10-11: Register{Request,Response}.NodeKeySignature
-const CurrentCapabilityVersion CapabilityVersion = 47
+//   - 48: 2022-11-02: Node.UnsignedPeerAPIOnly
+const CurrentCapabilityVersion CapabilityVersion = 48
 
 type StableID string
 
@@ -231,6 +232,14 @@ type Node struct {
 	//    "https://tailscale.com/cap/file-sharing"
 	Capabilities []string `json:",omitempty"`
 
+	// UnsignedPeerAPIOnly means that this node is not signed nor subject to TKA
+	// restrictions. However, in exchange for that privilege, it does not get
+	// network access. It can only access this node's peerapi, which may not let
+	// it do anything. It is the tailscaled client's job to double-check the
+	// MapResponse's PacketFilter to verify that its AllowedIPs will not be
+	// accepted by the packet filter.
+	UnsignedPeerAPIOnly bool `json:",omitempty"`
+
 	// The following three computed fields hold the various names that can
 	// be used for this node in UIs. They are populated from controlclient
 	// (not from control) by calling node.InitDisplayNames. These can be
@@ -1552,6 +1561,7 @@ func (n *Node) Equal(n2 *Node) bool {
 		n.Name == n2.Name &&
 		n.User == n2.User &&
 		n.Sharer == n2.Sharer &&
+		n.UnsignedPeerAPIOnly == n2.UnsignedPeerAPIOnly &&
 		n.Key == n2.Key &&
 		n.KeyExpiry.Equal(n2.KeyExpiry) &&
 		bytes.Equal(n.KeySignature, n2.KeySignature) &&

tailcfg/tailcfg_clone.go

@@ -92,6 +92,7 @@ var _NodeCloneNeedsRegeneration = Node(struct {
 	KeepAlive               bool
 	MachineAuthorized       bool
 	Capabilities            []string
+	UnsignedPeerAPIOnly     bool
 	ComputedName            string
 	computedHostIfDifferent string
 	ComputedNameWithHost    string

tailcfg/tailcfg_test.go

@@ -331,6 +331,7 @@ func TestNodeEqual(t *testing.T) {
 		"Created", "Tags", "PrimaryRoutes",
 		"LastSeen", "Online", "KeepAlive", "MachineAuthorized",
 		"Capabilities",
+		"UnsignedPeerAPIOnly",
 		"ComputedName", "computedHostIfDifferent", "ComputedNameWithHost",
 		"DataPlaneAuditLogID",
 	}

tailcfg/tailcfg_view.go

@@ -171,6 +171,7 @@ func (v NodeView) Online() *bool {
 func (v NodeView) KeepAlive() bool                   { return v.ж.KeepAlive }
 func (v NodeView) MachineAuthorized() bool           { return v.ж.MachineAuthorized }
 func (v NodeView) Capabilities() views.Slice[string] { return views.SliceOf(v.ж.Capabilities) }
+func (v NodeView) UnsignedPeerAPIOnly() bool         { return v.ж.UnsignedPeerAPIOnly }
 func (v NodeView) ComputedName() string              { return v.ж.ComputedName }
 func (v NodeView) ComputedNameWithHost() string      { return v.ж.ComputedNameWithHost }
 func (v NodeView) DataPlaneAuditLogID() string       { return v.ж.DataPlaneAuditLogID }
@@ -201,6 +202,7 @@ var _NodeViewNeedsRegeneration = Node(struct {
 	KeepAlive               bool
 	MachineAuthorized       bool
 	Capabilities            []string
+	UnsignedPeerAPIOnly     bool
 	ComputedName            string
 	computedHostIfDifferent string
 	ComputedNameWithHost    string

util/deephash/deephash_test.go

@@ -575,7 +575,7 @@ func TestGetTypeHasher(t *testing.T) {
 		{
 			name: "tailcfg.Node",
 			val:  &tailcfg.Node{},
-			out:  "\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\tn\x88\xf1\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\tn\x88\xf1\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
+			out:  "\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\tn\x88\xf1\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\tn\x88\xf1\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
 		},
 	}
 	for _, tt := range tests {