Skip to content

Commit

Permalink
Support for new Polkit (versions >= 124) (#1147)
Browse files Browse the repository at this point in the history
The new Polkit introduced quite some changes, even syntax ones. Since
then, users had to install an additional compatibility package to make
the privilege escalation policy work.

This PR adds support for the new Polkit versions whilst still supporting
clients that have the older versions and properly handling migration
between versions as well.

UDENG-5318
  • Loading branch information
denisonbarbosa authored Dec 2, 2024
2 parents 6f72f17 + 50df13d commit 2d2a5e1
Show file tree
Hide file tree
Showing 159 changed files with 795 additions and 285 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-group:sudo","unix-group:admin","unix-user:carole [email protected]"];
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-group:sudo","unix-group:admin","unix-user:carole [email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-group:sudo","unix-group:admin","unix-user:carole [email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@
# Do not edit this file manually.
# Any changes will be overwritten.

[Configuration]
AdminIdentities=unix-group:sudo;unix-group:admin

[Configuration]
AdminIdentities=unix-user:[email protected];unix-group:[email protected]

Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@

[Configuration]
AdminIdentities=unix-user:carole [email protected]

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# This file is managed by adsys.
# Do not edit this file manually.
# Any changes will be overwritten.

polkit.addAdminRule(function(action, subject){
return ["unix-user:[email protected]","unix-group:[email protected]"];
});
1 change: 0 additions & 1 deletion debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ Recommends: ${misc:Recommends},
Suggests: curlftpfs,
ubuntu-proxy-manager,
python3-cepces,
polkitd-pkla,
Description: ${source:Synopsis}
${source:Extended-Description}

Expand Down
9 changes: 8 additions & 1 deletion e2e/cmd/run_tests/11_test_pro_managers/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,15 @@ func action(ctx context.Context, cmd *command.Command) (err error) {
"[email protected]" ALL=(ALL:ALL) ALL`); err != nil {
return err
}

// Due to differences in polkit versions between Ubuntu versions, the file path is different
polkitFilePath := "/etc/polkit-1/rules.d/00-adsys-privilege-enforcement.rules"
if cmd.Inventory.Codename == "focal" || cmd.Inventory.Codename == "jammy" {
polkitFilePath = "/etc/polkit-1/localauthority.conf.d/99-adsys-privilege-enforcement.conf"
}

// Only partly assert the polkit file contents as there are differences in polkit configurations between Ubuntu versions
if err := rootClient.RequireContains(ctx, "cat /etc/polkit-1/localauthority.conf.d/99-adsys-privilege-enforcement.conf", "unix-user:[email protected]"); err != nil {
if err := rootClient.RequireContains(ctx, fmt.Sprintf("cat %s", polkitFilePath), "unix-user:[email protected]"); err != nil {
return err
}

Expand Down
8 changes: 8 additions & 0 deletions internal/policies/privilege/export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package privilege

// WithPolicyKitSystemDir sets the directory where the default policykit files are stored.
func WithPolicyKitSystemDir(dir string) func(*option) {
return func(o *option) {
o.policyKitSystemDir = dir
}
}
101 changes: 80 additions & 21 deletions internal/policies/privilege/internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package privilege

import (
"context"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/ubuntu/adsys/internal/testutils"
)

func TestSplitAndNormalizeUsersAndGroups(t *testing.T) {
Expand Down Expand Up @@ -62,43 +64,100 @@ func TestSplitAndNormalizeUsersAndGroups(t *testing.T) {
}
}

func TestGetSystemPolkitAdminIdentities(t *testing.T) {
func TestPolkitAdminIdentitiesFromConf(t *testing.T) {
t.Parallel()

tests := map[string]struct {
policyKitDir string

want string
wantErr bool
emptyReturn bool
}{
"Fetch previous admin identities": {policyKitDir: "testdata/existing-previous-local-admins-one/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities from highest ascii file": {policyKitDir: "testdata/existing-previous-local-admins-multi/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities ignoring adsys": {policyKitDir: "testdata/existing-previous-local-admins-with-adsys-file/polkit-1",
want: "unix-user:local50admin1;unix-user:local50admin2"},
"Fetch previous admin identities": {policyKitDir: "old-polkit/etc/polkit-1"},
"Fetch previous admin identities from highest ascii file": {policyKitDir: "old-polkit-multiple-files/etc/polkit-1"},
"Fetch previous admin identities ignoring adsys": {policyKitDir: "old-polkit/etc/polkit-1"},

// Edge cases
"No previous admin identities but regular directory structure": {policyKitDir: "testdata/existing-other-files/polkit-1",
want: ""},
"Returns an empty string if directory does not exists": {policyKitDir: "testdata/doesnotexists",
want: ""},
"Directory instead of a conf file is ignored": {policyKitDir: "testdata/incorrect-policikit-conf-is-dir/polkit-1",
want: ""},
"No previous admin identities but regular directory structure": {policyKitDir: "existing-other-files/etc/polkit-1", emptyReturn: true},
"Returns an empty string if directory does not exists": {policyKitDir: "doesnotexists", emptyReturn: true},
"Directory instead of a conf file is ignored": {policyKitDir: "incorrect-policikit-conf-is-dir/etc/polkit-1", emptyReturn: true},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()

got, err := getSystemPolkitAdminIdentities(context.Background(), tc.policyKitDir)
if tc.wantErr {
require.NotNil(t, err, "getSystemPolkitAdminIdentities should have failed but didn't")
got, err := polkitAdminIdentitiesFromConf(context.Background(), filepath.Join("testdata", tc.policyKitDir))
require.NoError(t, err, "polkitAdminIdentitiesFromConf failed but shouldn't have")

if tc.emptyReturn {
require.Empty(t, got)
return
}
require.NoError(t, err, "ApplyPolicy failed but shouldn't have")
want := testutils.LoadWithUpdateFromGolden(t, got)
require.Equal(t, want, got, "polkitAdminIdentitiesFromConf did not return expected value")
})
}
}

func TestPolkitAdminIdentitiesFromRules(t *testing.T) {
t.Parallel()

tests := map[string]struct {
policyKitDirs []string

emptyReturn bool
}{
"Fetch previous admin identities": {
policyKitDirs: []string{"existing-previous-local-admins-one/etc/polkit-1"},
},
"Fetch previous admin identities from lower ascii file": {
policyKitDirs: []string{"existing-previous-local-admins-multi/etc/polkit-1"},
},
"Fetch previous admin identities ignoring adsys": {
policyKitDirs: []string{"existing-previous-local-admins-with-adsys-file/etc/polkit-1"},
},

// Rules-specific cases
"Consider only first returned value": {
policyKitDirs: []string{"existing-previous-local-admins-return-early/etc/polkit-1"},
},
"Prioritize first specified directory if files have same ascii": {
policyKitDirs: []string{"multiple-polkit-dirs-same-file/etc/polkit-1", "multiple-polkit-dirs-same-file/etc/polkit-2"},
},
"Prioritize lower ascii file even if on second directory": {
policyKitDirs: []string{"multiple-polkit-dirs-diff-file/etc/polkit-1", "multiple-polkit-dirs-diff-file/etc/polkit-2"},
},

// Edge cases
"No previous admin identities but regular directory structure": {
policyKitDirs: []string{"existing-other-files/etc/polkit-1"},
emptyReturn: true,
},
"Returns an empty string if directory does not exists": {
policyKitDirs: []string{"doesnotexists"},
emptyReturn: true,
},
"Directory instead of a conf file is ignored": {
policyKitDirs: []string{"incorrect-policikit-conf-is-dir/etc/polkit-1"},
emptyReturn: true,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
t.Parallel()

for i, dir := range tc.policyKitDirs {
tc.policyKitDirs[i] = filepath.Join("testdata", dir)
}

got, err := polkitAdminIdentitiesFromRules(context.Background(), tc.policyKitDirs)
require.NoError(t, err, "getSystemPolkitAdminIdentities failed but shouldn't have")

assert.Equal(t, tc.want, got, "getSystemPolkitAdminIdentities returned expected value")
if tc.emptyReturn {
require.Empty(t, got)
return
}
want := testutils.LoadWithUpdateFromGolden(t, got)
require.Equal(t, want, got)
})
}
}
Loading

0 comments on commit 2d2a5e1

Please sign in to comment.