template: sandbox template rendering

The Nomad client renders templates in the same privileged process used for most
other client operations. During internal testing, we discovered that a malicious
task can create a symlink that can cause template rendering to read and write to
arbitrary files outside the allocation sandbox. Because the Nomad agent can be
restarted without restarting tasks, we can't simply check that the path is safe
at the time we write without encountering a time-of-check/time-of-use race.

To protect Nomad client hosts from this attack, we'll now read and write
templates in a subprocess:

* On Linux/Unix, this subprocess is sandboxed via chroot to the allocation
  directory. This requires that Nomad is running as a privileged process. A
  non-root Nomad agent will warn that it cannot sandbox the template renderer.

* On Windows, this process is sandboxed via a Windows AppContainer which has
  been granted access to only to the allocation directory. This does not require
  special privileges on Windows. (Creating symlinks in the first place can be
  prevented by running workloads as non-Administrator or
  non-ContainerAdministrator users.)

Both sandboxes cause encountered symlinks to be evaluated in the context of the
sandbox, which will result in a "file not found" or "access denied" error,
depending on the platform. This change will also require an update to
Consul-Template to allow callers to inject a custom `ReaderFunc` and
`RenderFunc`.

This design is intended as a workaround to allow us to fix this bug without
creating backwards compatibility issues for running tasks. A future version of
Nomad may introduce a read-only mount specifically for templates and artifacts
so that tasks cannot write into the same location that the Nomad agent is.

Fixes: #19888
Fixes: CVE-2024-1329

.changelog/19888.txt

@@ -0,0 +1,3 @@
+```release-note:security
+template: Fixed a bug where symlinks could force templates to read and write to arbitrary locations (CVE-2024-1329)
+```

.github/workflows/test-windows.yml

@@ -89,7 +89,9 @@ jobs:
             github.com/hashicorp/nomad/drivers/docker \
             github.com/hashicorp/nomad/client/lib/fifo \
             github.com/hashicorp/nomad/client/logmon \
-            github.com/hashicorp/nomad/client/allocrunner/taskrunner/template
+            github.com/hashicorp/nomad/client/allocrunner/taskrunner/template \
+            github.com/hashicorp/nomad/helper/winappcontainer \
+            github.com/hashicorp/nomad/helper/winexec
       - uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
         with:
           name: results.xml

client/allocrunner/taskrunner/template/renderer/doc.go

@@ -0,0 +1,13 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package renderer
+
+// This package implements a "hidden" command `nomad template-render`, similarly
+// to how we implement logmon, getter, docklog, and executor. This package's
+// init() function is evaluated before Nomad's top-level main.go gets a chance
+// to parse arguments. This bypasses loading in any behaviors other than the
+// small bit of code here.
+//
+// This command and its subcommands `write` and `read` are only invoked by the
+// template runner. See the parent package for the callers.

client/allocrunner/taskrunner/template/renderer/template_sandbox_default.go

@@ -0,0 +1,39 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+//go:build !windows
+
+package renderer
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// sandbox is the non-Windows sandbox implementation, which relies on chroot.
+// Although chroot is not an appropriate boundary for tasks (implicitly
+// untrusted), here the only code that's executing is Nomad itself. Returns the
+// new destPath inside the chroot.
+func sandbox(sandboxPath, destPath string) (string, error) {
+
+	err := syscall.Chroot(sandboxPath)
+	if err != nil {
+		// if the user is running in unsupported non-root configuration, we
+		// can't build the sandbox, but need to handle this gracefully
+		fmt.Fprintf(os.Stderr, "template-render sandbox %q not available: %v",
+			sandboxPath, err)
+		return destPath, nil
+	}
+
+	destPath, err = filepath.Rel(sandboxPath, destPath)
+	if err != nil {
+		return "", fmt.Errorf("could not find destination path relative to chroot: %w", err)
+	}
+	if !filepath.IsAbs(destPath) {
+		destPath = "/" + destPath
+	}
+
+	return destPath, nil
+}

client/allocrunner/taskrunner/template/renderer/template_sandbox_windows.go

@@ -0,0 +1,14 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+//go:build windows
+
+package renderer
+
+// sandbox is the Windows-specific sandbox implementation. Under Windows,
+// symlinks can only be written by the Administrator (including the
+// ContainerAdministrator user unfortunately used as the default for Docker). So
+// our sandboxing is done by creating an AppContainer in the caller.
+func sandbox(_, destPath string) (string, error) {
+	return destPath, nil
+}

client/allocrunner/taskrunner/template/renderer/z_template_render.go

