diff --git a/hscontrol/policy/v2/test.go b/hscontrol/policy/v2/test.go index a93b7c6b..c2b5f9b5 100644 --- a/hscontrol/policy/v2/test.go +++ b/hscontrol/policy/v2/test.go @@ -29,9 +29,15 @@ import ( // errors. The Error() prefix is "test(s) failed", the same string Tailscale // SaaS returns in the api_response_body.message — see // hscontrol/policy/v2/testdata/policytest_results/. +// +// errSSHPolicyTestsFailed wraps sshTests failures. Tailscale SaaS returns the +// same literal "test(s) failed" body for both ACL tests and SSH tests, but +// the two sentinels are kept as distinct values so callers can use errors.Is +// to tell them apart while still matching the SaaS body byte-for-byte. var ( - errPolicyTestsFailed = errors.New("test(s) failed") - errTestDestinationNoIP = errors.New("destination resolved to no IP addresses") + errPolicyTestsFailed = errors.New("test(s) failed") + errSSHPolicyTestsFailed = errors.New("test(s) failed") + errTestDestinationNoIP = errors.New("destination resolved to no IP addresses") ) // PolicyTest is one entry in the policy's `tests` block. @@ -53,6 +59,36 @@ type PolicyTest struct { Deny []string `json:"deny,omitempty"` } +// SSHPolicyTest is one entry in the policy's `sshTests` block. Unlike the +// ACL `tests` block, sshTests describe SSH login attempts: a source alias +// connects to each destination host and tries each named login user. The +// accept / deny / check arrays carry usernames, not destinations — every +// listed user is asserted against every entry in dst. +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"` + + // 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"` + + // 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"` + + // 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"` + + // 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"` +} + // PolicyTestResult is the outcome of a single PolicyTest. type PolicyTestResult struct { Src string `json:"src"` diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 3c1e1db6..adcfdad5 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -153,6 +153,10 @@ var ( ErrTestDestinationMultiPort = errors.New("test destination port must be a single port") ErrTestDestinationCIDR = errors.New("test destination must be a single host, not a CIDR range") ErrAutogroupInternetTestDst = errors.New("autogroup:internet not valid as a test destination") + ErrSSHTestEmptySrc = errors.New("SSH tests entry must have a non-empty src") + ErrSSHTestEmptyDst = errors.New("SSH tests entry must have at least one dst") + ErrSSHTestDstUnknownTag = errors.New("SSH tests dst contains unknown tag") + ErrSSHTestDstDisallowedElement = errors.New("SSH tests dst contains disallowed element") ) type resolved struct { @@ -2092,6 +2096,7 @@ type Policy struct { AutoApprovers AutoApproverPolicy `json:"autoApprovers"` SSHs []SSH `json:"ssh,omitempty"` Tests []PolicyTest `json:"tests,omitempty"` + SSHTests []SSHPolicyTest `json:"sshTests,omitempty"` RandomizeClientPort bool `json:"randomizeClientPort,omitempty"` } @@ -2910,6 +2915,10 @@ func (p *Policy) validate() error { errs = append(errs, err) } + if err := validateSSHTests(p, p.SSHTests); err != nil { //nolint:noinlineerr + errs = append(errs, err) + } + if len(errs) > 0 { return multierr.New(errs...) } @@ -3418,3 +3427,105 @@ func validateTestDestination(pol *Policy, dst string) error { return nil } + +// validateSSHTests enforces the parse-time shape rules an sshTests entry +// must satisfy: a non-empty src alias, at least one dst, and a dst list +// whose entries each describe a single SSH-reachable host. Login-user +// assertions (accept/deny/check) are not validated here — SaaS reports +// empty assertion arrays and empty user strings through the same +// "test(s) failed" body it uses for true evaluation failures, so those +// cases stay with the engine. Both the parse-time errors and the +// engine-time failures share the errSSHPolicyTestsFailed wrapper so +// callers see one consistent body. +func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error { + var errs []error + + for i, t := range tests { + if t.Src == "" { + errs = append(errs, fmt.Errorf("sshTest %d: %w", i, ErrSSHTestEmptySrc)) + } + + if len(t.Dst) == 0 { + errs = append(errs, fmt.Errorf("sshTest %d: %w", i, ErrSSHTestEmptyDst)) + } + + for _, dst := range t.Dst { + err := validateSSHTestDestination(pol, dst) + if err != nil { + errs = append(errs, fmt.Errorf("sshTest %d: %w", i, err)) + } + } + } + + if len(errs) > 0 { + return fmt.Errorf("%w:\n%w", errSSHPolicyTestsFailed, multierr.New(errs...)) + } + + return nil +} + +// 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) + } + + switch a := alias.(type) { + case *AutoGroup: + // autogroup:internet is the only autogroup SaaS rejects at parse. + // Other autogroups (member, tagged, self, nonroot) are valid SSH + // dst aliases and pass through to engine evaluation. + if *a == AutoGroupInternet { + return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) + } + + 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, "/") { + return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) + } + + case *Tag: + // A tag must be declared in tagOwners. The `tag:server:22` shape + // reaches this branch because isTag only checks the `tag:` prefix + // — the colon-port suffix is preserved in the Tag string and the + // tagOwners lookup misses, reproducing the SaaS error wording. + if pol == nil { + return fmt.Errorf("%w %q", ErrSSHTestDstUnknownTag, string(*a)) + } + + err := pol.TagOwners.Contains(a) + if err != nil { + return fmt.Errorf("%w %q", ErrSSHTestDstUnknownTag, string(*a)) + } + + case *Host: + // A hosts: entry that resolves to a multi-host prefix is a CIDR + // in disguise — reject it the same way as raw `/N`. + if pol == nil { + return nil + } + + pref, ok := pol.Hosts[*a] + if !ok { + return nil + } + + p := netip.Prefix(pref) + if p.Bits() < p.Addr().BitLen() { + return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst) + } + } + + return nil +} diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 5c4652c5..8dd06464 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -5973,3 +5973,225 @@ func TestUnmarshalPolicyEmptyArrays(t *testing.T) { }) } } + +// TestUnmarshalPolicySSHTests covers the parse-time shape rules for the +// sshTests block. Positive rows confirm the SSHPolicyTest struct fields +// round-trip through JSON. Rejection rows pin each parse-time sentinel +// against a representative malformed input. SaaS evaluation-time failures +// (empty assertions, empty user strings) are deliberately accepted at +// parse — they share the "test(s) failed" body with true failures and +// land with the engine. +func TestUnmarshalPolicySSHTests(t *testing.T) { + cases := []struct { + name string + input string + wantErr error // sentinel for errors.Is; nil means parse must succeed + extraSentinels []error // additional sentinels reachable via errors.Is + check func(t *testing.T, pol *Policy) + }{ + { + name: "valid-minimal-shape", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["tag:server"], "accept": ["root"]} + ] +} +`, + check: func(t *testing.T, pol *Policy) { + 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.Empty(t, got.Deny) + require.Empty(t, got.Check) + }, + }, + { + name: "valid-all-three-action-arrays", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + { + "src": "thor@example.org", + "dst": ["tag:server"], + "accept": ["root"], + "deny": ["nobody"], + "check": ["alice"] + } + ] +} +`, + check: func(t *testing.T, pol *Policy) { + 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) + }, + }, + { + // Empty accept+deny+check is rejected by SaaS at evaluation, + // not at parse — the captured body is the same "test(s) failed" + // that true evaluation failures emit. The parse layer must let + // this through so the engine reports it consistently. + name: "valid-empty-arrays-engine-deferred", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["tag:server"]} + ] +} +`, + check: func(t *testing.T, pol *Policy) { + t.Helper() + require.Len(t, pol.SSHTests, 1) + got := pol.SSHTests[0] + require.Empty(t, got.Accept) + require.Empty(t, got.Deny) + require.Empty(t, got.Check) + }, + }, + { + // `tag:server:22` parses as a Tag because isTag only checks + // the `tag:` prefix; the colon-port suffix is retained in the + // tag string and the tagOwners lookup misses. SaaS reports + // this as an unknown tag with the bad value quoted. + name: "dst-with-port", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["tag:server:22"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestDstUnknownTag, + }, + { + name: "dst-cidr", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["10.0.0.0/8"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestDstDisallowedElement, + }, + { + name: "dst-autogroup-internet", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["autogroup:internet"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestDstDisallowedElement, + }, + { + name: "dst-unknown-tag", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": ["tag:not-in-tagOwners"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestDstUnknownTag, + }, + { + name: "empty-src", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "", "dst": ["tag:server"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestEmptySrc, + }, + { + name: "empty-dst", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "thor@example.org", "dst": [], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestEmptyDst, + }, + { + // Multiple shape failures in one entry must aggregate through + // multierr.New under errSSHPolicyTestsFailed so the surfaced + // body matches the SaaS body byte-for-byte and every + // individual sentinel remains reachable via errors.Is. + name: "multierr-wrap", + input: ` +{ + "tagOwners": {"tag:server": ["admin@example.org"]}, + "sshTests": [ + {"src": "", "dst": ["10.0.0.0/8", "autogroup:internet"], "accept": ["root"]} + ] +} +`, + wantErr: ErrSSHTestEmptySrc, + extraSentinels: []error{ErrSSHTestDstDisallowedElement}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + pol, err := unmarshalPolicy([]byte(tc.input)) + + if tc.wantErr == nil { + require.NoError(t, err) + require.NotNil(t, pol) + + if tc.check != nil { + tc.check(t, pol) + } + + return + } + + require.Error(t, err) + require.ErrorIs( + t, + err, tc.wantErr, + "want errors.Is(err, %v); got %v", + tc.wantErr, + err, + ) + require.Contains( + t, + err.Error(), "test(s) failed", + `want err to contain "test(s) failed"; got %q`, + err.Error(), + ) + + for _, sentinel := range tc.extraSentinels { + require.ErrorIs( + t, + err, sentinel, + "want errors.Is(err, %v); got %v", + sentinel, + err, + ) + } + }) + } +}