From 5b1970468e485477d8d7a93ce79ed95d35d22bcc Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Thu, 16 Mar 2023 12:22:25 -0500 Subject: [PATCH] artifact: git needs more files for private repositories (#16508) * landlock: git needs more files for private repositories This PR fixes artifact downloading so that git may work when cloning from private repositories. It needs - file read on /etc/passwd - dir read on /root/.ssh - file write on /root/.ssh/known_hosts Add these rules to the landlock rules for the artifact sandbox. * cr: use nonexistent instead of devnull Co-authored-by: Michael Schurter * cr: use go-homdir for looking up home directory * pr: pull go-homedir into explicit require * cr: fixup homedir tests in homeless root cases * cl: fix root test for real --------- Co-authored-by: Michael Schurter --- .changelog/16495.txt | 3 + .../taskrunner/getter/util_linux.go | 69 +++++++++++++++++-- .../taskrunner/getter/util_linux_test.go | 48 ++++++++++--- .../taskrunner/getter/util_test.go | 54 +++++++++++++++ go.mod | 2 +- 5 files changed, 162 insertions(+), 14 deletions(-) create mode 100644 .changelog/16495.txt diff --git a/.changelog/16495.txt b/.changelog/16495.txt new file mode 100644 index 000000000..79aac2ce2 --- /dev/null +++ b/.changelog/16495.txt @@ -0,0 +1,3 @@ +```release-note:bug +artifact: Fixed a bug where artifact downloading failed when using git-ssh +``` diff --git a/client/allocrunner/taskrunner/getter/util_linux.go b/client/allocrunner/taskrunner/getter/util_linux.go index 092b8b59d..e2c52146c 100644 --- a/client/allocrunner/taskrunner/getter/util_linux.go +++ b/client/allocrunner/taskrunner/getter/util_linux.go @@ -7,7 +7,9 @@ import ( "path/filepath" "syscall" + "github.com/mitchellh/go-homedir" "github.com/shoenig/go-landlock" + "golang.org/x/sys/unix" ) var ( @@ -42,12 +44,35 @@ func credentials() (uint32, uint32) { return userUID, userGID } +// findHomeDir returns the home directory as provided by os.UserHomeDir. In case +// os.UserHomeDir returns an error, we return /root if the current process is being +// run by root, or /dev/null otherwise. +func findHomeDir() string { + // When running as a systemd unit the process may not have the $HOME + // environment variable set, and os.UserHomeDir will return an error. + + // if home is set, just use that + if home, err := homedir.Dir(); err == nil { + return home + } + + // if we are the root user, use "/root" + if unix.Getuid() == 0 { + return "/root" + } + + // nothing safe to do + return "/nonexistent" +} + // defaultEnvironment is the default minimal environment variables for Linux. func defaultEnvironment(taskDir string) map[string]string { tmpDir := filepath.Join(taskDir, "tmp") + homeDir := findHomeDir() return map[string]string{ "PATH": "/usr/local/bin:/usr/bin:/bin", "TMPDIR": tmpDir, + "HOME": homeDir, } } @@ -71,26 +96,60 @@ func lockdown(allocDir, taskDir string) error { landlock.Dir(allocDir, "rwc"), landlock.Dir(taskDir, "rwc"), } - paths = append(paths, systemVersionControlGlobalConfigs()...) + + paths = append(paths, additionalFilesForVCS()...) locker := landlock.New(paths...) return locker.Lock(landlock.Mandatory) } -func systemVersionControlGlobalConfigs() []*landlock.Path { +func additionalFilesForVCS() []*landlock.Path { const ( + sshDir = ".ssh" // git ssh + knownHosts = ".ssh/known_hosts" // git ssh + etcPasswd = "/etc/passwd" // git ssh gitGlobalFile = "/etc/gitconfig" // https://git-scm.com/docs/git-config#SCOPES hgGlobalFile = "/etc/mercurial/hgrc" // https://www.mercurial-scm.org/doc/hgrc.5.html#files hgGlobalDir = "/etc/mercurial/hgrc.d" // https://www.mercurial-scm.org/doc/hgrc.5.html#files ) - return loadVersionControlGlobalConfigs(gitGlobalFile, hgGlobalFile, hgGlobalDir) + return filesForVCS( + sshDir, + knownHosts, + etcPasswd, + gitGlobalFile, + hgGlobalFile, + hgGlobalDir, + ) } -func loadVersionControlGlobalConfigs(gitGlobalFile, hgGlobalFile, hgGlobalDir string) []*landlock.Path { +func filesForVCS( + sshDir, + knownHosts, + etcPasswd, + gitGlobalFile, + hgGlobalFile, + hgGlobalDir string) []*landlock.Path { + + // omit ssh if there is no home directory + home := findHomeDir() + sshDir = filepath.Join(home, sshDir) + knownHosts = filepath.Join(home, knownHosts) + + // only add if a path exists exists := func(p string) bool { _, err := os.Stat(p) return err == nil } - result := make([]*landlock.Path, 0, 3) + + result := make([]*landlock.Path, 0, 6) + if exists(sshDir) { + result = append(result, landlock.Dir(sshDir, "r")) + } + if exists(knownHosts) { + result = append(result, landlock.File(knownHosts, "rw")) + } + if exists(etcPasswd) { + result = append(result, landlock.File(etcPasswd, "r")) + } if exists(gitGlobalFile) { result = append(result, landlock.File(gitGlobalFile, "r")) } diff --git a/client/allocrunner/taskrunner/getter/util_linux_test.go b/client/allocrunner/taskrunner/getter/util_linux_test.go index 28cb857d8..863ea01eb 100644 --- a/client/allocrunner/taskrunner/getter/util_linux_test.go +++ b/client/allocrunner/taskrunner/getter/util_linux_test.go @@ -7,22 +7,42 @@ import ( "path/filepath" "testing" + "github.com/mitchellh/go-homedir" "github.com/shoenig/go-landlock" "github.com/shoenig/test/must" ) func TestUtil_loadVersionControlGlobalConfigs(t *testing.T) { + // not parallel + const filePerm = 0o644 const dirPerm = 0o755 - fakeEtc := t.TempDir() - var ( - gitFile = filepath.Join(fakeEtc, "gitconfig") - hgFile = filepath.Join(fakeEtc, "hgrc") - hgDir = filepath.Join(fakeEtc, "hgrc.d") + fakeEtc := t.TempDir() + fakeHome := t.TempDir() + + homedir.DisableCache = true + t.Cleanup(func() { + homedir.DisableCache = false + }) + + t.Setenv("HOME", fakeHome) + + const ( + ssh = ".ssh" + knownHosts = ".ssh/known_hosts" ) - err := os.WriteFile(gitFile, []byte("git"), filePerm) + var ( + gitConfig = filepath.Join(fakeEtc, "gitconfig") + hgFile = filepath.Join(fakeEtc, "hgrc") + hgDir = filepath.Join(fakeEtc, "hgrc.d") + etcPasswd = filepath.Join(fakeEtc, "passwd") + sshDir = filepath.Join(fakeHome, ssh) + knownHostsFile = filepath.Join(fakeHome, knownHosts) + ) + + err := os.WriteFile(gitConfig, []byte("git"), filePerm) must.NoError(t, err) err = os.WriteFile(hgFile, []byte("hg"), filePerm) @@ -31,9 +51,21 @@ func TestUtil_loadVersionControlGlobalConfigs(t *testing.T) { err = os.Mkdir(hgDir, dirPerm) must.NoError(t, err) - paths := loadVersionControlGlobalConfigs(gitFile, hgFile, hgDir) + err = os.WriteFile(etcPasswd, []byte("x:y:z"), filePerm) + must.NoError(t, err) + + err = os.Mkdir(sshDir, dirPerm) + must.NoError(t, err) + + err = os.WriteFile(knownHostsFile, []byte("abc123"), filePerm) + must.NoError(t, err) + + paths := filesForVCS(ssh, knownHosts, etcPasswd, gitConfig, hgFile, hgDir) must.SliceEqual(t, []*landlock.Path{ - landlock.File(gitFile, "r"), + landlock.Dir(sshDir, "r"), + landlock.File(knownHostsFile, "rw"), + landlock.File(etcPasswd, "r"), + landlock.File(gitConfig, "r"), landlock.File(hgFile, "r"), landlock.Dir(hgDir, "r"), }, paths) diff --git a/client/allocrunner/taskrunner/getter/util_test.go b/client/allocrunner/taskrunner/getter/util_test.go index 8f4fa62cb..73c382d03 100644 --- a/client/allocrunner/taskrunner/getter/util_test.go +++ b/client/allocrunner/taskrunner/getter/util_test.go @@ -2,12 +2,14 @@ package getter import ( "errors" + "fmt" "testing" "github.com/hashicorp/go-getter" "github.com/hashicorp/nomad/ci" "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/nomad/structs" + "github.com/mitchellh/go-homedir" "github.com/shoenig/test/must" ) @@ -142,21 +144,31 @@ func TestUtil_getTaskDir(t *testing.T) { func TestUtil_environment(t *testing.T) { // not parallel + testutil.RequireLinux(t) + homedir.DisableCache = true + t.Cleanup(func() { + homedir.DisableCache = false + }) + t.Run("default", func(t *testing.T) { + t.Setenv("HOME", "/test") result := environment("/a/b/c", "") must.Eq(t, []string{ + "HOME=/test", "PATH=/usr/local/bin:/usr/bin:/bin", "TMPDIR=/a/b/c/tmp", }, result) }) t.Run("append", func(t *testing.T) { + t.Setenv("HOME", "/test") t.Setenv("ONE", "1") t.Setenv("TWO", "2") result := environment("/a/b/c", "ONE,TWO") must.Eq(t, []string{ + "HOME=/test", "ONE=1", "PATH=/usr/local/bin:/usr/bin:/bin", "TMPDIR=/a/b/c/tmp", @@ -165,19 +177,61 @@ func TestUtil_environment(t *testing.T) { }) t.Run("override", func(t *testing.T) { + t.Setenv("HOME", "/test") t.Setenv("PATH", "/opt/bin") t.Setenv("TMPDIR", "/scratch") result := environment("/a/b/c", "PATH,TMPDIR") must.Eq(t, []string{ + "HOME=/test", "PATH=/opt/bin", "TMPDIR=/scratch", }, result) }) t.Run("missing", func(t *testing.T) { + t.Setenv("HOME", "/test") result := environment("/a/b/c", "DOES_NOT_EXIST") must.Eq(t, []string{ "DOES_NOT_EXIST=", + "HOME=/test", + "PATH=/usr/local/bin:/usr/bin:/bin", + "TMPDIR=/a/b/c/tmp", + }, result) + }) + + t.Run("homeless non-root", func(t *testing.T) { + testutil.RequireNonRoot(t) + + // assert we fallback via go-homdir ... + userHome, err := homedir.Dir() + must.NoError(t, err) + + // ... when HOME env var is not set, as is the case in some systemd setups + t.Setenv("HOME", "") + + result := environment("/a/b/c", "") + must.Eq(t, []string{ + fmt.Sprintf("HOME=%s", userHome), + "PATH=/usr/local/bin:/usr/bin:/bin", + "TMPDIR=/a/b/c/tmp", + }, result) + }) + + t.Run("homeless root", func(t *testing.T) { + testutil.RequireRoot(t) + + t.Setenv("HOME", "/root") // fake running as full root + + // assert we fallback via go-homdir ... + userHome, err := homedir.Dir() + must.NoError(t, err) + + // ... when HOME env var is not set, as is the case in some systemd setups + t.Setenv("HOME", "") + + result := environment("/a/b/c", "") + must.Eq(t, []string{ + fmt.Sprintf("HOME=%s", userHome), "PATH=/usr/local/bin:/usr/bin:/bin", "TMPDIR=/a/b/c/tmp", }, result) diff --git a/go.mod b/go.mod index cf5cdae32..4fa07c88f 100644 --- a/go.mod +++ b/go.mod @@ -89,6 +89,7 @@ require ( github.com/mitchellh/colorstring v0.0.0-20150917214807-8631ce90f286 github.com/mitchellh/copystructure v1.2.0 github.com/mitchellh/go-glint v0.0.0-20210722152315-6515ceb4a127 + github.com/mitchellh/go-homedir v1.1.0 github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b github.com/mitchellh/go-testing-interface v1.14.1 github.com/mitchellh/hashstructure v1.1.0 @@ -226,7 +227,6 @@ require ( github.com/mattn/go-isatty v0.0.16 // indirect github.com/mattn/go-runewidth v0.0.12 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect - github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect github.com/mitchellh/pointerstructure v1.2.1 github.com/morikuni/aec v1.0.0 // indirect