From c3df84e3547f0e558ac64f073ae4d59562e24226 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 11 May 2026 11:17:55 +0000 Subject: [PATCH] policy/matcher: include CapGrant.Dsts in match destinations MatchFromFilterRule only read DstPorts[].IP into the destination IPSet. Cap-grant-only filter rules (e.g. tailscale.com/cap/relay) carry their destinations in CapGrant[].Dsts, so the derived matchers had empty dest sets and BuildPeerMap / ReduceNodes never exposed the cap target to its source nodes. Without a companion IP-level grant the relay node stayed invisible, so clients never tried to use it and connections sat on DERP. Union CapGrant[].Dsts into the destination IPSet alongside DstPorts. Restores peer-visibility for any cap-grant-only relationship; the peer-relay flow is the most visible instance. Fixes #3256 --- hscontrol/policy/matcher/matcher.go | 35 ++++- hscontrol/policy/matcher/matcher_test.go | 72 +++++++++ hscontrol/policy/v2/policy_test.go | 178 +++++++++++++++++++++++ 3 files changed, 281 insertions(+), 4 deletions(-) diff --git a/hscontrol/policy/matcher/matcher.go b/hscontrol/policy/matcher/matcher.go index 275f2463..24f2b87a 100644 --- a/hscontrol/policy/matcher/matcher.go +++ b/hscontrol/policy/matcher/matcher.go @@ -44,13 +44,40 @@ func MatchesFromFilterRules(rules []tailcfg.FilterRule) []Match { return matches } +// MatchFromFilterRule derives a Match from a tailcfg.FilterRule. The +// destination IP set is the union of DstPorts[].IP and CapGrant[].Dsts: +// cap-grant-only rules (e.g. tailscale.com/cap/relay) carry their +// destinations in CapGrant.Dsts and would otherwise contribute nothing +// to peer-visibility derivation in BuildPeerMap / ReduceNodes, hiding +// the cap target from the source unless a companion IP-level rule +// also exists. func MatchFromFilterRule(rule tailcfg.FilterRule) Match { - dests := make([]string, 0, len(rule.DstPorts)) - for _, dest := range rule.DstPorts { - dests = append(dests, dest.IP) + srcs := new(netipx.IPSetBuilder) + dests := new(netipx.IPSetBuilder) + + for _, srcIP := range rule.SrcIPs { + set, _ := util.ParseIPSet(srcIP, nil) + srcs.AddSet(set) } - return MatchFromStrings(rule.SrcIPs, dests) + for _, dp := range rule.DstPorts { + set, _ := util.ParseIPSet(dp.IP, nil) + dests.AddSet(set) + } + + for _, cg := range rule.CapGrant { + for _, pref := range cg.Dsts { + dests.AddPrefix(pref) + } + } + + srcsSet, _ := srcs.IPSet() + destsSet, _ := dests.IPSet() + + return Match{ + srcs: srcsSet, + dests: destsSet, + } } // MatchFromStrings builds a Match from raw source and destination diff --git a/hscontrol/policy/matcher/matcher_test.go b/hscontrol/policy/matcher/matcher_test.go index 59c712c4..69ecbfaf 100644 --- a/hscontrol/policy/matcher/matcher_test.go +++ b/hscontrol/policy/matcher/matcher_test.go @@ -180,6 +180,78 @@ func TestMatchFromFilterRule(t *testing.T) { srcMatch: true, dstMatch: false, }, + { + // Regression: cap-grant-only rules (e.g. cap/relay) + // carry their destinations in CapGrant.Dsts. The + // matcher must surface those for peer-visibility + // derivation. https://github.com/juanfont/headscale/issues/3256 + name: "CapGrant Dsts populate destination set", + rule: tailcfg.FilterRule{ + SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32"}, + CapGrant: []tailcfg.CapGrant{ + { + Dsts: []netip.Prefix{ + netip.MustParsePrefix("100.64.0.3/32"), + }, + CapMap: tailcfg.PeerCapMap{ + tailcfg.PeerCapabilityRelay: nil, + }, + }, + }, + }, + checkSrc: netip.MustParseAddr("100.64.0.1"), + checkDst: netip.MustParseAddr("100.64.0.3"), + srcMatch: true, + dstMatch: true, + }, + { + // Companion cap-grant shape produced by + // companionCapGrantRules: SrcIPs are the original + // destinations, CapGrant.Dsts are the original sources. + name: "companion CapGrant Dsts populate destination set", + rule: tailcfg.FilterRule{ + SrcIPs: []string{"100.64.0.3/32"}, + CapGrant: []tailcfg.CapGrant{ + { + Dsts: []netip.Prefix{ + netip.MustParsePrefix("100.64.0.1/32"), + netip.MustParsePrefix("100.64.0.2/32"), + }, + CapMap: tailcfg.PeerCapMap{ + tailcfg.PeerCapabilityRelayTarget: nil, + }, + }, + }, + }, + checkSrc: netip.MustParseAddr("100.64.0.3"), + checkDst: netip.MustParseAddr("100.64.0.2"), + srcMatch: true, + dstMatch: true, + }, + { + // Mixed rule: DstPorts and CapGrant both contribute to dests. + name: "DstPorts and CapGrant Dsts both contribute", + rule: tailcfg.FilterRule{ + SrcIPs: []string{"100.64.0.1/32"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.0.0.0/8"}, + }, + CapGrant: []tailcfg.CapGrant{ + { + Dsts: []netip.Prefix{ + netip.MustParsePrefix("100.64.0.3/32"), + }, + CapMap: tailcfg.PeerCapMap{ + tailcfg.PeerCapabilityRelay: nil, + }, + }, + }, + }, + checkSrc: netip.MustParseAddr("100.64.0.1"), + checkDst: netip.MustParseAddr("100.64.0.3"), + srcMatch: true, + dstMatch: true, + }, } for _, tt := range tests { diff --git a/hscontrol/policy/v2/policy_test.go b/hscontrol/policy/v2/policy_test.go index 9495e110..a0c3cdb3 100644 --- a/hscontrol/policy/v2/policy_test.go +++ b/hscontrol/policy/v2/policy_test.go @@ -2022,3 +2022,181 @@ func TestValidateUserReferences_AllSites(t *testing.T) { }) } } + +// TestPeerRelayGrantMakesRelayVisible is a regression test for +// https://github.com/juanfont/headscale/issues/3256. +// +// A grant that uses only `app: { "tailscale.com/cap/relay": [] }` must +// make the relay node visible to the source nodes (and vice-versa). +// Before the fix, MatchFromFilterRule only considered DstPorts as +// destinations and ignored CapGrant.Dsts, so cap-grant-only rules +// produced matchers with an empty destination set and BuildPeerMap +// could not detect the cap-relay relationship. +// +// Sub-tests cover every alias shape documented for peer-relay grants +// at https://tailscale.com/docs/features/peer-relay: tag→tag, +// hostname→hostname (`hosts` block lookup), autogroup:member→hostname, +// and a direct Tailscale-IP destination. Each must establish mutual +// visibility between sources and the relay node without any companion +// IP-level grant. +func TestPeerRelayGrantMakesRelayVisible(t *testing.T) { + users := types.Users{ + {Model: gorm.Model{ID: 1}, Name: "alice", Email: "alice@headscale.net"}, + {Model: gorm.Model{ID: 2}, Name: "tagowner", Email: "tagowner@headscale.net"}, + } + + // Helper for tagged nodes belonging to the tag-owner user. + taggedNode := func(id types.NodeID, hostname, v4, v6 string, tags ...string) *types.Node { + return &types.Node{ + ID: id, + Hostname: hostname, + IPv4: ap(v4), + IPv6: ap(v6), + User: new(users[1]), + UserID: new(users[1].ID), + Tags: tags, + } + } + + userNode := func(id types.NodeID, hostname, v4, v6 string) *types.Node { + return &types.Node{ + ID: id, + Hostname: hostname, + IPv4: ap(v4), + IPv6: ap(v6), + User: new(users[0]), + UserID: new(users[0].ID), + } + } + + tests := []struct { + name string + nodes types.Nodes + policy string + srcIDs []types.NodeID // expected to see the relay + relayID types.NodeID + }{ + { + // Issue #3256 example: hosts block + autogroup:member src, + // hostname dst. + name: "hosts+autogroup_member src, hostname dst", + nodes: types.Nodes{ + userNode(1, "n1", "100.64.0.1", "fd7a:115c:a1e0::1"), + userNode(2, "n2", "100.64.0.2", "fd7a:115c:a1e0::2"), + userNode(3, "peer-relay", "100.64.0.3", "fd7a:115c:a1e0::3"), + }, + policy: `{ + "hosts": { + "n1": "100.64.0.1/32", + "n2": "100.64.0.2/32", + "peer-relay": "100.64.0.3/32" + }, + "grants": [ + {"src": ["n1"], "dst": ["n2"], "ip": ["*"]}, + { + "src": ["autogroup:member"], + "dst": ["peer-relay"], + "app": {"tailscale.com/cap/relay": []} + } + ] + }`, + srcIDs: []types.NodeID{1, 2}, + relayID: 3, + }, + { + // Tailscale docs example 1: tag → tag. + name: "tag src, tag dst", + nodes: types.Nodes{ + taggedNode(1, "vpc-a", "100.64.0.1", "fd7a:115c:a1e0::1", "tag:us-east-vpc"), + taggedNode(2, "vpc-b", "100.64.0.2", "fd7a:115c:a1e0::2", "tag:us-east-vpc"), + taggedNode(3, "relay-1", "100.64.0.3", "fd7a:115c:a1e0::3", "tag:us-east-relays"), + }, + policy: `{ + "tagOwners": { + "tag:us-east-vpc": ["tagowner@headscale.net"], + "tag:us-east-relays": ["tagowner@headscale.net"] + }, + "grants": [ + { + "src": ["tag:us-east-vpc"], + "dst": ["tag:us-east-relays"], + "app": {"tailscale.com/cap/relay": []} + } + ] + }`, + srcIDs: []types.NodeID{1, 2}, + relayID: 3, + }, + { + // Direct Tailscale-IP destination (no hosts alias). + name: "tag src, raw Tailscale IP dst", + nodes: types.Nodes{ + taggedNode(1, "client-a", "100.64.0.1", "fd7a:115c:a1e0::1", "tag:client"), + taggedNode(2, "client-b", "100.64.0.2", "fd7a:115c:a1e0::2", "tag:client"), + userNode(3, "peer-relay", "100.64.0.3", "fd7a:115c:a1e0::3"), + }, + policy: `{ + "tagOwners": { + "tag:client": ["tagowner@headscale.net"] + }, + "grants": [ + { + "src": ["tag:client"], + "dst": ["100.64.0.3/32"], + "app": {"tailscale.com/cap/relay": []} + } + ] + }`, + srcIDs: []types.NodeID{1, 2}, + relayID: 3, + }, + { + // User → hostname relay using `hosts` aliasing. + name: "user src, hostname dst via hosts block", + nodes: types.Nodes{ + userNode(1, "n1", "100.64.0.1", "fd7a:115c:a1e0::1"), + userNode(3, "peer-relay", "100.64.0.3", "fd7a:115c:a1e0::3"), + }, + policy: `{ + "hosts": { + "peer-relay": "100.64.0.3/32" + }, + "grants": [ + { + "src": ["alice@headscale.net"], + "dst": ["peer-relay"], + "app": {"tailscale.com/cap/relay": []} + } + ] + }`, + srcIDs: []types.NodeID{1}, + relayID: 3, + }, + } + + containsID := func(peers []types.NodeView, id types.NodeID) bool { + return slices.ContainsFunc(peers, func(nv types.NodeView) bool { + return nv.ID() == id + }) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pm, err := NewPolicyManager( + []byte(tt.policy), users, tt.nodes.ViewSlice(), + ) + require.NoError(t, err) + + peerMap := pm.BuildPeerMap(tt.nodes.ViewSlice()) + + for _, srcID := range tt.srcIDs { + require.True(t, containsID(peerMap[srcID], tt.relayID), + "node %d must see relay %d via cap/relay alone", + srcID, tt.relayID) + require.True(t, containsID(peerMap[tt.relayID], srcID), + "relay %d must see node %d via cap/relay alone", + tt.relayID, srcID) + } + }) + } +}