diff --git a/.changelog/14742.txt b/.changelog/14742.txt new file mode 100644 index 000000000000..4213bceaaccf --- /dev/null +++ b/.changelog/14742.txt @@ -0,0 +1,3 @@ +```release-note:improvement +client: re-enable nss-based user lookups +``` diff --git a/GNUmakefile b/GNUmakefile index 9707577d54cb..8f626e9b5dce 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -21,7 +21,7 @@ ifndef BIN BIN := $(GOPATH)/bin endif -GO_TAGS := osusergo $(GO_TAGS) +GO_TAGS := $(GO_TAGS) ifeq ($(CI),true) GO_TAGS := codegen_generated $(GO_TAGS) diff --git a/client/allocdir/fs_unix.go b/client/allocdir/fs_unix.go index 6010a2bf46d4..72ec9a1ea2ee 100644 --- a/client/allocdir/fs_unix.go +++ b/client/allocdir/fs_unix.go @@ -10,6 +10,7 @@ import ( "strconv" "syscall" + "github.com/hashicorp/nomad/helper/users" "golang.org/x/sys/unix" ) @@ -18,7 +19,7 @@ var ( // directory shared across tasks in a task group. SharedAllocContainerPath = filepath.Join("/", SharedAllocName) - // TaskLocalContainer is the path inside a container for mounted directory + // TaskLocalContainerPath is the path inside a container for mounted directory // for local storage. TaskLocalContainerPath = filepath.Join("/", TaskLocal) @@ -39,17 +40,14 @@ func dropDirPermissions(path string, desired os.FileMode) error { return nil } - u, err := user.Lookup("nobody") - if err != nil { - return err - } + nobody := users.Nobody() - uid, err := getUid(u) + uid, err := getUid(&nobody) if err != nil { return err } - gid, err := getGid(u) + gid, err := getGid(&nobody) if err != nil { return err } diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index d2c96fd93f52..cac6907f0d20 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "os/user" "path/filepath" "reflect" "regexp" @@ -28,6 +27,7 @@ import ( clienttestutil "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/helper/pointer" "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/helper/users" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -2391,10 +2391,10 @@ func TestTaskTemplateManager_writeToFile(t *testing.T) { ci.Parallel(t) - cu, err := user.Current() + cu, err := users.Current() require.NoError(t, err) - cg, err := user.LookupGroupId(cu.Gid) + cg, err := users.LookupGroupId(cu.Gid) require.NoError(t, err) file := "my.tmpl" diff --git a/command/agent/config.go b/command/agent/config.go index e69b827d87e5..df7dc91425ee 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -8,7 +8,6 @@ import ( "net" "os" "os/exec" - "os/user" "path/filepath" "runtime" "sort" @@ -23,6 +22,7 @@ import ( "github.com/hashicorp/nomad/client/fingerprint" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/pointer" + "github.com/hashicorp/nomad/helper/users" "github.com/hashicorp/nomad/nomad" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" @@ -1082,7 +1082,7 @@ func newDevModeConfig(devMode, connectMode bool) (*devModeConfig, error) { // come up and fail unexpectedly to run jobs return nil, fmt.Errorf("-dev-connect is only supported on linux.") } - u, err := user.Current() + u, err := users.Current() if err != nil { return nil, fmt.Errorf( "-dev-connect uses network namespaces and is only supported for root: %v", err) diff --git a/drivers/shared/executor/executor_universal_linux.go b/drivers/shared/executor/executor_universal_linux.go index f84411645003..2e6bf8791253 100644 --- a/drivers/shared/executor/executor_universal_linux.go +++ b/drivers/shared/executor/executor_universal_linux.go @@ -3,7 +3,6 @@ package executor import ( "fmt" "os/exec" - "os/user" "path/filepath" "strconv" "strings" @@ -13,6 +12,7 @@ import ( "github.com/hashicorp/nomad/client/lib/cgutil" "github.com/hashicorp/nomad/client/lib/resources" "github.com/hashicorp/nomad/client/taskenv" + "github.com/hashicorp/nomad/helper/users" "github.com/hashicorp/nomad/plugins/drivers" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/specconv" @@ -21,7 +21,7 @@ import ( // setCmdUser takes a user id as a string and looks up the user, and sets the command // to execute as that user. func setCmdUser(cmd *exec.Cmd, userid string) error { - u, err := user.Lookup(userid) + u, err := users.Lookup(userid) if err != nil { return fmt.Errorf("failed to identify user %v: %v", userid, err) } diff --git a/helper/users/lookup.go b/helper/users/lookup.go new file mode 100644 index 000000000000..bac6a6155054 --- /dev/null +++ b/helper/users/lookup.go @@ -0,0 +1,53 @@ +package users + +import ( + "fmt" + "os/user" + "sync" +) + +// lock is used to serialize all user lookup at the process level, because +// some NSS implementations are not concurrency safe +var lock *sync.Mutex + +// nobody is a cached copy of the nobody user, which is going to be looked-up +// frequently and is unlikely to be modified on the underlying system. +var nobody user.User + +// Nobody returns User data for the "nobody" user on the system, bypassing the +// locking / file read / NSS lookup. +func Nobody() user.User { + // original is immutable via copy by value + return nobody +} + +func init() { + lock = new(sync.Mutex) + u, err := Lookup("nobody") + if err != nil { + panic(fmt.Sprintf("unable to lookup the nobody user: %v", err)) + } + nobody = *u +} + +// Lookup username while holding a global process lock. +func Lookup(username string) (*user.User, error) { + lock.Lock() + defer lock.Unlock() + return user.Lookup(username) +} + +// LookupGroupId while holding a global process lock. +func LookupGroupId(gid string) (*user.Group, error) { + lock.Lock() + defer lock.Unlock() + return user.LookupGroupId(gid) +} + +// Current returns the current user, acquired while holding a global process +// lock. +func Current() (*user.User, error) { + lock.Lock() + defer lock.Unlock() + return user.Current() +} diff --git a/helper/users/lookup_test.go b/helper/users/lookup_test.go new file mode 100644 index 000000000000..c506ff5caa9c --- /dev/null +++ b/helper/users/lookup_test.go @@ -0,0 +1,35 @@ +//go:build linux + +package users + +import ( + "errors" + "os/user" + "testing" + + "github.com/shoenig/test/must" +) + +func TestLookup(t *testing.T) { + cases := []struct { + username string + + expErr error + expUser *user.User + }{ + {username: "nobody", expUser: &user.User{Username: "nobody", Uid: "65534", Gid: "65534", Name: "nobody", HomeDir: "/nonexistent"}}, // ubuntu + {username: "root", expUser: &user.User{Username: "root", Uid: "0", Gid: "0", Name: "root", HomeDir: "/root"}}, + {username: "doesnotexist", expErr: errors.New("user: unknown user doesnotexist")}, + } + + for _, tc := range cases { + t.Run(tc.username, func(t *testing.T) { + u, err := Lookup(tc.username) + if tc.expErr != nil { + must.EqError(t, tc.expErr, err.Error()) + } else { + must.Eq(t, tc.expUser, u) + } + }) + } +}