From ee0ef396a2e91413182c2b139b0b1ffb3aee4829 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 12 Sep 2025 09:12:30 +0200 Subject: [PATCH] policy: fix ssh usermap, fixing autogroup:nonroot (#2768) --- hscontrol/policy/policy_test.go | 309 +++++++++------------- hscontrol/policy/v2/filter.go | 24 +- hscontrol/policy/v2/filter_test.go | 409 +++++++++++++++++++++++++++++ hscontrol/policy/v2/types.go | 23 +- hscontrol/policy/v2/types_test.go | 128 +++++++++ 5 files changed, 698 insertions(+), 195 deletions(-) diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go index f19ac3d3..5d8f791d 100644 --- a/hscontrol/policy/policy_test.go +++ b/hscontrol/policy/policy_test.go @@ -1612,13 +1612,7 @@ func TestSSHPolicyRules(t *testing.T) { UserID: 2, User: users[1], } - taggedServer := types.Node{ - Hostname: "tagged-server", - IPv4: ap("100.64.0.3"), - UserID: 3, - User: users[2], - ForcedTags: []string{"tag:server"}, - } + taggedClient := types.Node{ Hostname: "tagged-client", IPv4: ap("100.64.0.4"), @@ -1659,149 +1653,14 @@ func TestSSHPolicyRules(t *testing.T) { {NodeIP: "100.64.0.2"}, }, SSHUsers: map[string]string{ - "autogroup:nonroot": "=", + "*": "=", + "root": "", }, Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, - }, - }, - }}, - }, - { - name: "group-to-tag", - targetNode: taggedServer, - peers: types.Nodes{&nodeUser1, &nodeUser2}, - policy: `{ - "tagOwners": { - "tag:server": ["user3@"], - }, - "groups": { - "group:users": ["user1@", "user2@"] - }, - "ssh": [ - { - "action": "accept", - "src": ["group:users"], - "dst": ["tag:server"], - "users": ["autogroup:nonroot"] - } - ] - }`, - wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ - { - Principals: []*tailcfg.SSHPrincipal{ - {NodeIP: "100.64.0.1"}, - {NodeIP: "100.64.0.2"}, - }, - SSHUsers: map[string]string{ - "autogroup:nonroot": "=", - }, - Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, - }, - }, - }}, - }, - { - name: "tag-to-user", - targetNode: nodeUser1, - peers: types.Nodes{&taggedClient}, - policy: `{ - "tagOwners": { - "tag:client": ["user1@"], - }, - "ssh": [ - { - "action": "accept", - "src": ["tag:client"], - "dst": ["user1@"], - "users": ["autogroup:nonroot"] - } - ] - }`, - wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ - { - Principals: []*tailcfg.SSHPrincipal{ - {NodeIP: "100.64.0.4"}, - }, - SSHUsers: map[string]string{ - "autogroup:nonroot": "=", - }, - Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, - }, - }, - }}, - }, - { - name: "tag-to-tag", - targetNode: taggedServer, - peers: types.Nodes{&taggedClient}, - policy: `{ - "tagOwners": { - "tag:client": ["user2@"], - "tag:server": ["user3@"], - }, - "ssh": [ - { - "action": "accept", - "src": ["tag:client"], - "dst": ["tag:server"], - "users": ["autogroup:nonroot"] - } - ] - }`, - wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ - { - Principals: []*tailcfg.SSHPrincipal{ - {NodeIP: "100.64.0.4"}, - }, - SSHUsers: map[string]string{ - "autogroup:nonroot": "=", - }, - Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, - }, - }, - }}, - }, - { - name: "group-to-wildcard", - targetNode: nodeUser1, - peers: types.Nodes{&nodeUser2, &taggedClient}, - policy: `{ - "groups": { - "group:admins": ["user2@"] - }, - "ssh": [ - { - "action": "accept", - "src": ["group:admins"], - "dst": ["*"], - "users": ["autogroup:nonroot"] - } - ] - }`, - wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ - { - Principals: []*tailcfg.SSHPrincipal{ - {NodeIP: "100.64.0.2"}, - }, - SSHUsers: map[string]string{ - "autogroup:nonroot": "=", - }, - Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, + Accept: true, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, }, }, }}, @@ -1830,13 +1689,15 @@ func TestSSHPolicyRules(t *testing.T) { {NodeIP: "100.64.0.4"}, }, SSHUsers: map[string]string{ - "autogroup:nonroot": "=", + "*": "=", + "root": "", }, Action: &tailcfg.SSHAction{ - Accept: true, - SessionDuration: 24 * time.Hour, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, + Accept: true, + SessionDuration: 24 * time.Hour, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, }, }, }}, @@ -1895,40 +1756,6 @@ func TestSSHPolicyRules(t *testing.T) { expectErr: true, errorMessage: "not a valid duration string", }, - { - name: "multiple-ssh-users-with-autogroup", - targetNode: nodeUser1, - peers: types.Nodes{&taggedClient}, - policy: `{ - "tagOwners": { - "tag:client": ["user1@"], - }, - "ssh": [ - { - "action": "accept", - "src": ["tag:client"], - "dst": ["user1@"], - "users": ["alice", "bob"] - } - ] - }`, - wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ - { - Principals: []*tailcfg.SSHPrincipal{ - {NodeIP: "100.64.0.4"}, - }, - SSHUsers: map[string]string{ - "alice": "=", - "bob": "=", - }, - Action: &tailcfg.SSHAction{ - Accept: true, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, - }, - }, - }}, - }, { name: "unsupported-autogroup", targetNode: nodeUser1, @@ -1946,6 +1773,114 @@ func TestSSHPolicyRules(t *testing.T) { expectErr: true, errorMessage: "autogroup \"autogroup:invalid\" is not supported", }, + { + name: "autogroup-nonroot-should-use-wildcard-with-root-excluded", + targetNode: nodeUser1, + peers: types.Nodes{&nodeUser2}, + policy: `{ + "groups": { + "group:admins": ["user2@"] + }, + "ssh": [ + { + "action": "accept", + "src": ["group:admins"], + "dst": ["user1@"], + "users": ["autogroup:nonroot"] + } + ] + }`, + // autogroup:nonroot should map to wildcard "*" with root excluded + wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ + { + Principals: []*tailcfg.SSHPrincipal{ + {NodeIP: "100.64.0.2"}, + }, + SSHUsers: map[string]string{ + "*": "=", + "root": "", + }, + Action: &tailcfg.SSHAction{ + Accept: true, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, + }, + }, + }}, + }, + { + name: "autogroup-nonroot-plus-root-should-use-wildcard-with-root-mapped", + targetNode: nodeUser1, + peers: types.Nodes{&nodeUser2}, + policy: `{ + "groups": { + "group:admins": ["user2@"] + }, + "ssh": [ + { + "action": "accept", + "src": ["group:admins"], + "dst": ["user1@"], + "users": ["autogroup:nonroot", "root"] + } + ] + }`, + // autogroup:nonroot + root should map to wildcard "*" with root mapped to itself + wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ + { + Principals: []*tailcfg.SSHPrincipal{ + {NodeIP: "100.64.0.2"}, + }, + SSHUsers: map[string]string{ + "*": "=", + "root": "root", + }, + Action: &tailcfg.SSHAction{ + Accept: true, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, + }, + }, + }}, + }, + { + name: "specific-users-should-map-to-themselves-not-equals", + targetNode: nodeUser1, + peers: types.Nodes{&nodeUser2}, + policy: `{ + "groups": { + "group:admins": ["user2@"] + }, + "ssh": [ + { + "action": "accept", + "src": ["group:admins"], + "dst": ["user1@"], + "users": ["ubuntu", "root"] + } + ] + }`, + // specific usernames should map to themselves, not "=" + wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{ + { + Principals: []*tailcfg.SSHPrincipal{ + {NodeIP: "100.64.0.2"}, + }, + SSHUsers: map[string]string{ + "root": "root", + "ubuntu": "ubuntu", + }, + Action: &tailcfg.SSHAction{ + Accept: true, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, + }, + }, + }}, + }, } for _, tt := range tests { diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 338e513b..5793d96c 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -89,11 +89,12 @@ func (pol *Policy) compileFilterRules( func sshAction(accept bool, duration time.Duration) tailcfg.SSHAction { return tailcfg.SSHAction{ - Reject: !accept, - Accept: accept, - SessionDuration: duration, - AllowAgentForwarding: true, - AllowLocalPortForwarding: true, + Reject: !accept, + Accept: accept, + SessionDuration: duration, + AllowAgentForwarding: true, + AllowLocalPortForwarding: true, + AllowRemotePortForwarding: true, } } @@ -153,8 +154,17 @@ func (pol *Policy) compileSSHPolicy( } userMap := make(map[string]string, len(rule.Users)) - for _, user := range rule.Users { - userMap[user.String()] = "=" + if rule.Users.ContainsNonRoot() { + userMap["*"] = "=" + + // by default, we do not allow root unless explicitly stated + userMap["root"] = "" + } + if rule.Users.ContainsRoot() { + userMap["root"] = "root" + } + for _, u := range rule.Users.NormalUsers() { + userMap[u.String()] = u.String() } rules = append(rules, &tailcfg.SSHRule{ Principals: principals, diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index 12c60fbb..8b73a6f5 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -1,10 +1,16 @@ package v2 import ( + "encoding/json" + "net/netip" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/juanfont/headscale/hscontrol/types" + "github.com/prometheus/common/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gorm.io/gorm" "tailscale.com/tailcfg" ) @@ -376,3 +382,406 @@ func TestParsing(t *testing.T) { }) } } + +func TestCompileSSHPolicy_UserMapping(t *testing.T) { + users := types.Users{ + {Name: "user1", Model: gorm.Model{ID: 1}}, + {Name: "user2", Model: gorm.Model{ID: 2}}, + } + + // Create test nodes + nodeUser1 := types.Node{ + Hostname: "user1-device", + IPv4: createAddr("100.64.0.1"), + UserID: 1, + User: users[0], + } + nodeUser2 := types.Node{ + Hostname: "user2-device", + IPv4: createAddr("100.64.0.2"), + UserID: 2, + User: users[1], + } + + nodes := types.Nodes{&nodeUser1, &nodeUser2} + + tests := []struct { + name string + targetNode types.Node + policy *Policy + wantSSHUsers map[string]string + wantEmpty bool + }{ + { + name: "specific user mapping", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{"ssh-it-user"}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "ssh-it-user": "ssh-it-user", + }, + }, + { + name: "multiple specific users", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{"ubuntu", "admin", "deploy"}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "ubuntu": "ubuntu", + "admin": "admin", + "deploy": "deploy", + }, + }, + { + name: "autogroup:nonroot only", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{SSHUser(AutoGroupNonRoot)}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "*": "=", + "root": "", + }, + }, + { + name: "root only", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{"root"}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "root": "root", + }, + }, + { + name: "autogroup:nonroot plus root", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{SSHUser(AutoGroupNonRoot), "root"}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "*": "=", + "root": "root", + }, + }, + { + name: "mixed specific users and autogroups", + targetNode: nodeUser1, + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{SSHUser(AutoGroupNonRoot), "root", "ubuntu", "admin"}, + }, + }, + }, + wantSSHUsers: map[string]string{ + "*": "=", + "root": "root", + "ubuntu": "ubuntu", + "admin": "admin", + }, + }, + { + name: "no matching destination", + targetNode: nodeUser2, // Target node2, but policy only allows user1 + policy: &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, // Only user1, not user2 + Users: []SSHUser{"ssh-it-user"}, + }, + }, + }, + wantEmpty: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Validate the policy + err := tt.policy.validate() + require.NoError(t, err) + + // Compile SSH policy + sshPolicy, err := tt.policy.compileSSHPolicy(users, tt.targetNode.View(), nodes.ViewSlice()) + require.NoError(t, err) + + if tt.wantEmpty { + if sshPolicy == nil { + return // Expected empty result + } + assert.Empty(t, sshPolicy.Rules, "SSH policy should be empty when no rules match") + return + } + + require.NotNil(t, sshPolicy) + require.Len(t, sshPolicy.Rules, 1, "Should have exactly one SSH rule") + + rule := sshPolicy.Rules[0] + assert.Equal(t, tt.wantSSHUsers, rule.SSHUsers, "SSH users mapping should match expected") + + // Verify principals are set correctly (should contain user2's IP since that's the source) + require.Len(t, rule.Principals, 1) + assert.Equal(t, "100.64.0.2", rule.Principals[0].NodeIP) + + // Verify action is set correctly + assert.True(t, rule.Action.Accept) + assert.True(t, rule.Action.AllowAgentForwarding) + assert.True(t, rule.Action.AllowLocalPortForwarding) + assert.True(t, rule.Action.AllowRemotePortForwarding) + }) + } +} + +func TestCompileSSHPolicy_CheckAction(t *testing.T) { + users := types.Users{ + {Name: "user1", Model: gorm.Model{ID: 1}}, + {Name: "user2", Model: gorm.Model{ID: 2}}, + } + + nodeUser1 := types.Node{ + Hostname: "user1-device", + IPv4: createAddr("100.64.0.1"), + UserID: 1, + User: users[0], + } + nodeUser2 := types.Node{ + Hostname: "user2-device", + IPv4: createAddr("100.64.0.2"), + UserID: 2, + User: users[1], + } + + nodes := types.Nodes{&nodeUser1, &nodeUser2} + + policy := &Policy{ + Groups: Groups{ + Group("group:admins"): []Username{Username("user2@")}, + }, + SSHs: []SSH{ + { + Action: "check", + CheckPeriod: model.Duration(24 * time.Hour), + Sources: SSHSrcAliases{gp("group:admins")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{"ssh-it-user"}, + }, + }, + } + + err := policy.validate() + require.NoError(t, err) + + sshPolicy, err := policy.compileSSHPolicy(users, nodeUser1.View(), nodes.ViewSlice()) + require.NoError(t, err) + require.NotNil(t, sshPolicy) + require.Len(t, sshPolicy.Rules, 1) + + rule := sshPolicy.Rules[0] + + // Verify SSH users are correctly mapped + expectedUsers := map[string]string{ + "ssh-it-user": "ssh-it-user", + } + assert.Equal(t, expectedUsers, rule.SSHUsers) + + // Verify check action with session duration + assert.True(t, rule.Action.Accept) + assert.Equal(t, 24*time.Hour, rule.Action.SessionDuration) +} + +// TestSSHIntegrationReproduction reproduces the exact scenario from the integration test +// TestSSHOneUserToAll that was failing with empty sshUsers +func TestSSHIntegrationReproduction(t *testing.T) { + // Create users matching the integration test + users := types.Users{ + {Name: "user1", Model: gorm.Model{ID: 1}}, + {Name: "user2", Model: gorm.Model{ID: 2}}, + } + + // Create simple nodes for testing + node1 := &types.Node{ + Hostname: "user1-node", + IPv4: createAddr("100.64.0.1"), + UserID: 1, + User: users[0], + } + + node2 := &types.Node{ + Hostname: "user2-node", + IPv4: createAddr("100.64.0.2"), + UserID: 2, + User: users[1], + } + + nodes := types.Nodes{node1, node2} + + // Create a simple policy that reproduces the issue + policy := &Policy{ + Groups: Groups{ + Group("group:integration-test"): []Username{Username("user1@")}, + }, + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{gp("group:integration-test")}, + Destinations: SSHDstAliases{up("user2@")}, // Target user2 + Users: []SSHUser{SSHUser("ssh-it-user")}, // This is the key - specific user + }, + }, + } + + // Validate policy + err := policy.validate() + require.NoError(t, err) + + // Test SSH policy compilation for node2 (target) + sshPolicy, err := policy.compileSSHPolicy(users, node2.View(), nodes.ViewSlice()) + require.NoError(t, err) + require.NotNil(t, sshPolicy) + require.Len(t, sshPolicy.Rules, 1) + + rule := sshPolicy.Rules[0] + + // This was the failing assertion in integration test - sshUsers was empty + assert.NotEmpty(t, rule.SSHUsers, "SSH users should not be empty") + assert.Contains(t, rule.SSHUsers, "ssh-it-user", "ssh-it-user should be present in SSH users") + assert.Equal(t, "ssh-it-user", rule.SSHUsers["ssh-it-user"], "ssh-it-user should map to itself") + + // Verify that ssh-it-user is correctly mapped + expectedUsers := map[string]string{ + "ssh-it-user": "ssh-it-user", + } + assert.Equal(t, expectedUsers, rule.SSHUsers, "ssh-it-user should be mapped to itself") +} + +// TestSSHJSONSerialization verifies that the SSH policy can be properly serialized +// to JSON and that the sshUsers field is not empty +func TestSSHJSONSerialization(t *testing.T) { + users := types.Users{ + {Name: "user1", Model: gorm.Model{ID: 1}}, + } + + node := &types.Node{ + Hostname: "test-node", + IPv4: createAddr("100.64.0.1"), + UserID: 1, + User: users[0], + } + + nodes := types.Nodes{node} + + policy := &Policy{ + SSHs: []SSH{ + { + Action: "accept", + Sources: SSHSrcAliases{up("user1@")}, + Destinations: SSHDstAliases{up("user1@")}, + Users: []SSHUser{"ssh-it-user", "ubuntu", "admin"}, + }, + }, + } + + err := policy.validate() + require.NoError(t, err) + + sshPolicy, err := policy.compileSSHPolicy(users, node.View(), nodes.ViewSlice()) + require.NoError(t, err) + require.NotNil(t, sshPolicy) + + // Serialize to JSON to verify structure + jsonData, err := json.MarshalIndent(sshPolicy, "", " ") + require.NoError(t, err) + + // Parse back to verify structure + var parsed tailcfg.SSHPolicy + err = json.Unmarshal(jsonData, &parsed) + require.NoError(t, err) + + // Verify the parsed structure has the expected SSH users + require.Len(t, parsed.Rules, 1) + rule := parsed.Rules[0] + + expectedUsers := map[string]string{ + "ssh-it-user": "ssh-it-user", + "ubuntu": "ubuntu", + "admin": "admin", + } + assert.Equal(t, expectedUsers, rule.SSHUsers, "SSH users should survive JSON round-trip") + + // Verify JSON contains the SSH users (not empty) + assert.Contains(t, string(jsonData), `"ssh-it-user"`) + assert.Contains(t, string(jsonData), `"ubuntu"`) + assert.Contains(t, string(jsonData), `"admin"`) + assert.NotContains(t, string(jsonData), `"sshUsers": {}`, "SSH users should not be empty") + assert.NotContains(t, string(jsonData), `"sshUsers": null`, "SSH users should not be null") +} + +// Helper function to create IP addresses for testing +func createAddr(ip string) *netip.Addr { + addr, _ := netip.ParseAddr(ip) + return &addr +} diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index a2541da6..80797e17 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -20,6 +20,7 @@ import ( "tailscale.com/types/ptr" "tailscale.com/types/views" "tailscale.com/util/multierr" + "tailscale.com/util/slicesx" ) const Wildcard = Asterix(0) @@ -506,6 +507,10 @@ func (ag *AutoGroup) UnmarshalJSON(b []byte) error { return nil } +func (ag AutoGroup) String() string { + return string(ag) +} + // MarshalJSON marshals the AutoGroup to JSON. func (ag AutoGroup) MarshalJSON() ([]byte, error) { return json.Marshal(string(ag)) @@ -1562,7 +1567,7 @@ type SSH struct { Action string `json:"action"` Sources SSHSrcAliases `json:"src"` Destinations SSHDstAliases `json:"dst"` - Users []SSHUser `json:"users"` + Users SSHUsers `json:"users"` CheckPeriod model.Duration `json:"checkPeriod,omitempty"` } @@ -1715,6 +1720,22 @@ func (a SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[t // It can be a list of usernames, tags or autogroups. type SSHDstAliases []Alias +type SSHUsers []SSHUser + +func (u SSHUsers) ContainsRoot() bool { + return slices.Contains(u, "root") +} + +func (u SSHUsers) ContainsNonRoot() bool { + return slices.Contains(u, SSHUser(AutoGroupNonRoot)) +} + +func (u SSHUsers) NormalUsers() []SSHUser { + return slicesx.Filter(nil, u, func(user SSHUser) bool { + return user != "root" && user != SSHUser(AutoGroupNonRoot) + }) +} + type SSHUser string func (u SSHUser) String() string { diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 5bdb7885..2a3ab578 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -1615,6 +1615,134 @@ func TestResolveAutoApprovers(t *testing.T) { } } +func TestSSHUsers_NormalUsers(t *testing.T) { + tests := []struct { + name string + users SSHUsers + expected []SSHUser + }{ + { + name: "empty users", + users: SSHUsers{}, + expected: []SSHUser{}, + }, + { + name: "only root", + users: SSHUsers{"root"}, + expected: []SSHUser{}, + }, + { + name: "only autogroup:nonroot", + users: SSHUsers{SSHUser(AutoGroupNonRoot)}, + expected: []SSHUser{}, + }, + { + name: "only normal user", + users: SSHUsers{"ssh-it-user"}, + expected: []SSHUser{"ssh-it-user"}, + }, + { + name: "multiple normal users", + users: SSHUsers{"ubuntu", "admin", "user1"}, + expected: []SSHUser{"ubuntu", "admin", "user1"}, + }, + { + name: "mixed users with root", + users: SSHUsers{"ubuntu", "root", "admin"}, + expected: []SSHUser{"ubuntu", "admin"}, + }, + { + name: "mixed users with autogroup:nonroot", + users: SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"}, + expected: []SSHUser{"ubuntu", "admin"}, + }, + { + name: "mixed users with both root and autogroup:nonroot", + users: SSHUsers{"ubuntu", "root", SSHUser(AutoGroupNonRoot), "admin"}, + expected: []SSHUser{"ubuntu", "admin"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.users.NormalUsers() + assert.ElementsMatch(t, tt.expected, result, "NormalUsers() should return expected normal users") + }) + } +} + +func TestSSHUsers_ContainsRoot(t *testing.T) { + tests := []struct { + name string + users SSHUsers + expected bool + }{ + { + name: "empty users", + users: SSHUsers{}, + expected: false, + }, + { + name: "contains root", + users: SSHUsers{"root"}, + expected: true, + }, + { + name: "does not contain root", + users: SSHUsers{"ubuntu", "admin"}, + expected: false, + }, + { + name: "contains root among others", + users: SSHUsers{"ubuntu", "root", "admin"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.users.ContainsRoot() + assert.Equal(t, tt.expected, result, "ContainsRoot() should return expected result") + }) + } +} + +func TestSSHUsers_ContainsNonRoot(t *testing.T) { + tests := []struct { + name string + users SSHUsers + expected bool + }{ + { + name: "empty users", + users: SSHUsers{}, + expected: false, + }, + { + name: "contains autogroup:nonroot", + users: SSHUsers{SSHUser(AutoGroupNonRoot)}, + expected: true, + }, + { + name: "does not contain autogroup:nonroot", + users: SSHUsers{"ubuntu", "admin", "root"}, + expected: false, + }, + { + name: "contains autogroup:nonroot among others", + users: SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"}, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.users.ContainsNonRoot() + assert.Equal(t, tt.expected, result, "ContainsNonRoot() should return expected result") + }) + } +} + func mustIPSet(prefixes ...string) *netipx.IPSet { var builder netipx.IPSetBuilder for _, p := range prefixes {