@@ -0,0 +1,152 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package renderer
+
+import (
+	"bytes"
+	"flag"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"strconv"
+
+	"github.com/hashicorp/consul-template/renderer"
+)
+
+const (
+	// DefaultFilePerms are the default file permissions for files rendered onto
+	// disk when a specific file permission has not already been specified.
+	DefaultFilePerms = 0o644
+
+	ExitDidRender           = 0
+	ExitError               = 1
+	ExitWouldRenderButDidnt = 117 // something unmistakeably belonging to Nomad
+)
+
+// This init() must be initialized last in package required by the child plugin
+// process. It's recommended to avoid any other `init()` or inline any necessary
+// calls here. See eeaa95d commit message for more details.
+func init() {
+	if len(os.Args) > 1 && os.Args[1] == "template-render" {
+
+		if len(os.Args) <= 3 {
+			// note: we don't use logger here as any message we send will get
+			// wrapped by CT's own logger, but it's important to keep Stderr and
+			// Stdout separate so that "read" has a clean output.
+			fmt.Fprintln(os.Stderr, `expected "read" or "write" argument`)
+		}
+
+		switch os.Args[2] {
+		case "read":
+			err := readTemplate()
+			if err != nil {
+				fmt.Fprintln(os.Stderr, err.Error())
+				os.Exit(ExitError)
+			}
+			os.Exit(0)
+
+		case "write":
+			result, err := writeTemplate()
+			if err != nil {
+				fmt.Fprintln(os.Stderr, err.Error())
+				os.Exit(ExitError)
+			}
+
+			if result.DidRender {
+				os.Exit(ExitDidRender)
+			}
+			if result.WouldRender {
+				os.Exit(ExitWouldRenderButDidnt)
+			}
+			os.Exit(ExitError)
+		default:
+			fmt.Fprintln(os.Stderr, `expected "read" or "write" argument`)
+			os.Exit(ExitError)
+		}
+	}
+}
+
+func readTemplate() error {
+	var (
+		sandboxPath, sourcePath string
+		err                     error
+	)
+
+	flags := flag.NewFlagSet("template-render", flag.ExitOnError)
+	flags.StringVar(&sandboxPath, "sandbox-path", "", "")
+	flags.StringVar(&sourcePath, "source-path", "", "")
+	flags.Parse(os.Args[3:])
+
+	sourcePath, err = sandbox(sandboxPath, sourcePath) // platform-specific sandboxing
+	if err != nil {
+		return fmt.Errorf("failed to sandbox alloc dir %q: %w", sandboxPath, err)
+	}
+
+	f, err := os.Open(sourcePath)
+	if err != nil {
+		return fmt.Errorf("failed to open source file %q: %w", sourcePath, err)
+	}
+	defer f.Close()
+
+	_, err = io.Copy(os.Stdout, f)
+	return err
+}
+
+func writeTemplate() (*renderer.RenderResult, error) {
+
+	var (
+		sandboxPath, destPath, perms, user, group string
+	)
+
+	flags := flag.NewFlagSet("template-render", flag.ExitOnError)
+	flags.StringVar(&sandboxPath, "sandbox-path", "", "")
+	flags.StringVar(&destPath, "dest-path", "", "")
+	flags.StringVar(&perms, "perms", "", "")
+	flags.StringVar(&user, "user", "", "")
+	flags.StringVar(&group, "group", "", "")
+
+	flags.Parse(os.Args[3:])
+
+	contents := new(bytes.Buffer)
+	_, err := io.Copy(contents, os.Stdin)
+	if err != nil {
+		return nil, fmt.Errorf("failed reading template contents: %w", err)
+	}
+
+	destPath, err = sandbox(sandboxPath, destPath) // platform-specific sandboxing
+	if err != nil {
+		return nil, fmt.Errorf("failed to sandbox alloc dir %q: %w", sandboxPath, err)
+	}
+
+	// perms must parse into a valid file permission
+	fileMode := os.FileMode(DefaultFilePerms)
+	if perms != "" {
+		fileModeInt, err := strconv.ParseUint(perms, 8, 32)
+		if err != nil {
+			return nil, fmt.Errorf(
+				"Invalid file mode %q: Must be a valid octal number: %w", perms, err)
+
+		}
+		fileMode = fs.FileMode(fileModeInt)
+		if fileMode.Perm() != fileMode {
+			return nil, fmt.Errorf(
+				"Invalid file mode %q: Must be a valid Unix permission: %w", perms, err)
+		}
+	}
+
+	input := &renderer.RenderInput{
+		Backup:         false,
+		Contents:       contents.Bytes(),
+		CreateDestDirs: true,
+		Dry:            false,
+		DryStream:      nil,
+		Path:           destPath,
+		Perms:          fileMode,
+		User:           user,
+		Group:          group,
+	}
+
+	return renderer.Render(input)
+}