Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v12] Order sudoers file lines by role name #24792

Merged
merged 6 commits into from Apr 25, 2023
Merged

[v12] Order sudoers file lines by role name #24792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2551bb7
Order sudoers lines by role name
lxea Mar 29, 2023
f412b49
Add a warning in the docs to explain order sudoers roles are written as
lxea Apr 3, 2023
444a3fe
Resolve comments
lxea Apr 3, 2023
22c0b2b
Resolve comments
lxea Apr 3, 2023
67ba52e
fix lints
lxea Apr 4, 2023
c55068a
Merge branch 'branch/v12' into bot/backport-23760-branch/v12
lxea Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/pages/server-access/guides/host-user-creation.mdx
View file
Open in desktop
Expand Up @@ -86,6 +86,13 @@ will be disabled. Roles that do not match the Node will not be checked.

</Admonition>

<Admonition type="warning">

When multiple roles contain `host_sudoers` entries, the sudoers file
will have the entries written to it ordered by role name

</Admonition>

If a role includes a `deny` rule that sets `host_sudoers` to `'*'`, the user will
have all sudoers entries removed when accessing matching Nodes, otherwise `deny`
rules are matched literally when filtering:
Expand Down
41 changes: 31 additions & 10 deletions lib/services/role.go
View file
Open in desktop
Expand Up @@ -2714,9 +2714,17 @@ func (set RoleSet) EnhancedRecordingSet() map[string]bool {
// a role disallows host user creation
func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
groups := make(map[string]struct{})
sudoers := make(map[string]struct{})
var sudoers []string
serverLabels := s.GetAllLabels()
for _, role := range set {

roleSet := make([]types.Role, len(set))
copy(roleSet, set)
slices.SortStableFunc(roleSet, func(a types.Role, b types.Role) bool {
return strings.Compare(a.GetName(), b.GetName()) == -1
})

seenSudoers := make(map[string]struct{})
for _, role := range roleSet {
result, _, err := MatchLabels(role.GetNodeLabels(types.Allow), serverLabels)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2735,10 +2743,16 @@ func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
groups[group] = struct{}{}
}
for _, sudoer := range role.GetHostSudoers(types.Allow) {
sudoers[sudoer] = struct{}{}
if _, ok := seenSudoers[sudoer]; ok {
continue
}
seenSudoers[sudoer] = struct{}{}
sudoers = append(sudoers, sudoer)
}
}
for _, role := range set {

var finalSudoers []string
for _, role := range roleSet {
result, _, err := MatchLabels(role.GetNodeLabels(types.Deny), serverLabels)
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -2749,18 +2763,25 @@ func (set RoleSet) HostUsers(s types.Server) (*HostUsersInfo, error) {
for _, group := range role.GetHostGroups(types.Deny) {
delete(groups, group)
}
for _, sudoer := range role.GetHostSudoers(types.Deny) {
if sudoer == "*" {
sudoers = nil
break

outer:
for _, sudoer := range sudoers {
for _, deniedSudoer := range role.GetHostSudoers(types.Deny) {
if deniedSudoer == "*" {
finalSudoers = nil
break outer
}
if sudoer != deniedSudoer {
finalSudoers = append(finalSudoers, sudoer)
}
}
delete(sudoers, sudoer)
}
sudoers = finalSudoers
}

return &HostUsersInfo{
Groups: utils.StringsSliceFromSet(groups),
Sudoers: utils.StringsSliceFromSet(sudoers),
Sudoers: sudoers,
}, nil
}

Expand Down
108 changes: 108 additions & 0 deletions lib/services/role_test.go
View file
Open in desktop
Expand Up @@ -6068,6 +6068,9 @@ func TestHostUsers_HostSudoers(t *testing.T) {
test: "multiple roles, one not matching",
sudoers: []string{"sudoers entry 1", "sudoers entry 2"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
Name: "a",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
Expand All @@ -6078,6 +6081,9 @@ func TestHostUsers_HostSudoers(t *testing.T) {
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
Name: "b",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
Expand Down Expand Up @@ -6173,6 +6179,108 @@ func TestHostUsers_HostSudoers(t *testing.T) {
},
},
},
{
test: "multiple roles, order preserved by role name",
sudoers: []string{"sudoers entry 1", "sudoers entry 2", "sudoers entry 3", "sudoers entry 4"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
Name: "a",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{"success": []string{"abc"}},
HostSudoers: []string{"sudoers entry 1"},
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
Name: "c",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}},
HostSudoers: []string{"sudoers entry 4", "sudoers entry 1"},
},
},
}, &types.RoleV6{
Metadata: types.Metadata{
Name: "b",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}},
HostSudoers: []string{"sudoers entry 2", "sudoers entry 3"},
},
},
}),
server: &types.ServerV2{
Metadata: types.Metadata{
Labels: map[string]string{
"success": "abc",
},
},
},
},
{
test: "duplication handled",
sudoers: []string{"sudoers entry 2"},
roles: NewRoleSet(&types.RoleV6{
Metadata: types.Metadata{
Name: "a",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{"success": []string{"abc"}},
HostSudoers: []string{"sudoers entry 1"},
},
},
}, &types.RoleV6{ // DENY sudoers entry 1
Metadata: types.Metadata{
Name: "d",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Deny: types.RoleConditions{
NodeLabels: types.Labels{"success": []string{"abc"}},
HostSudoers: []string{"sudoers entry 1"},
},
},
}, &types.RoleV6{ // duplicate sudoers entry 1 case also gets removed
Metadata: types.Metadata{
Name: "c",
},
Spec: types.RoleSpecV6{
Options: types.RoleOptions{
CreateHostUser: types.NewBoolOption(true),
},
Allow: types.RoleConditions{
NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}},
HostSudoers: []string{"sudoers entry 1", "sudoers entry 2"},
},
},
}),
server: &types.ServerV2{
Metadata: types.Metadata{
Labels: map[string]string{
"success": "abc",
},
},
},
},
} {
t.Run(tc.test, func(t *testing.T) {
info, err := tc.roles.HostUsers(tc.server)
Expand Down