Skip to content

Commit

Permalink
template: disallow writeToFile by default
Browse files Browse the repository at this point in the history
Resolves #12095 by WONTFIXing it.

This approach disables `writeToFile` as it allows arbitrary host
filesystem writes and is only a small quality of life improvement over
multiple `template` stanzas.

This approach has the significant downside of leaving people who have
altered their `template.function_denylist` *still vulnerable!* I added
an upgrade note, but we should have implemented the denylist as a
`map[string]bool` so that new funcs could be denied without overriding
custom configurations.
  • Loading branch information
schmichael committed Mar 16, 2022
1 parent 74c8860 commit a9833ce
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 101 deletions.
49 changes: 45 additions & 4 deletions client/allocrunner/taskrunner/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type MockTaskHooks struct {
UnblockCh chan struct{}

KillEvent *structs.TaskEvent
KillCh chan struct{}
KillCh chan *structs.TaskEvent

Events []*structs.TaskEvent
EmitEventCh chan *structs.TaskEvent
Expand All @@ -69,7 +69,7 @@ func NewMockTaskHooks() *MockTaskHooks {
UnblockCh: make(chan struct{}, 1),
RestartCh: make(chan struct{}, 1),
SignalCh: make(chan struct{}, 1),
KillCh: make(chan struct{}, 1),
KillCh: make(chan *structs.TaskEvent, 1),
EmitEventCh: make(chan *structs.TaskEvent, 1),
}
}
Expand Down Expand Up @@ -97,7 +97,7 @@ func (m *MockTaskHooks) Signal(event *structs.TaskEvent, s string) error {
func (m *MockTaskHooks) Kill(ctx context.Context, event *structs.TaskEvent) error {
m.KillEvent = event
select {
case m.KillCh <- struct{}{}:
case m.KillCh <- event:
default:
}
return nil
Expand Down Expand Up @@ -145,7 +145,7 @@ func newTestHarness(t *testing.T, templates []*structs.Template, consul, vault b
config: &config.Config{
Region: region,
TemplateConfig: &config.ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: config.DefaultTemplateFunctionDenylist,
DisableSandbox: false,
ConsulRetry: &config.RetryConfig{Backoff: helper.TimeToPtr(10 * time.Millisecond)},
}},
Expand Down Expand Up @@ -2157,3 +2157,44 @@ func TestTaskTemplateManager_Template_Wait_Set(t *testing.T) {
require.Equal(t, 10*time.Second, *k.Wait.Max)
}
}

// TestTaskTemplateManager_writeToFile asserts the consul-template function
// writeToFile is disabled by default.
func TestTaskTemplateManager_writeToFile(t *testing.T) {
ci.Parallel(t)

file := "my.tmpl"
template := &structs.Template{
EmbeddedTmpl: `Testing writeToFile...
{{ "if i exist writeToFile is enabled" | writeToFile "/tmp/NOMAD-TEST-SHOULD-NOT-EXIST" "" "" "0644" }}
...done
`,
DestPath: file,
ChangeMode: structs.TemplateChangeModeNoop,
}

harness := newTestHarness(t, []*structs.Template{template}, false, false)
require.NoError(t, harness.startWithErr(), "couldn't setup initial harness")
defer harness.stop()

// Using writeToFile should cause a kill
select {
case <-harness.mockHooks.UnblockCh:
t.Fatalf("Task unblock should have not have been called")
case <-harness.mockHooks.EmitEventCh:
t.Fatalf("Task event should not have been emitted")
case e := <-harness.mockHooks.KillCh:
require.Contains(t, e.DisplayMessage, "writeToFile: function is disabled")
case <-time.After(time.Duration(5*testutil.TestMultiplier()) * time.Second):
t.Fatalf("timeout")
}

// Check the file is there
path := filepath.Join(harness.taskDir, file)
_, err := ioutil.ReadFile(path)
require.Error(t, err)

harness.stop()

// Assert the
}
4 changes: 3 additions & 1 deletion client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ var (
}

DefaultTemplateMaxStale = 5 * time.Second

DefaultTemplateFunctionDenylist = []string{"plugin", "writeToFile"}
)

