Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading