From 2f7f90529aa85e8a6e66609c7737ac968ee1cf1d Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 13 May 2026 12:47:04 +0000 Subject: [PATCH] policy/v2: evaluate sshTests at write boundary SetPolicy and policy check now compile per-dst SSH rules and replay each sshTests entry. The accept assertion treats check-action rules as reachable; the check assertion requires HoldAndDelegate on the matching rule. Boot reload warns and continues. --- hscontrol/policy/v2/policy.go | 17 +- hscontrol/policy/v2/sshtest.go | 716 ++++++++++++ hscontrol/policy/v2/sshtest_test.go | 1000 +++++++++++++++++ .../v2/tailscale_ssh_data_compat_test.go | 55 - hscontrol/policy/v2/test.go | 85 +- hscontrol/policy/v2/types.go | 42 +- hscontrol/policy/v2/types_test.go | 61 +- 7 files changed, 1884 insertions(+), 92 deletions(-) create mode 100644 hscontrol/policy/v2/sshtest.go create mode 100644 hscontrol/policy/v2/sshtest_test.go diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index ad12c7c8..a602655f 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -208,6 +208,10 @@ func NewPolicyManager(b []byte, users []types.User, nodes views.Slice[types.Node log.Warn().Err(testErr).Msg("policy tests failed at boot; server starting anyway, fix the policy and reload") } + if testErr := pm.RunSSHTests(); testErr != nil { //nolint:noinlineerr // boot path: warn-and-continue, not return + log.Warn().Err(testErr).Msg("policy sshTests failed at boot; server starting anyway, fix the policy and reload") + } + return &pm, nil } @@ -482,9 +486,16 @@ func (pm *PolicyManager) SetPolicy(polB []byte) (bool, error) { // sandbox compiled from the new policy + current users/nodes; if // they fail, return without mutating the live PolicyManager so the // failed write does not knock the running config offline. - err = evaluateTests(pol, pm.users, pm.nodes) - if err != nil { - return false, err + // + // Aggregate ACL and SSH test failures via multierr so operators + // see both classes in a single response instead of having to + // fix-and-retry to discover the second one. + testErr := multierr.New( + evaluateTests(pol, pm.users, pm.nodes), + evaluateSSHTests(pol, pm.users, pm.nodes), + ) + if testErr != nil { + return false, testErr } // Log policy metadata for debugging diff --git a/hscontrol/policy/v2/sshtest.go b/hscontrol/policy/v2/sshtest.go new file mode 100644 index 00000000..fdb8ca0d --- /dev/null +++ b/hscontrol/policy/v2/sshtest.go @@ -0,0 +1,716 @@ +package v2 + +import ( + "fmt" + "net/netip" + "slices" + "strings" + + "github.com/juanfont/headscale/hscontrol/types" + "go4.org/netipx" + "tailscale.com/tailcfg" + "tailscale.com/types/views" +) + +// Each sshTests entry asserts that a source identity attempting SSH to +// one or more destination hosts can (or cannot) log in as the named +// users. Evaluation runs at user-initiated writes (SetPolicy, policy +// check, file reload); boot reload skips evaluation so a stale +// reference does not block startup. +// +// Three assertion kinds: +// +// - accept[user]: every (src, dst) must reach via an accept- or +// check-action rule. Both actions resolve to "session permitted" +// at the wire layer, so check counts as reachable for accept. +// - deny[user]: no (src, dst) reaches. Passes when no rule allows +// the user or every matching rule's SSHUsers map blocks them. +// - check[user]: every (src, dst) must reach via a check-action +// rule (HoldAndDelegate set; see filter.go sshCheck). An +// accept-only match fails — the two categories are kept distinct +// so policy authors can pin sensitive logins to check rules. + +// SSHPolicyTestResult is the outcome of a single SSHPolicyTest. Each +// map is keyed by login user with the per-dst breakdown. +type SSHPolicyTestResult struct { + Src string `json:"src"` + Passed bool `json:"passed"` + Errors []string `json:"errors,omitempty"` + + AcceptOK map[string][]string `json:"accept_ok,omitempty"` + AcceptFail map[string][]string `json:"accept_fail,omitempty"` + DenyOK map[string][]string `json:"deny_ok,omitempty"` + DenyFail map[string][]string `json:"deny_fail,omitempty"` + CheckOK map[string][]string `json:"check_ok,omitempty"` + CheckFail map[string][]string `json:"check_fail,omitempty"` +} + +// SSHPolicyTestResults aggregates one evaluation run. +type SSHPolicyTestResults struct { + AllPassed bool `json:"all_passed"` + Results []SSHPolicyTestResult `json:"results"` +} + +// Errors renders the per-test failure breakdown joined by newlines. +// Operators invoking SetPolicy from the CLI or file reload have no +// separate audit channel, so the rendered body is their only signal. +func (r SSHPolicyTestResults) Errors() string { + if r.AllPassed { + return "" + } + + var lines []string + + for _, res := range r.Results { + if res.Passed { + continue + } + + for _, e := range res.Errors { + lines = append(lines, fmt.Sprintf("%s: %s", res.Src, e)) + } + + for _, user := range sortedUsers(res.AcceptFail) { + for _, dst := range res.AcceptFail[user] { + lines = append(lines, fmt.Sprintf( + "%s/%s -> %s: expected ALLOWED, got DENIED", + res.Src, displayUser(user), dst, + )) + } + } + + for _, user := range sortedUsers(res.DenyFail) { + for _, dst := range res.DenyFail[user] { + lines = append(lines, fmt.Sprintf( + "%s/%s -> %s: expected DENIED, got ALLOWED", + res.Src, displayUser(user), dst, + )) + } + } + + for _, user := range sortedUsers(res.CheckFail) { + for _, dst := range res.CheckFail[user] { + lines = append(lines, fmt.Sprintf( + "%s/%s -> %s: expected ALLOWED via check, got %s", + res.Src, displayUser(user), dst, + checkFailReason(res, user, dst), + )) + } + } + } + + return strings.Join(lines, "\n") +} + +// sortedUsers returns the keys of m sorted by user name so error +// rendering is deterministic across runs. +func sortedUsers(m map[string][]string) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + + slices.Sort(keys) + + return keys +} + +// displayUser formats a login user for the rendered error. An empty +// string is shown as `""` so the operator can see that the assertion +// referenced an empty username (which is itself a failure case). +func displayUser(u string) string { + if u == "" { + return `""` + } + + return u +} + +// checkFailReason annotates a check-fail line with whether the user +// reached the dst via an accept rule (so the operator knows to flip the +// rule to action:check) or did not reach the dst at all. +func checkFailReason(res SSHPolicyTestResult, user, dst string) string { + if slices.Contains(res.AcceptOK[user], dst) { + return "ALLOWED via accept" + } + + return "DENIED" +} + +// RunSSHTests evaluates the policy's sshTests block against the live +// users and nodes and returns a wrapped error when any assertion fails. +// Callers that need the per-test breakdown can call runSSHPolicyTests +// directly with their own compile cache. +func (pm *PolicyManager) RunSSHTests() error { + if pm == nil || pm.pol == nil || len(pm.pol.SSHTests) == 0 { + return nil + } + + pm.mu.Lock() + defer pm.mu.Unlock() + + cache := make(map[types.NodeID]*tailcfg.SSHPolicy) + results := runSSHPolicyTests(pm.pol, pm.users, pm.nodes, cache) + + if results.AllPassed { + return nil + } + + return fmt.Errorf("%w:\n%s", errSSHPolicyTestsFailed, results.Errors()) +} + +// evaluateSSHTests is the user-write sandbox: run sshTests against pol +// + current users/nodes without mutating any live state. It mirrors +// evaluateTests for the ACL block. +func evaluateSSHTests( + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], +) error { + if pol == nil || len(pol.SSHTests) == 0 { + return nil + } + + cache := make(map[types.NodeID]*tailcfg.SSHPolicy) + results := runSSHPolicyTests(pol, users, nodes, cache) + + if results.AllPassed { + return nil + } + + return fmt.Errorf("%w:\n%s", errSSHPolicyTestsFailed, results.Errors()) +} + +// runSSHPolicyTests evaluates every sshTests entry against pol. The +// cache is keyed by destination node ID and reused across entries so a +// 10-entry block hitting 4 dst nodes pays 4 compiles, not 40. +func runSSHPolicyTests( + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], + cache map[types.NodeID]*tailcfg.SSHPolicy, +) SSHPolicyTestResults { + results := SSHPolicyTestResults{ + AllPassed: true, + Results: make([]SSHPolicyTestResult, 0, len(pol.SSHTests)), + } + + for _, test := range pol.SSHTests { + res := runSSHPolicyTest(test, pol, users, nodes, cache) + if !res.Passed { + results.AllPassed = false + } + + results.Results = append(results.Results, res) + } + + return results +} + +// runSSHPolicyTest evaluates one SSHPolicyTest entry against pol. +// +// Order of operations: resolve src → resolve dst nodes → reject empty +// assertion blocks → walk accept/deny/check arrays, asking the per-dst +// compiled SSH policy whether the user can reach the dst. +func runSSHPolicyTest( + test SSHPolicyTest, + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], + cache map[types.NodeID]*tailcfg.SSHPolicy, +) SSHPolicyTestResult { + srcLabel := "" + if test.Src != nil { + srcLabel = test.Src.String() + } + + res := SSHPolicyTestResult{ + Src: srcLabel, + Passed: true, + } + + srcAddrs, srcUserID, err := resolveSSHTestSource(test.Src, pol, users, nodes) + if err != nil { + res.Passed = false + res.Errors = append(res.Errors, + fmt.Sprintf("failed to resolve source %q: %v", srcLabel, err)) + + return res + } + + if len(srcAddrs) == 0 { + res.Passed = false + res.Errors = append(res.Errors, + fmt.Sprintf("source %q resolved to no IP addresses", srcLabel)) + + return res + } + + // An entry with no accept/deny/check arrays asserts nothing — flag + // it explicitly so a silent pass cannot mask misconfiguration. + if len(test.Accept) == 0 && len(test.Deny) == 0 && len(test.Check) == 0 { + res.Passed = false + res.Errors = append(res.Errors, + "no accept, deny, or check assertions specified") + + return res + } + + dstNodes, emptyDsts, err := resolveSSHTestDestNodes(test.Dst, pol, users, nodes, srcUserID) + if err != nil { + res.Passed = false + res.Errors = append(res.Errors, + fmt.Sprintf("failed to resolve destinations: %v", err)) + + return res + } + + // A dst alias resolving to no nodes makes the per-assertion loops + // below run zero iterations and the test pass silently — surface + // it as a failure instead. + for _, dst := range emptyDsts { + res.Passed = false + res.Errors = append(res.Errors, + fmt.Sprintf("dst alias %q resolved to no nodes", dst)) + } + + if len(dstNodes) == 0 { + return res + } + + for _, user := range test.Accept { + evaluateAssertion( + pol, users, nodes, cache, + srcAddrs, dstNodes, user.String(), + assertAccept, &res, + ) + } + + for _, user := range test.Deny { + evaluateAssertion( + pol, users, nodes, cache, + srcAddrs, dstNodes, user.String(), + assertDeny, &res, + ) + } + + for _, user := range test.Check { + evaluateAssertion( + pol, users, nodes, cache, + srcAddrs, dstNodes, user.String(), + assertCheck, &res, + ) + } + + return res +} + +// sshAssertion is the kind of assertion being evaluated for a single +// (src, dst, user) triple. +type sshAssertion int + +const ( + assertAccept sshAssertion = iota + assertDeny + assertCheck +) + +// evaluateAssertion walks every (srcAddr, dstNode) pair for one user +// and records the outcome in res. accept passes iff every pair reaches +// via an accept- or check-action rule; deny passes iff no pair +// reaches; check requires HoldAndDelegate on the matching rule. +// Empty username is parse-accepted but fails here because SSH login +// users cannot be empty. +func evaluateAssertion( + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], + cache map[types.NodeID]*tailcfg.SSHPolicy, + srcAddrs []netip.Addr, + dstNodes []types.NodeView, + user string, + kind sshAssertion, + res *SSHPolicyTestResult, +) { +dstLoop: + for _, dst := range dstNodes { + dstPol, err := compiledSSHPolicy(pol, users, nodes, cache, dst) + if err != nil { + res.Passed = false + res.Errors = append(res.Errors, + fmt.Sprintf("compiling SSH policy for %s: %v", + dst.Hostname(), err)) + + continue + } + + dstLabel := dst.Hostname() + + // acceptHit covers "any matching accept-or-check rule"; + // checkHit restricts to check-action matches only. + acceptHit := false + checkHit := false + + for _, srcAddr := range srcAddrs { + a, c := reachability(dstPol, srcAddr, user) + if a { + acceptHit = true + } + + if c { + checkHit = true + } + + // accept and deny require ALL src IPs to reach (or all + // to be blocked). A single counter-example fails the + // assertion. + switch kind { + case assertAccept: + if !a { + res.Passed = false + res.AcceptFail = appendUserDst(res.AcceptFail, user, dstLabel) + + continue dstLoop + } + case assertDeny: + if a { + res.Passed = false + res.DenyFail = appendUserDst(res.DenyFail, user, dstLabel) + + continue dstLoop + } + case assertCheck: + if !c { + res.Passed = false + res.CheckFail = appendUserDst(res.CheckFail, user, dstLabel) + + // Record whether the accept side passed so + // the rendered error can say "ALLOWED via + // accept" instead of "DENIED". + if a { + res.AcceptOK = appendUserDst(res.AcceptOK, user, dstLabel) + } + + continue dstLoop + } + } + } + + switch kind { + case assertAccept: + if acceptHit { + res.AcceptOK = appendUserDst(res.AcceptOK, user, dstLabel) + } + case assertDeny: + res.DenyOK = appendUserDst(res.DenyOK, user, dstLabel) + case assertCheck: + if checkHit { + res.CheckOK = appendUserDst(res.CheckOK, user, dstLabel) + } + } + } +} + +// appendUserDst appends dst to m[user], lazily allocating m. +func appendUserDst(m map[string][]string, user, dst string) map[string][]string { + if m == nil { + m = make(map[string][]string) + } + + m[user] = append(m[user], dst) + + return m +} + +// resolveSSHTestSource resolves the typed src alias into a list of +// netip.Addr (one per principal address the SSH compiler would emit +// for the same source). For user-shaped sources, srcUserID returns the +// resolved user's ID so autogroup:self destinations can scope to the +// same user. Returns ID 0 when the source is a tag, host, or IP. +func resolveSSHTestSource( + src Alias, + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], +) ([]netip.Addr, uint, error) { + if src == nil { + return nil, 0, nil + } + + addrs, err := src.Resolve(pol, users, nodes) + if err != nil { + return nil, 0, fmt.Errorf("resolving: %w", err) + } + + if addrs == nil || addrs.Empty() { + return nil, 0, nil + } + + out := make([]netip.Addr, 0) + for a := range addrs.Iter() { + out = append(out, a) + } + + var userID uint + + u, ok := src.(*Username) + if ok { + resolved, rErr := u.resolveUser(users) + if rErr == nil { + userID = resolved.ID + } + } + + return out, userID, nil +} + +// resolveSSHTestDestNodes resolves every dst alias to its destination +// NodeViews. autogroup:self is handled separately because it cannot +// resolve outside a per-node context. For every other alias, the +// resolved IPSet is matched against each node's IPs via InIPSet. +func resolveSSHTestDestNodes( + dsts SSHTestDestinations, + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], + srcUserID uint, +) ([]types.NodeView, []string, error) { + seen := make(map[types.NodeID]struct{}) + + var ( + out []types.NodeView + emptyDsts []string + ) + + for _, alias := range dsts { + dstLabel := alias.String() + matched := false + + if ag, ok := alias.(*AutoGroup); ok && ag.Is(AutoGroupSelf) { + // autogroup:self → non-tagged nodes owned by the same + // user as src. A tagged or IP-only src has no user + // identity, so the dst set is empty and the caller + // surfaces a failing assertion. + if srcUserID == 0 { + emptyDsts = append(emptyDsts, dstLabel) + + continue + } + + for _, n := range nodes.All() { + if n.IsTagged() { + continue + } + + if !n.User().Valid() { + continue + } + + if n.User().ID() != srcUserID { + continue + } + + matched = true + + if _, dup := seen[n.ID()]; dup { + continue + } + + seen[n.ID()] = struct{}{} + out = append(out, n) + } + + if !matched { + emptyDsts = append(emptyDsts, dstLabel) + } + + continue + } + + ips, err := alias.Resolve(pol, users, nodes) + if err != nil { + return nil, nil, fmt.Errorf("resolving destination %q: %w", dstLabel, err) + } + + if ips == nil || ips.Empty() { + emptyDsts = append(emptyDsts, dstLabel) + + continue + } + + // Compile to an IPSet for the InIPSet primitive. ResolvedAddresses + // already wraps one; expose it via the IPSet builder by walking + // the resolved prefixes. + set, err := prefixesToIPSet(ips.Prefixes()) + if err != nil { + return nil, nil, fmt.Errorf("building IPSet for %q: %w", dstLabel, err) + } + + for _, n := range nodes.All() { + if !n.InIPSet(set) { + continue + } + + matched = true + + if _, dup := seen[n.ID()]; dup { + continue + } + + seen[n.ID()] = struct{}{} + out = append(out, n) + } + + if !matched { + emptyDsts = append(emptyDsts, dstLabel) + } + } + + return out, emptyDsts, nil +} + +// prefixesToIPSet builds the IPSet that InIPSet expects on the node +// side. +func prefixesToIPSet(prefixes []netip.Prefix) (*netipx.IPSet, error) { + var b netipx.IPSetBuilder + + for _, p := range prefixes { + b.AddPrefix(p) + } + + return b.IPSet() +} + +// compiledSSHPolicy returns the per-node compiled SSH policy, populating +// cache on miss. baseURL is empty because the engine only needs the +// "is this rule a check rule" signal (HoldAndDelegate non-empty), not +// the actual URL contents. +func compiledSSHPolicy( + pol *Policy, + users []types.User, + nodes views.Slice[types.NodeView], + cache map[types.NodeID]*tailcfg.SSHPolicy, + node types.NodeView, +) (*tailcfg.SSHPolicy, error) { + if sshPol, ok := cache[node.ID()]; ok { + return sshPol, nil + } + + sshPol, err := pol.compileSSHPolicy("", users, node, nodes) + if err != nil { + return nil, err + } + + cache[node.ID()] = sshPol + + return sshPol, nil +} + +// reachability walks dstPolicy.Rules and reports whether srcAddr is +// allowed to log in as user via: +// +// - any rule (first return) — satisfies accept assertions +// - a check rule specifically (second return) — satisfies check assertions +// +// A nil policy is treated as "no rule matches", which is the right +// answer for both accept (DENIED) and check (DENIED) and for deny +// (PASS, because the deny assertion inverts). +func reachability( + dstPolicy *tailcfg.SSHPolicy, + srcAddr netip.Addr, + user string, +) (bool, bool) { + if dstPolicy == nil { + return false, false + } + + var acceptHit, checkHit bool + + for _, rule := range dstPolicy.Rules { + if !principalContainsAddr(rule.Principals, srcAddr) { + continue + } + + if !sshUserMapAllows(rule.SSHUsers, user) { + continue + } + + if rule.Action == nil { + continue + } + + acceptHit = true + + if rule.Action.HoldAndDelegate != "" { + checkHit = true + } + + // Early-out only when both bits are set; a rule that + // satisfies one assertion may not satisfy the other. + if acceptHit && checkHit { + return acceptHit, checkHit + } + } + + return acceptHit, checkHit +} + +// principalContainsAddr reports whether any principal has a NodeIP +// matching srcAddr. The SSH compiler emits one principal per source +// IP, so an exact-match comparison is correct. +func principalContainsAddr( + principals []*tailcfg.SSHPrincipal, + srcAddr netip.Addr, +) bool { + for _, p := range principals { + if p == nil { + continue + } + + if p.NodeIP == "" { + continue + } + + addr, err := netip.ParseAddr(p.NodeIP) + if err != nil { + continue + } + + if addr == srcAddr { + return true + } + } + + return false +} + +// sshUserMapAllows reports whether SSHUsers permits user. The wire +// shape (see filter.go compileSSHPolicy): +// +// - SSHUsers["root"] == "root" when the rule lists "root"; == "" +// means root is explicitly disallowed. +// - SSHUsers["*"] == "=" when the rule lists autogroup:nonroot — +// wildcard fallback for any non-root user. +// - SSHUsers[] == for every named SSH user. +// +// Empty user input (parse-accepted as a failure case) matches nothing. +func sshUserMapAllows(m map[string]string, user string) bool { + if user == "" { + return false + } + + if v, ok := m[user]; ok { + return v != "" + } + + if user == "root" { + return false + } + + // Wildcard fallback for non-root users. + if v, ok := m["*"]; ok { + return v != "" + } + + return false +} diff --git a/hscontrol/policy/v2/sshtest_test.go b/hscontrol/policy/v2/sshtest_test.go new file mode 100644 index 00000000..1fa2dd05 --- /dev/null +++ b/hscontrol/policy/v2/sshtest_test.go @@ -0,0 +1,1000 @@ +package v2 + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +// sshTestUsers/sshTestNodes are reused across the table below to keep +// each row focussed on the policy under exercise. Three users, six +// nodes: +// +// - alice (id 1) at headscale.net owns alice-laptop and alice-tablet +// - bob (id 2) at headscale.net owns bob-laptop +// - thor (id 3) at example.org owns thor-laptop +// - server (alice-created tagged node) → tag:server +// - prod (alice-created tagged node) → tag:prod +func sshTestUsers() types.Users { + return types.Users{ + {Model: gorm.Model{ID: 1}, Name: "alice", Email: "alice@headscale.net"}, + {Model: gorm.Model{ID: 2}, Name: "bob", Email: "bob@headscale.net"}, + {Model: gorm.Model{ID: 3}, Name: "thor", Email: "thor@example.org"}, + } +} + +func sshTestNodes(users types.Users) types.Nodes { + return types.Nodes{ + { + ID: 1, + Hostname: "alice-laptop", + IPv4: ap("100.64.0.1"), + IPv6: ap("fd7a:115c:a1e0::1"), + User: &users[0], + UserID: &users[0].ID, + }, + { + ID: 2, + Hostname: "bob-laptop", + IPv4: ap("100.64.0.2"), + IPv6: ap("fd7a:115c:a1e0::2"), + User: &users[1], + UserID: &users[1].ID, + }, + { + ID: 3, + Hostname: "server", + IPv4: ap("100.64.0.3"), + IPv6: ap("fd7a:115c:a1e0::3"), + User: &users[0], + UserID: &users[0].ID, + Tags: []string{"tag:server"}, + }, + { + ID: 4, + Hostname: "alice-tablet", + IPv4: ap("100.64.0.4"), + IPv6: ap("fd7a:115c:a1e0::4"), + User: &users[0], + UserID: &users[0].ID, + }, + { + ID: 5, + Hostname: "thor-laptop", + IPv4: ap("100.64.0.5"), + IPv6: ap("fd7a:115c:a1e0::5"), + User: &users[2], + UserID: &users[2].ID, + }, + { + ID: 6, + Hostname: "prod", + IPv4: ap("100.64.0.6"), + IPv6: ap("fd7a:115c:a1e0::6"), + User: &users[0], + UserID: &users[0].ID, + Tags: []string{"tag:prod"}, + }, + } +} + +// TestRunSSHTests covers the engine's per-test outcome reporting. Each +// row constructs a PolicyManager (whose SetPolicy sandbox also exercises +// evaluateSSHTests) and asserts on the resulting RunSSHTests behaviour. +// SetPolicy gating is exercised separately in +// TestSetPolicyRejectsFailingSSHTests below. +func TestRunSSHTests(t *testing.T) { + users := sshTestUsers() + nodes := sshTestNodes(users) + + tests := []struct { + name string + policy string + wantPass bool + wantErrSub []string + }{ + { + name: "accept-pass-basic", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "accept-pass-multi-user-in-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root", "ubuntu"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root", "ubuntu"] + }] + }`, + wantPass: true, + }, + { + name: "accept-fail-no-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "root", "expected ALLOWED"}, + }, + { + name: "accept-fail-user-not-allowed-by-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["mallory"] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "mallory", "expected ALLOWED"}, + }, + { + name: "accept-fail-different-src", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "bob@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"bob@headscale.net", "root", "expected ALLOWED"}, + }, + { + name: "deny-pass-no-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "deny": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "deny-pass-rule-blocks-user", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["autogroup:nonroot"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "deny": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "deny-fail-rule-allows", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "deny": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "root", "expected DENIED"}, + }, + { + name: "check-pass-rule-is-check", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "check", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "check": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "check-fail-rule-is-accept", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "check": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{ + "alice@headscale.net", + "root", + "via check", + "via accept", + }, + }, + { + name: "check-pass-and-accept-pass-coexist", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [ + { + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }, + { + "action": "check", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["ubuntu"] + } + ], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"], + "check": ["ubuntu"] + }] + }`, + wantPass: true, + }, + { + name: "accept-passes-on-check-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "check", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "multi-dst-all-must-reach", + policy: `{ + "tagOwners": { + "tag:server": ["alice@headscale.net"], + "tag:prod": ["alice@headscale.net"] + }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server", "tag:prod"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{ + "alice@headscale.net", + "root", + "prod", + "expected ALLOWED", + }, + }, + { + name: "multi-user-mixed-accept-deny-check-in-one-entry", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [ + { + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }, + { + "action": "check", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["ubuntu"] + } + ], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"], + "deny": ["mallory"], + "check": ["ubuntu"] + }] + }`, + wantPass: true, + }, + { + name: "nonroot-allows-alice", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["autogroup:nonroot"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["alice"] + }] + }`, + wantPass: true, + }, + { + name: "nonroot-denies-root", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["autogroup:nonroot"] + }], + "sshTests": [ + { + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }, + { + "src": "alice@headscale.net", + "dst": ["tag:server"], + "deny": ["root"] + } + ] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "root", "expected ALLOWED"}, + }, + { + name: "wildcard-user-allows-mallory", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["autogroup:nonroot"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["mallory"] + }] + }`, + wantPass: true, + }, + { + name: "root-only-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [ + { + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }, + { + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["alice"] + } + ] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "alice", "expected ALLOWED"}, + }, + { + name: "autogroup-self-same-user", + policy: `{ + "ssh": [{ + "action": "accept", + "src": ["autogroup:member"], + "dst": ["autogroup:self"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["autogroup:self"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "autogroup-self-cross-user-fails", + policy: `{ + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["autogroup:self"], + "users": ["root"] + }], + "sshTests": [{ + "src": "bob@headscale.net", + "dst": ["autogroup:self"], + "accept": ["root"] + }] + }`, + wantPass: false, + // autogroup:self for bob resolves to bob-laptop; the only + // rule allows alice as src, so reachability fails. + wantErrSub: []string{"bob@headscale.net", "root"}, + }, + { + name: "localpart-domain-match", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["localpart:*@headscale.net"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["alice"] + }] + }`, + wantPass: true, + }, + { + name: "localpart-domain-mismatch", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["thor@example.org"], + "dst": ["tag:server"], + "users": ["localpart:*@headscale.net"] + }], + "sshTests": [{ + "src": "thor@example.org", + "dst": ["tag:server"], + "accept": ["thor"] + }] + }`, + wantPass: false, + wantErrSub: []string{"thor@example.org", "thor", "expected ALLOWED"}, + }, + { + name: "tag-as-src", + policy: `{ + "tagOwners": { + "tag:server": ["alice@headscale.net"], + "tag:prod": ["alice@headscale.net"] + }, + "ssh": [{ + "action": "accept", + "src": ["tag:prod"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "tag:prod", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "acl-allows-tcp22-no-ssh-rule", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:22"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "root", "expected ALLOWED"}, + }, + { + // ACL grants only TCP:80 to alice; no rule grants TCP:22. + // SSH rule independently allows root@tag:server. The + // sshTests assertion must pass on the SSH layer alone, + // proving the engine does not require an ACL packet- + // filter rule for the SSH port. + name: "acl-denies-tcp22-ssh-rule-allows", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:80"] + }], + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "no-sshTests-block", + policy: `{ + "acls": [{ + "action": "accept", + "src": ["*"], + "dst": ["*:*"] + }] + }`, + wantPass: true, + }, + { + name: "both-tests-and-sshTests-both-pass", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:22"] + }], + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "tests": [{ + "src": "alice@headscale.net", + "accept": ["tag:server:22"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }`, + wantPass: true, + }, + { + name: "empty-accept-deny-check-in-entry", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "no accept, deny, or check"}, + }, + { + name: "empty-user-in-accept", + policy: `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": [""] + }] + }`, + wantPass: false, + wantErrSub: []string{"alice@headscale.net", "expected ALLOWED"}, + }, + { + // tag:empty has an owner but no tagged nodes, so the dst + // alias resolves to no nodes. Without the empty-dst guard + // the per-assertion loop runs zero iterations and the + // test silently passes — exactly the regression the + // guard exists to catch. + name: "dst-tag-with-no-tagged-nodes-fails", + policy: `{ + "tagOwners": { "tag:empty": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:empty"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"tag:empty", "resolved to no nodes"}, + }, + { + // autogroup:self from a tag src has no user identity to + // scope to, so the dst alias resolves to no nodes. Same + // empty-dst guard, distinct trigger path. + name: "dst-autogroup-self-from-tag-src-fails", + policy: `{ + "tagOwners": { "tag:prod": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "tag:prod", + "dst": ["autogroup:self"], + "accept": ["root"] + }] + }`, + wantPass: false, + wantErrSub: []string{"autogroup:self", "resolved to no nodes"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pm, err := NewPolicyManager([]byte(tt.policy), users, nodes.ViewSlice()) + require.NoError(t, err, "policy must parse and compile") + + runErr := pm.RunSSHTests() + if tt.wantPass { + require.NoError(t, runErr, "sshTests should pass") + + return + } + + require.Error(t, runErr, "sshTests should fail") + require.ErrorIs(t, runErr, errSSHPolicyTestsFailed) + + for _, sub := range tt.wantErrSub { + assert.Contains(t, runErr.Error(), sub, + "rendered error should mention %q", sub) + } + }) + } +} + +// TestRunSSHTestsBothTestsPassSSHTestsFail captures the distinction the +// caller cares about: a passing ACL `tests` block plus a failing +// `sshTests` block returns errSSHPolicyTestsFailed and NOT +// errPolicyTestsFailed. The two sentinels share a literal message but +// are distinct values. +func TestRunSSHTestsBothTestsPassSSHTestsFail(t *testing.T) { + users := sshTestUsers() + nodes := sshTestNodes(users) + + policy := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:22"] + }], + "tests": [{ + "src": "alice@headscale.net", + "accept": ["tag:server:22"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice()) + require.NoError(t, err) + + // ACL tests should pass. + require.NoError(t, pm.RunTests()) + + // SSH tests should fail because there's no SSH rule for root. + sshErr := pm.RunSSHTests() + require.Error(t, sshErr) + require.ErrorIs(t, sshErr, errSSHPolicyTestsFailed) + require.NotErrorIs(t, sshErr, errPolicyTestsFailed, + "ACL test sentinel must not appear on SSH-only failure") +} + +// TestSetPolicyRejectsFailingSSHTests asserts SetPolicy is the user-write +// boundary: a policy whose sshTests fail is rejected without mutating +// the live PolicyManager. SSHPolicy() output must remain the prior +// rules. +func TestSetPolicyRejectsFailingSSHTests(t *testing.T) { + users := sshTestUsers() + nodes := sshTestNodes(users) + + good := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + bad := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "sshTests": [{ + "src": "bob@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + pm, err := NewPolicyManager([]byte(good), users, nodes.ViewSlice()) + require.NoError(t, err) + + // Snapshot SSHPolicy output for alice-laptop before the rejected + // write — the live PolicyManager state must still describe the + // previous (good) rules afterwards. JSON-marshal the snapshot so + // the comparison sees rule content, not just object identity: a + // hypothetical mutation that preserves the slice length but + // rewrites principals or SSHUsers would slip past a count-only + // assertion. + aliceView := nodes.ViewSlice().At(0) + + beforePol, err := pm.SSHPolicy("", aliceView) + require.NoError(t, err) + + beforeJSON, err := json.Marshal(beforePol) + require.NoError(t, err) + + changed, err := pm.SetPolicy([]byte(bad)) + require.Error(t, err, "SetPolicy must reject a policy whose sshTests fail") + require.False(t, changed, "SetPolicy must report no change when rejected") + require.ErrorIs(t, err, errSSHPolicyTestsFailed) + require.Contains(t, err.Error(), "expected ALLOWED") + + afterPol, err := pm.SSHPolicy("", aliceView) + require.NoError(t, err) + + afterJSON, err := json.Marshal(afterPol) + require.NoError(t, err) + + require.JSONEq(t, string(beforeJSON), string(afterJSON), + "live SSH policy must not change after a rejected SetPolicy") +} + +// TestSetPolicyAggregatesACLAndSSHTestFailures exercises the multierr +// aggregation: when both layers fail, the returned error wraps both +// sentinels so operators see every failure in a single round trip. +func TestSetPolicyAggregatesACLAndSSHTestFailures(t *testing.T) { + users := sshTestUsers() + nodes := sshTestNodes(users) + + good := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:22"] + }], + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "tests": [{ + "src": "alice@headscale.net", + "accept": ["tag:server:22"] + }], + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + // Both blocks fail: acls only allow alice but tests assert bob; + // ssh only allows alice but sshTests assert bob. + bad := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "acls": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server:22"] + }], + "ssh": [{ + "action": "accept", + "src": ["alice@headscale.net"], + "dst": ["tag:server"], + "users": ["root"] + }], + "tests": [{ + "src": "bob@headscale.net", + "accept": ["tag:server:22"] + }], + "sshTests": [{ + "src": "bob@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + pm, err := NewPolicyManager([]byte(good), users, nodes.ViewSlice()) + require.NoError(t, err) + + _, err = pm.SetPolicy([]byte(bad)) + require.Error(t, err) + require.ErrorIs(t, err, errPolicyTestsFailed, + "aggregated error must wrap the ACL test sentinel") + require.ErrorIs(t, err, errSSHPolicyTestsFailed, + "aggregated error must wrap the SSH test sentinel") + + body := err.Error() + assert.Contains(t, body, "tag:server:22", + "aggregated error must include the ACL failure message") + assert.Contains(t, body, "bob@headscale.net", + "aggregated error must include the bob src") + // The SSH renderer emits "src/user -> dst" form; the ACL renderer + // emits "src -> dst". Substring "/root -> " is unique to the SSH + // body, so finding it inside the aggregated error proves the SSH + // failure rendering was concatenated alongside the ACL body. + assert.Contains(t, body, "/root -> ", + "aggregated error must include the SSH-shape src/user -> dst rendering") +} + +// TestNewPolicyManagerWarnsOnSSHTestsFailure asserts the boot path does +// not error on a failing sshTests block: warn-and-continue is the right +// behaviour for stale stored policy, mirroring the ACL tests handling. +func TestNewPolicyManagerWarnsOnSSHTestsFailure(t *testing.T) { + users := sshTestUsers() + nodes := sshTestNodes(users) + + // sshTests reference a user that does exist but no rule allows + // them — the test should fail at user-write but not at boot. + stale := `{ + "tagOwners": { "tag:server": ["alice@headscale.net"] }, + "sshTests": [{ + "src": "alice@headscale.net", + "dst": ["tag:server"], + "accept": ["root"] + }] + }` + + pm, err := NewPolicyManager([]byte(stale), users, nodes.ViewSlice()) + require.NoError(t, err, "boot must not error on sshTests failure") + require.NotNil(t, pm) + + // A subsequent SetPolicy of the same body must reject — that's + // the user-write path. + _, err = pm.SetPolicy([]byte(stale)) + require.Error(t, err) + require.ErrorIs(t, err, errSSHPolicyTestsFailed) +} + +// TestSSHPolicyTestResultsErrorsRendering checks the multi-line render +// layout. Because the body is the user-facing error, the format needs +// to identify (src, user, dst) cleanly across accept, deny, and check. +func TestSSHPolicyTestResultsErrorsRendering(t *testing.T) { + results := SSHPolicyTestResults{ + AllPassed: false, + Results: []SSHPolicyTestResult{ + { + Src: "alice@headscale.net", + AcceptFail: map[string][]string{ + "root": {"server"}, + }, + }, + { + Src: "bob@headscale.net", + DenyFail: map[string][]string{ + "root": {"alice-laptop"}, + }, + }, + { + Src: "alice@headscale.net", + CheckFail: map[string][]string{ + "ubuntu": {"server"}, + }, + AcceptOK: map[string][]string{ + "ubuntu": {"server"}, + }, + }, + }, + } + + rendered := results.Errors() + for _, sub := range []string{ + "alice@headscale.net/root -> server: expected ALLOWED, got DENIED", + "bob@headscale.net/root -> alice-laptop: expected DENIED, got ALLOWED", + "alice@headscale.net/ubuntu -> server: expected ALLOWED via check, got ALLOWED via accept", + } { + assert.Contains(t, rendered, sub) + } + + assert.Equal(t, 3, strings.Count(rendered, "\n")+1, + "expected one line per failing assertion") +} diff --git a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go index 74486af6..a6cb27c2 100644 --- a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go @@ -46,61 +46,6 @@ func setupSSHDataCompatUsers() types.Users { } } -// setupSSHDataCompatNodes returns the test nodes for SSH data-driven -// compatibility tests. Node GivenNames match the anonymized pokémon names: -// - bulbasaur (owned by odin) -// - ivysaur (owned by thor) -// - venusaur (owned by freya) -// - beedrill (tag:server) -// - kakuna (tag:prod) -func setupSSHDataCompatNodes(users types.Users) types.Nodes { - return types.Nodes{ - &types.Node{ - ID: 1, - GivenName: "bulbasaur", - User: &users[0], - UserID: &users[0].ID, - IPv4: ptrAddr("100.90.199.68"), - IPv6: ptrAddr("fd7a:115c:a1e0::2d01:c747"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - &types.Node{ - ID: 2, - GivenName: "ivysaur", - User: &users[1], - UserID: &users[1].ID, - IPv4: ptrAddr("100.110.121.96"), - IPv6: ptrAddr("fd7a:115c:a1e0::1737:7960"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - &types.Node{ - ID: 3, - GivenName: "venusaur", - User: &users[2], - UserID: &users[2].ID, - IPv4: ptrAddr("100.103.90.82"), - IPv6: ptrAddr("fd7a:115c:a1e0::9e37:5a52"), - Hostinfo: &tailcfg.Hostinfo{}, - }, - &types.Node{ - ID: 4, - GivenName: "beedrill", - IPv4: ptrAddr("100.108.74.26"), - IPv6: ptrAddr("fd7a:115c:a1e0::b901:4a87"), - Tags: []string{"tag:server"}, - Hostinfo: &tailcfg.Hostinfo{}, - }, - &types.Node{ - ID: 5, - GivenName: "kakuna", - IPv4: ptrAddr("100.103.8.15"), - IPv6: ptrAddr("fd7a:115c:a1e0::5b37:80f"), - Tags: []string{"tag:prod"}, - Hostinfo: &tailcfg.Hostinfo{}, - }, - } -} - // loadSSHTestFile loads and parses a single SSH capture HuJSON file. func loadSSHTestFile(t *testing.T, path string) *testcapture.Capture { t.Helper() diff --git a/hscontrol/policy/v2/test.go b/hscontrol/policy/v2/test.go index c2b5f9b5..6b27f068 100644 --- a/hscontrol/policy/v2/test.go +++ b/hscontrol/policy/v2/test.go @@ -7,6 +7,7 @@ import ( "slices" "strings" + "github.com/go-json-experiment/json" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/util" "tailscale.com/tailcfg" @@ -67,26 +68,92 @@ type PolicyTest struct { type SSHPolicyTest struct { // Src is a single source alias (user, group, tag, host, or IP). Same // shape as PolicyTest.Src — Tailscale only supports one src per entry. - Src string `json:"src"` + Src Alias `json:"src"` // Dst lists destination host aliases the test exercises. Tags, hosts, // and the SSH-compatible autogroups are valid; ports, CIDR ranges, and // autogroup:internet are rejected at parse time. - Dst []string `json:"dst"` + Dst SSHTestDestinations `json:"dst"` // Accept lists SSH login users that must be allowed by an action:accept // or action:check rule when Src connects to each entry in Dst. - Accept []string `json:"accept,omitempty"` + Accept []SSHUser `json:"accept,omitempty"` // Deny lists SSH login users that must NOT be allowed by any rule when // Src connects to each entry in Dst. - Deny []string `json:"deny,omitempty"` + Deny []SSHUser `json:"deny,omitempty"` - // Check lists SSH login users that must be allowed by an action:check - // rule specifically. action:accept matches do not satisfy a check - // assertion. Engine evaluation is not implemented yet; parse-time - // validation accepts the field so policies can be authored ahead of it. - Check []string `json:"check,omitempty"` + // Check lists SSH login users that must reach every dst via an + // action:check rule specifically (the HoldAndDelegate signal on the + // compiled SSH policy). An action:accept rule alone does not satisfy + // a check assertion — SaaS keeps the two categories distinct so + // policy authors can pin sensitive logins to check rules. + Check []SSHUser `json:"check,omitempty"` +} + +// SSHTestDestinations is the list of destination aliases an sshTests entry +// targets. Unmarshalling reuses the same alias parser the rest of the +// policy engine drives so each element lands as a typed Alias; the parse- +// time shape rules in validateSSHTestDestination continue to enforce the +// SSH-specific restrictions (no :port, no CIDR, no autogroup:internet, +// known tag). +type SSHTestDestinations []Alias + +// UnmarshalJSON walks the JSON array, dispatching each element through +// AliasEnc so trimming and prefix detection match the rest of the parser. +func (d *SSHTestDestinations) UnmarshalJSON(b []byte) error { + var aliases []AliasEnc + + err := json.Unmarshal(b, &aliases, policyJSONOpts...) + if err != nil { + return err + } + + *d = make([]Alias, len(aliases)) + for i, a := range aliases { + (*d)[i] = a.Alias + } + + return nil +} + +// UnmarshalJSON drives the typed shape of SSHPolicyTest. The wire format +// is unchanged: src is a JSON string parsed through parseAlias; dst is an +// array of strings handled by SSHTestDestinations; accept/deny/check are +// arrays of strings handled per element by SSHUser.UnmarshalJSON. An +// empty src string lands as a nil Alias so the empty-src case stays a +// validation-time error with the SaaS-aligned ErrSSHTestEmptySrc body +// rather than a raw parser failure. +func (t *SSHPolicyTest) UnmarshalJSON(b []byte) error { + var raw struct { + Src string `json:"src"` + Dst SSHTestDestinations `json:"dst"` + Accept []SSHUser `json:"accept,omitempty"` + Deny []SSHUser `json:"deny,omitempty"` + Check []SSHUser `json:"check,omitempty"` + } + + err := json.Unmarshal(b, &raw, policyJSONOpts...) + if err != nil { + return err + } + + trimmedSrc := strings.TrimSpace(raw.Src) + if trimmedSrc != "" { + alias, parseErr := parseAlias(trimmedSrc) + if parseErr != nil { + return parseErr + } + + t.Src = alias + } + + t.Dst = raw.Dst + t.Accept = raw.Accept + t.Deny = raw.Deny + t.Check = raw.Check + + return nil } // PolicyTestResult is the outcome of a single PolicyTest. diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index adcfdad5..8051ccd4 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -863,6 +863,12 @@ type Alias interface { Validate() error UnmarshalJSON(b []byte) error + // String renders the alias back to its policy-file form. Implementations + // are expected to return a value that round-trips through parseAlias for + // any alias the parser accepted, so callers can use it as a stable + // identity in rendered errors and logs. + String() string + // Resolve resolves the Alias to an IPSet. The IPSet will contain all the IP // addresses that the Alias represents within Headscale. It is the product // of the Alias and the Policy, Users and Nodes. @@ -2946,10 +2952,6 @@ func (p *SSHCheckPeriod) UnmarshalJSON(b []byte) error { return nil } - // time.ParseDuration produces error strings like - // `time: invalid duration "abc"` which match SaaS body wording - // exactly; model.ParseDuration wraps the same parse with custom - // phrasing and would diverge. d, err := time.ParseDuration(str) if err != nil { return err @@ -3441,7 +3443,7 @@ func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error { var errs []error for i, t := range tests { - if t.Src == "" { + if t.Src == nil { errs = append(errs, fmt.Errorf("sshTest %d: %w", i, ErrSSHTestEmptySrc)) } @@ -3467,16 +3469,14 @@ func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error { // validateSSHTestDestination enforces that an sshTests dst entry names a // single SSH-reachable host. Tailscale SaaS rejects three shapes at parse // time: a `:port` suffix (read by the parser as an unknown tag, hence the -// "unknown tag" error wording); a CIDR-shaped value (raw `/N` or a -// `hosts:` entry whose RHS is a multi-host prefix); and autogroup:internet -// (only valid in ACL destinations, not SSH ones). Tag entries must -// reference a tag that exists in tagOwners; bare hosts must resolve to a -// single-address prefix. -func validateSSHTestDestination(pol *Policy, dst string) error { - alias, err := parseAlias(dst) - if err != nil { - return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) - } +// "unknown tag" error wording); a multi-host CIDR (raw `/N` narrower than +// the address width, or a `hosts:` entry whose RHS is a multi-host prefix); +// and autogroup:internet (only valid in ACL destinations, not SSH ones). +// A bare IP literal — which parses to a `/BitLen` prefix — names a single +// host and is accepted. Tag entries must reference a tag that exists in +// tagOwners; bare hosts must resolve to a single-address prefix. +func validateSSHTestDestination(pol *Policy, alias Alias) error { + dst := alias.String() switch a := alias.(type) { case *AutoGroup: @@ -3488,10 +3488,14 @@ func validateSSHTestDestination(pol *Policy, dst string) error { } case *Prefix: - // A CIDR literal in dst is rejected. A bare IP parses as a Prefix - // with no slash in the input string — distinguish on the raw text - // the same way validateTestDestination does. - if strings.Contains(dst, "/") { + // SaaS accepts a bare IP (parsed to a `/BitLen` prefix) as a + // single SSH-reachable host but rejects a narrower CIDR like + // `10.0.0.0/24`. Distinguish the two by mask width: a prefix + // whose Bits() matches the address BitLen() is a single host + // and passes; anything narrower is a multi-host range and is + // rejected the same way as raw `/N`. + p := netip.Prefix(*a) + if p.Bits() < p.Addr().BitLen() { return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 8dd06464..1a046f12 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -6003,9 +6003,12 @@ func TestUnmarshalPolicySSHTests(t *testing.T) { t.Helper() require.Len(t, pol.SSHTests, 1) got := pol.SSHTests[0] - require.Equal(t, "thor@example.org", got.Src) - require.Equal(t, []string{"tag:server"}, got.Dst) - require.Equal(t, []string{"root"}, got.Accept) + require.IsType(t, (*Username)(nil), got.Src) + require.Equal(t, "thor@example.org", got.Src.String()) + require.Len(t, got.Dst, 1) + require.IsType(t, (*Tag)(nil), got.Dst[0]) + require.Equal(t, "tag:server", got.Dst[0].String()) + require.Equal(t, []SSHUser{"root"}, got.Accept) require.Empty(t, got.Deny) require.Empty(t, got.Check) }, @@ -6030,9 +6033,9 @@ func TestUnmarshalPolicySSHTests(t *testing.T) { t.Helper() require.Len(t, pol.SSHTests, 1) got := pol.SSHTests[0] - require.Equal(t, []string{"root"}, got.Accept) - require.Equal(t, []string{"nobody"}, got.Deny) - require.Equal(t, []string{"alice"}, got.Check) + require.Equal(t, []SSHUser{"root"}, got.Accept) + require.Equal(t, []SSHUser{"nobody"}, got.Deny) + require.Equal(t, []SSHUser{"alice"}, got.Check) //nolint:goconst }, }, { @@ -6086,6 +6089,52 @@ func TestUnmarshalPolicySSHTests(t *testing.T) { `, wantErr: ErrSSHTestDstDisallowedElement, }, + { + // SaaS accepts a bare IPv4 literal as a host address. The + // Prefix parser turns it into a /32 so validateSSHTestDestination + // must match Bits() against Addr().BitLen() rather than reject + // the whole *Prefix branch. + name: "dst-bare-ipv4-accepted", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["100.64.0.16"], "accept": ["root"]} + ] +} +`, + check: func(t *testing.T, pol *Policy) { + t.Helper() + require.Len(t, pol.SSHTests, 1) + got := pol.SSHTests[0] + require.Len(t, got.Dst, 1) + pref, ok := got.Dst[0].(*Prefix) + require.True(t, ok, "want *Prefix, got %T", got.Dst[0]) + require.Equal(t, "100.64.0.16/32", pref.String()) + }, + }, + { + // IPv6 mirror of the IPv4 case: bare `fd7a::10` parses to + // /128 and must pass the parse-time shape check. + name: "dst-bare-ipv6-accepted", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["fd7a:115c:a1e0::10"], "accept": ["root"]} + ] +} +`, + check: func(t *testing.T, pol *Policy) { + t.Helper() + require.Len(t, pol.SSHTests, 1) + got := pol.SSHTests[0] + require.Len(t, got.Dst, 1) + pref, ok := got.Dst[0].(*Prefix) + require.True(t, ok, "want *Prefix, got %T", got.Dst[0]) + require.Equal(t, "fd7a:115c:a1e0::10/128", pref.String()) + }, + }, { name: "dst-autogroup-internet", input: `