Skip to content

Commit

Permalink
func: Allow custom paths to be added the the getter landlock (#20349)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
astudentofblake authored Apr 11, 2024
1 parent 5612ab4 commit 7b7ed12
Show file tree
Hide file tree
Showing 14 changed files with 229 additions and 71 deletions.
3 changes: 3 additions & 0 deletions .changelog/20315.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
func: Allow custom paths to be added the the getter landlock
```
24 changes: 14 additions & 10 deletions client/allocrunner/taskrunner/getter/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"`
Expand Down Expand Up @@ -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:
Expand Down
11 changes: 10 additions & 1 deletion client/allocrunner/taskrunner/getter/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
Expand Down
21 changes: 11 additions & 10 deletions client/allocrunner/taskrunner/getter/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/taskrunner/getter/util_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

// lockdown is not implemented by default
func lockdown(string, string) error {
func lockdown(string, string, []string) error {
return nil
}

Expand Down
9 changes: 8 additions & 1 deletion client/allocrunner/taskrunner/getter/util_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/taskrunner/getter/util_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

// lockdown is not implemented on Windows
func lockdown(string, string) error {
func lockdown(string, string, []string) error {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/taskrunner/getter/z_getter_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 16 additions & 12 deletions client/config/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package config

import (
"fmt"
"slices"
"time"

"github.com/dustin/go-humanize"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 19 additions & 16 deletions client/config/artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
1 change: 1 addition & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 38 additions & 11 deletions nomad/structs/config/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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),
}
}

Expand All @@ -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),
Expand All @@ -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
}
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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(""),
}
Expand Down
Loading

0 comments on commit 7b7ed12

Please sign in to comment.