Skip to content

Commit bf8b2b0

Browse files
haojhcwahaojhcwa
authored
chore: use os.Chown instead of calling chown command (#524)
Use os.Chown instead of calling chown command when changing ownership of the aoc config files. Also fix the aoc running user to be `aoc` since the environment override is not very useful but create more complicated use cases.
1 parent ff835af commit bf8b2b0

File tree

2 files changed

+168
-47
lines changed

2 files changed

+168
-47
lines changed

‎pkg/userutils/userutil_linux.go‎

Lines changed: 61 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,23 @@
1818
package userutils
1919

2020
import (
21+
"fmt"
2122
"log"
2223
"os"
23-
"os/exec"
24-
gouser "os/user"
25-
"strconv"
24+
"path/filepath"
2625
"syscall"
2726

2827
"github.com/opencontainers/runc/libcontainer/user"
2928
"golang.org/x/sys/unix"
3029
)
3130

32-
var defaultUser = "aoc"
31+
var aocUserName = "aoc"
3332
var defaultInstallPath = "/opt/aws/aws-otel-collector/"
3433

34+
type ChownFunc func(name string, uid, gid int) error
35+
36+
var chown ChownFunc = os.Chown
37+
3538
// switchUser swithes the current process to new user
3639
func switchUser(execUser *user.ExecUser) error {
3740
if err := unix.Setgroups(execUser.Sgids); err != nil {
@@ -76,73 +79,84 @@ func getRunAsExecUser(runasuser string) (*user.ExecUser, error) {
7679
// ChangeUser allow customers to run the collector in selected user
7780
// by default it ran as 'aoc' user but can be set by environment variable
7881
func ChangeUser() (string, error) {
79-
runAsUser := getCustomUser()
80-
log.Printf("I! Detected runAsUser: %v", runAsUser)
8182

82-
_, err := user.LookupUser(runAsUser)
83+
_, err := user.LookupUser(aocUserName)
8384
if err != nil {
84-
log.Printf("E! User %s does not exist: %v", runAsUser, err)
85+
log.Printf("E! User %s does not exist: %v", aocUserName, err)
8586
return "root", err
8687
}
8788

88-
if runAsUser == "root" {
89-
return "root", nil
90-
}
91-
92-
execUser, err := getRunAsExecUser(runAsUser)
89+
execUser, err := getRunAsExecUser(aocUserName)
9390
if err != nil {
9491
log.Printf("E! Failed to getRunAsExecUser: %v", err)
95-
return runAsUser, err
92+
return aocUserName, err
9693
}
9794

98-
changeFileOwner(runAsUser, execUser.Gid)
95+
changeFileOwner(execUser.Uid, execUser.Gid)
9996

10097
if err := switchUser(execUser); err != nil {
101-
log.Printf("E! failed switching to %q: %v", runAsUser, err)
102-
return runAsUser, err
98+
log.Printf("E! failed switching to %q: %v", aocUserName, err)
99+
return aocUserName, err
103100
}
104101

105-
return runAsUser, nil
102+
return aocUserName, nil
106103
}
107104

108105
// changeFileOwner changes the folder to new user as owner
109-
func changeFileOwner(runAsUser string, groupId int) {
110-
group, err := gouser.LookupGroupId(strconv.Itoa(groupId))
111-
owner := runAsUser
112-
if err == nil {
113-
owner = owner + ":" + group.Name
114-
} else {
115-
log.Printf("I! Failed to get the group name: %v, it will just change the user, but not group.", err)
116-
}
117-
log.Printf("I! Change ownership to %v", owner)
118-
119-
chowncmd := exec.Command("chown", "-R", "-L", owner, getInstallPath()+"logs")
120-
stdoutStderr, err := chowncmd.CombinedOutput()
121-
if err != nil {
122-
log.Printf("E! Change ownership of %slogs: %s %v", getInstallPath(), stdoutStderr, err)
106+
func changeFileOwner(uid int, gid int) {
107+
if err := chownRecursive(uid, gid, filepath.Join(getInstallPath(), "logs")); err != nil {
108+
log.Printf("E! Change ownership of %slogs: %v", getInstallPath(), err)
123109
}
124110

125-
chowncmd = exec.Command("chown", "-R", "-L", owner, getInstallPath()+"etc")
126-
stdoutStderr, err = chowncmd.CombinedOutput()
127-
if err != nil {
128-
log.Printf("E! Change ownership of %setc: %s %v", getInstallPath(), stdoutStderr, err)
111+
if err := chownRecursive(uid, gid, filepath.Join(getInstallPath(), "etc")); err != nil {
112+
log.Printf("E! Change ownership of %setc: %v", getInstallPath(), err)
129113
}
130114

131-
chowncmd = exec.Command("chown", "-R", "-L", owner, getInstallPath()+"var")
132-
stdoutStderr, err = chowncmd.CombinedOutput()
133-
if err != nil {
134-
log.Printf("E! Change ownership of %svar: %s %v", getInstallPath(), stdoutStderr, err)
115+
if err := chownRecursive(uid, gid, filepath.Join(getInstallPath(), "var")); err != nil {
116+
log.Printf("E! Change ownership of %svar: %v", getInstallPath(), err)
135117
}
136-
}
137118

138-
// getCustomUser allows to override the default user
139-
func getCustomUser() string {
140-
if user, ok := os.LookupEnv("AOT_RUN_USER"); ok {
141-
defaultUser = user
142-
}
143-
return defaultUser
119+
log.Printf("I! Change ownership to %v:%v", uid, gid)
144120
}
145121

146122
func getInstallPath() string {
147123
return defaultInstallPath
148124
}
125+
126+
func chownRecursive(uid, gid int, dir string) error {
127+
128+
err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
129+
if err != nil {
130+
return err
131+
}
132+
133+
fmode := info.Mode()
134+
if fmode.IsRegular() {
135+
if fmode&os.ModeSetuid != 0 || fmode&os.ModeSetgid != 0 {
136+
return nil
137+
}
138+
139+
if fmode.Perm()&0111 != 0 {
140+
return nil
141+
}
142+
143+
if fmode.Perm()&0002 != 0 {
144+
return nil
145+
}
146+
}
147+
148+
if fmode&os.ModeSymlink != 0 {
149+
return nil
150+
}
151+
152+
if err := chown(path, uid, gid); err != nil {
153+
return err
154+
}
155+
return nil
156+
})
157+
158+
if err != nil {
159+
return fmt.Errorf("error change owner of dir %s to %v:%v due to error: %w", dir, uid, gid, err)
160+
}
161+
return nil
162+
}

‎pkg/userutils/userutil_test.go‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// +build linux
2+
3+
package userutils
4+
5+
import (
6+
"io/ioutil"
7+
"os"
8+
"path/filepath"
9+
"reflect"
10+
"testing"
11+
)
12+
13+
type MockChowner struct {
14+
chowns []chownEvent
15+
}
16+
17+
type chownEvent struct {
18+
path string
19+
uid, gid int
20+
}
21+
22+
func (c *MockChowner) Chown(path string, uid, gid int) error {
23+
c.chowns = append(c.chowns, chownEvent{path, uid, gid})
24+
return nil
25+
}
26+
27+
func TestChangeFileOwner(t *testing.T) {
28+
base, err := ioutil.TempDir("", "testChown")
29+
if err != nil {
30+
t.Fatalf("failed to crate temp test folder: %v", err)
31+
}
32+
defer os.RemoveAll(base) // Post test clean up
33+
34+
// Override the agent dirs
35+
defaultInstallPath = base
36+
agentLogDir := filepath.Join(base, "logs")
37+
agentEtcDir := filepath.Join(base, "etc")
38+
agentVarDir := filepath.Join(base, "var")
39+
40+
linkedTo := filepath.Join(base, "file-to-be-linked-to.txt")
41+
createFile(t, linkedTo, 0644)
42+
43+
// etc
44+
createFile(t, filepath.Join(agentEtcDir, "normal-config.yaml"), 0644)
45+
46+
// var
47+
createFile(t, filepath.Join(agentVarDir, "some-file-in-var"), 0644)
48+
49+
// Log files
50+
createFile(t, filepath.Join(agentLogDir, "normal-log-file.log"), 0644)
51+
52+
// Files that should be excluded
53+
createFile(t, filepath.Join(agentLogDir, "anyone-can-write.log"), 0666)
54+
createFile(t, filepath.Join(agentLogDir, "suid-file.log"), 0644|os.ModeSetuid)
55+
createFile(t, filepath.Join(agentLogDir, "sgid-file.log"), 0644|os.ModeSetgid)
56+
createFile(t, filepath.Join(agentLogDir, "suid-and-sgid-file.log"), 0644|os.ModeSetuid|os.ModeSetgid)
57+
createFile(t, filepath.Join(agentLogDir, "owner-executable.log"), 0744)
58+
createFile(t, filepath.Join(agentLogDir, "group-executable.log"), 0654)
59+
createFile(t, filepath.Join(agentLogDir, "other-executable.log"), 0645)
60+
createFile(t, filepath.Join(agentLogDir, "all-executable.log"), 0755)
61+
createSymlink(t, linkedTo, filepath.Join(agentLogDir, "link-to-other-file"))
62+
63+
// Call changeFileOwner
64+
var mc MockChowner
65+
chown = mc.Chown
66+
const someUid, someGid = 1111, 9999
67+
changeFileOwner(someUid, someGid)
68+
69+
expected := []chownEvent{
70+
{filepath.Join(base, "logs"), someUid, someGid},
71+
{filepath.Join(base, "logs/normal-log-file.log"), someUid, someGid},
72+
{filepath.Join(base, "etc"), someUid, someGid},
73+
{filepath.Join(base, "etc/normal-config.yaml"), someUid, someGid},
74+
{filepath.Join(base, "var"), someUid, someGid},
75+
{filepath.Join(base, "var/some-file-in-var"), someUid, someGid},
76+
}
77+
78+
if !reflect.DeepEqual(mc.chowns, expected) {
79+
t.Errorf("wrong files has been changed ownership, expecting\n%v\n\n but got\n%v", expected, mc.chowns)
80+
}
81+
}
82+
83+
func mkdir(t *testing.T, path string) {
84+
if err := os.MkdirAll(path, 0755); err != nil {
85+
t.Fatalf("failed to create dir %v: %v", path, err)
86+
}
87+
}
88+
89+
func createFile(t *testing.T, path string, mode os.FileMode) {
90+
dir, _ := filepath.Split(path)
91+
mkdir(t, dir)
92+
f, err := os.Create(path)
93+
if err != nil {
94+
t.Fatalf("failed to create file %v: %v", path, err)
95+
}
96+
f.Close()
97+
98+
if err := os.Chmod(path, mode); err != nil {
99+
t.Fatalf("failed to change file mode of %v to mode %o: %v", path, mode, err)
100+
}
101+
}
102+
103+
func createSymlink(t *testing.T, from, to string) {
104+
if err := os.Symlink(from, to); err != nil {
105+
t.Fatalf("failed to create symbolic link from %v to %v: %v", from, to, err)
106+
}
107+
}

0 commit comments

Comments
 (0)