// RPCHandler can be provided to the Client if there is a local server
Expand Down Expand Up @@ -766,7 +768,7 @@ func DefaultConfig() *Config {
NoHostUUID: true,
DisableRemoteExec: false,
TemplateConfig: &ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: DefaultTemplateFunctionDenylist,
DisableSandbox: false,
},
RPCHoldTimeout: 5 * time.Second,
Expand Down
4 changes: 2 additions & 2 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ func DevConfig(mode *devModeConfig) *Config {
conf.Client.GCInodeUsageThreshold = 99
conf.Client.GCMaxAllocs = 50
conf.Client.TemplateConfig = &client.ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: client.DefaultTemplateFunctionDenylist,
DisableSandbox: false,
}
conf.Client.BindWildcardDefaultHostNetwork = true
Expand Down Expand Up @@ -960,7 +960,7 @@ func DefaultConfig() *Config {
RetryMaxAttempts: 0,
},
TemplateConfig: &client.ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: client.DefaultTemplateFunctionDenylist,
DisableSandbox: false,
},
BindWildcardDefaultHostNetwork: true,
Expand Down
6 changes: 3 additions & 3 deletions command/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestConfig_Merge(t *testing.T) {
ClientMaxPort: 19996,
DisableRemoteExec: false,
TemplateConfig: &client.ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: client.DefaultTemplateFunctionDenylist,
DisableSandbox: false,
},
Reserved: &Resources{
Expand Down Expand Up @@ -304,7 +304,7 @@ func TestConfig_Merge(t *testing.T) {
MaxKillTimeout: "50s",
DisableRemoteExec: false,
TemplateConfig: &client.ClientTemplateConfig{
FunctionDenylist: []string{"plugin"},
FunctionDenylist: client.DefaultTemplateFunctionDenylist,
DisableSandbox: false,
},
Reserved: &Resources{
Expand Down Expand Up @@ -1471,7 +1471,7 @@ func TestConfig_LoadConsulTemplateBasic(t *testing.T) {
templateConfig := clientConfig.TemplateConfig
require.NotNil(t, templateConfig)
require.True(t, templateConfig.DisableSandbox)
require.Len(t, templateConfig.FunctionDenylist, 1)
require.Len(t, templateConfig.FunctionDenylist, 2)

// json
agentConfig, err = LoadConfig("test-resources/client_with_basic_template.json")
Expand Down
56 changes: 29 additions & 27 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ require (
github.com/gosuri/uilive v0.0.4
github.com/grpc-ecosystem/go-grpc-middleware v1.2.1-0.20200228141219-3ce3d519df39
github.com/hashicorp/consul v1.7.8
github.com/hashicorp/consul-template v0.25.2
github.com/hashicorp/consul/api v1.9.1
github.com/hashicorp/consul-template v0.28.0
github.com/hashicorp/consul/api v1.12.0
github.com/hashicorp/consul/sdk v0.8.0
github.com/hashicorp/cronexpr v1.1.1
github.com/hashicorp/go-bexpr v0.1.11
Expand All @@ -58,8 +58,8 @@ require (
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.5.11
github.com/hashicorp/go-hclog v1.1.0
github.com/hashicorp/go-immutable-radix v1.3.0
github.com/hashicorp/go-hclog v1.2.0
github.com/hashicorp/go-immutable-radix v1.3.1
github.com/hashicorp/go-memdb v1.3.2
github.com/hashicorp/go-msgpack v1.1.5
github.com/hashicorp/go-multierror v1.1.1
Expand All @@ -70,7 +70,7 @@ require (
github.com/hashicorp/go-uuid v1.0.2
github.com/hashicorp/go-version v1.4.0
github.com/hashicorp/golang-lru v0.5.4
github.com/hashicorp/hcl v1.0.1-0.20201016140508-a07e7d50bbee
github.com/hashicorp/hcl v1.0.1-vault-3
github.com/hashicorp/hcl/v2 v2.9.2-0.20210407182552-eb14f8319bdc
github.com/hashicorp/logutils v1.0.0
github.com/hashicorp/memberlist v0.3.1
Expand All @@ -79,21 +79,21 @@ require (
github.com/hashicorp/raft v1.3.5
github.com/hashicorp/raft-boltdb/v2 v2.2.0
github.com/hashicorp/serf v0.9.7
github.com/hashicorp/vault/api v1.0.5-0.20200805123347-1ef507638af6
github.com/hashicorp/vault/sdk v0.2.0
github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d
github.com/hashicorp/vault/api v1.4.1
github.com/hashicorp/vault/sdk v0.4.1
github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87
github.com/hpcloud/tail v1.0.1-0.20170814160653-37f427138745
github.com/kr/pretty v0.3.0
github.com/kr/text v0.2.0
github.com/mattn/go-colorable v0.1.9
github.com/mattn/go-colorable v0.1.12
github.com/miekg/dns v1.1.41
github.com/mitchellh/cli v1.1.2
github.com/mitchellh/colorstring v0.0.0-20150917214807-8631ce90f286
github.com/mitchellh/copystructure v1.2.0
github.com/mitchellh/go-glint v0.0.0-20210722152315-6515ceb4a127
github.com/mitchellh/go-ps v0.0.0-20190716172923-621e5597135b
github.com/mitchellh/go-testing-interface v1.14.1
github.com/mitchellh/hashstructure v1.0.0
github.com/mitchellh/hashstructure v1.1.0
github.com/mitchellh/mapstructure v1.4.3
github.com/mitchellh/reflectwalk v1.0.2
github.com/moby/sys/mount v0.3.0
Expand All @@ -116,12 +116,12 @@ require (
github.com/zclconf/go-cty-yaml v1.0.2
go.etcd.io/bbolt v1.3.5
go.uber.org/goleak v1.1.12
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
golang.org/x/net v0.0.0-20211216030914-fe4d6282115f
golang.org/x/crypto v0.0.0-20220315160706-3147a52a75dd
golang.org/x/net v0.0.0-20220225172249-27dd8689420f
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9
golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e
google.golang.org/grpc v1.44.0
golang.org/x/sys v0.0.0-20220315194320-039c03cc5b86
golang.org/x/time v0.0.0-20220224211638-0e9765cccd65
google.golang.org/grpc v1.45.0
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7
gopkg.in/tomb.v2 v2.0.0-20140626144623-14b3d72120e8
oss.indeed.com/go/libtime v1.5.0
Expand All @@ -142,9 +142,9 @@ require (
github.com/Azure/go-autorest/autorest/validation v0.3.0 // indirect
github.com/Azure/go-autorest/logger v0.2.1 // indirect
github.com/Azure/go-autorest/tracing v0.6.0 // indirect
github.com/BurntSushi/toml v0.4.1 // indirect
github.com/BurntSushi/toml v1.0.0 // indirect
github.com/DataDog/datadog-go v3.2.0+incompatible // indirect
github.com/Masterminds/goutils v1.1.0 // indirect
github.com/Masterminds/goutils v1.1.1 // indirect
github.com/Masterminds/semver v1.5.0 // indirect
github.com/Masterminds/sprig v2.22.0+incompatible // indirect
github.com/Microsoft/hcsshim v0.8.23 // indirect
Expand All @@ -159,6 +159,7 @@ require (
github.com/bits-and-blooms/bitset v1.2.0 // indirect
github.com/bmatcuk/doublestar v1.1.5 // indirect
github.com/boltdb/bolt v1.3.1 // indirect
github.com/cenkalti/backoff/v3 v3.2.2 // indirect
github.com/census-instrumentation/opencensus-proto v0.3.0 // indirect
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/checkpoint-restore/go-criu/v5 v5.0.0 // indirect
Expand Down Expand Up @@ -192,18 +193,19 @@ require (
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.2.0 // indirect
github.com/google/uuid v1.3.0 // indirect
github.com/googleapis/gax-go/v2 v2.1.1 // indirect
github.com/gookit/color v1.3.1 // indirect
github.com/gophercloud/gophercloud v0.1.0 // indirect
github.com/gorilla/mux v1.8.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-retryablehttp v0.6.7 // indirect
github.com/hashicorp/go-retryablehttp v0.7.0 // 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/parseutil v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/mlock v0.1.2 // indirect
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.4 // indirect
github.com/hashicorp/go-secure-stdlib/reloadutil v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.1 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
github.com/hashicorp/go-secure-stdlib/tlsutil v0.1.1 // indirect
github.com/hashicorp/mdns v1.0.4 // indirect
github.com/hashicorp/vic v1.5.1-0.20190403131502-bbfe86ec9443 // indirect
Expand All @@ -218,7 +220,6 @@ require (
github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
github.com/mattn/go-runewidth v0.0.7 // indirect
github.com/mattn/go-shellwords v1.0.12 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/go-wordwrap v1.0.1 // indirect
Expand All @@ -228,12 +229,12 @@ require (
github.com/mrunalp/fileutils v0.5.0 // indirect
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/oklog/run v1.1.0 // indirect
github.com/opencontainers/go-digest v1.0.0 // indirect
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/opencontainers/selinux v1.8.2 // indirect
github.com/packethost/packngo v0.1.1-0.20180711074735-b9cb5096f54c // indirect
github.com/pierrec/lz4 v2.5.2+incompatible // indirect
github.com/pierrec/lz4 v2.6.1+incompatible // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
github.com/prometheus/client_model v0.2.0 // indirect
Expand All @@ -257,19 +258,20 @@ require (
github.com/vmware/govmomi v0.18.0 // indirect
github.com/yusufpapurcu/wmi v1.2.2 // indirect
go.opencensus.io v0.23.0 // indirect
go.uber.org/atomic v1.9.0 // indirect
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 // indirect
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 // indirect
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
golang.org/x/text v0.3.7 // indirect
golang.org/x/tools v0.1.8-0.20211029000441-d6a9af8af023 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
google.golang.org/api v0.60.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-20220314164441-57ef72a4c106 // indirect
google.golang.org/protobuf v1.27.1 // 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/square/go-jose.v2 v2.6.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
)
Loading

0 comments on commit a9833ce

Please sign in to comment.