policy/v2: treat an ACL as one grant in ViaRoutesForPeer

Ports never steer routes, and one grant per destination
matched the sources once per destination.

hscontrol/policy/v2/filter_test.go

@@ -4260,3 +4260,26 @@ func TestDestinationsToNetPortRange_AutogroupInternet(t *testing.T) {
 		})
 	}
 }
+
+// TestACLSourcesResolvedOnce pins that the grants an ACL is split into, one
+// per destination, share the resolved sources instead of each holding a copy.
+func TestACLSourcesResolvedOnce(t *testing.T) {
+	t.Parallel()
+
+	pol, err := unmarshalPolicy([]byte(`{"acls":[{"action":"accept",
+		"src":["10.0.0.1","10.0.1.1","10.0.2.1"],
+		"dst":["100.64.0.1:80","100.64.0.2:443","100.64.0.3:22"]}]}`))
+	require.NoError(t, err)
+
+	grants := pol.compileGrants(nil, types.Nodes{}.ViewSlice())
+	require.Len(t, grants, 3)
+
+	for _, cg := range grants {
+		require.Equal(t, []string{"10.0.0.1", "10.0.1.1", "10.0.2.1"}, cg.srcIPStrings)
+		require.Same(t, &grants[0].srcIPStrings[0], &cg.srcIPStrings[0])
+	}
+
+	rules := globalFilterRules(grants)
+	require.Len(t, rules, 1)
+	require.Len(t, rules[0].DstPorts, 3)
+}

hscontrol/policy/v2/policy.go

@@ -1448,8 +1448,16 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
 	// Clip so the appends below allocate: callers share pm.pol under RLock,
 	// and writing into its spare capacity races between them.
 	grants := slices.Clip(pm.pol.Grants)
+
+	// Ports never steer routes, so an ACL is one grant here, not one per
+	// destination that would match its sources once per destination.
 	for _, acl := range pm.pol.ACLs {
-		grants = append(grants, aclToGrants(acl)...)
+		dsts := make(Aliases, 0, len(acl.Destinations))
+		for _, d := range acl.Destinations {
+			dsts = append(dsts, d.Alias)
+		}
+
+		grants = append(grants, Grant{Sources: acl.Sources, Destinations: dsts})
 	}
 
 	// matchViewer resolves a grant lazily and at most once: non-via

hscontrol/policy/v2/policy_test.go

@@ -1999,6 +1999,72 @@ func TestViaRoutesForPeer(t *testing.T) {
 		require.Empty(t, result.UsePrimary)
 	})
 
+	t.Run("acl_destinations_share_source_match", func(t *testing.T) {
+		t.Parallel()
+
+		nodes := types.Nodes{
+			{
+				ID:       1,
+				Hostname: "viewer",
+				IPv4:     ap("100.64.0.1"),
+				User:     new(users[0]),
+				UserID:   new(users[0].ID),
+				Hostinfo: &tailcfg.Hostinfo{},
+			},
+			{
+				ID:       2,
+				Hostname: "router",
+				IPv4:     ap("100.64.0.2"),
+				User:     new(users[0]),
+				UserID:   new(users[0].ID),
+				Tags:     []string{"tag:router"},
+				Hostinfo: &tailcfg.Hostinfo{
+					RoutableIPs: []netip.Prefix{mp("10.0.0.0/24")},
+				},
+				ApprovedRoutes: []netip.Prefix{mp("10.0.0.0/24")},
+			},
+		}
+
+		// Only the last destination of each ACL covers the route, and only
+		// the sources of the second ACL match the viewer.
+		pol := func(secondSrc string) string {
+			return `{
+			"tagOwners": {
+				"tag:router": ["user1@"]
+			},
+			"acls": [{
+				"action": "accept",
+				"src": ["user2@"],
+				"dst": ["192.168.0.0/24:*", "10.0.0.0/24:*"]
+			}, {
+				"action": "accept",
+				"src": ["user2@", "` + secondSrc + `"],
+				"dst": ["192.168.0.0/24:*", "10.0.0.0/24:*"]
+			}],
+			"grants": [{
+				"src": ["user1@"],
+				"dst": ["10.0.0.0/24"],
+				"ip": ["*"],
+				"via": ["tag:router"]
+			}]
+		}`
+		}
+
+		pm, err := NewPolicyManager([]byte(pol("user1@")), users, nodes.ViewSlice())
+		require.NoError(t, err)
+
+		result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
+		require.Equal(t, []netip.Prefix{mp("10.0.0.0/24")}, result.Include)
+		require.Equal(t, []netip.Prefix{mp("10.0.0.0/24")}, result.UsePrimary)
+
+		pm, err = NewPolicyManager([]byte(pol("user2@")), users, nodes.ViewSlice())
+		require.NoError(t, err)
+
+		result = pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
+		require.Equal(t, []netip.Prefix{mp("10.0.0.0/24")}, result.Include)
+		require.Empty(t, result.UsePrimary)
+	})
+
 	t.Run("peer_does_not_advertise_destination", func(t *testing.T) {
 		t.Parallel()