only send relevant filterrules to nodes

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

Author: kradalby

hscontrol/db/acls_test.go

@@ -125,122 +125,6 @@ func TestInvalidTagValidUser(t *testing.T) {
 	}
 }
 
-func TestPortGroup(t *testing.T) {
-	machine := types.Machine{
-		ID:         0,
-		MachineKey: "foo",
-		NodeKey:    "bar",
-		DiscoKey:   "faa",
-		Hostname:   "testmachine",
-		UserID:     0,
-		User: types.User{
-			Name: "testuser",
-		},
-		RegisterMethod: util.RegisterMethodAuthKey,
-		IPAddresses:    types.MachineAddresses{netip.MustParseAddr("100.64.0.5")},
-	}
-
-	acl := []byte(`
-{
-	"groups": {
-		"group:example": [
-			"testuser",
-		],
-	},
-
-	"hosts": {
-		"host-1": "100.100.100.100",
-		"subnet-1": "100.100.101.100/24",
-	},
-
-	"acls": [
-		{
-			"action": "accept",
-			"src": [
-				"group:example",
-			],
-			"dst": [
-				"host-1:*",
-			],
-		},
-	],
-}
-	`)
-	pol, err := policy.LoadACLPolicyFromBytes(acl, "hujson")
-	assert.NoError(t, err)
-
-	got, _, err := policy.GenerateFilterRules(pol, &machine, types.Machines{})
-	assert.NoError(t, err)
-
-	want := []tailcfg.FilterRule{
-		{
-			SrcIPs: []string{"100.64.0.5/32"},
-			DstPorts: []tailcfg.NetPortRange{
-				{IP: "100.100.100.100/32", Ports: tailcfg.PortRange{Last: 65535}},
-			},
-		},
-	}
-
-	if diff := cmp.Diff(want, got); diff != "" {
-		t.Errorf("TestPortGroup() unexpected result (-want +got):\n%s", diff)
-	}
-}
-
-func TestPortUser(t *testing.T) {
-	machine := types.Machine{
-		ID:         0,
-		MachineKey: "12345",
-		NodeKey:    "bar",
-		DiscoKey:   "faa",
-		Hostname:   "testmachine",
-		UserID:     0,
-		User: types.User{
-			Name: "testuser",
-		},
-		RegisterMethod: util.RegisterMethodAuthKey,
-		IPAddresses:    types.MachineAddresses{netip.MustParseAddr("100.64.0.9")},
-	}
-
-	acl := []byte(`
-{
-	"hosts": {
-		"host-1": "100.100.100.100",
-		"subnet-1": "100.100.101.100/24",
-	},
-
-	"acls": [
-		{
-			"action": "accept",
-			"src": [
-				"testuser",
-			],
-			"dst": [
-				"host-1:*",
-			],
-		},
-	],
-}
-	`)
-	pol, err := policy.LoadACLPolicyFromBytes(acl, "hujson")
-	assert.NoError(t, err)
-
-	got, _, err := policy.GenerateFilterRules(pol, &machine, types.Machines{})
-	assert.NoError(t, err)
-
-	want := []tailcfg.FilterRule{
-		{
-			SrcIPs: []string{"100.64.0.9/32"},
-			DstPorts: []tailcfg.NetPortRange{
-				{IP: "100.100.100.100/32", Ports: tailcfg.PortRange{Last: 65535}},
-			},
-		},
-	}
-
-	if diff := cmp.Diff(want, got); diff != "" {
-		t.Errorf("TestPortUser() unexpected result (-want +got):\n%s", diff)
-	}
-}
-
 // this test should validate that we can expand a group in a TagOWner section and
 // match properly the IP's of the related hosts. The owner is valid and the tag is also valid.
 // the tag is matched in the Destinations section.

hscontrol/mapper/mapper_test.go

@@ -403,8 +403,8 @@ func Test_fullMapResponse(t *testing.T) {
 				ACLs: []policy.ACL{
 					{
 						Action:       "accept",
-						Sources:      []string{"mini"},
-						Destinations: []string{"100.64.0.2:*"},
+						Sources:      []string{"100.64.0.2"},
+						Destinations: []string{"mini:*"},
 					},
 				},
 			},
