diff --git a/.circleci/config.yml b/.circleci/config.yml index 5ff002b6ec5..7a4120c0798 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -948,63 +948,6 @@ jobs: path: /tmp/test-reports - store_artifacts: path: /tmp/test-reports - build-darwin-binaries: - macos: - xcode: 11.3.1 - working_directory: ~/go/src/github.com/hashicorp/nomad - steps: - - checkout - - run: - command: echo 'export PATH="${GOPATH}/bin:${HOME}/goinstall/go/bin:$PATH"' >> ${BASH_ENV} - name: configure PATH - - run: - command: | - set -x - - echo installing golang ${GOLANG_VERSION} - - case "${OSTYPE}" in - linux*) os="linux" ;; - darwin*) os="darwin" ;; - msys*) os="windows" ;; - *) echo "unknown os: ${OSTYPE}"; exit 1 ;; - esac - - if [[ "${os}" != "windows" ]] - then - sudo rm -rf ~/goinstall/go - sudo mkdir -p ~/goinstall - curl -SL --fail -o /tmp/golang.tar.gz https://dl.google.com/go/go${GOLANG_VERSION}.${os}-amd64.tar.gz - sudo tar -C ~/goinstall -xzf /tmp/golang.tar.gz - rm -rf /tmp/golang.tar.gz - else - rm -rf ~/goinstall/go - mkdir -p ~/goinstall - curl -SL --fail -o /tmp/go.zip https://dl.google.com/go/go${GOLANG_VERSION}.windows-amd64.zip - unzip -q -o /tmp/go.zip -d ~/goinstall - rm -rf /tmp/go.zip - fi - name: Install golang - - run: - command: source ${BASH_ENV} && make deps - - run: - command: brew install protobuf - - run: - command: sudo -E PATH="$GOPATH/bin:${HOME}/goinstall/go/bin:$PATH" make generate-structs - - run: - command: source ${BASH_ENV} && make pkg/darwin_amd64.zip - - store_artifacts: - destination: /builds/nomad_darwin_amd64.zip - path: pkg/darwin_amd64.zip - environment: - - GIT_PAGER: cat - - GOLANG_VERSION: 1.14.6 - - GOMAXPROCS: 1 - - GOPATH: /Users/distiller/go - - GOTESTSUM_JSONFILE: /tmp/test-reports/testjsonfile.json - - GOTESTSUM_JUNITFILE: /tmp/test-reports/results.xml - - NOMAD_SLOW_TEST: 1 - - PAGER: cat test-docker: machine: image: circleci/classic:201808-01 @@ -1307,13 +1250,6 @@ workflows: - /^.-ui\b.*/ - /^docs-.*/ - stable-website - - build-darwin-binaries: - filters: - branches: - ignore: - - /^.-ui\b.*/ - - /^docs-.*/ - - stable-website - test-windows: filters: branches: diff --git a/.circleci/config/workflows/build-test.yml b/.circleci/config/workflows/build-test.yml index edc16ecb713..2efea1f40f4 100644 --- a/.circleci/config/workflows/build-test.yml +++ b/.circleci/config/workflows/build-test.yml @@ -15,8 +15,6 @@ jobs: - /^docs-.*/ - stable-website - - build-darwin-binaries: - filters: *backend_branches_filter - test-windows: filters: *backend_branches_filter diff --git a/CHANGELOG.md b/CHANGELOG.md index 34336f15e33..f8c59475420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 0.11.5 (October 21, 2020) + +SECURITY: + + * artifact: Fixed a bug where interpolation can be used in the artifact `destination` field to write artifact payloads outside the allocation directory. CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + * template: Fixed a bug where interpolation can be used in the template `source` and `destination` fields to read or write files outside the allocation directory even when `disable_file_sandbox` was set to `false` (the default). CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + * template: Fixed a bug where the `disable_file_sandbox` configuration was only respected for the template `file` function and not the template `source` and `destination` fields. CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + ## 0.11.4 (August 7, 2020) SECURITY: @@ -140,6 +148,14 @@ BUG FIXES: * scheduler: Fixed a bug where changes to task group `shutdown_delay` were not persisted or displayed in plan output [[GH-7618](https://github.com/hashicorp/nomad/issues/7618)] * ui: Fixed handling of multi-byte unicode characters in allocation log view [[GH-7470](https://github.com/hashicorp/nomad/issues/7470)] [[GH-7551](https://github.com/hashicorp/nomad/pull/7551)] +## 0.10.6 (October 21, 2020) + +SECURITY: + + * artifact: _Backport from v0.12.6_ - Fixed a bug where interpolation can be used in the artifact `destination` field to write artifact payloads outside the allocation directory. CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + * template: _Backport from v0.12.6_ - Fixed a bug where interpolation can be used in the template `source` and `destination` fields to read or write files outside the allocation directory even when `disable_file_sandbox` was set to `false` (the default). CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + * template: _Backport from v0.12.6_ - Fixed a bug where the `disable_file_sandbox` configuration was only respected for the template `file` function and not the template `source` and `destination` fields. CVE-2020-27195 [[GH-9129](https://github.com/hashicorp/nomad/issues/9129)] + ## 0.10.5 (March 24, 2020) SECURITY: diff --git a/client/allocrunner/taskrunner/getter/getter.go b/client/allocrunner/taskrunner/getter/getter.go index c896d6f9292..f40d4f306ba 100644 --- a/client/allocrunner/taskrunner/getter/getter.go +++ b/client/allocrunner/taskrunner/getter/getter.go @@ -1,13 +1,14 @@ package getter import ( + "errors" "fmt" "net/url" - "path/filepath" "strings" "sync" gg "github.com/hashicorp/go-getter" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" ) @@ -96,8 +97,12 @@ func GetArtifact(taskEnv EnvReplacer, artifact *structs.TaskArtifact, taskDir st return newGetError(artifact.GetterSource, err, false) } - // Download the artifact - dest := filepath.Join(taskDir, artifact.RelativeDest) + // Verify the destination is still in the task sandbox after interpolation + dest, err := helper.GetPathInSandbox(taskDir, artifact.RelativeDest) + if err != nil { + 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 diff --git a/client/allocrunner/taskrunner/getter/getter_test.go b/client/allocrunner/taskrunner/getter/getter_test.go index 2816be10b1e..cfac7a0106e 100644 --- a/client/allocrunner/taskrunner/getter/getter_test.go +++ b/client/allocrunner/taskrunner/getter/getter_test.go @@ -93,6 +93,36 @@ func TestGetArtifact_File_RelativeDest(t *testing.T) { } } +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, err := ioutil.TempDir("", "nomad-test") + if err != nil { + t.Fatalf("failed to make temp directory: %v", err) + } + defer os.RemoveAll(taskDir) + + // 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 + err = GetArtifact(taskEnv, artifact, taskDir) + 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{ diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go index a0e7c2bbd6b..fa5881abbbf 100644 --- a/client/allocrunner/taskrunner/template/template.go +++ b/client/allocrunner/taskrunner/template/template.go @@ -28,10 +28,6 @@ const ( // consulTemplateSourceName is the source name when using the TaskHooks. consulTemplateSourceName = "Template" - // hostSrcOption is the Client option that determines whether the template - // source may be from the host - hostSrcOption = "template.allow_host_source" - // missingDepEventLimit is the number of missing dependencies that will be // logged before we switch to showing just the number of missing // dependencies. @@ -546,25 +542,27 @@ func maskProcessEnv(env map[string]string) map[string]string { // parseTemplateConfigs converts the tasks templates in the config into // consul-templates func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.TemplateConfig]*structs.Template, error) { - allowAbs := config.ClientConfig.ReadBoolDefault(hostSrcOption, true) + sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox taskEnv := config.EnvBuilder.Build() ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates)) for _, tmpl := range config.Templates { var src, dest string + var err error if tmpl.SourcePath != "" { - if filepath.IsAbs(tmpl.SourcePath) { - if !allowAbs { - return nil, fmt.Errorf("Specifying absolute template paths disallowed by client config: %q", tmpl.SourcePath) - } - - src = tmpl.SourcePath - } else { - src = filepath.Join(config.TaskDir, taskEnv.ReplaceEnv(tmpl.SourcePath)) + src = taskEnv.ReplaceEnv(tmpl.SourcePath) + src, err = helper.GetPathInSandbox(config.TaskDir, src) + if err != nil && sandboxEnabled { + return nil, fmt.Errorf("template source path escapes alloc directory") } } + if tmpl.DestPath != "" { - dest = filepath.Join(config.TaskDir, taskEnv.ReplaceEnv(tmpl.DestPath)) + dest = taskEnv.ReplaceEnv(tmpl.DestPath) + dest, err = helper.GetPathInSandbox(config.TaskDir, dest) + if err != nil && sandboxEnabled { + return nil, fmt.Errorf("template destination path escapes alloc directory") + } } ct := ctconf.DefaultTemplateConfig() @@ -574,7 +572,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa ct.LeftDelim = &tmpl.LeftDelim ct.RightDelim = &tmpl.RightDelim ct.FunctionBlacklist = config.ClientConfig.TemplateConfig.FunctionBlacklist - if !config.ClientConfig.TemplateConfig.DisableSandbox { + if sandboxEnabled { ct.SandboxPath = &config.TaskDir } diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 3b51af3f4b3..7a15835ae39 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -380,7 +380,11 @@ func TestTaskTemplateManager_HostPath(t *testing.T) { } harness := newTestHarness(t, []*structs.Template{template}, false, false) - harness.start(t) + harness.config.TemplateConfig.DisableSandbox = true + err = harness.startWithErr() + if err != nil { + t.Fatalf("couldn't setup initial harness: %v", err) + } defer harness.stop() // Wait for the unblock @@ -403,12 +407,46 @@ func TestTaskTemplateManager_HostPath(t *testing.T) { // Change the config to disallow host sources harness = newTestHarness(t, []*structs.Template{template}, false, false) - harness.config.Options = map[string]string{ - hostSrcOption: "false", + err = harness.startWithErr() + if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") { + t.Fatalf("Expected absolute template path disallowed for %q: %v", + template.SourcePath, err) + } + + template.SourcePath = "../../../../../../" + file + harness = newTestHarness(t, []*structs.Template{template}, false, false) + err = harness.startWithErr() + if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") { + t.Fatalf("Expected directory traversal out of %q disallowed for %q: %v", + harness.taskDir, template.SourcePath, err) + } + + // Build a new task environment + a := mock.Alloc() + task := a.Job.TaskGroups[0].Tasks[0] + task.Name = TestTaskName + task.Meta = map[string]string{"ESCAPE": "../"} + + template.SourcePath = "${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}" + file + harness = newTestHarness(t, []*structs.Template{template}, false, false) + harness.envBuilder = taskenv.NewBuilder(harness.node, a, task, "global") + err = harness.startWithErr() + if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") { + t.Fatalf("Expected directory traversal out of %q via interpolation disallowed for %q: %v", + harness.taskDir, template.SourcePath, err) } - if err := harness.startWithErr(); err == nil || !strings.Contains(err.Error(), "absolute") { - t.Fatalf("Expected absolute template path disallowed: %v", err) + + // Test with desination too + template.SourcePath = f.Name() + template.DestPath = "../../../../../../" + file + harness = newTestHarness(t, []*structs.Template{template}, false, false) + harness.envBuilder = taskenv.NewBuilder(harness.node, a, task, "global") + err = harness.startWithErr() + if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") { + t.Fatalf("Expected directory traversal out of %q via interpolation disallowed for %q: %v", + harness.taskDir, template.SourcePath, err) } + } func TestTaskTemplateManager_Unblock_Static(t *testing.T) { diff --git a/helper/funcs.go b/helper/funcs.go index c75294a1b69..a0b6d1c2b3b 100644 --- a/helper/funcs.go +++ b/helper/funcs.go @@ -3,6 +3,7 @@ package helper import ( "crypto/sha512" "fmt" + "path/filepath" "reflect" "regexp" "strings" @@ -461,3 +462,22 @@ func RemoveEqualFold(xs *[]string, search string) { } } } + +// GetPathInSandbox returns a cleaned path inside the sandbox directory +// (typically this will be the allocation directory). Relative paths will be +// joined to the sandbox directory. Returns an error if the path escapes the +// sandbox directory. +func GetPathInSandbox(sandboxDir, path string) (string, error) { + if !filepath.IsAbs(path) { + path = filepath.Join(sandboxDir, path) + } + path = filepath.Clean(path) + rel, err := filepath.Rel(sandboxDir, path) + if err != nil { + return path, err + } + if strings.HasPrefix(rel, "..") { + return path, fmt.Errorf("%q escapes sandbox directory", path) + } + return path, nil +} diff --git a/helper/funcs_test.go b/helper/funcs_test.go index cff3e9c21cf..84be51cca95 100644 --- a/helper/funcs_test.go +++ b/helper/funcs_test.go @@ -5,6 +5,8 @@ import ( "reflect" "sort" "testing" + + "github.com/stretchr/testify/require" ) func TestSliceStringIsSubset(t *testing.T) { @@ -157,3 +159,72 @@ func BenchmarkCleanEnvVar(b *testing.B) { CleanEnvVar(in, replacement) } } + +func TestGetPathInSandbox(t *testing.T) { + cases := []struct { + name string + path string + dir string + expected string + expectedErr string + }{ + { + name: "ok absolute path inside sandbox", + path: "/alloc/safe", + dir: "/alloc", + expected: "/alloc/safe", + }, + { + name: "ok relative path inside sandbox", + path: "./safe", + dir: "/alloc", + expected: "/alloc/safe", + }, + { + name: "ok relative path traversal constrained to sandbox", + path: "../../alloc/safe", + dir: "/alloc", + expected: "/alloc/safe", + }, + { + name: "ok absolute path traversal constrained to sandbox", + path: "/../alloc/safe", + dir: "/alloc", + expected: "/alloc/safe", + }, + { + name: "fail absolute path outside sandbox", + path: "/unsafe", + dir: "/alloc", + expected: "/unsafe", + expectedErr: "\"/unsafe\" escapes sandbox directory", + }, + { + name: "fail relative path traverses outside sandbox", + path: "../../../unsafe", + dir: "/alloc", + expected: "/unsafe", + expectedErr: "\"/unsafe\" escapes sandbox directory", + }, + { + name: "fail absolute path tries to transverse outside sandbox", + path: "/alloc/../unsafe", + dir: "/alloc", + expected: "/unsafe", + expectedErr: "\"/unsafe\" escapes sandbox directory", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + caseMsg := fmt.Sprintf("path: %v\ndir: %v", tc.path, tc.dir) + escapes, err := GetPathInSandbox(tc.dir, tc.path) + if tc.expectedErr != "" { + require.EqualError(t, err, tc.expectedErr, caseMsg) + } else { + require.NoError(t, err, caseMsg) + } + require.Equal(t, tc.expected, escapes, caseMsg) + }) + } +} diff --git a/version/version.go b/version/version.go index ab1e6b52d43..baeca72dcb5 100644 --- a/version/version.go +++ b/version/version.go @@ -11,7 +11,7 @@ var ( GitDescribe string // The main version number that is being run at the moment. - Version = "0.11.4" + Version = "0.11.5" // A pre-release marker for the version. If this is "" (empty string) // then it means that it is a final release. Otherwise, this is a pre-release