homedir: remove idtools and libcontainer's user package dependencies

Tibor Vass · Tibor Vass · commit a8608b5b67c7 · 2019-09-24T00:32:13.000Z
About github.com/opencontainers/runc/libcontainer/user: According to opencontainers/runc@195d8d5 this package has two functions: - Have a static implementation of user lookup, which is now supported in the os/user stdlib package with the osusergo build tag, but wasn't at the time. - Have extra functions that os/user doesn't have, but none of those are used in homedir. Since moby#11287, homedir depended directly on libcontainer's user package for CurrentUser(). This is being replaced with os/user.Current(), because all of our static binaries are compiled with the osusergo tag, and for dynamic libraries it is more correct to use libc's implementation than parsing /etc/passwd. About github.com/docker/docker/pkg/idtools: Only dependency was from GetStatic() which uses idtools.LookupUID(uid). The implementation of idtools.LookupUID just calls to github.com/opencontainers/runc/libcontainer/user.LookupUid or fallbacks to exec-ing to getent (since moby#27599). This patch replaces calls to homedir.GetStatic by homedir.Get(), opting out of supporting nss lookups in static binaries via exec-ing to getent for the homedir package. If homedir package users need to support nss lookups, they are advised to compile dynamically instead. Signed-off-by: Tibor Vass <tibor@docker.com>
diff --git a/daemon/logger/gcplogs/gcplogging_linux.go b/daemon/logger/gcplogs/gcplogging_linux.go
@@ -11,17 +11,15 @@ import (
 // ensureHomeIfIAmStatic ensure $HOME to be set if dockerversion.IAmStatic is "true".
 // See issue #29344: gcplogs segfaults (static binary)
 // If HOME is not set, logging.NewClient() will call os/user.Current() via oauth2/google.
-// However, in static binary, os/user.Current() leads to segfault due to a glibc issue that won't be fixed
-// in a short term. (golang/go#13470, https://sourceware.org/bugzilla/show_bug.cgi?id=19341)
+// If compiling statically, make sure osusergo build tag is also used to prevent a segfault
+// due to a glibc issue that won't be fixed in a short term
+// (see golang/go#13470, https://sourceware.org/bugzilla/show_bug.cgi?id=19341).
 // So we forcibly set HOME so as to avoid call to os/user/Current()
 func ensureHomeIfIAmStatic() error {
-	// Note: dockerversion.IAmStatic and homedir.GetStatic() is only available for linux.
+	// Note: dockerversion.IAmStatic is only available for linux.
 	// So we need to use them in this gcplogging_linux.go rather than in gcplogging.go
 	if dockerversion.IAmStatic == "true" && os.Getenv("HOME") == "" {
-		home, err := homedir.GetStatic()
-		if err != nil {
-			return err
-		}
+		home := homedir.Get()
 		logrus.Warnf("gcplogs requires HOME to be set for static daemon binary. Forcibly setting HOME to %s.", home)
 		os.Setenv("HOME", home)
 	}
diff --git a/pkg/homedir/homedir_linux.go b/pkg/homedir/homedir_linux.go
@@ -5,24 +5,8 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-
-	"github.com/docker/docker/pkg/idtools"
 )
 
-// GetStatic returns the home directory for the current user without calling
-// os/user.Current(). This is useful for static-linked binary on glibc-based
-// system, because a call to os/user.Current() in a static binary leads to
-// segfault due to a glibc issue that won't be fixed in a short term.
-// (#29344, golang/go#13470, https://sourceware.org/bugzilla/show_bug.cgi?id=19341)
-func GetStatic() (string, error) {
-	uid := os.Getuid()
-	usr, err := idtools.LookupUID(uid)
-	if err != nil {
-		return "", err
-	}
-	return usr.Home, nil
-}
-
 // GetRuntimeDir returns XDG_RUNTIME_DIR.
 // XDG_RUNTIME_DIR is typically configured via pam_systemd.
 // GetRuntimeDir returns non-nil error if XDG_RUNTIME_DIR is not set.
diff --git a/pkg/homedir/homedir_others.go b/pkg/homedir/homedir_others.go
@@ -6,12 +6,6 @@ import (
 	"errors"
 )
 
-// GetStatic is not needed for non-linux systems.
-// (Precisely, it is needed only for glibc-based linux systems.)
-func GetStatic() (string, error) {
-	return "", errors.New("homedir.GetStatic() is not supported on this system")
-}
-
 // GetRuntimeDir is unsupported on non-linux system.
 func GetRuntimeDir() (string, error) {
 	return "", errors.New("homedir.GetRuntimeDir() is not supported on this system")
diff --git a/pkg/homedir/homedir_unix.go b/pkg/homedir/homedir_unix.go
@@ -4,8 +4,7 @@ package homedir // import "github.com/docker/docker/pkg/homedir"
 
 import (
 	"os"
-
-	"github.com/opencontainers/runc/libcontainer/user"
+	"os/user"
 )
 
 // Key returns the env var name for the user's home dir based on
@@ -17,11 +16,13 @@ func Key() string {
 // Get returns the home directory of the current user with the help of
 // environment variables depending on the target operating system.
 // Returned path should be used with "path/filepath" to form new paths.
+// If compiling statically, ensure the osusergo build tag is used.
+// If needing to do nss lookups, do not compile statically.
 func Get() string {
 	home := os.Getenv(Key())
 	if home == "" {
-		if u, err := user.CurrentUser(); err == nil {
-			return u.Home
+		if u, err := user.Current(); err == nil {
+			return u.HomeDir
 		}
 	}
 	return home
