Skip to content

Commit

Permalink
artifact: protect against unbounded artifact decompression (1.4.x) (#…
Browse files Browse the repository at this point in the history
…16126) (#16158)

* artifact: protect against unbounded artifact decompression

This PR enables mitigations provided by go-getter against payloads which
decompress into an unbounded size or file count.

There are two new client config options under the artifact block:

artifact.decompression_size_limit (e.g. "10GB") - the maximum amount of
data that will be decompressed before triggering an error and cancelling
the operation

artifact.decompression_file_count_limit (e.g. 1024) - the maximum number
of files that will be decompressed before triggering ana error and
cancelling the operation.

* fixup CR comments

* deps: update to go-getter 1.7.0
  • Loading branch information
shoenig authored and tgross committed Feb 14, 2023
1 parent 6c90f7f commit 3bf45af
Show file tree
Hide file tree
Showing 11 changed files with 810 additions and 195 deletions.
3 changes: 3 additions & 0 deletions .changelog/16126.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
artifact: Provide mitigations against unbounded artifact decompression
```
1 change: 1 addition & 0 deletions client/alloc_watcher_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestPrevAlloc_StreamAllocDir_TLS(t *testing.T) {
CertFile: clientCertFn,
KeyFile: clientKeyFn,
}

c.Client.Enabled = true
c.Client.Servers = []string{server.GetConfig().RPCAddr.String()}
}
Expand Down
6 changes: 5 additions & 1 deletion client/allocrunner/taskrunner/getter/getter.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ func (g *Getter) GetArtifact(taskEnv interfaces.EnvReplacer, artifact *structs.T
}

headers := getHeaders(taskEnv, artifact.GetterHeaders)

if err := g.getClient(ggURL, headers, mode, dest).Get(); err != nil {
return newGetError(ggURL, err, true)
}
Expand All @@ -99,8 +100,11 @@ func (g *Getter) getClient(src string, headers http.Header, mode gg.ClientMode,
Umask: 060000000,
Getters: g.createGetters(headers),

// This will prevent copying or writing files through symlinks
// This will prevent copying or writing files through symlinks.
DisableSymlinks: true,

// This will protect against decompression bombs.
Decompressors: gg.LimitedDecompressors(g.config.DecompressionLimitFileCount, g.config.DecompressionLimitSize),
}
}

Expand Down
30 changes: 23 additions & 7 deletions client/allocrunner/taskrunner/getter/getter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,36 @@ func removeAllT(t *testing.T, path string) {
}

func TestGetter_getClient(t *testing.T) {
const fileCountLimit = 555
const fileSizeLimit = int64(666)
getter := NewGetter(hclog.NewNullLogger(), &clientconfig.ArtifactConfig{
HTTPReadTimeout: time.Minute,
HTTPMaxBytes: 100_000,
GCSTimeout: 1 * time.Minute,
GitTimeout: 2 * time.Minute,
HgTimeout: 3 * time.Minute,
S3Timeout: 4 * time.Minute,
HTTPReadTimeout: time.Minute,
HTTPMaxBytes: 100_000,
GCSTimeout: 1 * time.Minute,
GitTimeout: 2 * time.Minute,
HgTimeout: 3 * time.Minute,
S3Timeout: 4 * time.Minute,
DecompressionLimitFileCount: fileCountLimit,
DecompressionLimitSize: fileSizeLimit,
})

client := getter.getClient("src", nil, gg.ClientModeAny, "dst")

t.Run("check symlink config", func(t *testing.T) {
require.True(t, client.DisableSymlinks)
})

t.Run("check file size limits", func(t *testing.T) {
require.Equal(t, fileSizeLimit, client.Decompressors["zip"].(*gg.ZipDecompressor).FileSizeLimit)
require.Equal(t, fileCountLimit, client.Decompressors["zip"].(*gg.ZipDecompressor).FilesLimit)

require.Equal(t, fileSizeLimit, client.Decompressors["tar.gz"].(*gg.TarGzipDecompressor).FileSizeLimit)
require.Equal(t, fileCountLimit, client.Decompressors["tar.gz"].(*gg.TarGzipDecompressor).FilesLimit)

require.Equal(t, fileSizeLimit, client.Decompressors["xz"].(*gg.XzDecompressor).FileSizeLimit)
// xz does not support files count limit
})

t.Run("check http config", func(t *testing.T) {
require.True(t, client.Getters["http"].(*gg.HttpGetter).XTerraformGetDisabled)
require.Equal(t, time.Minute, client.Getters["http"].(*gg.HttpGetter).ReadTimeout)
Expand Down Expand Up @@ -155,7 +171,7 @@ func TestGetArtifact_getHeaders(t *testing.T) {
}

func TestGetArtifact_Headers(t *testing.T) {
file := "output.txt"
const file = "output.txt"

// Create the test server with a handler that will validate headers are set.
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down
12 changes: 8 additions & 4 deletions client/allocrunner/taskrunner/getter/testing.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//go:build !release
// +build !release

package getter

Expand All @@ -8,12 +7,17 @@ import (

"github.com/hashicorp/go-hclog"
clientconfig "github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/helper/pointer"
"github.com/hashicorp/nomad/nomad/structs/config"
"github.com/stretchr/testify/require"
"github.com/shoenig/test/must"
)

// TestDefaultGetter creates a Getter suitable for unit test cases.
func TestDefaultGetter(t *testing.T) *Getter {
getterConf, err := clientconfig.ArtifactConfigFromAgent(config.DefaultArtifactConfig())
require.NoError(t, err)
defaultConfig := config.DefaultArtifactConfig()
defaultConfig.DecompressionSizeLimit = pointer.Of("1MB")
defaultConfig.DecompressionFileCountLimit = pointer.Of(10)
getterConf, err := clientconfig.ArtifactConfigFromAgent(defaultConfig)
must.NoError(t, err)
return NewGetter(hclog.NewNullLogger(), getterConf)
}
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/task_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func (tr *TaskRunner) initHooks() {
// Create the task directory hook. This is run first to ensure the
// directory path exists for other hooks.
alloc := tr.Alloc()

tr.runnerHooks = []interfaces.TaskHook{
newValidateHook(tr.clientConfig, hookLogger),
newTaskDirHook(tr, hookLogger),
Expand Down
12 changes: 12 additions & 0 deletions client/config/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ type ArtifactConfig struct {
GitTimeout time.Duration
HgTimeout time.Duration
S3Timeout time.Duration

DecompressionLimitSize int64
DecompressionLimitFileCount int
}

// ArtifactConfigFromAgent creates a new internal readonly copy of the client
Expand Down Expand Up @@ -61,6 +64,15 @@ func ArtifactConfigFromAgent(c *config.ArtifactConfig) (*ArtifactConfig, error)
}
newConfig.S3Timeout = t

s, err = humanize.ParseBytes(*c.DecompressionSizeLimit)
if err != nil {
return nil, fmt.Errorf("error parsing DecompressionLimitSize: %w", err)
}
newConfig.DecompressionLimitSize = int64(s)

// no parsing its just an int
newConfig.DecompressionLimitFileCount = *c.DecompressionFileCountLimit

return newConfig, nil
}

Expand Down
102 changes: 47 additions & 55 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ replace github.com/hashicorp/nomad/api => ./api

require (
github.com/LK4D4/joincontext v0.0.0-20171026170139-1724345da6d5
github.com/Microsoft/go-winio v0.4.15-0.20200113171025-3fe6c5262873
github.com/Microsoft/go-winio v0.4.17
github.com/NYTimes/gziphandler v1.0.1
github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e
github.com/armon/go-metrics v0.4.0
github.com/aws/aws-sdk-go v1.44.84
github.com/aws/aws-sdk-go v1.44.122
github.com/boltdb/bolt v1.3.1
github.com/container-storage-interface/spec v1.4.0
github.com/containerd/go-cni v0.0.0-20190904155053-d20b7eebc7ee
github.com/containernetworking/cni v0.7.2-0.20190612152420-dc953e2fd91f
github.com/containernetworking/plugins v0.7.3-0.20190501191748-2d6d46d308b2
github.com/coreos/go-iptables v0.4.3-0.20190724151750-969b135e941d
github.com/containerd/go-cni v1.0.2
github.com/containernetworking/cni v0.8.1
github.com/containernetworking/plugins v0.9.1
github.com/coreos/go-iptables v0.5.0
github.com/coreos/go-semver v0.3.0
github.com/docker/cli v0.0.0-20200303215952-eb310fca4956
github.com/docker/distribution v2.7.1+incompatible
Expand Down Expand Up @@ -57,7 +57,7 @@ require (
// versions.
github.com/hashicorp/go-discover v0.0.0-20210818145131-c573d69da192
github.com/hashicorp/go-envparse v0.0.0-20180119215841-310ca1881b22
github.com/hashicorp/go-getter v1.6.1
github.com/hashicorp/go-getter v1.7.0
github.com/hashicorp/go-hclog v1.2.2
github.com/hashicorp/go-immutable-radix v1.3.1
github.com/hashicorp/go-memdb v1.3.4
Expand All @@ -68,7 +68,7 @@ require (
github.com/hashicorp/go-sockaddr v1.0.2
github.com/hashicorp/go-syslog v1.0.0
github.com/hashicorp/go-uuid v1.0.2
github.com/hashicorp/go-version v1.3.0
github.com/hashicorp/go-version v1.6.0
github.com/hashicorp/golang-lru v0.5.4
github.com/hashicorp/hcl v1.0.1-0.20201016140508-a07e7d50bbee
github.com/hashicorp/hcl/v2 v2.9.2-0.20210407182552-eb14f8319bdc
Expand Down Expand Up @@ -118,24 +118,26 @@ require (
go.uber.org/goleak v1.1.12
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d
golang.org/x/exp v0.0.0-20220921164117-439092de6870
golang.org/x/net v0.0.0-20220906165146-f3363e06e74c
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
golang.org/x/sys v0.1.0
golang.org/x/net v0.1.0
golang.org/x/sync v0.0.0-20220929204114-8fcdb60fdcc0
golang.org/x/sys v0.2.0
golang.org/x/time v0.0.0-20220224211638-0e9765cccd65
google.golang.org/grpc v1.45.0
google.golang.org/grpc v1.50.1
google.golang.org/protobuf v1.28.1
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
gopkg.in/tomb.v2 v2.0.0-20140626144623-14b3d72120e8
)

require (
cloud.google.com/go v0.97.0 // indirect
cloud.google.com/go/storage v1.18.2 // indirect
cloud.google.com/go v0.104.0 // indirect
cloud.google.com/go/compute v1.10.0 // indirect
cloud.google.com/go/iam v0.5.0 // indirect
cloud.google.com/go/storage v1.27.0 // indirect
github.com/Azure/azure-sdk-for-go v44.0.0+incompatible // indirect
github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect
github.com/Azure/go-autorest v14.2.0+incompatible // indirect
github.com/Azure/go-autorest/autorest v0.11.4 // indirect
github.com/Azure/go-autorest/autorest/adal v0.9.2 // indirect
github.com/Azure/go-autorest/autorest/adal v0.9.5 // indirect
github.com/Azure/go-autorest/autorest/azure/auth v0.5.1 // indirect
github.com/Azure/go-autorest/autorest/azure/cli v0.4.0 // indirect
github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
Expand All @@ -148,7 +150,7 @@ require (
github.com/Masterminds/goutils v1.1.0 // indirect
github.com/Masterminds/semver/v3 v3.1.1 // indirect
github.com/Masterminds/sprig/v3 v3.2.0 // indirect
github.com/Microsoft/hcsshim v0.8.9 // indirect
github.com/Microsoft/hcsshim v0.8.16 // indirect
github.com/VividCortex/ewma v1.1.1 // indirect
github.com/agext/levenshtein v1.2.1 // indirect
github.com/apparentlymart/go-cidr v1.0.1 // indirect
Expand All @@ -158,55 +160,53 @@ require (
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
github.com/bgentry/speakeasy v0.1.0 // indirect
github.com/bmatcuk/doublestar v1.1.5 // indirect
github.com/census-instrumentation/opencensus-proto v0.3.0 // indirect
github.com/cenkalti/backoff/v3 v3.0.0 // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/checkpoint-restore/go-criu/v4 v4.1.0 // indirect
github.com/cheggaaa/pb/v3 v3.0.5 // indirect
github.com/cilium/ebpf v0.2.0 // indirect
github.com/cilium/ebpf v0.4.0 // indirect
github.com/circonus-labs/circonus-gometrics v2.3.1+incompatible // indirect
github.com/circonus-labs/circonusllhist v0.1.3 // indirect
github.com/cncf/udpa/go v0.0.0-20210930031921-04548b0d99d4 // indirect
github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1 // indirect
github.com/containerd/console v1.0.1 // indirect
github.com/containerd/containerd v1.3.4 // indirect
github.com/containerd/continuity v0.0.0-20200709052629-daa8e1ccc0bc // indirect
github.com/coreos/go-systemd/v22 v22.1.0 // indirect
github.com/cyphar/filepath-securejoin v0.2.3-0.20190205144030-7efe413b52e1 // indirect
github.com/containerd/console v1.0.2 // indirect
github.com/containerd/containerd v1.5.1 // indirect
github.com/containerd/continuity v0.1.0 // indirect
github.com/coreos/go-systemd/v22 v22.3.2 // indirect
github.com/cyphar/filepath-securejoin v0.2.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/denverdino/aliyungo v0.0.0-20190125010748-a747050bb1ba // indirect
github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect
github.com/digitalocean/godo v1.10.0 // indirect
github.com/dimchansky/utfbom v1.1.0 // indirect
github.com/docker/docker-credential-helpers v0.6.2-0.20180719074751-73e5f5dbfea3 // indirect
github.com/docker/docker-credential-helpers v0.7.0 // indirect
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-metrics v0.0.1 // indirect
github.com/docker/libtrust v0.0.0-20160708172513-aabc10ec26b7 // indirect
github.com/envoyproxy/go-control-plane v0.10.0 // indirect
github.com/envoyproxy/protoc-gen-validate v0.6.2 // indirect
github.com/fsnotify/fsnotify v1.4.9 // indirect
github.com/form3tech-oss/jwt-go v3.2.2+incompatible // indirect
github.com/go-ole/go-ole v1.2.6 // indirect
github.com/godbus/dbus/v5 v5.0.3 // indirect
github.com/gogo/protobuf v1.3.1 // indirect
github.com/godbus/dbus/v5 v5.0.4 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/google/btree v1.0.0 // indirect
github.com/google/go-querystring v0.0.0-20170111101155-53e6ce116135 // indirect
github.com/google/uuid v1.1.2 // indirect
github.com/googleapis/gax-go/v2 v2.1.1 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/googleapis/enterprise-certificate-proxy v0.2.0 // indirect
github.com/googleapis/gax-go/v2 v2.6.0 // indirect
github.com/gookit/color v1.3.1 // indirect
github.com/gophercloud/gophercloud v0.1.0 // indirect
github.com/gorilla/mux v1.7.4 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-retryablehttp v0.6.7 // indirect
github.com/hashicorp/go-rootcerts v1.0.2 // indirect
github.com/hashicorp/go-safetemp v1.0.0 // indirect
github.com/hashicorp/go-secure-stdlib/mlock v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.6 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
github.com/hashicorp/mdns v1.0.4 // indirect
github.com/hashicorp/vic v1.5.1-0.20190403131502-bbfe86ec9443 // indirect
github.com/huandu/xstrings v1.3.2 // indirect
github.com/imdario/mergo v0.3.11 // indirect
github.com/ishidawataru/sctp v0.0.0-20191218070446-00ab2ac2db07 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/joyent/triton-go v0.0.0-20190112182421-51ffac552869 // indirect
github.com/klauspost/compress v1.13.6 // indirect
github.com/klauspost/compress v1.15.11 // indirect
github.com/linode/linodego v0.7.1 // indirect
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
Expand All @@ -220,7 +220,6 @@ require (
github.com/nicolai86/scaleway-sdk v1.10.2-0.20180628010248-798f60e20bb2 // indirect
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/oklog/run v1.0.1-0.20180308005104-6934b124db28 // indirect
github.com/onsi/gomega v1.9.0 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.0.1 // indirect
github.com/opencontainers/selinux v1.8.0 // indirect
Expand All @@ -245,36 +244,29 @@ require (
github.com/tklauser/numcpus v0.4.0 // indirect
github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926 // indirect
github.com/ulikunitz/xz v0.5.10 // indirect
github.com/vishvananda/netlink v1.1.0 // indirect
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df // indirect
github.com/vishvananda/netlink v1.1.1-0.20201029203352-d40f9887b852 // indirect
github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae // indirect
github.com/vmihailenco/msgpack/v4 v4.3.12 // indirect
github.com/vmihailenco/tagparser v0.1.1 // indirect
github.com/vmware/govmomi v0.18.0 // indirect
github.com/willf/bitset v1.1.11 // indirect
github.com/yusufpapurcu/wmi v1.2.2 // indirect
go.opencensus.io v0.23.0 // indirect
golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b // indirect
golang.org/x/term v0.0.0-20220526004731-065cf7ba2467 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/api v0.60.0 // indirect
go.uber.org/atomic v1.9.0 // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/oauth2 v0.1.0 // indirect
golang.org/x/term v0.1.0 // indirect
golang.org/x/text v0.4.0 // indirect
golang.org/x/tools v0.1.12 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/api v0.100.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20211104193956-4c6863e31247 // indirect
google.golang.org/genproto v0.0.0-20221025140454-527a21cfbd71 // indirect
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
gopkg.in/fsnotify.v1 v1.4.7 // indirect
gopkg.in/resty.v1 v1.12.0 // indirect
gopkg.in/square/go-jose.v2 v2.5.1 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

require (
github.com/cenkalti/backoff/v3 v3.0.0 // indirect
github.com/hashicorp/go-secure-stdlib/mlock v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.6 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
go.uber.org/atomic v1.9.0 // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/tools v0.1.12 // indirect
gotest.tools/v3 v3.4.0 // indirect
)

0 comments on commit 3bf45af

Please sign in to comment.