@@ -430,8 +430,9 @@ func Test_fullMapResponse(t *testing.T) {
 				CollectServices: "false",
 				PacketFilter: []tailcfg.FilterRule{
 					{
-						SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32"},
+						SrcIPs: []string{"100.64.0.2/32"},
 						DstPorts: []tailcfg.NetPortRange{
+							{IP: "100.64.0.1/32", Ports: tailcfg.PortRangeAny},
 							{IP: "100.64.0.2/32", Ports: tailcfg.PortRangeAny},
 						},
 					},

hscontrol/policy/acls.go

@@ -177,33 +177,60 @@ func (pol *ACLPolicy) generateFilterRules(
 			srcIPs = append(srcIPs, srcs...)
 		}
 
-		protocols, needsWildcard, err := parseProtocol(acl.Protocol)
+		protocols, isWildcard, err := parseProtocol(acl.Protocol)
 		if err != nil {
 			log.Error().
 				Msgf("Error parsing ACL %d. protocol unknown %s", index, acl.Protocol)
 
 			return nil, err
 		}
 
+		// record if the rule is actually relevant for the given machine.
+		isRelevant := false
+
 		destPorts := []tailcfg.NetPortRange{}
-		for destIndex, dest := range acl.Destinations {
-			dests, err := pol.getNetPortRangeFromDestination(
-				dest,
+		for _, dest := range acl.Destinations {
+			alias, port, err := parseDestination(dest)
+			if err != nil {
+				return nil, err
+			}
+
+			expanded, err := pol.ExpandAlias(
 				machines,
-				needsWildcard,
+				alias,
 			)
 			if err != nil {
-				log.Error().
-					Interface("dest", dest).
-					Int("ACL index", index).
-					Int("dest index", destIndex).
-					Msgf("Error parsing ACL")
+				return nil, err
+			}
 
+			if machine.IPAddresses.InIPSet(expanded) {
+				isRelevant = true
+			}
+
+			ports, err := expandPorts(port, isWildcard)
+			if err != nil {
 				return nil, err
 			}
+
+			dests := []tailcfg.NetPortRange{}
+			for _, dest := range expanded.Prefixes() {
+				for _, port := range *ports {
+					pr := tailcfg.NetPortRange{
+						IP:    dest.String(),
+						Ports: port,
+					}
+					dests = append(dests, pr)
+				}
+			}
 			destPorts = append(destPorts, dests...)
 		}
 
+		// if the rule does not apply to the machine we are evaluating,
+		// do not add it to the list and continue.
+		if !isRelevant {
+			continue
+		}
+
 		rules = append(rules, tailcfg.FilterRule{
 			SrcIPs:   srcIPs,
 			DstPorts: destPorts,
@@ -368,44 +395,6 @@ func (pol *ACLPolicy) getIPsFromSource(
 	return prefixes, nil
 }
 
-// getNetPortRangeFromDestination returns a set of tailcfg.NetPortRange
-// which are associated with the dest alias.
-func (pol *ACLPolicy) getNetPortRangeFromDestination(
-	dest string,
-	machines types.Machines,
-	needsWildcard bool,
-) ([]tailcfg.NetPortRange, error) {
-	alias, port, err := parseDestination(dest)
-	if err != nil {
-		return nil, err
-	}
-
-	expanded, err := pol.ExpandAlias(
-		machines,
-		alias,
-	)
-	if err != nil {
-		return nil, err
-	}
-	ports, err := expandPorts(port, needsWildcard)
-	if err != nil {
-		return nil, err
-	}
-
-	dests := []tailcfg.NetPortRange{}
-	for _, dest := range expanded.Prefixes() {
-		for _, port := range *ports {
-			pr := tailcfg.NetPortRange{
-				IP:    dest.String(),
-				Ports: port,
-			}
-			dests = append(dests, pr)
-		}
-	}
-
-	return dests, nil
-}
-
 func parseDestination(dest string) (string, string, error) {
 	var tokens []string
 
@@ -605,14 +594,14 @@ func excludeCorrectlyTaggedNodes(
 	return out
 }
 
-func expandPorts(portsStr string, needsWildcard bool) (*[]tailcfg.PortRange, error) {
+func expandPorts(portsStr string, isWild bool) (*[]tailcfg.PortRange, error) {
 	if isWildcard(portsStr) {
 		return &[]tailcfg.PortRange{
 			{First: portRangeBegin, Last: portRangeEnd},
 		}, nil
 	}
 
-	if needsWildcard {
+	if isWild {
 		return nil, ErrWildcardIsNeeded
 	}