validate policy against nodes, error if not valid

.github/workflows/test-integration.yaml

@@ -37,6 +37,7 @@ jobs:
           - TestNodeRenameCommand
           - TestNodeMoveCommand
           - TestPolicyCommand
+          - TestPolicyBrokenConfigCommand
           - TestResolveMagicDNS
           - TestValidateResolvConf
           - TestDERPServerScenario

hscontrol/app.go

@@ -1001,6 +1001,32 @@ func (h *Headscale) loadACLPolicy() error {
 		if err != nil {
 			return fmt.Errorf("failed to load ACL policy from file: %w", err)
 		}
+
+		// Validate and reject configuration that would error when applied
+		// when creating a map response. This requires nodes, so there is still
+		// a scenario where they might be allowed if the server has no nodes
+		// yet, but it should help for the general case and for hot reloading
+		// configurations.
+		// Note that this check is only done for file-based policies in this function
+		// as the database-based policies are checked in the gRPC API where it is not
+		// allowed to be written to the database.
+		nodes, err := h.db.ListNodes()
+		if err != nil {
+			return fmt.Errorf("loading nodes from database to validate policy: %w", err)
+		}
+
+		_, err = pol.CompileFilterRules(nodes)
+		if err != nil {
+			return fmt.Errorf("verifying policy rules: %w", err)
+		}
+
+		if len(nodes) > 0 {
+			_, err = pol.CompileSSHPolicy(nodes[0], nodes)
+			if err != nil {
+				return fmt.Errorf("verifying SSH rules: %w", err)
+			}
+		}
+
 	case types.PolicyModeDB:
 		p, err := h.db.GetPolicy()
 		if err != nil {

hscontrol/grpcv1.go

@@ -4,6 +4,7 @@ package hscontrol
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"os"
 	"sort"
@@ -721,17 +722,39 @@ func (api headscaleV1APIServer) SetPolicy(
 
 	p := request.GetPolicy()
 
-	valid, err := policy.LoadACLPolicyFromBytes([]byte(p))
+	pol, err := policy.LoadACLPolicyFromBytes([]byte(p))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("loading ACL policy file: %w", err)
+	}
+
+	// Validate and reject configuration that would error when applied
+	// when creating a map response. This requires nodes, so there is still
+	// a scenario where they might be allowed if the server has no nodes
+	// yet, but it should help for the general case and for hot reloading
+	// configurations.
+	nodes, err := api.h.db.ListNodes()
+	if err != nil {
+		return nil, fmt.Errorf("loading nodes from database to validate policy: %w", err)
+	}
+
+	_, err = pol.CompileFilterRules(nodes)
+	if err != nil {
+		return nil, fmt.Errorf("verifying policy rules: %w", err)
+	}
+
+	if len(nodes) > 0 {
+		_, err = pol.CompileSSHPolicy(nodes[0], nodes)
+		if err != nil {
+			return nil, fmt.Errorf("verifying SSH rules: %w", err)
+		}
 	}
 
 	updated, err := api.h.db.SetPolicy(p)
 	if err != nil {
 		return nil, err
 	}
 
-	api.h.ACLPolicy = valid
+	api.h.ACLPolicy = pol
 
 	ctx := types.NotifyCtx(context.Background(), "acl-update", "na")
 	api.h.nodeNotifier.NotifyAll(ctx, types.StateUpdate{

integration/cli_test.go

@@ -1676,3 +1676,77 @@ func TestPolicyCommand(t *testing.T) {
 	assert.Len(t, output.ACLs, 1)
 	assert.Equal(t, output.TagOwners["tag:exists"], []string{"policy-user"})
 }
+
+func TestPolicyBrokenConfigCommand(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	scenario, err := NewScenario(dockertestMaxWait())
+	assertNoErr(t, err)
+	defer scenario.Shutdown()
+
+	spec := map[string]int{
+		"policy-user": 1,
+	}
+
+	err = scenario.CreateHeadscaleEnv(
+		spec,
+		[]tsic.Option{},
+		hsic.WithTestName("clins"),
+		hsic.WithConfigEnv(map[string]string{
+			"HEADSCALE_POLICY_MODE": "database",
+		}),
+	)
+	assertNoErr(t, err)
+
+	headscale, err := scenario.Headscale()
+	assertNoErr(t, err)
+
+	p := policy.ACLPolicy{
+		ACLs: []policy.ACL{
+			{
+				// This is an unknown action, so it will return an error
+				// and the config will not be applied.
+				Action:       "acccept",
+				Sources:      []string{"*"},
+				Destinations: []string{"*:*"},
+			},
+		},
+		TagOwners: map[string][]string{
+			"tag:exists": {"policy-user"},
+		},
+	}
+
+	pBytes, _ := json.Marshal(p)
+
+	policyFilePath := "/etc/headscale/policy.json"
+
+	err = headscale.WriteFile(policyFilePath, pBytes)
+	assertNoErr(t, err)
+
+	// No policy is present at this time.
+	// Add a new policy from a file.
+	_, err = headscale.Execute(
+		[]string{
+			"headscale",
+			"policy",
+			"set",
+			"-f",
+			policyFilePath,
+		},
+	)
+	assert.ErrorContains(t, err, "verifying policy rules: invalid action")
+
+	// The new policy was invalid, the old one should still be in place, which
+	// is none.
+	_, err = headscale.Execute(
+		[]string{
+			"headscale",
+			"policy",
+			"get",
+			"--output",
+			"json",
+		},
+	)
+	assert.ErrorContains(t, err, "acl policy not found")
+}

integration/dockertestutil/execute.go

@@ -62,7 +62,7 @@ func ExecuteCommand(
 		exitCode, err := resource.Exec(
 			cmd,
 			dockertest.ExecOptions{
-				Env:    append(env, "HEADSCALE_LOG_LEVEL=disabled"),
+				Env:    append(env, "HEADSCALE_LOG_LEVEL=info"),
 				StdOut: &stdout,
 				StdErr: &stderr,
 			},

integration/hsic/hsic.go

@@ -551,7 +551,7 @@ func (t *HeadscaleInContainer) Execute(
 			log.Printf("command stdout: %s\n", stdout)
 		}
 
-		return "", err
+		return stdout, fmt.Errorf("executing command in docker: %w, stderr: %s", err, stderr)
 	}
 
 	return stdout, nil