mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-07 16:40:21 +09:00
policy/v2: align SSH rule validation with Tailscale SaaS
Drop autogroup/localpart strictness on SSH users. Drop checkPeriod minimum. Reject empty/wildcard users and empty acceptEnv at parse. Match SaaS wording for action and checkPeriod errors. 28 captures under testdata/ssh_results/ssh-malformed-*.hujson lock the surface.
This commit is contained in:
@@ -1317,7 +1317,7 @@ func TestSSHPolicyRules(t *testing.T) {
|
||||
]
|
||||
}`,
|
||||
expectErr: true,
|
||||
errorMessage: `invalid SSH action: "invalid", must be one of: accept, check`,
|
||||
errorMessage: `"invalid" is not a valid action`,
|
||||
},
|
||||
{
|
||||
name: "invalid-check-period",
|
||||
@@ -1341,10 +1341,15 @@ func TestSSHPolicyRules(t *testing.T) {
|
||||
]
|
||||
}`,
|
||||
expectErr: true,
|
||||
errorMessage: "not a valid duration string",
|
||||
errorMessage: `time: invalid duration "invalid"`,
|
||||
},
|
||||
// `autogroup:invalid` as an SSH user is no longer rejected:
|
||||
// SaaS treats every `autogroup:*` user-string as a literal
|
||||
// label and compiles it into the SSHUsers map. The compat
|
||||
// suite covers this via ssh-malformed-user-autogroup-* — no
|
||||
// dedicated case is needed here.
|
||||
{
|
||||
name: "unsupported-autogroup",
|
||||
name: "ssh-user-unknown-autogroup-as-literal",
|
||||
targetNode: taggedClient,
|
||||
peers: types.Nodes{&nodeUser2},
|
||||
policy: `{
|
||||
@@ -1363,8 +1368,23 @@ func TestSSHPolicyRules(t *testing.T) {
|
||||
}
|
||||
]
|
||||
}`,
|
||||
expectErr: true,
|
||||
errorMessage: "autogroup not supported for SSH user",
|
||||
wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
|
||||
{
|
||||
Principals: []*tailcfg.SSHPrincipal{
|
||||
{NodeIP: "100.64.0.2"},
|
||||
},
|
||||
SSHUsers: map[string]string{
|
||||
"autogroup:invalid": "autogroup:invalid",
|
||||
"root": "",
|
||||
},
|
||||
Action: &tailcfg.SSHAction{
|
||||
Accept: true,
|
||||
AllowAgentForwarding: true,
|
||||
AllowLocalPortForwarding: true,
|
||||
AllowRemotePortForwarding: true,
|
||||
},
|
||||
},
|
||||
}},
|
||||
},
|
||||
{
|
||||
name: "autogroup-nonroot-should-use-wildcard-with-root-excluded",
|
||||
|
||||
@@ -31,14 +31,7 @@ import (
|
||||
// knownSSHTesterDivergences tracks scenarios where headscale and SaaS
|
||||
// disagree on whether a policy is accepted. Each entry should describe
|
||||
// the engine area a follow-up PR needs to touch.
|
||||
var knownSSHTesterDivergences = map[string]string{
|
||||
// SaaS rejects `users: ["*"]` on an `ssh` rule at policy-parse
|
||||
// time with `user "*" is not valid`; headscale accepts the
|
||||
// wildcard and proceeds to evaluate sshTests against it. Fix in
|
||||
// hscontrol/policy/v2/types.go — reject `*` as an SSH login user
|
||||
// during SSH-rule validation.
|
||||
"sshtest-user-wildcard": "headscale accepts `users: [\"*\"]` on ssh rules; SaaS rejects with `user \"*\" is not valid`",
|
||||
}
|
||||
var knownSSHTesterDivergences = map[string]string{}
|
||||
|
||||
func TestSSHTesterCompat(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -87,75 +87,12 @@ var sshSkipReasons = map[string]string{
|
||||
// equivalent for the user:*@passkey wildcard pattern.
|
||||
"ssh-b5": "user:*@passkey wildcard not supported in headscale",
|
||||
"ssh-d10": "user:*@passkey wildcard not supported in headscale",
|
||||
|
||||
// AUTOGROUP_AS_SSH_USER (4 tests)
|
||||
//
|
||||
// SaaS accepts `users: ["autogroup:X"]` on an `ssh` rule for
|
||||
// every autogroup name (member, self, tagged, internet); the
|
||||
// resolution then produces zero principals at compile time for
|
||||
// the ones that don't map to actual logins. headscale's
|
||||
// validate() restricts users-side autogroups to just
|
||||
// `autogroup:nonroot` via autogroupForSSHUser, so the policy
|
||||
// fails to parse. Either widen autogroupForSSHUser to mirror
|
||||
// SaaS (and let compilation drop unsupported entries) or
|
||||
// document that headscale is intentionally stricter.
|
||||
"ssh-malformed-user-autogroup-internet": "headscale rejects `users: [autogroup:internet]`; SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-autogroup-member": "headscale rejects `users: [autogroup:member]`; SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-autogroup-self": "headscale rejects `users: [autogroup:self]`; SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-autogroup-tagged": "headscale rejects `users: [autogroup:tagged]`; SaaS accepts and compiles to zero principals",
|
||||
|
||||
// LOCALPART_SHAPE (4 tests)
|
||||
//
|
||||
// SaaS accepts every shape under the `localpart:` prefix
|
||||
// (`localpart:`, `localpart:foo`, `localpart:*@`, and
|
||||
// `localpart:foo@example.com`) and compiles each to zero
|
||||
// principals. headscale's SSHUser.ParseLocalpart enforces the
|
||||
// strict `localpart:*@<domain>` shape and returns errors for
|
||||
// every other form — see hscontrol/policy/v2/types.go
|
||||
// ParseLocalpart and the call site in validate(). The strict
|
||||
// path is arguably correct (these strings can never produce a
|
||||
// useful principal) but it surfaces as a parse error rather
|
||||
// than the SaaS behaviour of silently producing zero
|
||||
// principals.
|
||||
"ssh-malformed-user-localpart-empty": "headscale rejects `localpart:` (missing @); SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-localpart-no-at": "headscale rejects `localpart:foo` (missing @); SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-localpart-no-domain": "headscale rejects `localpart:*@` (empty domain); SaaS accepts and compiles to zero principals",
|
||||
"ssh-malformed-user-localpart-no-glob": "headscale rejects `localpart:foo@example.com` (local part not *); SaaS accepts and compiles to zero principals",
|
||||
|
||||
// CHECK_PERIOD_MIN (2 tests)
|
||||
//
|
||||
// SaaS allows `checkPeriod: "0s"` and any sub-minute value;
|
||||
// headscale's SSHCheckPeriodMin = 1 minute rejects both. The
|
||||
// minimum may be a deliberate hardening, but it diverges from
|
||||
// SaaS — needs an explicit decision.
|
||||
"ssh-malformed-checkperiod-zero": "headscale rejects checkPeriod `0s`; SaaS accepts",
|
||||
"ssh-malformed-checkperiod-too-short": "headscale rejects checkPeriod `30s`; SaaS accepts (no minimum)",
|
||||
}
|
||||
|
||||
// sshRejectSkipReasons documents APIResponseCode != 200 captures where
|
||||
// headscale and SaaS legitimately disagree on whether the policy should
|
||||
// be rejected (or where headscale rejects with different wording).
|
||||
var sshRejectSkipReasons = map[string]string{
|
||||
// WORDING_DIFFERS (4 tests)
|
||||
//
|
||||
// headscale rejects these inputs but with different error
|
||||
// text, so the SaaS message is not a substring of headscale's.
|
||||
"ssh-malformed-action-deny": `headscale rejects "deny" with 'invalid SSH action: "deny", must be one of: accept, check' vs SaaS '"deny" is not a valid action'`,
|
||||
"ssh-malformed-action-empty": `headscale rejects empty action with 'invalid SSH action: "", must be one of: accept, check' vs SaaS 'action must be specified'`,
|
||||
"ssh-malformed-checkperiod-malformed": `headscale rejects malformed duration with 'not a valid duration string: "abc"' vs SaaS 'time: invalid duration "abc"'`,
|
||||
"ssh-malformed-checkperiod-too-long": "headscale rejects 200h with 'checkPeriod above maximum of 168 hours (1 week)' vs SaaS 'checkPeriod 200h0m0s is above the max (168h)'",
|
||||
|
||||
// MISSING_VALIDATIONS (5 tests)
|
||||
//
|
||||
// SaaS rejects these inputs; headscale accepts them today.
|
||||
// Fixes belong in hscontrol/policy/v2/types.go validate().
|
||||
"ssh-malformed-action-missing": "headscale accepts missing `action`; SaaS rejects with `action must be specified`",
|
||||
"ssh-malformed-acceptenv-empty": "headscale accepts `acceptEnv: [\"\"]`; SaaS rejects with `acceptEnv values cannot be empty`",
|
||||
"ssh-malformed-users-empty-array": "headscale accepts empty `users: []`; SaaS rejects with `users must be specified`",
|
||||
"ssh-malformed-users-missing": "headscale accepts missing `users`; SaaS rejects with `users must be specified`",
|
||||
"ssh-malformed-user-empty": `headscale accepts empty username ""; SaaS rejects with 'user "" is not valid'`,
|
||||
"ssh-malformed-user-wildcard": `headscale accepts wildcard "*"; SaaS rejects with 'user "*" is not valid' (same gap as sshtester_compat_test.go's sshtest-user-wildcard)`,
|
||||
|
||||
// DOMAIN_NOT_ASSOCIATED (5 tests)
|
||||
//
|
||||
// SaaS validates that email domains in user:*@domain and
|
||||
|
||||
@@ -14,7 +14,6 @@ import (
|
||||
"github.com/go-json-experiment/json"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/prometheus/common/model"
|
||||
"github.com/tailscale/hujson"
|
||||
"go4.org/netipx"
|
||||
"tailscale.com/net/tsaddr"
|
||||
@@ -46,17 +45,21 @@ var (
|
||||
ErrSSHAutogroupSelfRequiresUserSource = errors.New("autogroup:self destination requires source to contain only users or groups, not tags or autogroup:tagged")
|
||||
ErrSSHTagSourceToAutogroupMember = errors.New("tags in SSH source cannot access autogroup:member (user-owned devices)")
|
||||
ErrSSHWildcardDestination = errors.New("wildcard (*) is not supported as SSH destination")
|
||||
ErrSSHCheckPeriodBelowMin = errors.New("checkPeriod below minimum of 1 minute")
|
||||
ErrSSHCheckPeriodAboveMax = errors.New("checkPeriod above maximum of 168 hours (1 week)")
|
||||
ErrSSHCheckPeriodAboveMax = errors.New("is above the max (168h)")
|
||||
ErrSSHCheckPeriodOnNonCheck = errors.New("checkPeriod is only valid with action \"check\"")
|
||||
ErrInvalidLocalpart = errors.New("invalid localpart format, must be localpart:*@<domain>")
|
||||
ErrSSHUsersMustBeSpecified = errors.New("users must be specified")
|
||||
ErrSSHUserInvalid = errors.New("is not valid")
|
||||
ErrSSHAcceptEnvEmpty = errors.New("acceptEnv values cannot be empty")
|
||||
ErrSSHActionMustBeSpecified = errors.New("action must be specified")
|
||||
ErrSSHActionInvalid = errors.New("is not a valid action")
|
||||
)
|
||||
|
||||
// SSH check period constants per Tailscale docs:
|
||||
// https://tailscale.com/docs/features/tailscale-ssh#checkperiod
|
||||
// SaaS imposes no minimum (0s is accepted) so headscale matches.
|
||||
const (
|
||||
SSHCheckPeriodDefault = 12 * time.Hour
|
||||
SSHCheckPeriodMin = time.Minute
|
||||
SSHCheckPeriodMax = 168 * time.Hour
|
||||
)
|
||||
|
||||
@@ -117,7 +120,6 @@ var (
|
||||
ErrAutogroupDangerAllDst = errors.New("cannot use autogroup:danger-all as a dst")
|
||||
ErrAutogroupNotSupportedSSHSrc = errors.New("autogroup not supported for SSH sources")
|
||||
ErrAutogroupNotSupportedSSHDst = errors.New("autogroup not supported for SSH destinations")
|
||||
ErrAutogroupNotSupportedSSHUsr = errors.New("autogroup not supported for SSH user")
|
||||
ErrHostNotDefined = errors.New("host not defined in policy")
|
||||
ErrSSHSourceAliasNotSupported = errors.New("alias not supported for SSH source")
|
||||
ErrSSHDestAliasNotSupported = errors.New("alias not supported for SSH destination")
|
||||
@@ -1631,15 +1633,21 @@ func (a *SSHAction) String() string {
|
||||
}
|
||||
|
||||
// UnmarshalJSON implements JSON unmarshaling for SSHAction.
|
||||
//
|
||||
// Empty strings are accepted at parse time; the per-rule validate()
|
||||
// pass surfaces them with `action must be specified` to match SaaS.
|
||||
// Non-empty unknown values fail here with `"foo" is not a valid action`.
|
||||
func (a *SSHAction) UnmarshalJSON(b []byte) error {
|
||||
str := strings.Trim(string(b), `"`)
|
||||
switch str {
|
||||
case "":
|
||||
*a = SSHAction("")
|
||||
case "accept":
|
||||
*a = SSHActionAccept
|
||||
case "check":
|
||||
*a = SSHActionCheck
|
||||
default:
|
||||
return fmt.Errorf("%w: %q, must be one of: accept, check", ErrInvalidSSHAction, str)
|
||||
return fmt.Errorf("%q %w", str, ErrSSHActionInvalid)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -2022,7 +2030,6 @@ var (
|
||||
autogroupForDst = []AutoGroup{AutoGroupInternet, AutoGroupMember, AutoGroupTagged, AutoGroupSelf}
|
||||
autogroupForSSHSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged}
|
||||
autogroupForSSHDst = []AutoGroup{AutoGroupMember, AutoGroupTagged, AutoGroupSelf}
|
||||
autogroupForSSHUser = []AutoGroup{AutoGroupNonRoot}
|
||||
autogroupNotSupported = []AutoGroup{}
|
||||
|
||||
errUnknownProtocolWildcard = errors.New("proto name \"*\" not known; use protocol number 0-255 or protocol name (icmp, tcp, udp, etc.)")
|
||||
@@ -2108,18 +2115,6 @@ func validateAutogroupForSSHDst(dst *AutoGroup) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateAutogroupForSSHUser(user *AutoGroup) error {
|
||||
if user == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
if !slices.Contains(autogroupForSSHUser, *user) {
|
||||
return fmt.Errorf("%w: %q, can be %v", ErrAutogroupNotSupportedSSHUsr, *user, autogroupForSSHUser)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateSSHSrcDstCombination validates that SSH source/destination combinations
|
||||
// follow Tailscale's security model:
|
||||
// - Destination can be: tags, autogroup:self (if source is users/groups), or same-user
|
||||
@@ -2395,23 +2390,37 @@ func (p *Policy) validate() error {
|
||||
}
|
||||
|
||||
for _, ssh := range p.SSHs {
|
||||
// SaaS rejects empty/missing `action` with `action must be
|
||||
// specified`; an empty SSHAction survives parse intentionally
|
||||
// so this validate pass surfaces the SaaS-aligned wording.
|
||||
if ssh.Action == "" {
|
||||
errs = append(errs, ErrSSHActionMustBeSpecified)
|
||||
}
|
||||
|
||||
// SaaS rejects empty/missing `users` with `users must be
|
||||
// specified`; non-canonical user strings (autogroup:*, group:,
|
||||
// tag:, malformed localpart:) are accepted and flow through to
|
||||
// compileSSHPolicy as literals — matching SaaS compile output.
|
||||
if len(ssh.Users) == 0 {
|
||||
errs = append(errs, ErrSSHUsersMustBeSpecified)
|
||||
}
|
||||
|
||||
for _, user := range ssh.Users {
|
||||
if strings.HasPrefix(string(user), "autogroup:") {
|
||||
maybeAuto := AutoGroup(user)
|
||||
|
||||
err := validateAutogroupForSSHUser(&maybeAuto)
|
||||
if err != nil {
|
||||
errs = append(errs, err)
|
||||
continue
|
||||
}
|
||||
// SaaS rejects `""` and `"*"` as user logins; everything
|
||||
// else (including autogroup:*, group:, tag:, malformed
|
||||
// localpart:) is accepted and treated as a literal.
|
||||
switch user {
|
||||
case "", "*":
|
||||
errs = append(errs, fmt.Errorf("user %q %w", user, ErrSSHUserInvalid))
|
||||
}
|
||||
}
|
||||
|
||||
if user.IsLocalpart() {
|
||||
_, err := user.ParseLocalpart()
|
||||
if err != nil {
|
||||
errs = append(errs, err)
|
||||
continue
|
||||
}
|
||||
// SaaS rejects empty entries in `acceptEnv` with
|
||||
// `acceptEnv values cannot be empty`. The wildcard `*` and
|
||||
// double-glob `**` are accepted (only empty string is invalid).
|
||||
for _, env := range ssh.AcceptEnv {
|
||||
if env == "" {
|
||||
errs = append(errs, ErrSSHAcceptEnvEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2735,12 +2744,16 @@ func (p *SSHCheckPeriod) UnmarshalJSON(b []byte) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
d, err := model.ParseDuration(str)
|
||||
// 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 fmt.Errorf("parsing checkPeriod %q: %w", str, err)
|
||||
return err
|
||||
}
|
||||
|
||||
p.Duration = time.Duration(d)
|
||||
p.Duration = d
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -2755,25 +2768,15 @@ func (p SSHCheckPeriod) MarshalJSON() ([]byte, error) {
|
||||
}
|
||||
|
||||
// Validate checks that the SSHCheckPeriod is within allowed bounds.
|
||||
// SaaS imposes no minimum; the only ceiling is 168h.
|
||||
func (p *SSHCheckPeriod) Validate() error {
|
||||
if p.Always {
|
||||
return nil
|
||||
}
|
||||
|
||||
if p.Duration < SSHCheckPeriodMin {
|
||||
return fmt.Errorf(
|
||||
"%w: got %s",
|
||||
ErrSSHCheckPeriodBelowMin,
|
||||
p.Duration,
|
||||
)
|
||||
}
|
||||
|
||||
if p.Duration > SSHCheckPeriodMax {
|
||||
return fmt.Errorf(
|
||||
"%w: got %s",
|
||||
ErrSSHCheckPeriodAboveMax,
|
||||
p.Duration,
|
||||
)
|
||||
// SaaS body: `checkPeriod 200h0m0s is above the max (168h)`.
|
||||
return fmt.Errorf("checkPeriod %s %w", p.Duration, ErrSSHCheckPeriodAboveMax)
|
||||
}
|
||||
|
||||
return nil
|
||||
@@ -2951,25 +2954,33 @@ func (u SSHUsers) ContainsNonRoot() bool {
|
||||
return slices.Contains(u, SSHUser(AutoGroupNonRoot))
|
||||
}
|
||||
|
||||
// ContainsLocalpart returns true if any entry has the localpart: prefix.
|
||||
// ContainsLocalpart returns true if any entry is a canonical
|
||||
// `localpart:*@<domain>` form. Non-canonical strings that merely start
|
||||
// with `localpart:` (e.g. `localpart:`, `localpart:foo`) are treated as
|
||||
// literal user names per SaaS behaviour.
|
||||
func (u SSHUsers) ContainsLocalpart() bool {
|
||||
return slices.ContainsFunc(u, func(user SSHUser) bool {
|
||||
return user.IsLocalpart()
|
||||
return user.IsCanonicalLocalpart()
|
||||
})
|
||||
}
|
||||
|
||||
// NormalUsers returns all SSH users that are not root, autogroup:nonroot,
|
||||
// or localpart: entries.
|
||||
// NormalUsers returns SSH users handled by the literal user map: every
|
||||
// entry except root, autogroup:nonroot, and canonical
|
||||
// `localpart:*@<domain>`. Malformed `localpart:` strings flow through
|
||||
// here so they end up in the compiled SSHUsers map literally — matching
|
||||
// SaaS, which also keeps them verbatim.
|
||||
func (u SSHUsers) NormalUsers() []SSHUser {
|
||||
return slicesx.Filter(nil, u, func(user SSHUser) bool {
|
||||
return user != "root" && user != SSHUser(AutoGroupNonRoot) && !user.IsLocalpart()
|
||||
return user != "root" && user != SSHUser(AutoGroupNonRoot) && !user.IsCanonicalLocalpart()
|
||||
})
|
||||
}
|
||||
|
||||
// LocalpartEntries returns only the localpart: prefixed entries.
|
||||
// LocalpartEntries returns only canonical `localpart:*@<domain>` entries.
|
||||
// Non-canonical localpart strings are excluded so they do not trigger
|
||||
// the resolution path; they are emitted literally by NormalUsers.
|
||||
func (u SSHUsers) LocalpartEntries() []SSHUser {
|
||||
return slicesx.Filter(nil, u, func(user SSHUser) bool {
|
||||
return user.IsLocalpart()
|
||||
return user.IsCanonicalLocalpart()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -2979,11 +2990,25 @@ func (u SSHUser) String() string {
|
||||
return string(u)
|
||||
}
|
||||
|
||||
// IsLocalpart returns true if the SSHUser has the localpart: prefix.
|
||||
// IsLocalpart returns true if the SSHUser has the literal `localpart:`
|
||||
// prefix. It is a syntactic check only — non-canonical shapes still
|
||||
// pass.
|
||||
func (u SSHUser) IsLocalpart() bool {
|
||||
return strings.HasPrefix(string(u), SSHUserLocalpartPrefix)
|
||||
}
|
||||
|
||||
// IsCanonicalLocalpart reports whether the SSHUser parses as the
|
||||
// canonical `localpart:*@<domain>` form that resolution acts on.
|
||||
func (u SSHUser) IsCanonicalLocalpart() bool {
|
||||
if !u.IsLocalpart() {
|
||||
return false
|
||||
}
|
||||
|
||||
_, err := u.ParseLocalpart()
|
||||
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// ParseLocalpart validates and extracts the domain from a localpart: entry.
|
||||
// The expected format is localpart:*@<domain>.
|
||||
// Returns the domain part or an error if the format is invalid.
|
||||
|
||||
@@ -660,7 +660,7 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ssh-with-tag-and-user",
|
||||
name: "ssh-with-tag-and-wildcard-user",
|
||||
input: `
|
||||
{
|
||||
"tagOwners": {
|
||||
@@ -681,26 +681,7 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
]
|
||||
}
|
||||
`,
|
||||
want: &Policy{
|
||||
TagOwners: TagOwners{
|
||||
Tag("tag:web"): Owners{new(Username("admin@example.com"))},
|
||||
Tag("tag:server"): Owners{new(Username("admin@example.com"))},
|
||||
},
|
||||
SSHs: []SSH{
|
||||
{
|
||||
Action: "accept",
|
||||
Sources: SSHSrcAliases{
|
||||
tp("tag:web"),
|
||||
},
|
||||
Destinations: SSHDstAliases{
|
||||
tp("tag:server"),
|
||||
},
|
||||
Users: []SSHUser{
|
||||
SSHUser("*"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
wantErr: `user "*" is not valid`,
|
||||
},
|
||||
{
|
||||
name: "ssh-with-check-period",
|
||||
@@ -2006,8 +1987,11 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
`,
|
||||
wantErr: "square brackets are only valid around IPv6 addresses",
|
||||
},
|
||||
// Non-canonical `localpart:` strings flow through as literal
|
||||
// user names per SaaS behaviour — captured in
|
||||
// ssh-malformed-user-localpart-{no-at,no-glob,no-domain}.
|
||||
{
|
||||
name: "ssh-localpart-invalid-no-at-sign",
|
||||
name: "ssh-localpart-non-canonical-no-at-sign",
|
||||
input: `
|
||||
{
|
||||
"tagOwners": {"tag:prod": ["admin@"]},
|
||||
@@ -2019,10 +2003,22 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
}]
|
||||
}
|
||||
`,
|
||||
wantErr: "invalid localpart format",
|
||||
want: &Policy{
|
||||
TagOwners: TagOwners{
|
||||
Tag("tag:prod"): Owners{new(Username("admin@"))},
|
||||
},
|
||||
SSHs: []SSH{
|
||||
{
|
||||
Action: "accept",
|
||||
Sources: SSHSrcAliases{agp("autogroup:member")},
|
||||
Destinations: SSHDstAliases{tp("tag:prod")},
|
||||
Users: []SSHUser{SSHUser("localpart:foo")},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ssh-localpart-invalid-non-wildcard",
|
||||
name: "ssh-localpart-non-canonical-non-wildcard",
|
||||
input: `
|
||||
{
|
||||
"tagOwners": {"tag:prod": ["admin@"]},
|
||||
@@ -2034,10 +2030,22 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
}]
|
||||
}
|
||||
`,
|
||||
wantErr: "invalid localpart format",
|
||||
want: &Policy{
|
||||
TagOwners: TagOwners{
|
||||
Tag("tag:prod"): Owners{new(Username("admin@"))},
|
||||
},
|
||||
SSHs: []SSH{
|
||||
{
|
||||
Action: "accept",
|
||||
Sources: SSHSrcAliases{agp("autogroup:member")},
|
||||
Destinations: SSHDstAliases{tp("tag:prod")},
|
||||
Users: []SSHUser{SSHUser("localpart:alice@example.com")},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "ssh-localpart-invalid-empty-domain",
|
||||
name: "ssh-localpart-non-canonical-empty-domain",
|
||||
input: `
|
||||
{
|
||||
"tagOwners": {"tag:prod": ["admin@"]},
|
||||
@@ -2049,7 +2057,19 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
}]
|
||||
}
|
||||
`,
|
||||
wantErr: "invalid localpart format",
|
||||
want: &Policy{
|
||||
TagOwners: TagOwners{
|
||||
Tag("tag:prod"): Owners{new(Username("admin@"))},
|
||||
},
|
||||
SSHs: []SSH{
|
||||
{
|
||||
Action: "accept",
|
||||
Sources: SSHSrcAliases{agp("autogroup:member")},
|
||||
Destinations: SSHDstAliases{tp("tag:prod")},
|
||||
Users: []SSHUser{SSHUser("localpart:*@")},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
// A test entry with neither accept nor deny asserts nothing
|
||||
// and is silently accepted today. Tailscale rejects the policy.
|
||||
@@ -4364,18 +4384,21 @@ func TestSSHCheckPeriodValidate(t *testing.T) {
|
||||
period: SSHCheckPeriod{Always: true},
|
||||
},
|
||||
{
|
||||
name: "1m minimum valid",
|
||||
name: "zero duration is valid",
|
||||
period: SSHCheckPeriod{Duration: 0},
|
||||
},
|
||||
{
|
||||
name: "30s below previous minimum is valid (matches SaaS)",
|
||||
period: SSHCheckPeriod{Duration: 30 * time.Second},
|
||||
},
|
||||
{
|
||||
name: "1m valid",
|
||||
period: SSHCheckPeriod{Duration: time.Minute},
|
||||
},
|
||||
{
|
||||
name: "168h maximum valid",
|
||||
period: SSHCheckPeriod{Duration: 168 * time.Hour},
|
||||
},
|
||||
{
|
||||
name: "30s below minimum",
|
||||
period: SSHCheckPeriod{Duration: 30 * time.Second},
|
||||
wantErr: ErrSSHCheckPeriodBelowMin,
|
||||
},
|
||||
{
|
||||
name: "169h above maximum",
|
||||
period: SSHCheckPeriod{Duration: 169 * time.Hour},
|
||||
@@ -4444,7 +4467,7 @@ func TestSSHCheckPeriodPolicyValidation(t *testing.T) {
|
||||
wantErr: ErrSSHCheckPeriodOnNonCheck,
|
||||
},
|
||||
{
|
||||
name: "check with 30s is invalid",
|
||||
name: "check with 30s is valid (matches SaaS, no minimum)",
|
||||
ssh: SSH{
|
||||
Action: SSHActionCheck,
|
||||
Sources: SSHSrcAliases{up("user@")},
|
||||
@@ -4452,7 +4475,17 @@ func TestSSHCheckPeriodPolicyValidation(t *testing.T) {
|
||||
Users: SSHUsers{"root"},
|
||||
CheckPeriod: &SSHCheckPeriod{Duration: 30 * time.Second},
|
||||
},
|
||||
wantErr: ErrSSHCheckPeriodBelowMin,
|
||||
},
|
||||
{
|
||||
name: "check with 200h above max is invalid",
|
||||
ssh: SSH{
|
||||
Action: SSHActionCheck,
|
||||
Sources: SSHSrcAliases{up("user@")},
|
||||
Destinations: SSHDstAliases{agp("autogroup:member")},
|
||||
Users: SSHUsers{"root"},
|
||||
CheckPeriod: &SSHCheckPeriod{Duration: 200 * time.Hour},
|
||||
},
|
||||
wantErr: ErrSSHCheckPeriodAboveMax,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -4472,6 +4505,115 @@ func TestSSHCheckPeriodPolicyValidation(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSSHRuleSaaSValidation exercises the SaaS-aligned rejections
|
||||
// added to match the API body strings exactly.
|
||||
func TestSSHRuleSaaSValidation(t *testing.T) {
|
||||
baseSSH := func(modify func(*SSH)) SSH {
|
||||
ssh := SSH{
|
||||
Action: SSHActionAccept,
|
||||
Sources: SSHSrcAliases{up("user@")},
|
||||
Destinations: SSHDstAliases{agp("autogroup:member")},
|
||||
Users: SSHUsers{"root"},
|
||||
}
|
||||
if modify != nil {
|
||||
modify(&ssh)
|
||||
}
|
||||
|
||||
return ssh
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
ssh SSH
|
||||
wantErr error
|
||||
}{
|
||||
{
|
||||
name: "users empty rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.Users = nil }),
|
||||
wantErr: ErrSSHUsersMustBeSpecified,
|
||||
},
|
||||
{
|
||||
name: "users empty array rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.Users = SSHUsers{} }),
|
||||
wantErr: ErrSSHUsersMustBeSpecified,
|
||||
},
|
||||
{
|
||||
name: "user empty string rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.Users = SSHUsers{""} }),
|
||||
wantErr: ErrSSHUserInvalid,
|
||||
},
|
||||
{
|
||||
name: "user wildcard rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.Users = SSHUsers{"*"} }),
|
||||
wantErr: ErrSSHUserInvalid,
|
||||
},
|
||||
{
|
||||
name: "acceptEnv empty entry rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.AcceptEnv = []string{"FOO", ""} }),
|
||||
wantErr: ErrSSHAcceptEnvEmpty,
|
||||
},
|
||||
{
|
||||
name: "action empty rejected",
|
||||
ssh: baseSSH(func(s *SSH) { s.Action = "" }),
|
||||
wantErr: ErrSSHActionMustBeSpecified,
|
||||
},
|
||||
{
|
||||
name: "user autogroup non-nonroot accepted (literal)",
|
||||
ssh: baseSSH(func(s *SSH) {
|
||||
s.Users = SSHUsers{"autogroup:internet"}
|
||||
}),
|
||||
},
|
||||
{
|
||||
name: "user malformed localpart accepted (literal)",
|
||||
ssh: baseSSH(func(s *SSH) {
|
||||
s.Users = SSHUsers{"localpart:foo"}
|
||||
}),
|
||||
},
|
||||
{
|
||||
name: "acceptEnv double-glob accepted",
|
||||
ssh: baseSSH(func(s *SSH) {
|
||||
s.AcceptEnv = []string{"**"}
|
||||
}),
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
pol := &Policy{SSHs: []SSH{tt.ssh}}
|
||||
err := pol.validate()
|
||||
|
||||
if tt.wantErr != nil {
|
||||
require.ErrorIs(t, err, tt.wantErr)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSSHActionInvalidUnmarshal verifies the SaaS-aligned wording for
|
||||
// non-empty unknown actions surfaces at JSON parse time.
|
||||
func TestSSHActionInvalidUnmarshal(t *testing.T) {
|
||||
var a SSHAction
|
||||
|
||||
err := json.Unmarshal([]byte(`"deny"`), &a)
|
||||
require.Error(t, err)
|
||||
require.ErrorIs(t, err, ErrSSHActionInvalid)
|
||||
require.Contains(t, err.Error(), `"deny" is not a valid action`)
|
||||
}
|
||||
|
||||
// TestSSHCheckPeriodInvalidDuration verifies the SaaS body for the
|
||||
// malformed-duration case (`time: invalid duration "abc"`).
|
||||
func TestSSHCheckPeriodInvalidDuration(t *testing.T) {
|
||||
var p SSHCheckPeriod
|
||||
|
||||
err := json.Unmarshal([]byte(`"abc"`), &p)
|
||||
require.Error(t, err)
|
||||
require.Contains(t, err.Error(), `time: invalid duration "abc"`)
|
||||
}
|
||||
|
||||
func TestUnmarshalGrants(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in New Issue
Block a user