func: Allow custom paths to be added the the getter landlock (#20349)

* func: Allow custom paths to be added the the getter landlock

Fixes: 20315

* fix: slices imports
fix: more meaningful examples
fix: improve documentation
fix: quote error output

.changelog/20315.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+func: Allow custom paths to be added the the getter landlock
+```

client/allocrunner/taskrunner/getter/params.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-getter"
+	"github.com/hashicorp/nomad/helper"
 )
 
 // parameters is encoded by the Nomad client and decoded by the getter sub-process
@@ -22,16 +23,17 @@ import (
 // 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"`
-	DecompressionLimitFileCount int           `json:"decompression_limit_file_count"`
-	DecompressionLimitSize      int64         `json:"decompression_limit_size"`
-	DisableFilesystemIsolation  bool          `json:"disable_filesystem_isolation"`
-	SetEnvironmentVariables     string        `json:"set_environment_variables"`
+	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"`
+	DecompressionLimitFileCount   int           `json:"decompression_limit_file_count"`
+	DecompressionLimitSize        int64         `json:"decompression_limit_size"`
+	DisableFilesystemIsolation    bool          `json:"disable_filesystem_isolation"`
+	FilesystemIsolationExtraPaths []string      `json:"filesystem_isolation_extra_paths"`
+	SetEnvironmentVariables       string        `json:"set_environment_variables"`
 
 	// Artifact
 	Mode        getter.ClientMode   `json:"artifact_mode"`
@@ -98,6 +100,8 @@ func (p *parameters) Equal(o *parameters) bool {
 		return false
 	case p.DisableFilesystemIsolation != o.DisableFilesystemIsolation:
 		return false
+	case !helper.SliceSetEq(p.FilesystemIsolationExtraPaths, o.FilesystemIsolationExtraPaths):
+		return false
 	case p.SetEnvironmentVariables != o.SetEnvironmentVariables:
 		return false
 	case p.Mode != o.Mode:

client/allocrunner/taskrunner/getter/params_test.go

@@ -25,6 +25,11 @@ const paramsAsJSON = `
   "decompression_limit_file_count": 3,
   "decompression_limit_size": 98765,
   "disable_filesystem_isolation": true,
+  "filesystem_isolation_extra_paths": [
+    "f:r:/dev/urandom",
+    "d:rx:/opt/bin",
+    "d:r:/tmp/stash"
+  ],
   "set_environment_variables": "",
   "artifact_mode": 2,
   "artifact_insecure": false,
@@ -47,7 +52,11 @@ var paramsAsStruct = &parameters{
 	DecompressionLimitFileCount: 3,
 	DecompressionLimitSize:      98765,
 	DisableFilesystemIsolation:  true,
-
+	FilesystemIsolationExtraPaths: []string{
+		"f:r:/dev/urandom",
+		"d:rx:/opt/bin",
+		"d:r:/tmp/stash",
+	},
 	Mode:        getter.ClientModeFile,
 	Source:      "https://example.com/file.txt",
 	Destination: "local/out.txt",

client/allocrunner/taskrunner/getter/sandbox.go

@@ -44,16 +44,17 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact
 
 	params := &parameters{
 		// downloader configuration
-		HTTPReadTimeout:             s.ac.HTTPReadTimeout,
-		HTTPMaxBytes:                s.ac.HTTPMaxBytes,
-		GCSTimeout:                  s.ac.GCSTimeout,
-		GitTimeout:                  s.ac.GitTimeout,
-		HgTimeout:                   s.ac.HgTimeout,
-		S3Timeout:                   s.ac.S3Timeout,
-		DecompressionLimitFileCount: s.ac.DecompressionLimitFileCount,
-		DecompressionLimitSize:      s.ac.DecompressionLimitSize,
-		DisableFilesystemIsolation:  s.ac.DisableFilesystemIsolation,
-		SetEnvironmentVariables:     s.ac.SetEnvironmentVariables,
+		HTTPReadTimeout:               s.ac.HTTPReadTimeout,
+		HTTPMaxBytes:                  s.ac.HTTPMaxBytes,
+		GCSTimeout:                    s.ac.GCSTimeout,
+		GitTimeout:                    s.ac.GitTimeout,
+		HgTimeout:                     s.ac.HgTimeout,
+		S3Timeout:                     s.ac.S3Timeout,
+		DecompressionLimitFileCount:   s.ac.DecompressionLimitFileCount,
+		DecompressionLimitSize:        s.ac.DecompressionLimitSize,
+		DisableFilesystemIsolation:    s.ac.DisableFilesystemIsolation,
+		FilesystemIsolationExtraPaths: s.ac.FilesystemIsolationExtraPaths,
+		SetEnvironmentVariables:       s.ac.SetEnvironmentVariables,
 
 		// artifact configuration
 		Mode:        mode,

client/allocrunner/taskrunner/getter/util_default.go

@@ -10,7 +10,7 @@ import (
 )
 
 // lockdown is not implemented by default
-func lockdown(string, string) error {
+func lockdown(string, string, []string) error {
 	return nil
 }
 

client/allocrunner/taskrunner/getter/util_linux.go

@@ -51,7 +51,7 @@ func defaultEnvironment(taskDir string) map[string]string {
 // dir - the task directory
 //
 // Only applies to Linux, when available.
-func lockdown(allocDir, taskDir string) error {
+func lockdown(allocDir, taskDir string, extra []string) error {
 	// landlock not present in the kernel, do not sandbox
 	if !landlock.Available() {
 		return nil
@@ -68,6 +68,13 @@ func lockdown(allocDir, taskDir string) error {
 		landlock.Dir(taskDir, "rwc"),
 	}
 
+	for _, p := range extra {
+		path, err := landlock.ParsePath(p)
+		if err != nil {
+			return err
+		}
+		paths = append(paths, path)
+	}
 	paths = append(paths, additionalFilesForVCS()...)
 	locker := landlock.New(paths...)
 	return locker.Lock(landlock.Mandatory)

client/allocrunner/taskrunner/getter/util_windows.go

@@ -11,7 +11,7 @@ import (
 )
 
 // lockdown is not implemented on Windows
-func lockdown(string, string) error {
+func lockdown(string, string, []string) error {
 	return nil
 }
 

client/allocrunner/taskrunner/getter/z_getter_cmd.go

@@ -34,7 +34,7 @@ func init() {
 
 		// sandbox the host filesystem for this process
 		if !env.DisableFilesystemIsolation {
-			if err := lockdown(env.AllocDir, env.TaskDir); err != nil {
+			if err := lockdown(env.AllocDir, env.TaskDir, env.FilesystemIsolationExtraPaths); err != nil {
 				subproc.Print("failed to sandbox %s process: %v", SubCommand, err)
 				return subproc.ExitFailure
 			}

client/config/artifact.go

@@ -5,6 +5,7 @@ package config
 
 import (
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/dustin/go-humanize"
@@ -25,8 +26,9 @@ type ArtifactConfig struct {
 	DecompressionLimitFileCount int
 	DecompressionLimitSize      int64
 
-	DisableFilesystemIsolation bool
-	SetEnvironmentVariables    string
+	DisableFilesystemIsolation    bool
+	FilesystemIsolationExtraPaths []string
+	SetEnvironmentVariables       string
 }
 
 // ArtifactConfigFromAgent creates a new internal readonly copy of the client
@@ -68,17 +70,19 @@ func ArtifactConfigFromAgent(c *config.ArtifactConfig) (*ArtifactConfig, error)
 	}
 
 	return &ArtifactConfig{
-		HTTPReadTimeout:             httpReadTimeout,
-		HTTPMaxBytes:                int64(httpMaxSize),
-		GCSTimeout:                  gcsTimeout,
-		GitTimeout:                  gitTimeout,
-		HgTimeout:                   hgTimeout,
-		S3Timeout:                   s3Timeout,
-		DecompressionLimitFileCount: *c.DecompressionFileCountLimit,
-		DecompressionLimitSize:      int64(decompressionSizeLimit),
-		DisableFilesystemIsolation:  *c.DisableFilesystemIsolation,
-		SetEnvironmentVariables:     *c.SetEnvironmentVariables,
+		HTTPReadTimeout:               httpReadTimeout,
+		HTTPMaxBytes:                  int64(httpMaxSize),
+		GCSTimeout:                    gcsTimeout,
+		GitTimeout:                    gitTimeout,
+		HgTimeout:                     hgTimeout,
+		S3Timeout:                     s3Timeout,
+		DecompressionLimitFileCount:   *c.DecompressionFileCountLimit,
+		DecompressionLimitSize:        int64(decompressionSizeLimit),
+		DisableFilesystemIsolation:    *c.DisableFilesystemIsolation,
+		FilesystemIsolationExtraPaths: slices.Clone(c.FilesystemIsolationExtraPaths),
+		SetEnvironmentVariables:       *c.SetEnvironmentVariables,
 	}, nil
+
 }
 
 func (a *ArtifactConfig) Copy() *ArtifactConfig {

client/config/artifact_test.go

@@ -129,14 +129,15 @@ func TestArtifactConfig_Copy(t *testing.T) {
 	ci.Parallel(t)
 
 	ac := &ArtifactConfig{
-		HTTPReadTimeout:            time.Minute,
-		HTTPMaxBytes:               1000,
-		GCSTimeout:                 2 * time.Minute,
-		GitTimeout:                 time.Second,
-		HgTimeout:                  time.Hour,
-		S3Timeout:                  5 * time.Minute,
-		DisableFilesystemIsolation: true,
-		SetEnvironmentVariables:    "FOO,BAR",
+		HTTPReadTimeout:               time.Minute,
+		HTTPMaxBytes:                  1000,
+		GCSTimeout:                    2 * time.Minute,
+		GitTimeout:                    time.Second,
+		HgTimeout:                     time.Hour,
+		S3Timeout:                     5 * time.Minute,
+		DisableFilesystemIsolation:    true,
+		FilesystemIsolationExtraPaths: []string{"f:r:/dev/urandom"},
+		SetEnvironmentVariables:       "FOO,BAR",
 	}
 
 	// make sure values are copied.
@@ -151,16 +152,18 @@ func TestArtifactConfig_Copy(t *testing.T) {
 	configCopy.HgTimeout = 2 * time.Hour
 	configCopy.S3Timeout = 10 * time.Minute
 	configCopy.DisableFilesystemIsolation = false
+	configCopy.FilesystemIsolationExtraPaths = []string{"f:rx:/opt/bin/runme"}
 	configCopy.SetEnvironmentVariables = "BAZ"
 
 	must.Eq(t, &ArtifactConfig{
-		HTTPReadTimeout:            time.Minute,
-		HTTPMaxBytes:               1000,
-		GCSTimeout:                 2 * time.Minute,
-		GitTimeout:                 time.Second,
-		HgTimeout:                  time.Hour,
-		S3Timeout:                  5 * time.Minute,
-		DisableFilesystemIsolation: true,
-		SetEnvironmentVariables:    "FOO,BAR",
+		HTTPReadTimeout:               time.Minute,
+		HTTPMaxBytes:                  1000,
+		GCSTimeout:                    2 * time.Minute,
+		GitTimeout:                    time.Second,
+		HgTimeout:                     time.Hour,
+		S3Timeout:                     5 * time.Minute,
+		DisableFilesystemIsolation:    true,
+		FilesystemIsolationExtraPaths: []string{"f:r:/dev/urandom"},
+		SetEnvironmentVariables:       "FOO,BAR",
 	}, ac)
 }

command/agent/agent.go

@@ -898,6 +898,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) {
 	if err != nil {
 		return nil, fmt.Errorf("invalid artifact config: %v", err)
 	}
+
 	conf.Artifact = artifactConfig
 
 	drainConfig, err := clientconfig.DrainConfigFromAgent(agentConfig.Client.Drain)

nomad/structs/config/artifact.go

@@ -6,10 +6,13 @@ package config
 import (
 	"fmt"
 	"math"
+	"slices"
 	"time"
 
 	"github.com/dustin/go-humanize"
+	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/helper/pointer"
+	"github.com/shoenig/go-landlock"
 )
 
 // ArtifactConfig is the configuration specific to the Artifact block
@@ -55,6 +58,10 @@ type ArtifactConfig struct {
 	// read only from specific locations on the host filesystem.
 	DisableFilesystemIsolation *bool `hcl:"disable_filesystem_isolation"`
 
+	// FilesystemIsolationExtraPaths allows extra paths to be included in
+	// the sandbox used by the artifact downloader
+	FilesystemIsolationExtraPaths []string `hcl:"filesystem_isolation_extra_paths"`
+
 	// SetEnvironmentVariables is a comma-separated list of environment
 	// variable names to inherit from the Nomad Client and set in the artifact
 	// download sandbox process.
@@ -66,16 +73,17 @@ func (a *ArtifactConfig) Copy() *ArtifactConfig {
 		return nil
 	}
 	return &ArtifactConfig{
-		HTTPReadTimeout:             pointer.Copy(a.HTTPReadTimeout),
-		HTTPMaxSize:                 pointer.Copy(a.HTTPMaxSize),
-		GCSTimeout:                  pointer.Copy(a.GCSTimeout),
-		GitTimeout:                  pointer.Copy(a.GitTimeout),
-		HgTimeout:                   pointer.Copy(a.HgTimeout),
-		S3Timeout:                   pointer.Copy(a.S3Timeout),
-		DecompressionFileCountLimit: pointer.Copy(a.DecompressionFileCountLimit),
-		DecompressionSizeLimit:      pointer.Copy(a.DecompressionSizeLimit),
-		DisableFilesystemIsolation:  pointer.Copy(a.DisableFilesystemIsolation),
-		SetEnvironmentVariables:     pointer.Copy(a.SetEnvironmentVariables),
+		HTTPReadTimeout:               pointer.Copy(a.HTTPReadTimeout),
+		HTTPMaxSize:                   pointer.Copy(a.HTTPMaxSize),
+		GCSTimeout:                    pointer.Copy(a.GCSTimeout),
+		GitTimeout:                    pointer.Copy(a.GitTimeout),
+		HgTimeout:                     pointer.Copy(a.HgTimeout),
+		S3Timeout:                     pointer.Copy(a.S3Timeout),
+		DecompressionFileCountLimit:   pointer.Copy(a.DecompressionFileCountLimit),
+		DecompressionSizeLimit:        pointer.Copy(a.DecompressionSizeLimit),
+		DisableFilesystemIsolation:    pointer.Copy(a.DisableFilesystemIsolation),
+		FilesystemIsolationExtraPaths: slices.Clone(a.FilesystemIsolationExtraPaths),
+		SetEnvironmentVariables:       pointer.Copy(a.SetEnvironmentVariables),
 	}
 }
 
@@ -86,7 +94,7 @@ func (a *ArtifactConfig) Merge(o *ArtifactConfig) *ArtifactConfig {
 	case o == nil:
 		return a.Copy()
 	default:
-		return &ArtifactConfig{
+		result := &ArtifactConfig{
 			HTTPReadTimeout:             pointer.Merge(a.HTTPReadTimeout, o.HTTPReadTimeout),
 			HTTPMaxSize:                 pointer.Merge(a.HTTPMaxSize, o.HTTPMaxSize),
 			GCSTimeout:                  pointer.Merge(a.GCSTimeout, o.GCSTimeout),
@@ -98,6 +106,14 @@ func (a *ArtifactConfig) Merge(o *ArtifactConfig) *ArtifactConfig {
 			DisableFilesystemIsolation:  pointer.Merge(a.DisableFilesystemIsolation, o.DisableFilesystemIsolation),
 			SetEnvironmentVariables:     pointer.Merge(a.SetEnvironmentVariables, o.SetEnvironmentVariables),
 		}
+
+		if o.FilesystemIsolationExtraPaths != nil {
+			result.FilesystemIsolationExtraPaths = slices.Clone(o.FilesystemIsolationExtraPaths)
+		} else {
+			result.FilesystemIsolationExtraPaths = slices.Clone(a.FilesystemIsolationExtraPaths)
+		}
+
+		return result
 	}
 }
 
@@ -124,6 +140,8 @@ func (a *ArtifactConfig) Equal(o *ArtifactConfig) bool {
 		return false
 	case !pointer.Eq(a.DisableFilesystemIsolation, o.DisableFilesystemIsolation):
 		return false
+	case !helper.SliceSetEq(a.FilesystemIsolationExtraPaths, o.FilesystemIsolationExtraPaths):
+		return false
 	case !pointer.Eq(a.SetEnvironmentVariables, o.SetEnvironmentVariables):
 		return false
 	}
@@ -209,6 +227,12 @@ func (a *ArtifactConfig) Validate() error {
 		return fmt.Errorf("disable_filesystem_isolation must be set")
 	}
 
+	for _, p := range a.FilesystemIsolationExtraPaths {
+		if _, err := landlock.ParsePath(p); err != nil {
+			return fmt.Errorf("filesystem_isolation_extra_paths contains invalid lockdown path %q", p)
+		}
+	}
+
 	if a.SetEnvironmentVariables == nil {
 		return fmt.Errorf("set_environment_variables must be set")
 	}
@@ -254,6 +278,9 @@ func DefaultArtifactConfig() *ArtifactConfig {
 		// Toggle for disabling filesystem isolation, where available.
 		DisableFilesystemIsolation: pointer.Of(false),
 
+		// No Filesystem Isolation Extra Locations by default
+		FilesystemIsolationExtraPaths: nil,
+
 		// No environment variables are inherited from Client by default.
 		SetEnvironmentVariables: pointer.Of(""),
 	}