From cfc67c3422c428aa9637a7158f3b36abb38b8758 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 7 Dec 2022 16:02:25 -0600 Subject: [PATCH] client: sandbox go-getter subprocess with landlock (#15328) * client: sandbox go-getter subprocess with landlock This PR re-implements the getter package for artifact downloads as a subprocess. Key changes include On all platforms, run getter as a child process of the Nomad agent. On Linux platforms running as root, run the child process as the nobody user. On supporting Linux kernels, uses landlock for filesystem isolation (via go-landlock). On all platforms, restrict environment variables of the child process to a static set. notably TMP/TEMP now points within the allocation's task directory kernel.landlock attribute is fingerprinted (version number or unavailable) These changes make Nomad client more resilient against a faulty go-getter implementation that may panic, and more secure against bad actors attempting to use artifact downloads as a privilege escalation vector. Adds new e2e/artifact suite for ensuring artifact downloading works. TODO: Windows git test (need to modify the image, etc... followup PR) * landlock: fixup items from cr * cr: fixup tests and go.mod file --- .changelog/15328.txt | 3 + .../allocrunner/taskrunner/artifact_hook.go | 3 +- .../taskrunner/artifact_hook_test.go | 20 +- client/allocrunner/taskrunner/getter/error.go | 37 ++ .../taskrunner/getter/error_test.go | 90 +++ .../allocrunner/taskrunner/getter/getter.go | 221 ------- .../taskrunner/getter/getter_test.go | 565 ------------------ .../allocrunner/taskrunner/getter/params.go | 157 +++++ .../taskrunner/getter/params_test.go | 119 ++++ .../taskrunner/getter/replacer_test.go | 59 ++ .../allocrunner/taskrunner/getter/sandbox.go | 59 ++ .../taskrunner/getter/sandbox_test.go | 50 ++ .../getter/test-fixtures/archive.tar.gz | Bin 246 -> 0 bytes .../test-fixtures/archive/exist/my.config | 1 - .../test-fixtures/archive/new/my.config | 1 - .../getter/test-fixtures/archive/test.sh | 1 - .../getter/test-fixtures/setuid.tgz | Bin 199 -> 0 bytes .../taskrunner/getter/test-fixtures/test.sh | 1 - .../allocrunner/taskrunner/getter/testing.go | 46 +- client/allocrunner/taskrunner/getter/util.go | 127 ++++ .../taskrunner/getter/util_default.go | 43 ++ .../taskrunner/getter/util_linux.go | 83 +++ .../taskrunner/getter/util_test.go | 148 +++++ .../taskrunner/getter/util_windows.go | 37 ++ .../taskrunner/getter/z_getter_cmd.go | 53 ++ .../taskrunner/task_runner_test.go | 5 +- client/allocrunner/testing.go | 2 +- client/client.go | 2 +- client/config/testing.go | 5 + client/fingerprint/fingerprint.go | 1 + client/fingerprint/landlock.go | 42 ++ client/fingerprint/landlock_test.go | 70 +++ client/interfaces/client.go | 5 +- e2e/artifact/artifact_test.go | 108 ++++ e2e/artifact/input/artifact_darwin.nomad | 97 +++ e2e/artifact/input/artifact_linux.nomad | 265 ++++++++ e2e/artifact/input/artifact_windows.nomad | 95 +++ go.mod | 2 + go.sum | 4 + helper/subproc/doc.go | 11 + helper/subproc/self.go | 34 ++ helper/subproc/subproc.go | 69 +++ helper/users/lookup_test.go | 6 + helper/users/lookup_unix.go | 32 +- main.go | 1 + .../docs/job-specification/artifact.mdx | 6 +- .../content/docs/upgrade/upgrade-specific.mdx | 48 ++ 47 files changed, 2012 insertions(+), 822 deletions(-) create mode 100644 .changelog/15328.txt create mode 100644 client/allocrunner/taskrunner/getter/error.go create mode 100644 client/allocrunner/taskrunner/getter/error_test.go delete mode 100644 client/allocrunner/taskrunner/getter/getter.go delete mode 100644 client/allocrunner/taskrunner/getter/getter_test.go create mode 100644 client/allocrunner/taskrunner/getter/params.go create mode 100644 client/allocrunner/taskrunner/getter/params_test.go create mode 100644 client/allocrunner/taskrunner/getter/replacer_test.go create mode 100644 client/allocrunner/taskrunner/getter/sandbox.go create mode 100644 client/allocrunner/taskrunner/getter/sandbox_test.go delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/archive.tar.gz delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/archive/exist/my.config delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/archive/new/my.config delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/archive/test.sh delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/setuid.tgz delete mode 100644 client/allocrunner/taskrunner/getter/test-fixtures/test.sh create mode 100644 client/allocrunner/taskrunner/getter/util.go create mode 100644 client/allocrunner/taskrunner/getter/util_default.go create mode 100644 client/allocrunner/taskrunner/getter/util_linux.go create mode 100644 client/allocrunner/taskrunner/getter/util_test.go create mode 100644 client/allocrunner/taskrunner/getter/util_windows.go create mode 100644 client/allocrunner/taskrunner/getter/z_getter_cmd.go create mode 100644 client/fingerprint/landlock.go create mode 100644 client/fingerprint/landlock_test.go create mode 100644 e2e/artifact/artifact_test.go create mode 100644 e2e/artifact/input/artifact_darwin.nomad create mode 100644 e2e/artifact/input/artifact_linux.nomad create mode 100644 e2e/artifact/input/artifact_windows.nomad create mode 100644 helper/subproc/doc.go create mode 100644 helper/subproc/self.go create mode 100644 helper/subproc/subproc.go diff --git a/.changelog/15328.txt b/.changelog/15328.txt new file mode 100644 index 000000000..98ad8fde5 --- /dev/null +++ b/.changelog/15328.txt @@ -0,0 +1,3 @@ +```release-note:improvement +client: execute artifact downloads in sandbox process +``` diff --git a/client/allocrunner/taskrunner/artifact_hook.go b/client/allocrunner/taskrunner/artifact_hook.go index dae238ecf..de31ad7ff 100644 --- a/client/allocrunner/taskrunner/artifact_hook.go +++ b/client/allocrunner/taskrunner/artifact_hook.go @@ -41,9 +41,8 @@ func (h *artifactHook) doWork(req *interfaces.TaskPrestartRequest, resp *interfa } h.logger.Debug("downloading artifact", "artifact", artifact.GetterSource, "aid", aid) - //XXX add ctx to GetArtifact to allow cancelling long downloads - if err := h.getter.GetArtifact(req.TaskEnv, artifact); err != nil { + if err := h.getter.Get(req.TaskEnv, artifact); err != nil { wrapped := structs.NewRecoverableError( fmt.Errorf("failed to download artifact %q: %v", artifact.GetterSource, err), true, diff --git a/client/allocrunner/taskrunner/artifact_hook_test.go b/client/allocrunner/taskrunner/artifact_hook_test.go index 92e28d104..917e0411a 100644 --- a/client/allocrunner/taskrunner/artifact_hook_test.go +++ b/client/allocrunner/taskrunner/artifact_hook_test.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/allocrunner/taskrunner/getter" "github.com/hashicorp/nomad/client/taskenv" + "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/require" @@ -39,7 +40,8 @@ func TestTaskRunner_ArtifactHook_Recoverable(t *testing.T) { ci.Parallel(t) me := &mockEmitter{} - artifactHook := newArtifactHook(me, getter.TestDefaultGetter(t), testlog.HCLogger(t)) + sbox := getter.TestSandbox(t) + artifactHook := newArtifactHook(me, sbox, testlog.HCLogger(t)) req := &interfaces.TaskPrestartRequest{ TaskEnv: taskenv.NewEmptyTaskEnv(), @@ -69,10 +71,12 @@ func TestTaskRunner_ArtifactHook_Recoverable(t *testing.T) { // already downloaded artifacts when subsequent artifacts fail and cause a // restart. func TestTaskRunner_ArtifactHook_PartialDone(t *testing.T) { + testutil.RequireRoot(t) ci.Parallel(t) me := &mockEmitter{} - artifactHook := newArtifactHook(me, getter.TestDefaultGetter(t), testlog.HCLogger(t)) + sbox := getter.TestSandbox(t) + artifactHook := newArtifactHook(me, sbox, testlog.HCLogger(t)) // Create a source directory with 1 of the 2 artifacts srcdir := t.TempDir() @@ -86,7 +90,7 @@ func TestTaskRunner_ArtifactHook_PartialDone(t *testing.T) { defer ts.Close() // Create the target directory. - destdir := t.TempDir() + _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), @@ -160,7 +164,8 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadSuccess(t *testing.T) { t.Parallel() me := &mockEmitter{} - artifactHook := newArtifactHook(me, getter.TestDefaultGetter(t), testlog.HCLogger(t)) + sbox := getter.TestSandbox(t) + artifactHook := newArtifactHook(me, sbox, testlog.HCLogger(t)) // Create a source directory all 7 artifacts srcdir := t.TempDir() @@ -176,7 +181,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadSuccess(t *testing.T) { defer ts.Close() // Create the target directory. - destdir := t.TempDir() + _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), @@ -247,7 +252,8 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadFailure(t *testing.T) { t.Parallel() me := &mockEmitter{} - artifactHook := newArtifactHook(me, getter.TestDefaultGetter(t), testlog.HCLogger(t)) + sbox := getter.TestSandbox(t) + artifactHook := newArtifactHook(me, sbox, testlog.HCLogger(t)) // Create a source directory with 3 of the 4 artifacts srcdir := t.TempDir() @@ -266,7 +272,7 @@ func TestTaskRunner_ArtifactHook_ConcurrentDownloadFailure(t *testing.T) { defer ts.Close() // Create the target directory. - destdir := t.TempDir() + _, destdir := getter.SetupDir(t) req := &interfaces.TaskPrestartRequest{ TaskEnv: taskenv.NewTaskEnv(nil, nil, nil, nil, destdir, ""), diff --git a/client/allocrunner/taskrunner/getter/error.go b/client/allocrunner/taskrunner/getter/error.go new file mode 100644 index 000000000..5e84ecb0c --- /dev/null +++ b/client/allocrunner/taskrunner/getter/error.go @@ -0,0 +1,37 @@ +package getter + +// Error is a RecoverableError used to include the URL along with the underlying +// fetching error. +type Error struct { + URL string + Err error + Recoverable bool +} + +func (e *Error) Error() string { + if e == nil || e.Err == nil { + return "" + } + return e.Err.Error() +} + +func (e *Error) IsRecoverable() bool { + return e.Recoverable +} + +func (e *Error) Equal(o *Error) bool { + if e == nil || o == nil { + return e == o + } + + switch { + case e.URL != o.URL: + return false + case e.Recoverable != o.Recoverable: + return false + case e.Error() != o.Error(): + return false + default: + return true + } +} diff --git a/client/allocrunner/taskrunner/getter/error_test.go b/client/allocrunner/taskrunner/getter/error_test.go new file mode 100644 index 000000000..94d90940f --- /dev/null +++ b/client/allocrunner/taskrunner/getter/error_test.go @@ -0,0 +1,90 @@ +package getter + +import ( + "errors" + "testing" + + "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" +) + +func TestError_Error(t *testing.T) { + cases := []struct { + name string + err *Error + exp string + }{ + {"object nil", nil, ""}, + {"error nil", new(Error), ""}, + {"has error", &Error{Err: errors.New("oops")}, "oops"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + e := Error{Err: tc.err} + result := e.Error() + must.Eq(t, tc.exp, result) + }) + } +} + +func TestError_IsRecoverable(t *testing.T) { + var _ structs.Recoverable = (*Error)(nil) + must.True(t, (&Error{Recoverable: true}).IsRecoverable()) + must.False(t, (&Error{Recoverable: false}).IsRecoverable()) +} + +func TestError_Equal(t *testing.T) { + cases := []struct { + name string + a *Error + b *Error + exp bool + }{ + {name: "both nil", a: nil, b: nil, exp: true}, + {name: "one nil", a: new(Error), b: nil, exp: false}, + { + name: "different url", + a: &Error{URL: "example.com/a"}, + b: &Error{URL: "example.com/b"}, + exp: false, + }, + { + name: "different err", + a: &Error{URL: "example.com/z", Err: errors.New("b")}, + b: &Error{URL: "example.com/z", Err: errors.New("a")}, + exp: false, + }, + { + name: "nil vs not nil err", + a: &Error{URL: "example.com/z", Err: errors.New("b")}, + b: &Error{URL: "example.com/z", Err: nil}, + exp: false, + }, + { + name: "different recoverable", + a: &Error{URL: "example.com", Err: errors.New("a"), Recoverable: false}, + b: &Error{URL: "example.com", Err: errors.New("b"), Recoverable: true}, + exp: false, + }, + { + name: "same no error", + a: &Error{URL: "example.com", Err: nil, Recoverable: true}, + b: &Error{URL: "example.com", Err: nil, Recoverable: true}, + exp: true, + }, + { + name: "same with error", + a: &Error{URL: "example.com", Err: errors.New("a"), Recoverable: true}, + b: &Error{URL: "example.com", Err: errors.New("a"), Recoverable: true}, + exp: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := tc.a.Equal(tc.b) + must.Eq(t, tc.exp, result) + }) + } +} diff --git a/client/allocrunner/taskrunner/getter/getter.go b/client/allocrunner/taskrunner/getter/getter.go deleted file mode 100644 index d46c730fc..000000000 --- a/client/allocrunner/taskrunner/getter/getter.go +++ /dev/null @@ -1,221 +0,0 @@ -package getter - -import ( - "errors" - "fmt" - "net/http" - "net/url" - "runtime/debug" - "strings" - - "github.com/hashicorp/go-cleanhttp" - gg "github.com/hashicorp/go-getter" - "github.com/hashicorp/go-hclog" - - "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/interfaces" - "github.com/hashicorp/nomad/nomad/structs" -) - -const ( - // gitSSHPrefix is the prefix for downloading via git using ssh - gitSSHPrefix = "git@github.com:" -) - -// Getter wraps go-getter calls in an artifact configuration. -type Getter struct { - logger hclog.Logger - - // httpClient is a shared HTTP client for use across all http/https - // Getter instantiations. The HTTP client is designed to be - // thread-safe, and using a pooled transport will help reduce excessive - // connections when clients are downloading lots of artifacts. - httpClient *http.Client - config *config.ArtifactConfig -} - -// NewGetter returns a new Getter instance. This function is called once per -// client and shared across alloc and task runners. -func NewGetter(logger hclog.Logger, config *config.ArtifactConfig) *Getter { - return &Getter{ - logger: logger, - httpClient: &http.Client{ - Transport: cleanhttp.DefaultPooledTransport(), - }, - config: config, - } -} - -// GetArtifact downloads an artifact into the specified task directory. -func (g *Getter) GetArtifact(taskEnv interfaces.EnvReplacer, artifact *structs.TaskArtifact) (returnErr error) { - // Recover from panics to avoid crashing the entire Nomad client due to - // artifact download failures, such as bugs in go-getter. - defer func() { - if r := recover(); r != nil { - g.logger.Error("panic while downloading artifact", - "artifact", artifact.GetterSource, - "error", r, - "stack", string(debug.Stack())) - returnErr = fmt.Errorf("getter panic: %v", r) - } - }() - - ggURL, err := getGetterUrl(taskEnv, artifact) - if err != nil { - return newGetError(artifact.GetterSource, err, false) - } - - dest, escapes := taskEnv.ClientPath(artifact.RelativeDest, true) - // Verify the destination is still in the task sandbox after interpolation - if escapes { - return newGetError(artifact.RelativeDest, - errors.New("artifact destination path escapes the alloc directory"), - false) - } - - // Convert from string getter mode to go-getter const - mode := gg.ClientModeAny - switch artifact.GetterMode { - case structs.GetterModeFile: - mode = gg.ClientModeFile - case structs.GetterModeDir: - mode = gg.ClientModeDir - } - - headers := getHeaders(taskEnv, artifact.GetterHeaders) - if err := g.getClient(ggURL, headers, mode, dest).Get(); err != nil { - return newGetError(ggURL, err, true) - } - - return nil -} - -// getClient returns a client that is suitable for Nomad downloading artifacts. -func (g *Getter) getClient(src string, headers http.Header, mode gg.ClientMode, dst string) *gg.Client { - return &gg.Client{ - Src: src, - Dst: dst, - Mode: mode, - Umask: 060000000, - Getters: g.createGetters(headers), - - // This will prevent copying or writing files through symlinks - DisableSymlinks: true, - } -} - -func (g *Getter) createGetters(header http.Header) map[string]gg.Getter { - httpGetter := &gg.HttpGetter{ - Netrc: true, - Client: g.httpClient, - Header: header, - - // Do not support the custom X-Terraform-Get header and - // associated logic. - XTerraformGetDisabled: true, - - // Disable HEAD requests as they can produce corrupt files when - // retrying a download of a resource that has changed. - // hashicorp/go-getter#219 - DoNotCheckHeadFirst: true, - - // Read timeout for HTTP operations. Must be long enough to - // accommodate large/slow downloads. - ReadTimeout: g.config.HTTPReadTimeout, - - // Maximum download size. Must be large enough to accommodate - // large downloads. - MaxBytes: g.config.HTTPMaxBytes, - } - - // Explicitly create fresh set of supported Getter for each Client, because - // go-getter is not thread-safe. Use a shared HTTP client for http/https Getter, - // with pooled transport which is thread-safe. - // - // If a getter type is not listed here, it is not supported (e.g. file). - return map[string]gg.Getter{ - "git": &gg.GitGetter{ - Timeout: g.config.GitTimeout, - }, - "hg": &gg.HgGetter{ - Timeout: g.config.HgTimeout, - }, - "gcs": &gg.GCSGetter{ - Timeout: g.config.GCSTimeout, - }, - "s3": &gg.S3Getter{ - Timeout: g.config.S3Timeout, - }, - "http": httpGetter, - "https": httpGetter, - } -} - -// getGetterUrl returns the go-getter URL to download the artifact. -func getGetterUrl(taskEnv interfaces.EnvReplacer, artifact *structs.TaskArtifact) (string, error) { - source := taskEnv.ReplaceEnv(artifact.GetterSource) - - // Handle an invalid URL when given a go-getter url such as - // git@github.com:hashicorp/nomad.git - gitSSH := false - if strings.HasPrefix(source, gitSSHPrefix) { - gitSSH = true - source = source[len(gitSSHPrefix):] - } - - u, err := url.Parse(source) - if err != nil { - return "", fmt.Errorf("failed to parse source URL %q: %v", artifact.GetterSource, err) - } - - // Build the url - q := u.Query() - for k, v := range artifact.GetterOptions { - q.Add(k, taskEnv.ReplaceEnv(v)) - } - u.RawQuery = q.Encode() - - // Add the prefix back - ggURL := u.String() - if gitSSH { - ggURL = fmt.Sprintf("%s%s", gitSSHPrefix, ggURL) - } - - return ggURL, nil -} - -func getHeaders(env interfaces.EnvReplacer, m map[string]string) http.Header { - if len(m) == 0 { - return nil - } - - headers := make(http.Header, len(m)) - for k, v := range m { - headers.Set(k, env.ReplaceEnv(v)) - } - return headers -} - -// GetError wraps the underlying artifact fetching error with the URL. It -// implements the RecoverableError interface. -type GetError struct { - URL string - Err error - recoverable bool -} - -func newGetError(url string, err error, recoverable bool) *GetError { - return &GetError{ - URL: url, - Err: err, - recoverable: recoverable, - } -} - -func (g *GetError) Error() string { - return g.Err.Error() -} - -func (g *GetError) IsRecoverable() bool { - return g.recoverable -} diff --git a/client/allocrunner/taskrunner/getter/getter_test.go b/client/allocrunner/taskrunner/getter/getter_test.go deleted file mode 100644 index 7bae67bee..000000000 --- a/client/allocrunner/taskrunner/getter/getter_test.go +++ /dev/null @@ -1,565 +0,0 @@ -package getter - -import ( - "fmt" - "io" - "io/ioutil" - "mime" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "reflect" - "runtime" - "strings" - "testing" - "time" - - gg "github.com/hashicorp/go-getter" - "github.com/hashicorp/go-hclog" - clientconfig "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/interfaces" - "github.com/hashicorp/nomad/client/taskenv" - "github.com/hashicorp/nomad/helper/escapingfs" - "github.com/hashicorp/nomad/nomad/mock" - "github.com/hashicorp/nomad/nomad/structs" - "github.com/stretchr/testify/require" -) - -// noopReplacer is a noop version of taskenv.TaskEnv.ReplaceEnv. -type noopReplacer struct { - taskDir string -} - -func clientPath(taskDir, path string, join bool) (string, bool) { - if !filepath.IsAbs(path) || (escapingfs.PathEscapesSandbox(taskDir, path) && join) { - path = filepath.Join(taskDir, path) - } - path = filepath.Clean(path) - if taskDir != "" && !escapingfs.PathEscapesSandbox(taskDir, path) { - return path, false - } - return path, true -} - -func (noopReplacer) ReplaceEnv(s string) string { - return s -} - -func (r noopReplacer) ClientPath(p string, join bool) (string, bool) { - path, escapes := clientPath(r.taskDir, r.ReplaceEnv(p), join) - return path, escapes -} - -func noopTaskEnv(taskDir string) interfaces.EnvReplacer { - return noopReplacer{ - taskDir: taskDir, - } -} - -// panicReplacer is a version of taskenv.TaskEnv.ReplaceEnv that panics. -type panicReplacer struct{} - -func (panicReplacer) ReplaceEnv(_ string) string { - panic("panic") -} -func (panicReplacer) ClientPath(_ string, _ bool) (string, bool) { - panic("panic") -} -func panicTaskEnv() interfaces.EnvReplacer { - return panicReplacer{} -} - -// upperReplacer is a version of taskenv.TaskEnv.ReplaceEnv that upper-cases -// the given input. -type upperReplacer struct { - taskDir string -} - -func (upperReplacer) ReplaceEnv(s string) string { - return strings.ToUpper(s) -} - -func (u upperReplacer) ClientPath(p string, join bool) (string, bool) { - path, escapes := clientPath(u.taskDir, u.ReplaceEnv(p), join) - return path, escapes -} - -func TestGetter_getClient(t *testing.T) { - getter := NewGetter(hclog.NewNullLogger(), &clientconfig.ArtifactConfig{ - HTTPReadTimeout: time.Minute, - HTTPMaxBytes: 100_000, - GCSTimeout: 1 * time.Minute, - GitTimeout: 2 * time.Minute, - HgTimeout: 3 * time.Minute, - S3Timeout: 4 * time.Minute, - }) - client := getter.getClient("src", nil, gg.ClientModeAny, "dst") - - t.Run("check symlink config", func(t *testing.T) { - require.True(t, client.DisableSymlinks) - }) - - t.Run("check http config", func(t *testing.T) { - require.True(t, client.Getters["http"].(*gg.HttpGetter).XTerraformGetDisabled) - require.Equal(t, time.Minute, client.Getters["http"].(*gg.HttpGetter).ReadTimeout) - require.Equal(t, int64(100_000), client.Getters["http"].(*gg.HttpGetter).MaxBytes) - }) - - t.Run("check https config", func(t *testing.T) { - require.True(t, client.Getters["https"].(*gg.HttpGetter).XTerraformGetDisabled) - require.Equal(t, time.Minute, client.Getters["https"].(*gg.HttpGetter).ReadTimeout) - require.Equal(t, int64(100_000), client.Getters["https"].(*gg.HttpGetter).MaxBytes) - }) - - t.Run("check gcs config", func(t *testing.T) { - require.Equal(t, client.Getters["gcs"].(*gg.GCSGetter).Timeout, 1*time.Minute) - }) - - t.Run("check git config", func(t *testing.T) { - require.Equal(t, client.Getters["git"].(*gg.GitGetter).Timeout, 2*time.Minute) - }) - - t.Run("check hg config", func(t *testing.T) { - require.Equal(t, client.Getters["hg"].(*gg.HgGetter).Timeout, 3*time.Minute) - }) - - t.Run("check s3 config", func(t *testing.T) { - require.Equal(t, client.Getters["s3"].(*gg.S3Getter).Timeout, 4*time.Minute) - }) - -} - -func TestGetArtifact_getHeaders(t *testing.T) { - t.Run("nil", func(t *testing.T) { - require.Nil(t, getHeaders(noopTaskEnv(""), nil)) - }) - - t.Run("empty", func(t *testing.T) { - require.Nil(t, getHeaders(noopTaskEnv(""), make(map[string]string))) - }) - - t.Run("set", func(t *testing.T) { - upperTaskEnv := new(upperReplacer) - expected := make(http.Header) - expected.Set("foo", "BAR") - result := getHeaders(upperTaskEnv, map[string]string{ - "foo": "bar", - }) - require.Equal(t, expected, result) - }) -} - -func TestGetArtifact_Headers(t *testing.T) { - file := "output.txt" - - // Create the test server with a handler that will validate headers are set. - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Validate the expected value for our header. - value := r.Header.Get("X-Some-Value") - require.Equal(t, "FOOBAR", value) - - // Write the value to the file that is our artifact, for fun. - w.Header().Set("Content-Type", mime.TypeByExtension(filepath.Ext(file))) - w.WriteHeader(http.StatusOK) - _, err := io.Copy(w, strings.NewReader(value)) - require.NoError(t, err) - })) - defer ts.Close() - - // Create a temp directory to download into. - taskDir := t.TempDir() - - // Create the artifact. - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterHeaders: map[string]string{ - "X-Some-Value": "foobar", - }, - RelativeDest: file, - GetterMode: "file", - } - - // Download the artifact. - getter := TestDefaultGetter(t) - taskEnv := upperReplacer{ - taskDir: taskDir, - } - - err := getter.GetArtifact(taskEnv, artifact) - require.NoError(t, err) - - // Verify artifact exists. - b, err := ioutil.ReadFile(filepath.Join(taskDir, taskEnv.ReplaceEnv(file))) - require.NoError(t, err) - - // Verify we wrote the interpolated header value into the file that is our - // artifact. - require.Equal(t, "FOOBAR", string(b)) -} - -func TestGetArtifact_FileAndChecksum(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into - taskDir := t.TempDir() - - // Create the artifact - file := "test.sh" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "md5:bce963762aa2dbfed13caf492a45fb72", - }, - } - - // Download the artifact - getter := TestDefaultGetter(t) - if err := getter.GetArtifact(noopTaskEnv(taskDir), artifact); err != nil { - t.Fatalf("GetArtifact failed: %v", err) - } - - // Verify artifact exists - if _, err := os.Stat(filepath.Join(taskDir, file)); err != nil { - t.Fatalf("file not found: %s", err) - } -} - -func TestGetArtifact_File_RelativeDest(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into - taskDir := t.TempDir() - - // Create the artifact - file := "test.sh" - relative := "foo/" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "md5:bce963762aa2dbfed13caf492a45fb72", - }, - RelativeDest: relative, - } - - // Download the artifact - getter := TestDefaultGetter(t) - if err := getter.GetArtifact(noopTaskEnv(taskDir), artifact); err != nil { - t.Fatalf("GetArtifact failed: %v", err) - } - - // Verify artifact was downloaded to the correct path - if _, err := os.Stat(filepath.Join(taskDir, relative, file)); err != nil { - t.Fatalf("file not found: %s", err) - } -} - -func TestGetArtifact_File_EscapeDest(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into - taskDir := t.TempDir() - - // Create the artifact - file := "test.sh" - relative := "../../../../foo/" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "md5:bce963762aa2dbfed13caf492a45fb72", - }, - RelativeDest: relative, - } - - // attempt to download the artifact - getter := TestDefaultGetter(t) - err := getter.GetArtifact(noopTaskEnv(taskDir), artifact) - if err == nil || !strings.Contains(err.Error(), "escapes") { - t.Fatalf("expected GetArtifact to disallow sandbox escape: %v", err) - } -} - -func TestGetGetterUrl_Interpolation(t *testing.T) { - // Create the artifact - artifact := &structs.TaskArtifact{ - GetterSource: "${NOMAD_META_ARTIFACT}", - } - - url := "foo.com" - alloc := mock.Alloc() - task := alloc.Job.TaskGroups[0].Tasks[0] - task.Meta = map[string]string{"artifact": url} - taskEnv := taskenv.NewBuilder(mock.Node(), alloc, task, "global").Build() - - act, err := getGetterUrl(taskEnv, artifact) - if err != nil { - t.Fatalf("getGetterUrl() failed: %v", err) - } - - if act != url { - t.Fatalf("getGetterUrl() returned %q; want %q", act, url) - } -} - -func TestGetArtifact_InvalidChecksum(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into - taskDir := t.TempDir() - - // Create the artifact with an incorrect checksum - file := "test.sh" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "md5:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - }, - } - - // Download the artifact and expect an error - getter := TestDefaultGetter(t) - if err := getter.GetArtifact(noopTaskEnv(taskDir), artifact); err == nil { - t.Fatalf("GetArtifact should have failed") - } -} - -func createContents(basedir string, fileContents map[string]string, t *testing.T) { - for relPath, content := range fileContents { - folder := basedir - if strings.Index(relPath, "/") != -1 { - // Create the folder. - folder = filepath.Join(basedir, filepath.Dir(relPath)) - if err := os.Mkdir(folder, 0777); err != nil { - t.Fatalf("failed to make directory: %v", err) - } - } - - // Create a file in the existing folder. - file := filepath.Join(folder, filepath.Base(relPath)) - if err := ioutil.WriteFile(file, []byte(content), 0777); err != nil { - t.Fatalf("failed to write data to file %v: %v", file, err) - } - } -} - -func checkContents(basedir string, fileContents map[string]string, t *testing.T) { - for relPath, content := range fileContents { - path := filepath.Join(basedir, relPath) - actual, err := ioutil.ReadFile(path) - if err != nil { - t.Fatalf("failed to read file %q: %v", path, err) - } - - if !reflect.DeepEqual(actual, []byte(content)) { - t.Fatalf("%q: expected %q; got %q", path, content, string(actual)) - } - } -} - -func TestGetArtifact_Archive(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into and create some of the same - // files that exist in the artifact to ensure they are overridden - taskDir := t.TempDir() - - create := map[string]string{ - "exist/my.config": "to be replaced", - "untouched": "existing top-level", - } - createContents(taskDir, create, t) - - file := "archive.tar.gz" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "sha1:20bab73c72c56490856f913cf594bad9a4d730f6", - }, - } - - getter := TestDefaultGetter(t) - if err := getter.GetArtifact(noopTaskEnv(taskDir), artifact); err != nil { - t.Fatalf("GetArtifact failed: %v", err) - } - - // Verify the unarchiving overrode files properly. - expected := map[string]string{ - "untouched": "existing top-level", - "exist/my.config": "hello world\n", - "new/my.config": "hello world\n", - "test.sh": "sleep 1\n", - } - checkContents(taskDir, expected, t) -} - -func TestGetArtifact_Setuid(t *testing.T) { - // Create the test server hosting the file to download - ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/")))) - defer ts.Close() - - // Create a temp directory to download into and create some of the same - // files that exist in the artifact to ensure they are overridden - taskDir := t.TempDir() - - file := "setuid.tgz" - artifact := &structs.TaskArtifact{ - GetterSource: fmt.Sprintf("%s/%s", ts.URL, file), - GetterOptions: map[string]string{ - "checksum": "sha1:e892194748ecbad5d0f60c6c6b2db2bdaa384a90", - }, - } - - getter := TestDefaultGetter(t) - require.NoError(t, getter.GetArtifact(noopTaskEnv(taskDir), artifact)) - - var expected map[string]int - - if runtime.GOOS == "windows" { - // windows doesn't support Chmod changing file permissions. - expected = map[string]int{ - "public": 0666, - "private": 0666, - "setuid": 0666, - } - } else { - // Verify the unarchiving masked files properly. - expected = map[string]int{ - "public": 0666, - "private": 0600, - "setuid": 0755, - } - } - - for file, perm := range expected { - path := filepath.Join(taskDir, "setuid", file) - s, err := os.Stat(path) - require.NoError(t, err) - p := os.FileMode(perm) - o := s.Mode() - require.Equalf(t, p, o, "%s expected %o found %o", file, p, o) - } -} - -// TestGetArtifact_handlePanic tests that a panic during the getter execution -// does not cause its goroutine to crash. -func TestGetArtifact_handlePanic(t *testing.T) { - getter := TestDefaultGetter(t) - err := getter.GetArtifact(panicTaskEnv(), &structs.TaskArtifact{}) - require.Error(t, err) - require.Contains(t, err.Error(), "panic") -} - -func TestGetGetterUrl_Queries(t *testing.T) { - cases := []struct { - name string - artifact *structs.TaskArtifact - output string - }{ - { - name: "adds query parameters", - artifact: &structs.TaskArtifact{ - GetterSource: "https://foo.com?test=1", - GetterOptions: map[string]string{ - "foo": "bar", - "bam": "boom", - }, - }, - output: "https://foo.com?bam=boom&foo=bar&test=1", - }, - { - name: "git without http", - artifact: &structs.TaskArtifact{ - GetterSource: "github.com/hashicorp/nomad", - GetterOptions: map[string]string{ - "ref": "abcd1234", - }, - }, - output: "github.com/hashicorp/nomad?ref=abcd1234", - }, - { - name: "git using ssh", - artifact: &structs.TaskArtifact{ - GetterSource: "git@github.com:hashicorp/nomad?sshkey=1", - GetterOptions: map[string]string{ - "ref": "abcd1234", - }, - }, - output: "git@github.com:hashicorp/nomad?ref=abcd1234&sshkey=1", - }, - { - name: "s3 scheme 1", - artifact: &structs.TaskArtifact{ - GetterSource: "s3::https://s3.amazonaws.com/bucket/foo", - GetterOptions: map[string]string{ - "aws_access_key_id": "abcd1234", - }, - }, - output: "s3::https://s3.amazonaws.com/bucket/foo?aws_access_key_id=abcd1234", - }, - { - name: "s3 scheme 2", - artifact: &structs.TaskArtifact{ - GetterSource: "s3::https://s3-eu-west-1.amazonaws.com/bucket/foo", - GetterOptions: map[string]string{ - "aws_access_key_id": "abcd1234", - }, - }, - output: "s3::https://s3-eu-west-1.amazonaws.com/bucket/foo?aws_access_key_id=abcd1234", - }, - { - name: "s3 scheme 3", - artifact: &structs.TaskArtifact{ - GetterSource: "bucket.s3.amazonaws.com/foo", - GetterOptions: map[string]string{ - "aws_access_key_id": "abcd1234", - }, - }, - output: "bucket.s3.amazonaws.com/foo?aws_access_key_id=abcd1234", - }, - { - name: "s3 scheme 4", - artifact: &structs.TaskArtifact{ - GetterSource: "bucket.s3-eu-west-1.amazonaws.com/foo/bar", - GetterOptions: map[string]string{ - "aws_access_key_id": "abcd1234", - }, - }, - output: "bucket.s3-eu-west-1.amazonaws.com/foo/bar?aws_access_key_id=abcd1234", - }, - { - name: "gcs", - artifact: &structs.TaskArtifact{ - GetterSource: "gcs::https://www.googleapis.com/storage/v1/b/d/f", - }, - output: "gcs::https://www.googleapis.com/storage/v1/b/d/f", - }, - { - name: "local file", - artifact: &structs.TaskArtifact{ - GetterSource: "/foo/bar", - }, - output: "/foo/bar", - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - act, err := getGetterUrl(noopTaskEnv(""), c.artifact) - if err != nil { - t.Fatalf("want %q; got err %v", c.output, err) - } else if act != c.output { - t.Fatalf("want %q; got %q", c.output, act) - } - }) - } -} diff --git a/client/allocrunner/taskrunner/getter/params.go b/client/allocrunner/taskrunner/getter/params.go new file mode 100644 index 000000000..55d40a997 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/params.go @@ -0,0 +1,157 @@ +package getter + +import ( + "context" + "encoding/json" + "io" + "io/fs" + "strings" + "time" + + "github.com/hashicorp/go-cleanhttp" + "github.com/hashicorp/go-getter" + "github.com/hashicorp/nomad/helper" + "golang.org/x/exp/maps" + "golang.org/x/exp/slices" +) + +// parameters is encoded by the Nomad client and decoded by the getter sub-process +// so it can know what to do. We use standard IO to pass configuration to achieve +// better control over input sanitization risks. +// e.g. https://www.opencve.io/cve/CVE-2022-41716 +type parameters struct { + // Config + HTTPReadTimeout time.Duration `json:"http_read_timeout"` + HTTPMaxBytes int64 `json:"http_max_bytes"` + GCSTimeout time.Duration `json:"gcs_timeout"` + GitTimeout time.Duration `json:"git_timeout"` + HgTimeout time.Duration `json:"hg_timeout"` + S3Timeout time.Duration `json:"s3_timeout"` + + // Artifact + Mode getter.ClientMode `json:"artifact_mode"` + Source string `json:"artifact_source"` + Destination string `json:"artifact_destination"` + Headers map[string][]string `json:"artifact_headers"` + + // Task Environment + TaskDir string `json:"task_dir"` +} + +func (p *parameters) reader() io.Reader { + b, err := json.Marshal(p) + if err != nil { + b = nil + } + return strings.NewReader(string(b)) +} + +func (p *parameters) read(r io.Reader) error { + return json.NewDecoder(r).Decode(p) +} + +// deadline returns an absolute deadline before the artifact download +// sub-process forcefully terminates. The default is 1/2 hour, unless one or +// more getter configurations is set higher. A 1 minute grace period is added +// so that an internal timeout has a moment to complete before the process is +// terminated via signal. +func (p *parameters) deadline() time.Duration { + const minimum = 30 * time.Minute + max := minimum + max = helper.Max(max, p.HTTPReadTimeout) + max = helper.Max(max, p.GCSTimeout) + max = helper.Max(max, p.GitTimeout) + max = helper.Max(max, p.HgTimeout) + max = helper.Max(max, p.S3Timeout) + return max + 1*time.Minute +} + +// Equal returns whether p and o are the same. +func (p *parameters) Equal(o *parameters) bool { + if p == nil || o == nil { + return p == o + } + + switch { + case p.HTTPReadTimeout != o.HTTPReadTimeout: + return false + case p.HTTPMaxBytes != o.HTTPMaxBytes: + return false + case p.GCSTimeout != o.GCSTimeout: + return false + case p.GitTimeout != o.GitTimeout: + return false + case p.HgTimeout != o.HgTimeout: + return false + case p.S3Timeout != o.S3Timeout: + return false + case p.Mode != o.Mode: + return false + case p.Source != o.Source: + return false + case p.Destination != o.Destination: + return false + case p.TaskDir != o.TaskDir: + return false + case !maps.EqualFunc(p.Headers, o.Headers, slices.Equal[string]): + return false + } + + return true +} + +const ( + // stop privilege escalation via setuid/setgid + // https://github.com/hashicorp/nomad/issues/6176 + umask = fs.ModeSetuid | fs.ModeSetgid +) + +func (p *parameters) client(ctx context.Context) *getter.Client { + httpGetter := &getter.HttpGetter{ + Netrc: true, + Client: cleanhttp.DefaultClient(), + Header: p.Headers, + + // Do not support the custom X-Terraform-Get header and + // associated logic. + XTerraformGetDisabled: true, + + // Disable HEAD requests as they can produce corrupt files when + // retrying a download of a resource that has changed. + // hashicorp/go-getter#219 + DoNotCheckHeadFirst: true, + + // Read timeout for HTTP operations. Must be long enough to + // accommodate large/slow downloads. + ReadTimeout: p.HTTPReadTimeout, + + // Maximum download size. Must be large enough to accommodate + // large downloads. + MaxBytes: p.HTTPMaxBytes, + } + return &getter.Client{ + Ctx: ctx, + Src: p.Source, + Dst: p.Destination, + Mode: p.Mode, + Umask: umask, + Insecure: false, + DisableSymlinks: true, + Getters: map[string]getter.Getter{ + "git": &getter.GitGetter{ + Timeout: p.GitTimeout, + }, + "hg": &getter.HgGetter{ + Timeout: p.HgTimeout, + }, + "gcs": &getter.GCSGetter{ + Timeout: p.GCSTimeout, + }, + "s3": &getter.S3Getter{ + Timeout: p.S3Timeout, + }, + "http": httpGetter, + "https": httpGetter, + }, + } +} diff --git a/client/allocrunner/taskrunner/getter/params_test.go b/client/allocrunner/taskrunner/getter/params_test.go new file mode 100644 index 000000000..4252339bb --- /dev/null +++ b/client/allocrunner/taskrunner/getter/params_test.go @@ -0,0 +1,119 @@ +package getter + +import ( + "context" + "io" + "strings" + "testing" + "time" + + "github.com/hashicorp/go-getter" + "github.com/shoenig/test/must" +) + +const paramsAsJSON = ` +{ + "http_read_timeout": 1000000000, + "http_max_bytes": 2000, + "gcs_timeout": 2000000000, + "git_timeout": 3000000000, + "hg_timeout": 4000000000, + "s3_timeout": 5000000000, + "artifact_mode": 2, + "artifact_source": "https://example.com/file.txt", + "artifact_destination": "local/out.txt", + "artifact_headers": { + "X-Nomad-Artifact": ["hi"] + }, + "task_dir": "/path/to/task" +}` + +var paramsAsStruct = ¶meters{ + HTTPReadTimeout: 1 * time.Second, + HTTPMaxBytes: 2000, + GCSTimeout: 2 * time.Second, + GitTimeout: 3 * time.Second, + HgTimeout: 4 * time.Second, + S3Timeout: 5 * time.Second, + + Mode: getter.ClientModeFile, + Source: "https://example.com/file.txt", + Destination: "local/out.txt", + TaskDir: "/path/to/task", + Headers: map[string][]string{ + "X-Nomad-Artifact": {"hi"}, + }, +} + +func TestParameters_reader(t *testing.T) { + p := paramsAsStruct + reader := p.reader() + b, err := io.ReadAll(reader) + must.NoError(t, err) + must.EqJSON(t, paramsAsJSON, string(b)) +} + +func TestParameters_read(t *testing.T) { + reader := strings.NewReader(paramsAsJSON) + p := new(parameters) + err := p.read(reader) + must.NoError(t, err) + must.Equal(t, paramsAsStruct, p) +} + +func TestParameters_deadline(t *testing.T) { + t.Run("typical", func(t *testing.T) { + dur := paramsAsStruct.deadline() + must.Eq(t, 31*time.Minute, dur) + }) + + t.Run("long", func(t *testing.T) { + params := ¶meters{ + HTTPReadTimeout: 1 * time.Hour, + GCSTimeout: 2 * time.Hour, + GitTimeout: 3 * time.Hour, + HgTimeout: 4 * time.Hour, + S3Timeout: 5 * time.Hour, + } + dur := params.deadline() + must.Eq(t, 5*time.Hour+1*time.Minute, dur) + }) +} + +func TestParameters_client(t *testing.T) { + ctx := context.Background() + c := paramsAsStruct.client(ctx) + must.NotNil(t, c) + + // security options + must.False(t, c.Insecure) + must.True(t, c.DisableSymlinks) + must.Eq(t, umask, c.Umask) + + // artifact options + must.Eq(t, "https://example.com/file.txt", c.Src) + must.Eq(t, "local/out.txt", c.Dst) +} + +func TestParameters_Equal_headers(t *testing.T) { + p1 := ¶meters{ + Headers: map[string][]string{ + "East": []string{"New York", "Florida"}, + "West": []string{"California"}, + }, + } + + p2 := ¶meters{ + Headers: map[string][]string{ + "East": []string{"New York", "Florida"}, + "West": []string{"California"}, + }, + } + + // equal + must.Equal(t, p1, p2) + + // not equal + p2.Headers["East"] = []string{"New York"} + must.NotEqual(t, p1, p2) +} diff --git a/client/allocrunner/taskrunner/getter/replacer_test.go b/client/allocrunner/taskrunner/getter/replacer_test.go new file mode 100644 index 000000000..3eb5d0ee7 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/replacer_test.go @@ -0,0 +1,59 @@ +package getter + +import ( + "path/filepath" + "strings" + + "github.com/hashicorp/nomad/client/interfaces" + "github.com/hashicorp/nomad/helper/escapingfs" +) + +// noopReplacer is a noop version of taskenv.TaskEnv.ReplaceEnv. +type noopReplacer struct { + taskDir string +} + +// noopTaskEnv creates a new noopReplacer with the given taskDir. +func noopTaskEnv(taskDir string) interfaces.EnvReplacer { + return &noopReplacer{taskDir: taskDir} +} + +func (*noopReplacer) ReplaceEnv(s string) string { + return s +} + +func (r *noopReplacer) ClientPath(p string, join bool) (string, bool) { + path, escapes := clientPath(r.taskDir, r.ReplaceEnv(p), join) + return path, escapes +} + +// type upReplacer is a version of taskenv.TaskEnv.ReplaceEnv +// that uppercases all the things. +type upReplacer struct { + taskDir string +} + +// upTaskEnv creates a new noopReplacer with the given taskDir. +func upTaskEnv(taskDir string) interfaces.EnvReplacer { + return &upReplacer{taskDir: taskDir} +} + +func (*upReplacer) ReplaceEnv(s string) string { + return strings.ToUpper(s) +} + +func (r *upReplacer) ClientPath(p string, join bool) (string, bool) { + path, escapes := clientPath(r.taskDir, r.ReplaceEnv(p), join) + return path, escapes +} + +func clientPath(taskDir, path string, join bool) (string, bool) { + if !filepath.IsAbs(path) || (escapingfs.PathEscapesSandbox(taskDir, path) && join) { + path = filepath.Join(taskDir, path) + } + path = filepath.Clean(path) + if taskDir != "" && !escapingfs.PathEscapesSandbox(taskDir, path) { + return path, false + } + return path, true +} diff --git a/client/allocrunner/taskrunner/getter/sandbox.go b/client/allocrunner/taskrunner/getter/sandbox.go new file mode 100644 index 000000000..02d993e1d --- /dev/null +++ b/client/allocrunner/taskrunner/getter/sandbox.go @@ -0,0 +1,59 @@ +package getter + +import ( + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/interfaces" + "github.com/hashicorp/nomad/nomad/structs" +) + +// New creates a Sandbox with the given ArtifactConfig. +func New(ac *config.ArtifactConfig, logger hclog.Logger) *Sandbox { + return &Sandbox{ + logger: logger.Named("artifact"), + ac: ac, + } +} + +// A Sandbox is used to download artifacts. +type Sandbox struct { + logger hclog.Logger + ac *config.ArtifactConfig +} + +func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) error { + s.logger.Debug("get", "source", artifact.GetterSource, "destination", artifact.RelativeDest) + + source, err := getURL(env, artifact) + if err != nil { + return err + } + + destination, err := getDestination(env, artifact) + if err != nil { + return err + } + + mode := getMode(artifact) + headers := getHeaders(env, artifact) + dir := getTaskDir(env) + + params := ¶meters{ + HTTPReadTimeout: s.ac.HTTPReadTimeout, + HTTPMaxBytes: s.ac.HTTPMaxBytes, + GCSTimeout: s.ac.GCSTimeout, + GitTimeout: s.ac.GitTimeout, + HgTimeout: s.ac.HgTimeout, + S3Timeout: s.ac.S3Timeout, + Mode: mode, + Source: source, + Destination: destination, + Headers: headers, + TaskDir: dir, + } + + if err = runCmd(params, s.logger); err != nil { + return err + } + return nil +} diff --git a/client/allocrunner/taskrunner/getter/sandbox_test.go b/client/allocrunner/taskrunner/getter/sandbox_test.go new file mode 100644 index 000000000..7ffe83d69 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/sandbox_test.go @@ -0,0 +1,50 @@ +package getter + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" +) + +func artifactConfig(timeout time.Duration) *config.ArtifactConfig { + return &config.ArtifactConfig{ + HTTPReadTimeout: timeout, + HTTPMaxBytes: 1e6, + GCSTimeout: timeout, + GitTimeout: timeout, + HgTimeout: timeout, + S3Timeout: timeout, + } +} + +// comprehensive scenarios tested in e2e/artifact + +func TestSandbox_Get_http(t *testing.T) { + testutil.RequireRoot(t) + logger := testlog.HCLogger(t) + + ac := artifactConfig(10 * time.Second) + sbox := New(ac, logger) + + _, taskDir := SetupDir(t) + env := noopTaskEnv(taskDir) + + artifact := &structs.TaskArtifact{ + GetterSource: "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod", + RelativeDest: "local/downloads", + } + + err := sbox.Get(env, artifact) + must.NoError(t, err) + + b, err := os.ReadFile(filepath.Join(taskDir, "local", "downloads", "go.mod")) + must.NoError(t, err) + must.StrContains(t, string(b), "module github.com/hashicorp/go-set") +} diff --git a/client/allocrunner/taskrunner/getter/test-fixtures/archive.tar.gz b/client/allocrunner/taskrunner/getter/test-fixtures/archive.tar.gz deleted file mode 100644 index 2b1446bc5ac859dbec953d7334da06c76a1eced1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 246 zcmVD1MSpJ3c@fDhT$G1H*lIxlJit;r63~x@%+X#2ySXO5rp^I ze2WHVnvZts=4Ch^s**$`IA>CNpHi%@nVGtA)`@efYNWQRp)#)MR@pMm?HE_NwztdH zxp=JaH=gJBc;}y(*8cCy@j9Fe&4~BmbMN2#uS4v=@oFn0h4%SC|Ed3Rc(ij@X2z@k zd^J1!SF!Z}6S@C23e6DFo&WzPSoP`q_e%c&00000000000001cJORi@7NGzr004is BWcL67 diff --git a/client/allocrunner/taskrunner/getter/test-fixtures/test.sh b/client/allocrunner/taskrunner/getter/test-fixtures/test.sh deleted file mode 100644 index 08bfc4afe..000000000 --- a/client/allocrunner/taskrunner/getter/test-fixtures/test.sh +++ /dev/null @@ -1 +0,0 @@ -sleep 1 diff --git a/client/allocrunner/taskrunner/getter/testing.go b/client/allocrunner/taskrunner/getter/testing.go index 9dc974802..5095c4725 100644 --- a/client/allocrunner/taskrunner/getter/testing.go +++ b/client/allocrunner/taskrunner/getter/testing.go @@ -1,19 +1,43 @@ -//go:build !release -// +build !release - package getter import ( + "os" + "path/filepath" "testing" - "github.com/hashicorp/go-hclog" - clientconfig "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/nomad/structs/config" - "github.com/stretchr/testify/require" + cconfig "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/helper/testlog" + sconfig "github.com/hashicorp/nomad/nomad/structs/config" + "github.com/shoenig/test/must" ) -func TestDefaultGetter(t *testing.T) *Getter { - getterConf, err := clientconfig.ArtifactConfigFromAgent(config.DefaultArtifactConfig()) - require.NoError(t, err) - return NewGetter(hclog.NewNullLogger(), getterConf) +// TestSandbox creates a real artifact downloader configured via the default +// artifact config. It is good enough for tests so no mock implementation exists. +func TestSandbox(t *testing.T) *Sandbox { + ac, err := cconfig.ArtifactConfigFromAgent(sconfig.DefaultArtifactConfig()) + must.NoError(t, err) + return New(ac, testlog.HCLogger(t)) +} + +// SetupDir creates a directory suitable for testing artifact - i.e. it is +// owned by the nobody user as would be the case in a normal client operation. +// +// returns alloc_dir, task_dir +func SetupDir(t *testing.T) (string, string) { + uid, gid := credentials() + + allocDir := t.TempDir() + taskDir := filepath.Join(allocDir, "local") + topDir := filepath.Dir(allocDir) + + must.NoError(t, os.Chown(topDir, int(uid), int(gid))) + must.NoError(t, os.Chmod(topDir, 0o755)) + + must.NoError(t, os.Chown(allocDir, int(uid), int(gid))) + must.NoError(t, os.Chmod(allocDir, 0o755)) + + must.NoError(t, os.Mkdir(taskDir, 0o755)) + must.NoError(t, os.Chown(taskDir, int(uid), int(gid))) + must.NoError(t, os.Chmod(taskDir, 0o755)) + return allocDir, taskDir } diff --git a/client/allocrunner/taskrunner/getter/util.go b/client/allocrunner/taskrunner/getter/util.go new file mode 100644 index 000000000..90944dad4 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/util.go @@ -0,0 +1,127 @@ +package getter + +import ( + "bytes" + "fmt" + "net/http" + "net/url" + "os/exec" + "path/filepath" + "strings" + + "github.com/hashicorp/go-getter" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/nomad/client/interfaces" + "github.com/hashicorp/nomad/helper/subproc" + "github.com/hashicorp/nomad/nomad/structs" +) + +const ( + // githubPrefixSSH is the prefix for downloading via git using ssh from GitHub. + githubPrefixSSH = "git@github.com:" +) + +func getURL(taskEnv interfaces.EnvReplacer, artifact *structs.TaskArtifact) (string, error) { + source := taskEnv.ReplaceEnv(artifact.GetterSource) + + // fixup GitHub SSH URL such as git@github.com:hashicorp/nomad.git + gitSSH := false + if strings.HasPrefix(source, githubPrefixSSH) { + gitSSH = true + source = source[len(githubPrefixSSH):] + } + + u, err := url.Parse(source) + if err != nil { + return "", &Error{ + URL: artifact.GetterSource, + Err: fmt.Errorf("failed to parse source URL %q: %v", artifact.GetterSource, err), + Recoverable: false, + } + } + + // build the URL by substituting as necessary + q := u.Query() + for k, v := range artifact.GetterOptions { + q.Set(k, taskEnv.ReplaceEnv(v)) + } + u.RawQuery = q.Encode() + + // add the prefix back if necessary + sourceURL := u.String() + if gitSSH { + sourceURL = fmt.Sprintf("%s%s", githubPrefixSSH, sourceURL) + } + + return sourceURL, nil +} + +func getDestination(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) (string, error) { + destination, escapes := env.ClientPath(artifact.RelativeDest, true) + if escapes { + return "", &Error{ + URL: artifact.GetterSource, + Err: fmt.Errorf("artifact destination path escapes alloc directory"), + Recoverable: false, + } + } + return destination, nil +} + +func getMode(artifact *structs.TaskArtifact) getter.ClientMode { + switch artifact.GetterMode { + case structs.GetterModeFile: + return getter.ClientModeFile + case structs.GetterModeDir: + return getter.ClientModeDir + default: + return getter.ClientModeAny + } +} + +func getHeaders(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) map[string][]string { + m := artifact.GetterHeaders + if len(m) == 0 { + return nil + } + headers := make(http.Header, len(m)) + for k, v := range m { + headers.Set(k, env.ReplaceEnv(v)) + } + return headers +} + +func getTaskDir(env interfaces.EnvReplacer) string { + p, _ := env.ClientPath("stub", false) + return filepath.Dir(p) +} + +func runCmd(env *parameters, logger hclog.Logger) error { + // find the nomad process + bin := subproc.Self() + + // final method of ensuring subprocess termination + ctx, cancel := subproc.Context(env.deadline()) + defer cancel() + + // start the subprocess, passing in parameters via stdin + output := new(bytes.Buffer) + cmd := exec.CommandContext(ctx, bin, SubCommand) + cmd.Env = minimalVars(env.TaskDir) + cmd.Stdin = env.reader() + cmd.Stdout = output + cmd.Stderr = output + cmd.SysProcAttr = attributes() + + // start & wait for the subprocess to terminate + if err := cmd.Run(); err != nil { + subproc.Log(output, logger.Error) + return &Error{ + URL: env.Source, + Err: fmt.Errorf("getter subprocess failed: %v", err), + Recoverable: true, + } + } + subproc.Log(output, logger.Debug) + return nil +} diff --git a/client/allocrunner/taskrunner/getter/util_default.go b/client/allocrunner/taskrunner/getter/util_default.go new file mode 100644 index 000000000..d025c09d1 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/util_default.go @@ -0,0 +1,43 @@ +//go:build !linux && !windows + +package getter + +import ( + "fmt" + "path/filepath" + "syscall" +) + +// attributes returns the system process attributes to run +// the sandbox process with +func attributes() *syscall.SysProcAttr { + uid, gid := credentials() + return &syscall.SysProcAttr{ + Credential: &syscall.Credential{ + Uid: uid, + Gid: gid, + }, + } +} + +// credentials returns the credentials of the user Nomad is running as +func credentials() (uint32, uint32) { + uid := syscall.Getuid() + gid := syscall.Getgid() + return uint32(uid), uint32(gid) +} + +// minimalVars returns the minimal environment set for artifact +// downloader sandbox +func minimalVars(taskDir string) []string { + tmpDir := filepath.Join(taskDir, "tmp") + return []string{ + fmt.Sprintf("PATH=/usr/local/bin:/usr/bin:/bin"), + fmt.Sprintf("TMPDIR=%s", tmpDir), + } +} + +// lockdown applies only to Linux +func lockdown(string) error { + return nil +} diff --git a/client/allocrunner/taskrunner/getter/util_linux.go b/client/allocrunner/taskrunner/getter/util_linux.go new file mode 100644 index 000000000..b02841ec1 --- /dev/null +++ b/client/allocrunner/taskrunner/getter/util_linux.go @@ -0,0 +1,83 @@ +//go:build linux + +package getter + +import ( + "fmt" + "path/filepath" + "syscall" + + "github.com/hashicorp/nomad/helper/users" + "github.com/shoenig/go-landlock" +) + +var ( + // userUID is the current user's uid + userUID uint32 + + // userGID is the current user's gid + userGID uint32 +) + +func init() { + userUID = uint32(syscall.Getuid()) + userGID = uint32(syscall.Getgid()) +} + +// attributes returns the system process attributes to run +// the sandbox process with +func attributes() *syscall.SysProcAttr { + uid, gid := credentials() + return &syscall.SysProcAttr{ + Credential: &syscall.Credential{ + Uid: uid, + Gid: gid, + }, + } +} + +// credentials returns the UID and GID of the user the child process +// will run as. On Linux systems this will be the nobody user if Nomad +// is being run as the root user, or the user Nomad is being run as +// otherwise. +func credentials() (uint32, uint32) { + switch userUID { + case 0: + return users.NobodyIDs() + default: + return userUID, userGID + } +} + +// minimalVars returns the minimal environment set for artifact +// downloader sandbox +func minimalVars(taskDir string) []string { + tmpDir := filepath.Join(taskDir, "tmp") + return []string{ + "PATH=/usr/local/bin:/usr/bin:/bin", + fmt.Sprintf("TMPDIR=%s", tmpDir), + } +} + +// lockdown isolates this process to only be able to write and +// create files in the task's task directory. +// dir - the task directory +// +// Only applies to Linux, when available. +func lockdown(dir string) error { + // landlock not present in the kernel, do not sandbox + if !landlock.Available() { + return nil + } + paths := []*landlock.Path{ + landlock.DNS(), + landlock.Certs(), + landlock.Shared(), + landlock.Dir("/bin", "rx"), + landlock.Dir("/usr/bin", "rx"), + landlock.Dir("/usr/local/bin", "rx"), + landlock.Dir(dir, "rwc"), + } + locker := landlock.New(paths...) + return locker.Lock(landlock.Mandatory) +} diff --git a/client/allocrunner/taskrunner/getter/util_test.go b/client/allocrunner/taskrunner/getter/util_test.go new file mode 100644 index 000000000..b637c3f8b --- /dev/null +++ b/client/allocrunner/taskrunner/getter/util_test.go @@ -0,0 +1,148 @@ +package getter + +import ( + "errors" + "runtime" + "testing" + + "github.com/hashicorp/go-getter" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/shoenig/test/must" +) + +func TestUtil_getURL(t *testing.T) { + cases := []struct { + name string + artifact *structs.TaskArtifact + expURL string + expErr *Error + }{{ + name: "basic http", + artifact: &structs.TaskArtifact{GetterSource: "example.com"}, + expURL: "example.com", + expErr: nil, + }, { + name: "bad url", + artifact: &structs.TaskArtifact{GetterSource: "::example.com"}, + expURL: "", + expErr: &Error{ + URL: "::example.com", + Err: errors.New(`failed to parse source URL "::example.com": parse "::example.com": missing protocol scheme`), + Recoverable: false, + }, + }, { + name: "option", + artifact: &structs.TaskArtifact{ + GetterSource: "git::github.com/hashicorp/nomad", + GetterOptions: map[string]string{"sshkey": "abc123"}, + }, + expURL: "git::github.com/hashicorp/nomad?sshkey=abc123", + expErr: nil, + }, { + name: "github case", + artifact: &structs.TaskArtifact{ + GetterSource: "git@github.com:hashicorp/nomad.git", + GetterOptions: map[string]string{"sshkey": "abc123"}, + }, + expURL: "git@github.com:hashicorp/nomad.git?sshkey=abc123", + expErr: nil, + }} + + env := noopTaskEnv("/path/to/task") + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result, err := getURL(env, tc.artifact) + err2, _ := err.(*Error) + must.Equal(t, tc.expErr, err2) + must.Eq(t, tc.expURL, result) + }) + } +} + +func TestUtil_getDestination(t *testing.T) { + env := noopTaskEnv("/path/to/task") + t.Run("ok", func(t *testing.T) { + result, err := getDestination(env, &structs.TaskArtifact{ + RelativeDest: "local/downloads", + }) + must.NoError(t, err) + must.Eq(t, "/path/to/task/local/downloads", result) + }) + + t.Run("escapes", func(t *testing.T) { + result, err := getDestination(env, &structs.TaskArtifact{ + RelativeDest: "../../../../../../../etc", + }) + must.EqError(t, err, "artifact destination path escapes alloc directory") + must.Eq(t, "", result) + }) +} + +func TestUtil_getMode(t *testing.T) { + cases := []struct { + mode string + exp getter.ClientMode + }{ + {mode: structs.GetterModeFile, exp: getter.ClientModeFile}, + {mode: structs.GetterModeDir, exp: getter.ClientModeDir}, + {mode: structs.GetterModeAny, exp: getter.ClientModeAny}, + } + + for _, tc := range cases { + t.Run(tc.mode, func(t *testing.T) { + artifact := &structs.TaskArtifact{GetterMode: tc.mode} + result := getMode(artifact) + must.Eq(t, tc.exp, result) + }) + } +} + +func TestUtil_getHeaders(t *testing.T) { + env := upTaskEnv("/path/to/task") + + t.Run("empty", func(t *testing.T) { + result := getHeaders(env, &structs.TaskArtifact{ + GetterHeaders: nil, + }) + must.Nil(t, result) + }) + + t.Run("replacments", func(t *testing.T) { + result := getHeaders(env, &structs.TaskArtifact{ + GetterHeaders: map[string]string{ + "color": "red", + "number": "six", + }, + }) + must.MapEq(t, map[string][]string{ + "Color": {"RED"}, + "Number": {"SIX"}, + }, result) + }) +} + +func TestUtil_getTaskDir(t *testing.T) { + env := noopTaskEnv("/path/to/task") + result := getTaskDir(env) + must.Eq(t, "/path/to/task", result) +} + +func TestUtil_minimalVars(t *testing.T) { + var exp []string + switch runtime.GOOS { + case "windows": + exp = []string{ + `PATH=C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;`, + `TMP=C:\path\to\task\tmp`, + `TEMP=C:\path\to\task\tmp`, + } + default: + exp = []string{ + "PATH=/usr/local/bin:/usr/bin:/bin", + "TMPDIR=/path/to/task/tmp", + } + } + result := minimalVars("/path/to/task") + must.Eq(t, exp, result) +} diff --git a/client/allocrunner/taskrunner/getter/util_windows.go b/client/allocrunner/taskrunner/getter/util_windows.go new file mode 100644 index 000000000..67c38b7ab --- /dev/null +++ b/client/allocrunner/taskrunner/getter/util_windows.go @@ -0,0 +1,37 @@ +//go:build windows + +package getter + +import ( + "fmt" + "os" + "path/filepath" + "syscall" +) + +// attributes returns the system process attributes to run +// the sandbox process with +func attributes() *syscall.SysProcAttr { + return &syscall.SysProcAttr{} +} + +func credentials() (uint32, uint32) { + return 0, 0 +} + +// lockdown has no effect on windows +func lockdown(string) error { + return nil +} + +func minimalVars(taskDir string) []string { + tmpDir := filepath.Join(taskDir, "tmp") + return []string{ + fmt.Sprintf("HOMEPATH=%s", os.Getenv("HOMEPATH")), + fmt.Sprintf("HOMEDRIVE=%s", os.Getenv("HOMEDRIVE")), + fmt.Sprintf("USERPROFILE=%s", os.Getenv("USERPROFILE")), + fmt.Sprintf("PATH=%s", os.Getenv("PATH")), + fmt.Sprintf("TMP=%s", tmpDir), + fmt.Sprintf("TEMP=%s", tmpDir), + } +} diff --git a/client/allocrunner/taskrunner/getter/z_getter_cmd.go b/client/allocrunner/taskrunner/getter/z_getter_cmd.go new file mode 100644 index 000000000..76a9aa5aa --- /dev/null +++ b/client/allocrunner/taskrunner/getter/z_getter_cmd.go @@ -0,0 +1,53 @@ +package getter + +import ( + "os" + + "github.com/hashicorp/nomad/helper/subproc" +) + +const ( + // SubCommand is the first argument to the clone of the nomad + // agent process for downloading artifacts. + SubCommand = "artifact-isolation" +) + +func init() { + subproc.Do(SubCommand, func() int { + + // get client and artifact configuration from standard IO + env := new(parameters) + if err := env.read(os.Stdin); err != nil { + subproc.Print("failed to read configuration: %v", err) + return subproc.ExitFailure + } + + // create context with the overall timeout + ctx, cancel := subproc.Context(env.deadline()) + defer cancel() + + // force quit after maximum timeout exceeded + subproc.SetExpiration(ctx) + + // sandbox the host filesystem for this process + dir := env.TaskDir + if err := lockdown(dir); err != nil { + subproc.Print("failed to sandbox getter process: %v", err) + return subproc.ExitFailure + } + + // create the go-getter client + // options were already transformed into url query parameters + // headers were already replaced and are usable now + c := env.client(ctx) + + // run the go-getter client + if err := c.Get(); err != nil { + subproc.Print("failed to download artifact: %v", err) + return subproc.ExitFailure + } + + subproc.Print("artifact download was a success") + return subproc.ExitSuccess + }) +} diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 78870b04f..357e0d2ab 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -146,7 +146,7 @@ func testTaskRunnerConfig(t *testing.T, alloc *structs.Allocation, taskName stri ShutdownDelayCtx: shutdownDelayCtx, ShutdownDelayCancelFn: shutdownDelayCancelFn, ServiceRegWrapper: wrapperMock, - Getter: getter.TestDefaultGetter(t), + Getter: getter.TestSandbox(t), } // Set the cgroup path getter if we are in v2 mode @@ -1716,7 +1716,7 @@ func TestTaskRunner_DeriveToken_Unrecoverable(t *testing.T) { require.True(t, state.Events[2].FailsTask) } -// TestTaskRunner_Download_Exec asserts that downloaded artifacts may be +// TestTaskRunner_Download_RawExec asserts that downloaded artifacts may be // executed in a driver without filesystem isolation. func TestTaskRunner_Download_RawExec(t *testing.T) { ci.Parallel(t) @@ -1727,6 +1727,7 @@ func TestTaskRunner_Download_RawExec(t *testing.T) { // Create a task that downloads a script and executes it. alloc := mock.BatchAlloc() alloc.Job.TaskGroups[0].RestartPolicy = &structs.RestartPolicy{} + task := alloc.Job.TaskGroups[0].Tasks[0] task.RestartPolicy = &structs.RestartPolicy{} task.Driver = "raw_exec" diff --git a/client/allocrunner/testing.go b/client/allocrunner/testing.go index 44e3eb524..44dc79a59 100644 --- a/client/allocrunner/testing.go +++ b/client/allocrunner/testing.go @@ -90,7 +90,7 @@ func testAllocRunnerConfig(t *testing.T, alloc *structs.Allocation) (*Config, fu ServersContactedCh: make(chan struct{}), ServiceRegWrapper: wrapper.NewHandlerWrapper(clientConf.Logger, consulRegMock, nomadRegMock), CheckStore: checkstore.NewStore(clientConf.Logger, stateDB), - Getter: getter.TestDefaultGetter(t), + Getter: getter.TestSandbox(t), } return conf, cleanup diff --git a/client/client.go b/client/client.go index 04348ea40..5c691a0c9 100644 --- a/client/client.go +++ b/client/client.go @@ -399,7 +399,7 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulProxie serversContactedCh: make(chan struct{}), serversContactedOnce: sync.Once{}, cpusetManager: cgutil.CreateCPUSetManager(cfg.CgroupParent, cfg.ReservableCores, logger), - getter: getter.NewGetter(logger.Named("artifact_getter"), cfg.Artifact), + getter: getter.New(cfg.Artifact, logger), EnterpriseClient: newEnterpriseClient(logger), } diff --git a/client/config/testing.go b/client/config/testing.go index 511505044..02f87984f 100644 --- a/client/config/testing.go +++ b/client/config/testing.go @@ -39,6 +39,11 @@ func TestClientConfig(t testing.T) (*Config, func()) { os.RemoveAll(parent) } + // Fixup nomadtest dir permissions + if err = os.Chmod(parent, 0777); err != nil { + t.Fatalf("error updating permissions on nomadtest dir") + } + allocDir := filepath.Join(parent, "allocs") if err := os.Mkdir(allocDir, 0777); err != nil { cleanup() diff --git a/client/fingerprint/fingerprint.go b/client/fingerprint/fingerprint.go index 39c8dcba9..c15448b12 100644 --- a/client/fingerprint/fingerprint.go +++ b/client/fingerprint/fingerprint.go @@ -34,6 +34,7 @@ var ( "cni": NewCNIFingerprint, // networks "cpu": NewCPUFingerprint, "host": NewHostFingerprint, + "landlock": NewLandlockFingerprint, "memory": NewMemoryFingerprint, "network": NewNetworkFingerprint, "nomad": NewNomadFingerprint, diff --git a/client/fingerprint/landlock.go b/client/fingerprint/landlock.go new file mode 100644 index 000000000..9c0d6eb5b --- /dev/null +++ b/client/fingerprint/landlock.go @@ -0,0 +1,42 @@ +package fingerprint + +import ( + "fmt" + + "github.com/hashicorp/go-hclog" + "github.com/shoenig/go-landlock" +) + +const ( + landlockKey = "kernel.landlock" +) + +// LandlockFingerprint is used to fingerprint the kernel landlock feature. +type LandlockFingerprint struct { + StaticFingerprinter + logger hclog.Logger + detector func() (int, error) +} + +func NewLandlockFingerprint(logger hclog.Logger) Fingerprint { + return &LandlockFingerprint{ + logger: logger.Named("landlock"), + detector: landlock.Detect, + } +} + +func (f *LandlockFingerprint) Fingerprint(_ *FingerprintRequest, resp *FingerprintResponse) error { + version, err := f.detector() + if err != nil { + f.logger.Warn("failed to fingerprint kernel landlock feature", "error", err) + version = 0 + } + switch version { + case 0: + // do not set any attribute + default: + v := fmt.Sprintf("v%d", version) + resp.AddAttribute(landlockKey, v) + } + return nil +} diff --git a/client/fingerprint/landlock_test.go b/client/fingerprint/landlock_test.go new file mode 100644 index 000000000..3091e85bf --- /dev/null +++ b/client/fingerprint/landlock_test.go @@ -0,0 +1,70 @@ +package fingerprint + +import ( + "errors" + "testing" + + "github.com/hashicorp/nomad/ci" + "github.com/hashicorp/nomad/client/testutil" + "github.com/hashicorp/nomad/helper/testlog" + "github.com/shoenig/go-landlock" + "github.com/shoenig/test/must" +) + +func TestLandlockFingerprint(t *testing.T) { + testutil.RequireLinux(t) + ci.Parallel(t) + + version, err := landlock.Detect() + must.NoError(t, err) + + logger := testlog.HCLogger(t) + f := NewLandlockFingerprint(logger) + + var response FingerprintResponse + err = f.Fingerprint(nil, &response) + must.NoError(t, err) + + result := response.Attributes[landlockKey] + exp := map[int]string{ + 0: "", // unavailable + 1: "v1", + 2: "v2", + 3: "v3", + } + must.Eq(t, exp[version], result) +} + +func TestLandlockFingerprint_absent(t *testing.T) { + ci.Parallel(t) + + logger := testlog.HCLogger(t) + f := NewLandlockFingerprint(logger) + f.(*LandlockFingerprint).detector = func() (int, error) { + return 0, nil + } + + var response FingerprintResponse + err := f.Fingerprint(nil, &response) + must.NoError(t, err) + + _, exists := response.Attributes[landlockKey] + must.False(t, exists) +} + +func TestLandlockFingerprint_error(t *testing.T) { + ci.Parallel(t) + + logger := testlog.HCLogger(t) + f := NewLandlockFingerprint(logger) + f.(*LandlockFingerprint).detector = func() (int, error) { + return 0, errors.New("oops") + } + + var response FingerprintResponse + err := f.Fingerprint(nil, &response) + must.NoError(t, err) + + _, exists := response.Attributes[landlockKey] + must.False(t, exists) +} diff --git a/client/interfaces/client.go b/client/interfaces/client.go index f3fc4a5a8..241fcedbe 100644 --- a/client/interfaces/client.go +++ b/client/interfaces/client.go @@ -32,7 +32,8 @@ type EnvReplacer interface { ClientPath(string, bool) (string, bool) } -// ArtifactGetter is an interface satisfied by the helper/getter package. +// ArtifactGetter is an interface satisfied by the getter package. type ArtifactGetter interface { - GetArtifact(taskEnv EnvReplacer, artifact *structs.TaskArtifact) error + // Get artifact and put it in the task directory. + Get(taskEnv EnvReplacer, artifact *structs.TaskArtifact) error } diff --git a/e2e/artifact/artifact_test.go b/e2e/artifact/artifact_test.go new file mode 100644 index 000000000..7fc3ac90d --- /dev/null +++ b/e2e/artifact/artifact_test.go @@ -0,0 +1,108 @@ +package artifact + +import ( + "testing" + + "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/e2e/e2eutil" + "github.com/hashicorp/nomad/helper/uuid" + "github.com/shoenig/test/must" +) + +func TestArtifact(t *testing.T) { + nomad := e2eutil.NomadClient(t) + + e2eutil.WaitForLeader(t, nomad) + e2eutil.WaitForNodesReady(t, nomad, 1) + + t.Run("testLinux", testLinux) + t.Run("testWindows", testWindows) +} + +// artifactCheckLogContents verifies expected logs for artifact downloader tests. +// +// each task is designed to download the artifact in some way then cat the go.mod +// file, so we just need to read the logs +// +// note: git requires the use of destination (hence no default form) +func artifactCheckLogContents(t *testing.T, nomad *api.Client, group, task string, allocations []map[string]string) { + var allocID string + for _, alloc := range allocations { + if alloc["Task Group"] == group { + allocID = alloc["ID"] + break + } + } + e2eutil.WaitForAllocStopped(t, nomad, allocID) + t.Run(task, func(t *testing.T) { + logs, err := e2eutil.AllocTaskLogs(allocID, task, e2eutil.LogsStdOut) + must.NoError(t, err) + must.StrContains(t, logs, "module github.com/hashicorp/go-set") + }) +} + +func testWindows(t *testing.T) { + nomad := e2eutil.NomadClient(t) + + jobID := "artifact-linux-" + uuid.Short() + jobIDs := []string{jobID} + t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs)) + + // start job + e2eutil.RegisterAndWaitForAllocs(t, nomad, "./input/artifact_windows.nomad", jobID, "") + + // get allocations + allocations, err := e2eutil.AllocsForJob(jobID, "") + must.NoError(t, err) + must.Len(t, 1, allocations) + + // assert log contents for each task + check := func(group, task string) { + artifactCheckLogContents(t, nomad, group, task, allocations) + } + + check("rawexec", "rawexec_file_default") + check("rawexec", "rawexec_file_custom") + check("rawexec", "rawexec_zip_default") + check("rawexec", "rawexec_zip_custom") + check("rawexec", "rawexec_git_custom") +} + +func testLinux(t *testing.T) { + nomad := e2eutil.NomadClient(t) + + jobID := "artifact-linux-" + uuid.Short() + jobIDs := []string{jobID} + t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs)) + + // start job + e2eutil.RegisterAndWaitForAllocs(t, nomad, "./input/artifact_linux.nomad", jobID, "") + + // get allocations + allocations, err := e2eutil.AllocsForJob(jobID, "") + must.NoError(t, err) + must.Len(t, 3, allocations) + + // assert log contents for each task + check := func(group, task string) { + artifactCheckLogContents(t, nomad, group, task, allocations) + } + + check("rawexec", "rawexec_file_default") + check("rawexec", "rawexec_file_custom") + check("rawexec", "rawexec_zip_default") + check("rawexec", "rawexec_zip_custom") + check("rawexec", "rawexec_git_custom") + + check("exec", "exec_file_default") + check("exec", "exec_file_custom") + check("exec", "exec_zip_default") + check("exec", "exec_zip_custom") + check("exec", "exec_git_custom") + + check("docker", "docker_file_default") + check("docker", "docker_file_custom") + check("docker", "docker_zip_default") + check("docker", "docker_zip_custom") + check("docker", "docker_git_custom") +} diff --git a/e2e/artifact/input/artifact_darwin.nomad b/e2e/artifact/input/artifact_darwin.nomad new file mode 100644 index 000000000..2ea05427c --- /dev/null +++ b/e2e/artifact/input/artifact_darwin.nomad @@ -0,0 +1,97 @@ +# note: this job is not run regularly in e2e, +# where we have no macOS runner. + +job "darwin" { + datacenters = ["dc1"] + type = "batch" + + constraint { + attribute = "${attr.kernel.name}" + value = "darwin" + } + + group "rawexec" { + task "rawexec" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_file_custom" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + destination = "local/my/path" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/my/path/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_default" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_custom" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + destination = "local/my/zip" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/my/zip/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_git_custom" { + artifact { + source = "git::https://github.com/hashicorp/go-set" + destination = "local/repository" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/repository/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + } +} diff --git a/e2e/artifact/input/artifact_linux.nomad b/e2e/artifact/input/artifact_linux.nomad new file mode 100644 index 000000000..bc6f8105f --- /dev/null +++ b/e2e/artifact/input/artifact_linux.nomad @@ -0,0 +1,265 @@ +job "linux" { + datacenters = ["dc1"] + type = "batch" + + constraint { + attribute = "${attr.kernel.name}" + value = "linux" + } + + group "rawexec" { + + task "rawexec_file_default" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_file_custom" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + destination = "local/my/path" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/my/path/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_default" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_custom" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + destination = "local/my/zip" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/my/zip/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_git_custom" { + artifact { + source = "git::https://github.com/hashicorp/go-set" + destination = "local/repository" + } + driver = "raw_exec" + config { + command = "cat" + args = ["local/repository/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + } + + group "exec" { + task "exec_file_default" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + } + driver = "exec" + config { + command = "cat" + args = ["local/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 256 + } + } + + task "exec_file_custom" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + destination = "local/my/path" + } + driver = "exec" + config { + command = "cat" + args = ["local/my/path/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 256 + } + } + + task "exec_zip_default" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + } + driver = "exec" + config { + command = "cat" + args = ["local/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 256 + } + } + + task "exec_zip_custom" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + destination = "local/my/zip" + } + driver = "exec" + config { + command = "cat" + args = ["local/my/zip/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 256 + } + } + + task "exec_git_custom" { + artifact { + source = "git::https://github.com/hashicorp/go-set" + destination = "local/repository" + } + driver = "exec" + config { + command = "cat" + args = ["local/repository/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 256 + } + } + } + + group "docker" { + task "docker_file_default" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + } + driver = "docker" + config { + image = "bash:5" + args = ["cat", "local/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "docker_file_custom" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + destination = "local/my/path" + } + driver = "docker" + config { + image = "bash:5" + args = ["cat", "local/my/path/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "docker_zip_default" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + } + driver = "docker" + config { + image = "bash:5" + args = ["cat", "local/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "docker_zip_custom" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + destination = "local/my/zip" + } + driver = "docker" + config { + image = "bash:5" + args = ["cat", "local/my/zip/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "docker_git_custom" { + artifact { + source = "git::https://github.com/hashicorp/go-set" + destination = "local/repository" + } + driver = "docker" + config { + image = "bash:5" + args = ["cat", "local/repository/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + } +} diff --git a/e2e/artifact/input/artifact_windows.nomad b/e2e/artifact/input/artifact_windows.nomad new file mode 100644 index 000000000..ab9756905 --- /dev/null +++ b/e2e/artifact/input/artifact_windows.nomad @@ -0,0 +1,95 @@ +job "windows" { + datacenters = ["dc1"] + type = "batch" + + constraint { + attribute = "${attr.kernel.name}" + value = "windows" + } + + group "rawexec" { + task "rawexec" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + } + driver = "raw_exec" + config { + command = "powershell" + args = ["type", "local/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_file_custom" { + artifact { + source = "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod" + destination = "local/my/path" + } + driver = "raw_exec" + config { + command = "powershell" + args = ["type", "local/my/path/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_default" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + } + driver = "raw_exec" + config { + command = "powershell" + args = ["type", "local/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + task "rawexec_zip_custom" { + artifact { + source = "https://github.com/hashicorp/go-set/archive/refs/heads/main.zip" + destination = "local/my/zip" + } + driver = "raw_exec" + config { + command = "powershell" + args = ["type", "local/my/zip/go-set-main/go.mod"] + } + resources { + cpu = 16 + memory = 32 + disk = 64 + } + } + + # TODO(shoenig) setup git in our Windows e2e client + # task "rawexec_git_custom" { + # artifact { + # source = "git::https://github.com/hashicorp/go-set" + # destination = "local/repository" + # } + # driver = "raw_exec" + # config { + # command = "powershell" + # args = ["type", "local/repository/go.mod"] + # } + # resources { + # cpu = 16 + # memory = 32 + # disk = 64 + # } + # } + } +} diff --git a/go.mod b/go.mod index 7d3ee5ad4..378bab244 100644 --- a/go.mod +++ b/go.mod @@ -110,6 +110,7 @@ require ( github.com/ryanuber/go-glob v1.0.0 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 github.com/shirou/gopsutil/v3 v3.22.10 + github.com/shoenig/go-landlock v0.1.3 github.com/shoenig/test v0.4.6 github.com/skratchdot/open-golang v0.0.0-20160302144031-75fb7ed4208c github.com/stretchr/testify v1.8.1 @@ -282,4 +283,5 @@ require ( gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect + kernel.org/pub/linux/libs/security/libcap/psx v1.2.66 // indirect ) diff --git a/go.sum b/go.sum index 9ec5d91fa..1bb59133d 100644 --- a/go.sum +++ b/go.sum @@ -1173,6 +1173,8 @@ github.com/shirou/gopsutil/v3 v3.22.10 h1:4KMHdfBRYXGF9skjDWiL4RA2N+E8dRdodU/bOZ github.com/shirou/gopsutil/v3 v3.22.10/go.mod h1:QNza6r4YQoydyCfo6rH0blGfKahgibh4dQmV5xdFkQk= github.com/shoenig/test v0.4.6 h1:S1pAVs5L1TSRen3N1YQNtBZIh9Z6d1PyQSUDUweMTqk= github.com/shoenig/test v0.4.6/go.mod h1:xYtyGBC5Q3kzCNyJg/SjgNpfAa2kvmgA0i5+lQso8x0= +github.com/shoenig/go-landlock v0.1.3 h1:C2msGjuYBCtyJ8+0m7BlcuQgypsiW1jEIKENL/lJANg= +github.com/shoenig/go-landlock v0.1.3/go.mod h1:y7C09FIsHOe9nM586kp08p6cLvJFDR0ogvCG/LY/Gjk= github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ= github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o= github.com/shurcooL/sanitized_anchor_name v1.0.0/go.mod h1:1NzhyTcUVG4SuEtjjoZeVRXNmyL/1OwPU0+IJeTBvfc= @@ -1963,6 +1965,8 @@ k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd/go.mod h1:WOJ3KddDSol4tAG k8s.io/kubernetes v1.13.0/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk= k8s.io/utils v0.0.0-20200324210504-a9aa75ae1b89/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= +kernel.org/pub/linux/libs/security/libcap/psx v1.2.66 h1:ikIhPzfkSSAEwBOU+2DWhoF+xnGUhvlMTfQjBVhvzQY= +kernel.org/pub/linux/libs/security/libcap/psx v1.2.66/go.mod h1:+l6Ee2F59XiJ2I6WR5ObpC1utCQJZ/VLsEbQCD8RG24= oss.indeed.com/go/libtime v1.6.0 h1:XQyczJihse/wQGo59OfPF3f4f+Sywv4R8vdGB3S9BfU= oss.indeed.com/go/libtime v1.6.0/go.mod h1:B2sdEcuzB0zhTKkAuHy4JInKRc7Al3tME4qWam6R7mA= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= diff --git a/helper/subproc/doc.go b/helper/subproc/doc.go new file mode 100644 index 000000000..b6344c2af --- /dev/null +++ b/helper/subproc/doc.go @@ -0,0 +1,11 @@ +// Package subproc provides helper utilities for executing the Nomad binary as +// a child process of the Nomad agent. +// +// The main entrypoint is the Do function, in which the given MainFunc will be +// executed as a sub-process if the first argument matches the subcommand. +// +// Context can be used to create a context.Context object with a given timeout, +// and is expected to be used in conjunction with SetExpiration which uses the +// context's termination to forcefully terminate the child process if it has not +// exited by itself. +package subproc diff --git a/helper/subproc/self.go b/helper/subproc/self.go new file mode 100644 index 000000000..3f9442c99 --- /dev/null +++ b/helper/subproc/self.go @@ -0,0 +1,34 @@ +package subproc + +import ( + "fmt" + "os" + "os/exec" + "strings" +) + +var ( + // executable is the executable of this process + executable string +) + +func init() { + s, err := os.Executable() + if err != nil { + panic(fmt.Sprintf("failed to detect executable: %v", err)) + } + + // when running tests, we need to use the real nomad binary, + // and make sure you recompile between changes! + if strings.HasSuffix(s, ".test") { + if s, err = exec.LookPath("nomad"); err != nil { + panic(fmt.Sprintf("failed to find nomad binary: %v", err)) + } + } + executable = s +} + +// Self returns the path to the executable of this process. +func Self() string { + return executable +} diff --git a/helper/subproc/subproc.go b/helper/subproc/subproc.go new file mode 100644 index 000000000..ad32c8109 --- /dev/null +++ b/helper/subproc/subproc.go @@ -0,0 +1,69 @@ +package subproc + +import ( + "bufio" + "context" + "fmt" + "io" + "os" + "time" +) + +const ( + // ExitSuccess indicates the subprocess completed successfully. + ExitSuccess = iota + + // ExitFailure indicates the subprocess terminated unsuccessfully. + ExitFailure + + // ExitTimeout indicates the subprocess timed out before completion. + ExitTimeout +) + +// MainFunc is the function that runs for this sub-process. +// +// The return value is a process exit code. +type MainFunc func() int + +// Do f if nomad was launched as, "nomad [name]". This process will exit without +// running any other part of Nomad. +func Do(name string, f MainFunc) { + if len(os.Args) > 1 && os.Args[1] == name { + os.Exit(f()) + } +} + +// Print the given message to standard error. +func Print(format string, args ...any) { + _, _ = fmt.Fprintf(os.Stderr, format+"\n", args...) +} + +// Log the given output to the logger. +// +// r should be a buffer containing output (typically combined stdin + stdout) +// f should be an HCLogger Print method (e.g. log.Debug) +func Log(r io.Reader, f func(msg string, args ...any)) { + scanner := bufio.NewScanner(r) + for scanner.Scan() { + line := scanner.Text() + f("sub-process", "OUTPUT", line) + } +} + +// Context creates a context setup with the given timeout. +func Context(timeout time.Duration) (context.Context, context.CancelFunc) { + ctx := context.Background() + return context.WithTimeout(ctx, timeout) +} + +// SetExpiration is used to ensure the process terminates, once ctx +// is complete. A short grace period is added to allow any cleanup +// to happen first. +func SetExpiration(ctx context.Context) { + const graceful = 5 * time.Second + go func() { + <-ctx.Done() + time.Sleep(graceful) + os.Exit(ExitTimeout) + }() +} diff --git a/helper/users/lookup_test.go b/helper/users/lookup_test.go index c506ff5ca..8189adf3b 100644 --- a/helper/users/lookup_test.go +++ b/helper/users/lookup_test.go @@ -33,3 +33,9 @@ func TestLookup(t *testing.T) { }) } } + +func TestLookup_NobodyIDs(t *testing.T) { + uid, gid := NobodyIDs() + must.Eq(t, 65534, uid) // ubuntu + must.Eq(t, 65534, gid) // ubuntu +} diff --git a/helper/users/lookup_unix.go b/helper/users/lookup_unix.go index 28b9f9235..7dc9da81b 100644 --- a/helper/users/lookup_unix.go +++ b/helper/users/lookup_unix.go @@ -5,11 +5,20 @@ package users import ( "fmt" "os/user" + "strconv" ) -// 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 +var ( + // 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. + nobody user.User + + // nobodyUID is a cached copy of the int value of the nobody user's UID. + nobodyUID uint32 + + // nobodyGID int is a cached copy of the int value of the nobody users's GID. + nobodyGID uint32 +) // Nobody returns User data for the "nobody" user on the system, bypassing the // locking / file read / NSS lookup. @@ -17,10 +26,27 @@ func Nobody() user.User { return nobody } +// NobodyIDs returns the integer UID and GID of the nobody user. +func NobodyIDs() (uint32, uint32) { + return nobodyUID, nobodyGID +} + func init() { u, err := Lookup("nobody") if err != nil { panic(fmt.Sprintf("failed to lookup nobody user: %v", err)) } nobody = *u + + uid, err := strconv.Atoi(u.Uid) + if err != nil { + panic(fmt.Sprintf("failed to parse nobody UID: %v", err)) + } + + gid, err := strconv.Atoi(u.Gid) + if err != nil { + panic(fmt.Sprintf("failed to parse nobody GID: %v", err)) + } + + nobodyUID, nobodyGID = uint32(uid), uint32(gid) } diff --git a/main.go b/main.go index baeefc107..9102b0366 100644 --- a/main.go +++ b/main.go @@ -13,6 +13,7 @@ import ( // into their command logic. This is because they are run as separate // processes along side of a task. By early importing them we can avoid // additional code being imported and thus reserving memory. + _ "github.com/hashicorp/nomad/client/allocrunner/taskrunner/getter" _ "github.com/hashicorp/nomad/client/logmon" _ "github.com/hashicorp/nomad/drivers/docker/docklog" _ "github.com/hashicorp/nomad/drivers/shared/executor" diff --git a/website/content/docs/job-specification/artifact.mdx b/website/content/docs/job-specification/artifact.mdx index a87b46e3d..e41e2d679 100644 --- a/website/content/docs/job-specification/artifact.mdx +++ b/website/content/docs/job-specification/artifact.mdx @@ -138,10 +138,10 @@ artifact { source = "git::git@bitbucket.org:example/nomad-examples" destination = "local/repo" options { - # Make sure that the Nomad user's known hosts file is populated: - # ssh-keyscan github.com | sudo tee -a /root/.ssh/known_hosts + # Make sure that the system known hosts file is populated: + # ssh-keyscan github.com | sudo tee -a /etc/ssh/ssh_known_hosts # https://github.com/hashicorp/go-getter/issues/55 - sshkey = "${base64encode(file(pathexpand("~/.ssh/id_rsa")))}" + sshkey = "${base64encode(file("/etc/ssh/ssh_known_hosts"))}" } } ``` diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx index b76d37e3b..40927e809 100644 --- a/website/content/docs/upgrade/upgrade-specific.mdx +++ b/website/content/docs/upgrade/upgrade-specific.mdx @@ -13,6 +13,50 @@ upgrade. However, specific versions of Nomad may have more details provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Nomad 1.5.0 + +#### Artifact Download Sandboxing + +Nomad 1.5.0 changes the way [artifacts] are downloaded when specifying an `artifact` +in a task configuration. Previously the Nomad Client would download artifacts +in-process. External commands used to facilitate the download (e.g. `git`, `hg`) +would be run as `root`, and the resulting payload would be owned as `root` in the +allocation's task directory. + +In an effort to improve the resilience and security model of the Nomad Client, +in 1.5.0 artifact downloads occur in a sub-process. Where possible, that +sub-process is run as the `nobody` user, and on modern Linux systems will +be isolated from the filesystem via the kernel's [landlock] capabilitiy. + +Operators are encouraged to ensure jobs making use of artifacts continue to work +as expected. In particular, git-ssh users will need to make sure the system-wide +`/etc/ssh/ssh_known_hosts` file is populated with any necessary remote hosts. +Previously, Nomad's documentation suggested configuring +`/root/.ssh/known_hosts` which would apply only to the `root` user. + +The artifact downloader no longer inherits all environment variables available +to the Nomad Client. The downloader sub-process environment is set as follows on +Linux / macOS: + +``` +PATH=/usr/local/bin:/usr/bin:/bin +TMPDIR=/tmp +``` + +and as follows on Windows: + +``` +TMP=\tmp +TEMP=\tmp +PATH= +HOMEPATH= +HOMEDRIVE= +USERPROFILE= +``` + +Configuration of the artifact downloader should happen through the [`options`][artifact_params] +and [`headers`][artifact_params] fields of the `artifact` block. + ## Nomad 1.4.0 #### Possible Panic During Upgrades @@ -1466,6 +1510,8 @@ state. Once that is done the client can be killed, the `data_dir` should be deleted and then Nomad 0.3.0 can be launched. [api_jobs_parse]: /api-docs/jobs#parse-job +[artifacts]: /docs/job-specification/artifact +[artifact_params]: /docs/job-specification/artifact#artifact-parameters [cgroups2]: https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html [cgroup_parent]: /docs/configuration/client#cgroup_parent [client_artifact]: /docs/configuration/client#artifact-parameters @@ -1530,3 +1576,5 @@ deleted and then Nomad 0.3.0 can be launched. [gh_10446]: https://github.com/hashicorp/nomad/pull/10446#issuecomment-1224833906 [gh_issue]: https://github.com/hashicorp/nomad/issues/new/choose [upgrade process]: /docs/upgrade#upgrade-process +[landlock]: https://docs.kernel.org/userspace-api/landlock.html +