Skip to content

Commit

Permalink
Merge pull request #5772 from hashicorp/f-disable-nomad-exec
Browse files Browse the repository at this point in the history
client config flag to disable remote exec
  • Loading branch information
Mahmood Ali committed Jun 4, 2019
2 parents 0a29d67 + b170ef9 commit fc9f753
Show file tree
Hide file tree
Showing 12 changed files with 101 additions and 13 deletions.
4 changes: 4 additions & 0 deletions client/alloc_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions client/alloc_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -249,6 +252,7 @@ func DefaultConfig() *Config {
GCMaxAllocs: 50,
NoHostUUID: true,
DisableTaggedMetrics: false,
DisableRemoteExec: false,
BackwardsCompatibleMetrics: false,
RPCHoldTimeout: 5 * time.Second,
}
Expand Down
1 change: 1 addition & 0 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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...)

Expand Down
1 change: 1 addition & 0 deletions command/agent/config_parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ var basicConfig = &Config{
GCInodeUsageThreshold: 91,
GCMaxAllocs: 50,
NoHostUUID: helper.BoolToPtr(false),
DisableRemoteExec: true,
},
Server: &ServerConfig{
Enabled: true,
Expand Down
26 changes: 14 additions & 12 deletions command/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions command/agent/testdata/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion command/agent/testdata/basic.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"network_interface": "eth0",
"network_speed": 100,
"no_host_uuid": false,
"disable_remote_exec": true,
"node_class": "linux-medium-64bit",
"options": [
{
Expand Down Expand Up @@ -285,4 +286,4 @@
"token": "12345"
}
]
}
}
1 change: 1 addition & 0 deletions website/source/api/agent.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ $ curl \
"ChrootEnv": {},
"ClientMaxPort": 14512,
"ClientMinPort": 14000,
"DisableRemoteExec": false,
"Enabled": true,
"GCDiskUsageThreshold": 99,
"GCInodeUsageThreshold": 99,
Expand Down
6 changes: 6 additions & 0 deletions website/source/docs/commands/alloc/exec.html.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ nomad alloc exec -job <job-id> <command> [<args>...]
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
3 changes: 3 additions & 0 deletions website/source/docs/configuration/client.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down

0 comments on commit fc9f753

Please sign in to comment.