From a9f81f2daae8e83fd49cbeb969891c691cb0c89d Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 3 Jun 2019 15:31:39 -0400 Subject: [PATCH 1/2] client config flag to disable remote exec This exposes a client flag to disable nomad remote exec support in environments where access to tasks ought to be restricted. I used `disable_remote_exec` client flag that defaults to allowing remote exec. Opted for a client config that can be used to disable remote exec globally, or to a subset of the cluster if necessary. --- client/alloc_endpoint.go | 4 ++ client/alloc_endpoint_test.go | 56 +++++++++++++++++++ client/config/config.go | 4 ++ command/agent/agent.go | 1 + command/agent/config.go | 8 +++ command/agent/config_parse_test.go | 1 + command/agent/config_test.go | 26 +++++---- command/agent/testdata/basic.hcl | 1 + command/agent/testdata/basic.json | 3 +- website/source/api/agent.html.md | 1 + .../source/docs/configuration/client.html.md | 3 + 11 files changed, 95 insertions(+), 13 deletions(-) diff --git a/client/alloc_endpoint.go b/client/alloc_endpoint.go index 26c3c06a8..e97e98812 100644 --- a/client/alloc_endpoint.go +++ b/client/alloc_endpoint.go @@ -144,6 +144,10 @@ func (a *Allocations) execImpl(encoder *codec.Encoder, decoder *codec.Decoder, e return helper.Int64ToPtr(500), err } + if a.c.GetConfig().DisableRemoteExec { + return nil, structs.ErrPermissionDenied + } + aclObj, token, err := a.c.resolveTokenAndACL(req.QueryOptions.AuthToken) { // log access diff --git a/client/alloc_endpoint_test.go b/client/alloc_endpoint_test.go index 56b8cdeca..4031aa737 100644 --- a/client/alloc_endpoint_test.go +++ b/client/alloc_endpoint_test.go @@ -619,6 +619,62 @@ func TestAlloc_ExecStreaming_NoAllocation(t *testing.T) { } } +func TestAlloc_ExecStreaming_DisableRemoteExec(t *testing.T) { + t.Parallel() + require := require.New(t) + + // Start a server and client + s := nomad.TestServer(t, nil) + defer s.Shutdown() + testutil.WaitForLeader(t, s.RPC) + + c, cleanup := TestClient(t, func(c *config.Config) { + c.Servers = []string{s.GetConfig().RPCAddr.String()} + c.DisableRemoteExec = true + }) + defer cleanup() + + // Make the request + req := &cstructs.AllocExecRequest{ + AllocID: uuid.Generate(), + Task: "testtask", + Tty: true, + Cmd: []string{"placeholder command"}, + QueryOptions: nstructs.QueryOptions{Region: "global"}, + } + + // Get the handler + handler, err := c.StreamingRpcHandler("Allocations.Exec") + require.Nil(err) + + // Create a pipe + p1, p2 := net.Pipe() + defer p1.Close() + defer p2.Close() + + errCh := make(chan error) + frames := make(chan *drivers.ExecTaskStreamingResponseMsg) + + // Start the handler + go handler(p2) + go decodeFrames(t, p1, frames, errCh) + + // Send the request + encoder := codec.NewEncoder(p1, nstructs.MsgpackHandle) + require.Nil(encoder.Encode(req)) + + timeout := time.After(3 * time.Second) + + select { + case <-timeout: + require.FailNow("timed out") + case err := <-errCh: + require.True(nstructs.IsErrPermissionDenied(err), "expected permission denied error but found: %v", err) + case f := <-frames: + require.Fail("received unexpected frame", "frame: %#v", f) + } +} + func TestAlloc_ExecStreaming_ACL_Basic(t *testing.T) { t.Parallel() require := require.New(t) diff --git a/client/config/config.go b/client/config/config.go index 287d8826d..da1980e4e 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -198,6 +198,9 @@ type Config struct { // key/value/tag format, or simply a key/value format DisableTaggedMetrics bool + // DisableRemoteExec disables remote exec targeting tasks on this client + DisableRemoteExec bool + // BackwardsCompatibleMetrics determines whether to show methods of // displaying metrics for older versions, or to only show the new format BackwardsCompatibleMetrics bool @@ -249,6 +252,7 @@ func DefaultConfig() *Config { GCMaxAllocs: 50, NoHostUUID: true, DisableTaggedMetrics: false, + DisableRemoteExec: false, BackwardsCompatibleMetrics: false, RPCHoldTimeout: 5 * time.Second, } diff --git a/command/agent/agent.go b/command/agent/agent.go index ec4d356ca..cd8cd9085 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -460,6 +460,7 @@ func convertClientConfig(agentConfig *Config) (*clientconfig.Config, error) { } conf.ClientMaxPort = uint(agentConfig.Client.ClientMaxPort) conf.ClientMinPort = uint(agentConfig.Client.ClientMinPort) + conf.DisableRemoteExec = agentConfig.Client.DisableRemoteExec // Setup the node conf.Node = new(structs.Node) diff --git a/command/agent/config.go b/command/agent/config.go index dea781065..773adbc17 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -239,6 +239,9 @@ type ClientConfig struct { // random UUID. NoHostUUID *bool `hcl:"no_host_uuid"` + // DisableRemoteExec disables remote exec targeting tasks on this client + DisableRemoteExec bool `hcl:"disable_remote_exec"` + // ServerJoin contains information that is used to attempt to join servers ServerJoin *ServerJoin `hcl:"server_join"` @@ -686,6 +689,7 @@ func DefaultConfig() *Config { GCInodeUsageThreshold: 70, GCMaxAllocs: 50, NoHostUUID: helper.BoolToPtr(true), + DisableRemoteExec: false, ServerJoin: &ServerJoin{ RetryJoin: []string{}, RetryInterval: 30 * time.Second, @@ -1245,6 +1249,10 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { result.NoHostUUID = b.NoHostUUID } + if b.DisableRemoteExec { + result.DisableRemoteExec = b.DisableRemoteExec + } + // Add the servers result.Servers = append(result.Servers, b.Servers...) diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 4c8bb0c1c..e16e7a8f3 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -78,6 +78,7 @@ var basicConfig = &Config{ GCInodeUsageThreshold: 91, GCMaxAllocs: 50, NoHostUUID: helper.BoolToPtr(false), + DisableRemoteExec: true, }, Server: &ServerConfig{ Enabled: true, diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 7d7128d5f..0e93a5478 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -88,11 +88,12 @@ func TestConfig_Merge(t *testing.T) { Options: map[string]string{ "foo": "bar", }, - NetworkSpeed: 100, - CpuCompute: 100, - MemoryMB: 100, - MaxKillTimeout: "20s", - ClientMaxPort: 19996, + NetworkSpeed: 100, + CpuCompute: 100, + MemoryMB: 100, + MaxKillTimeout: "20s", + ClientMaxPort: 19996, + DisableRemoteExec: false, Reserved: &Resources{ CPU: 10, MemoryMB: 10, @@ -244,13 +245,14 @@ func TestConfig_Merge(t *testing.T) { "foo": "bar", "baz": "zip", }, - ChrootEnv: map[string]string{}, - ClientMaxPort: 20000, - ClientMinPort: 22000, - NetworkSpeed: 105, - CpuCompute: 105, - MemoryMB: 105, - MaxKillTimeout: "50s", + ChrootEnv: map[string]string{}, + ClientMaxPort: 20000, + ClientMinPort: 22000, + NetworkSpeed: 105, + CpuCompute: 105, + MemoryMB: 105, + MaxKillTimeout: "50s", + DisableRemoteExec: false, Reserved: &Resources{ CPU: 15, MemoryMB: 15, diff --git a/command/agent/testdata/basic.hcl b/command/agent/testdata/basic.hcl index a80ff634a..3c56fa244 100644 --- a/command/agent/testdata/basic.hcl +++ b/command/agent/testdata/basic.hcl @@ -68,6 +68,7 @@ client { gc_inode_usage_threshold = 91 gc_max_allocs = 50 no_host_uuid = false + disable_remote_exec = true } server { enabled = true diff --git a/command/agent/testdata/basic.json b/command/agent/testdata/basic.json index ef1ec6dba..cf0c4645a 100644 --- a/command/agent/testdata/basic.json +++ b/command/agent/testdata/basic.json @@ -60,6 +60,7 @@ "network_interface": "eth0", "network_speed": 100, "no_host_uuid": false, + "disable_remote_exec": true, "node_class": "linux-medium-64bit", "options": [ { @@ -285,4 +286,4 @@ "token": "12345" } ] -} \ No newline at end of file +} diff --git a/website/source/api/agent.html.md b/website/source/api/agent.html.md index 4ff04e1cb..0fc937b20 100644 --- a/website/source/api/agent.html.md +++ b/website/source/api/agent.html.md @@ -179,6 +179,7 @@ $ curl \ "ChrootEnv": {}, "ClientMaxPort": 14512, "ClientMinPort": 14000, + "DisableRemoteExec": false, "Enabled": true, "GCDiskUsageThreshold": 99, "GCInodeUsageThreshold": 99, diff --git a/website/source/docs/configuration/client.html.md b/website/source/docs/configuration/client.html.md index 7a52da877..d5a8f3cd7 100644 --- a/website/source/docs/configuration/client.html.md +++ b/website/source/docs/configuration/client.html.md @@ -55,6 +55,9 @@ driver) but will be removed in a future release. job is allowed to wait to exit. Individual jobs may customize their own kill timeout, but it may not exceed this value. +- `disable_remote_exec` `(bool: false)` - Specifies if the client should disable + remote task execution to tasks running on this client. + - `meta` `(map[string]string: nil)` - Specifies a key-value map that annotates with user-defined metadata. From 89930873dae27f288809b9df1a474870627d608f Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 4 Jun 2019 14:37:56 -0400 Subject: [PATCH 2/2] link to flag from alloc exec doc --- website/source/docs/commands/alloc/exec.html.md.erb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/source/docs/commands/alloc/exec.html.md.erb b/website/source/docs/commands/alloc/exec.html.md.erb index 733bd86d8..30cbfcc07 100644 --- a/website/source/docs/commands/alloc/exec.html.md.erb +++ b/website/source/docs/commands/alloc/exec.html.md.erb @@ -117,3 +117,9 @@ nomad alloc exec -job [...] Choosing a specific allocation is useful for debugging issues with a specific instance of a service. For other operations using the `-job` flag may be more convenient than looking up an allocation ID to use. + +## Disabling remote execution + +`alloc exec` is enabled by default to aid with debugging. Operators can disable the feature by setting [`disable_remote_exec` client config option][disable_remote_exec_flag] on all clients, or a subset of clients that run sensitive workloads. + +[disable_remote_exec_flag]: /docs/configuration/client.html#disable_remote_exec