Update ACL when node advertise route

Fixes #1604

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

.github/workflows/test-integration-v2-TestSubnetRouteACL.yaml

@@ -0,0 +1,67 @@
+# DO NOT EDIT, generated with cmd/gh-action-integration-generator/main.go
+# To regenerate, run "go generate" in cmd/gh-action-integration-generator/
+
+name: Integration Test v2 - TestSubnetRouteACL
+
+on: [pull_request]
+
+concurrency:
+  group: ${{ github.workflow }}-$${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  TestSubnetRouteACL:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+
+      - uses: DeterminateSystems/nix-installer-action@main
+      - uses: DeterminateSystems/magic-nix-cache-action@main
+      - uses: satackey/action-docker-layer-caching@main
+        continue-on-error: true
+
+      - name: Get changed files
+        id: changed-files
+        uses: tj-actions/changed-files@v34
+        with:
+          files: |
+            *.nix
+            go.*
+            **/*.go
+            integration_test/
+            config-example.yaml
+
+      - name: Run TestSubnetRouteACL
+        uses: Wandalen/wretry.action@master
+        if: steps.changed-files.outputs.any_changed == 'true'
+        with:
+          attempt_limit: 5
+          command: |
+            nix develop --command -- docker run \
+              --tty --rm \
+              --volume ~/.cache/hs-integration-go:/go \
+              --name headscale-test-suite \
+              --volume $PWD:$PWD -w $PWD/integration \
+              --volume /var/run/docker.sock:/var/run/docker.sock \
+              --volume $PWD/control_logs:/tmp/control \
+              golang:1 \
+                go run gotest.tools/gotestsum@latest -- ./... \
+                  -failfast \
+                  -timeout 120m \
+                  -parallel 1 \
+                  -run "^TestSubnetRouteACL$"
+
+      - uses: actions/upload-artifact@v3
+        if: always() && steps.changed-files.outputs.any_changed == 'true'
+        with:
+          name: logs
+          path: "control_logs/*.log"
+
+      - uses: actions/upload-artifact@v3
+        if: always() && steps.changed-files.outputs.any_changed == 'true'
+        with:
+          name: pprof
+          path: "control_logs/*.pprof.tar"

hscontrol/db/node.go

@@ -739,6 +739,19 @@ func (hsdb *HSDatabase) enableRoutes(node *types.Node, routeStrs ...string) erro
 			stateUpdate, node.MachineKey.String())
 	}
 
+	// Send an update to the node itself with to ensure it
+	// has an updated packetfilter allowing the new route
+	// if it is defined in the ACL.
+	selfUpdate := types.StateUpdate{
+		Type:        types.StateSelfUpdate,
+		ChangeNodes: types.Nodes{node},
+	}
+	if selfUpdate.Valid() {
+		hsdb.notifier.NotifyByMachineKey(
+			selfUpdate,
+			node.MachineKey)
+	}
+
 	return nil
 }
 

hscontrol/mapper/mapper.go

@@ -278,6 +278,18 @@ func (m *Mapper) LiteMapResponse(
 		return nil, err
 	}
 
+	rules, sshPolicy, err := policy.GenerateFilterAndSSHRules(
+		pol,
+		node,
+		nodeMapToList(m.peers),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	resp.PacketFilter = policy.ReduceFilterRules(node, rules)
+	resp.SSHPolicy = sshPolicy
+
 	return m.marshalMapResponse(mapRequest, resp, node, mapRequest.Compress)
 }
 

hscontrol/policy/acls.go

@@ -250,6 +250,21 @@ func ReduceFilterRules(node *types.Node, rules []tailcfg.FilterRule) []tailcfg.F
 			if node.IPAddresses.InIPSet(expanded) {
 				dests = append(dests, dest)
 			}
+
+			// If the node exposes routes, ensure they are note removed
+			// when the filters are reduced.
+			if node.Hostinfo != nil {
+				// TODO(kradalby): Evaluate if we should only keep
+				// the routes if the route is enabled. This will
+				// require database access in this part of the code.
+				if len(node.Hostinfo.RoutableIPs) > 0 {
+					for _, routableIP := range node.Hostinfo.RoutableIPs {
+						if expanded.ContainsPrefix(routableIP) {
+							dests = append(dests, dest)
+						}
+					}
+				}
+			}
 		}
 
 		if len(dests) > 0 {

hscontrol/policy/acls_test.go

@@ -1901,6 +1901,81 @@ func TestReduceFilterRules(t *testing.T) {
 			},
 			want: []tailcfg.FilterRule{},
 		},
+		{
+			name: "1604-subnet-routers-are-preserved",
+			pol: ACLPolicy{
+				Groups: Groups{
+					"group:admins": {"user1"},
+				},
+				ACLs: []ACL{
+					{
+						Action:       "accept",
+						Sources:      []string{"group:admins"},
+						Destinations: []string{"group:admins:*"},
+					},
+					{
+						Action:       "accept",
+						Sources:      []string{"group:admins"},
+						Destinations: []string{"10.33.0.0/16:*"},
+					},
+				},
+			},
+			node: &types.Node{
+				IPAddresses: types.NodeAddresses{
+					netip.MustParseAddr("100.64.0.1"),
+					netip.MustParseAddr("fd7a:115c:a1e0::1"),
+				},
+				User: types.User{Name: "user1"},
+				Hostinfo: &tailcfg.Hostinfo{
+					RoutableIPs: []netip.Prefix{
+						netip.MustParsePrefix("10.33.0.0/16"),
+					},
+				},
+			},
+			peers: types.Nodes{
+				&types.Node{
+					IPAddresses: types.NodeAddresses{
+						netip.MustParseAddr("100.64.0.2"),
+						netip.MustParseAddr("fd7a:115c:a1e0::2"),
+					},
+					User: types.User{Name: "user1"},
+				},
+			},
+			want: []tailcfg.FilterRule{
+				{
+					SrcIPs: []string{
+						"100.64.0.1/32",
+						"100.64.0.2/32",
+						"fd7a:115c:a1e0::1/128",
+						"fd7a:115c:a1e0::2/128",
+					},
+					DstPorts: []tailcfg.NetPortRange{
+						{
+							IP:    "100.64.0.1/32",
+							Ports: tailcfg.PortRangeAny,
+						},
+						{
+							IP:    "fd7a:115c:a1e0::1/128",
+							Ports: tailcfg.PortRangeAny,
+						},
+					},
+				},
+				{
+					SrcIPs: []string{
+						"100.64.0.1/32",
+						"100.64.0.2/32",
+						"fd7a:115c:a1e0::1/128",
+						"fd7a:115c:a1e0::2/128",
+					},
+					DstPorts: []tailcfg.NetPortRange{
+						{
+							IP:    "10.33.0.0/16",
+							Ports: tailcfg.PortRangeAny,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {

hscontrol/poll.go

@@ -153,6 +153,8 @@ func (h *Headscale) handlePoll(
 					return
 				}
 
+				// Send an update to all peers to propagate the new routes
+				// available.
 				stateUpdate := types.StateUpdate{
 					Type:        types.StatePeerChanged,
 					ChangeNodes: types.Nodes{node},
@@ -164,6 +166,19 @@ func (h *Headscale) handlePoll(
 						node.MachineKey.String())
 				}
 
+				// Send an update to the node itself with to ensure it
+				// has an updated packetfilter allowing the new route
+				// if it is defined in the ACL.
+				selfUpdate := types.StateUpdate{
+					Type:        types.StateSelfUpdate,
+					ChangeNodes: types.Nodes{node},
+				}
+				if selfUpdate.Valid() {
+					h.nodeNotifier.NotifyByMachineKey(
+						selfUpdate,
+						node.MachineKey)
+				}
+
 				return
 			}
 		}
@@ -378,6 +393,16 @@ func (h *Headscale) handlePoll(
 			var data []byte
 			var err error
 
+			// Ensure the node object is updated, for example, there
+			// might have been a hostinfo update in a sidechannel
+			// which contains data needed to generate a map response.
+			node, err = h.db.GetNodeByMachineKey(node.MachineKey)
+			if err != nil {
+				logErr(err, "Could not get machine from db")
+
+				return
+			}
+
 			switch update.Type {
 			case types.StateFullUpdate:
 				logInfo("Sending Full MapResponse")