policy/v2: add SSH src/dst validation for Tailscale parity

Add validateSSHSrcDstCombination() to enforce Tailscale's SSH policy rules
at validation time:

- dst=tag: any src allowed
- dst=autogroup:self: handled at runtime (different rules per node)
- dst=username: src must contain ONLY the same username

Per Tailscale docs (https://tailscale.com/kb/1337/policy-syntax#dst-1):
"The destination can be a tag, autogroup:self (if the source contains only
users or groups), or a single named user (if the source contains only the
same named user)."

Invalid combinations now rejected with appropriate error messages:
- Tag src -> User dst: "tags in src cannot SSH to user-owned devices"
- Group src -> User dst: "groups in src cannot SSH to user-owned devices"
- Different user src -> User dst: "src X cannot SSH to dst users Y"
- Autogroup src -> User dst: "autogroup X in src cannot SSH to user-owned devices"

Updates #3010
This commit is contained in:
Kristoffer Dalby 2026-01-16 14:39:13 +00:00
parent 515a22e696
commit 60dc17bab6
2 changed files with 352 additions and 23 deletions

View file

@ -37,6 +37,11 @@ var ErrCircularReference = errors.New("circular reference detected")
var ErrUndefinedTagReference = errors.New("references undefined tag")
// ErrSSHUserDstRequiresSameUserSrc is returned when an SSH rule has a username
// destination but the source is not the same username.
// Per Tailscale docs: "users in dst are only allowed from the same user".
var ErrSSHUserDstRequiresSameUserSrc = errors.New("users in dst are only allowed from the same user")
type Asterix int
func (a Asterix) Validate() error {
@ -1613,6 +1618,68 @@ func validateAutogroupForSSHUser(user *AutoGroup) error {
return nil
}
// validateSSHSrcDstCombination validates that SSH rule src/dst combinations
// follow Tailscale's rules:
// - dst=tag: any src allowed
// - dst=autogroup:self: src must be users or groups (no tags) - handled at runtime in filter.go
// - dst=username: src must be ONLY the same username
//
// See: https://tailscale.com/kb/1337/policy-syntax#dst-1
// "The destination can be a tag, autogroup:self (if the source contains only users or groups),
// or a single named user (if the source contains only the same named user)."
//
// This function validates explicit username destinations.
// The autogroup:self case is handled at runtime in filter.go because it results in
// different rules for different nodes (tagged nodes get no rules).
// The * (Asterix) case is also handled at runtime in filter.go - see #3009.
func validateSSHSrcDstCombination(ssh SSH) error {
// Find all usernames in dst
var dstUsernames []Username
for _, dst := range ssh.Destinations {
if u, ok := dst.(*Username); ok {
dstUsernames = append(dstUsernames, *u)
}
}
// If no usernames in dst, no restriction on src (validation-wise)
// Note: * and autogroup:self are handled at runtime in filter.go
if len(dstUsernames) == 0 {
return nil
}
// dst has username(s) - src must contain ONLY the same username(s)
for _, src := range ssh.Sources {
switch s := src.(type) {
case *Username:
// Check if this username is in dstUsernames
if !slices.Contains(dstUsernames, *s) {
return fmt.Errorf(
"%w; src %q cannot SSH to dst users %v",
ErrSSHUserDstRequiresSameUserSrc, *s, dstUsernames,
)
}
case *Tag:
return fmt.Errorf(
"%w; tags in src cannot SSH to user-owned devices",
ErrSSHUserDstRequiresSameUserSrc,
)
case *Group:
return fmt.Errorf(
"%w; groups in src cannot SSH to user-owned devices",
ErrSSHUserDstRequiresSameUserSrc,
)
case *AutoGroup:
return fmt.Errorf(
"%w; autogroup %q in src cannot SSH to user-owned devices",
ErrSSHUserDstRequiresSameUserSrc, *s,
)
}
}
return nil
}
// validate reports if there are any errors in a policy after
// the unmarshaling process.
// It runs through all rules and checks if there are any inconsistencies
@ -1754,6 +1821,13 @@ func (p *Policy) validate() error {
}
}
}
// Validate SSH src/dst combination rules (#3010)
// This rejects policies where tags/groups/different-users are in src when dst is a username
err := validateSSHSrcDstCombination(ssh)
if err != nil {
errs = append(errs, err)
}
}
for _, tagOwners := range p.TagOwners {

View file

@ -660,7 +660,9 @@ func TestUnmarshalPolicy(t *testing.T) {
},
},
{
name: "ssh-with-tag-and-user",
// #3010: Tag src -> User dst should be rejected
// Tailscale rule: "users in dst are only allowed from the same user"
name: "ssh-tag-src-user-dst-rejected",
input: `
{
"tagOwners": {
@ -680,32 +682,16 @@ func TestUnmarshalPolicy(t *testing.T) {
]
}
`,
want: &Policy{
TagOwners: TagOwners{
Tag("tag:web"): Owners{ptr.To(Username("admin@example.com"))},
},
SSHs: []SSH{
{
Action: "accept",
Sources: SSHSrcAliases{
tp("tag:web"),
},
Destinations: SSHDstAliases{
ptr.To(Username("admin@example.com")),
},
Users: []SSHUser{
SSHUser("*"),
},
},
},
},
wantErr: "users in dst are only allowed from the same user",
},
{
name: "ssh-with-check-period",
// #3010: Group src -> User dst should be rejected
// Groups contain multiple users, so they can't SSH to a single user's devices
name: "ssh-group-src-user-dst-rejected",
input: `
{
"groups": {
"group:admins": ["admin@example.com"]
"group:admins": ["admin@example.com", "other@example.com"]
},
"ssh": [
{
@ -716,6 +702,272 @@ func TestUnmarshalPolicy(t *testing.T) {
"dst": [
"admin@example.com"
],
"users": ["*"]
}
]
}
`,
wantErr: "users in dst are only allowed from the same user",
},
{
// #3010: Different user src -> User dst should be rejected
name: "ssh-different-user-src-dst-rejected",
input: `
{
"ssh": [
{
"action": "accept",
"src": [
"bob@example.com"
],
"dst": [
"alice@example.com"
],
"users": ["*"]
}
]
}
`,
wantErr: "users in dst are only allowed from the same user",
},
{
// #3010: Same user src -> Same user dst should be allowed
name: "ssh-same-user-src-dst-allowed",
input: `
{
"ssh": [
{
"action": "accept",
"src": [
"alice@example.com"
],
"dst": [
"alice@example.com"
],
"users": ["*"]
}
]
}
`,
want: &Policy{
SSHs: []SSH{
{
Action: "accept",
Sources: SSHSrcAliases{
ptr.To(Username("alice@example.com")),
},
Destinations: SSHDstAliases{
ptr.To(Username("alice@example.com")),
},
Users: []SSHUser{
SSHUser("*"),
},
},
},
},
},
{
// #3010: Tag src -> Tag dst should be allowed
name: "ssh-tag-src-tag-dst-allowed",
input: `
{
"tagOwners": {
"tag:web": ["admin@example.com"],
"tag:server": ["admin@example.com"]
},
"ssh": [
{
"action": "accept",
"src": [
"tag:web"
],
"dst": [
"tag:server"
],
"users": ["root"]
}
]
}
`,
want: &Policy{
TagOwners: TagOwners{
Tag("tag:web"): Owners{ptr.To(Username("admin@example.com"))},
Tag("tag:server"): Owners{ptr.To(Username("admin@example.com"))},
},
SSHs: []SSH{
{
Action: "accept",
Sources: SSHSrcAliases{
tp("tag:web"),
},
Destinations: SSHDstAliases{
tp("tag:server"),
},
Users: []SSHUser{
SSHUser("root"),
},
},
},
},
},
{
// #3010: User src -> Tag dst should be allowed
name: "ssh-user-src-tag-dst-allowed",
input: `
{
"tagOwners": {
"tag:server": ["admin@example.com"]
},
"ssh": [
{
"action": "accept",
"src": [
"admin@example.com"
],
"dst": [
"tag:server"
],
"users": ["root"]
}
]
}
`,
want: &Policy{
TagOwners: TagOwners{
Tag("tag:server"): Owners{ptr.To(Username("admin@example.com"))},
},
SSHs: []SSH{
{
Action: "accept",
Sources: SSHSrcAliases{
ptr.To(Username("admin@example.com")),
},
Destinations: SSHDstAliases{
tp("tag:server"),
},
Users: []SSHUser{
SSHUser("root"),
},
},
},
},
},
{
// #3010: Group src -> Tag dst should be allowed
name: "ssh-group-src-tag-dst-allowed",
input: `
{
"groups": {
"group:admins": ["admin@example.com"]
},
"tagOwners": {
"tag:server": ["admin@example.com"]
},
"ssh": [
{
"action": "accept",
"src": [
"group:admins"
],
"dst": [
"tag:server"
],
"users": ["root"]
}
]
}
`,
want: &Policy{
Groups: Groups{
Group("group:admins"): []Username{Username("admin@example.com")},
},
TagOwners: TagOwners{
Tag("tag:server"): Owners{ptr.To(Username("admin@example.com"))},
},
SSHs: []SSH{
{
Action: "accept",
Sources: SSHSrcAliases{
gp("group:admins"),
},
Destinations: SSHDstAliases{
tp("tag:server"),
},
Users: []SSHUser{
SSHUser("root"),
},
},
},
},
},
{
// #3010: Mixed src (tag + user) -> User dst should be rejected
// Even if the user matches, having a tag in src invalidates it
name: "ssh-mixed-src-user-dst-rejected",
input: `
{
"tagOwners": {
"tag:web": ["admin@example.com"]
},
"ssh": [
{
"action": "accept",
"src": [
"tag:web",
"admin@example.com"
],
"dst": [
"admin@example.com"
],
"users": ["*"]
}
]
}
`,
wantErr: "users in dst are only allowed from the same user",
},
{
// #3010: Autogroup in src -> User dst should be rejected
// autogroup:member contains multiple users
name: "ssh-autogroup-src-user-dst-rejected",
input: `
{
"ssh": [
{
"action": "accept",
"src": [
"autogroup:member"
],
"dst": [
"admin@example.com"
],
"users": ["*"]
}
]
}
`,
wantErr: "users in dst are only allowed from the same user",
},
{
// Test checkPeriod with valid src/dst combination (group -> tag)
name: "ssh-with-check-period",
input: `
{
"groups": {
"group:admins": ["admin@example.com"]
},
"tagOwners": {
"tag:server": ["admin@example.com"]
},
"ssh": [
{
"action": "accept",
"src": [
"group:admins"
],
"dst": [
"tag:server"
],
"users": ["root"],
"checkPeriod": "24h"
}
@ -726,6 +978,9 @@ func TestUnmarshalPolicy(t *testing.T) {
Groups: Groups{
Group("group:admins"): []Username{Username("admin@example.com")},
},
TagOwners: TagOwners{
Tag("tag:server"): Owners{ptr.To(Username("admin@example.com"))},
},
SSHs: []SSH{
{
Action: "accept",
@ -733,7 +988,7 @@ func TestUnmarshalPolicy(t *testing.T) {
gp("group:admins"),
},
Destinations: SSHDstAliases{
ptr.To(Username("admin@example.com")),
tp("tag:server"),
},
Users: []SSHUser{
SSHUser("root"),