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

Changes disable_remote_exec default to true so remote exec is opt-in. #2854

Merged
merged 1 commit into from
Mar 30, 2017
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
7 changes: 4 additions & 3 deletions command/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,7 @@ type Config struct {

// DisableRemoteExec is used to turn off the remote execution
// feature. This is for security to prevent unknown scripts from running.
DisableRemoteExec bool `mapstructure:"disable_remote_exec"`
DisableRemoteExec *bool `mapstructure:"disable_remote_exec"`

// DisableUpdateCheck is used to turn off the automatic update and
// security bulletin checking.
Expand Down Expand Up @@ -828,6 +828,7 @@ func DefaultConfig() *Config {
ACLDefaultPolicy: "allow",
ACLDisabledTTL: 120 * time.Second,
ACLEnforceVersion8: Bool(true),
DisableRemoteExec: Bool(true),
RetryInterval: 30 * time.Second,
RetryIntervalWan: 30 * time.Second,

Expand Down Expand Up @@ -1696,8 +1697,8 @@ func MergeConfig(a, b *Config) *Config {
if len(b.WatchPlans) != 0 {
result.WatchPlans = append(result.WatchPlans, b.WatchPlans...)
}
if b.DisableRemoteExec {
result.DisableRemoteExec = true
if b.DisableRemoteExec != nil {
result.DisableRemoteExec = b.DisableRemoteExec
}
if b.DisableUpdateCheck {
result.DisableUpdateCheck = true
Expand Down
15 changes: 10 additions & 5 deletions command/agent/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -783,14 +783,19 @@ func TestDecodeConfig(t *testing.T) {
t.Fatalf("bad: %#v", config)
}

// remote exec
input = `{"disable_remote_exec": true}`
// Remote exec is disabled by default.
config = DefaultConfig()
if *config.DisableRemoteExec != true {
t.Fatalf("bad: %#v", config)
}

// Test re-enabling remote exec.
input = `{"disable_remote_exec": false}`
config, err = DecodeConfig(bytes.NewReader([]byte(input)))
if err != nil {
t.Fatalf("err: %s", err)
}

if !config.DisableRemoteExec {
if *config.DisableRemoteExec != false {
t.Fatalf("bad: %#v", config)
}

Expand Down Expand Up @@ -1723,7 +1728,7 @@ func TestMergeConfig(t *testing.T) {
"handler": "foobar",
},
},
DisableRemoteExec: true,
DisableRemoteExec: Bool(true),
Telemetry: Telemetry{
StatsiteAddr: "127.0.0.1:7250",
StatsitePrefix: "stats_prefix",
Expand Down
2 changes: 1 addition & 1 deletion command/agent/user_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func (a *Agent) ingestUserEvent(msg *UserEvent) {
// Special handling for internal events
switch msg.Name {
case remoteExecName:
if a.config.DisableRemoteExec {
if *a.config.DisableRemoteExec {
a.logger.Printf("[INFO] agent: ignoring remote exec event (%s), disabled.", msg.ID)
} else {
go a.handleRemoteExec(msg)
Expand Down
25 changes: 19 additions & 6 deletions command/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ func TestExecCommand_implements(t *testing.T) {
}

func TestExecCommandRun(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()
waitForLeader(t, a1.httpAddr)

Expand All @@ -46,11 +48,14 @@ func TestExecCommandRun(t *testing.T) {
}

func TestExecCommandRun_CrossDC(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()

a2 := testAgentWithConfig(t, func(c *agent.Config) {
c.Datacenter = "dc2"
c.DisableRemoteExec = agent.Bool(false)
})
defer a2.Shutdown()

Expand Down Expand Up @@ -136,7 +141,9 @@ func TestExecCommand_Validate(t *testing.T) {
}

func TestExecCommand_Sessions(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()
waitForLeader(t, a1.httpAddr)

Expand Down Expand Up @@ -177,7 +184,9 @@ func TestExecCommand_Sessions(t *testing.T) {
}

func TestExecCommand_Sessions_Foreign(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()
waitForLeader(t, a1.httpAddr)

Expand Down Expand Up @@ -228,7 +237,9 @@ func TestExecCommand_Sessions_Foreign(t *testing.T) {
}

func TestExecCommand_UploadDestroy(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()
waitForLeader(t, a1.httpAddr)

Expand Down Expand Up @@ -285,7 +296,9 @@ func TestExecCommand_UploadDestroy(t *testing.T) {
}

func TestExecCommand_StreamResults(t *testing.T) {
a1 := testAgent(t)
a1 := testAgentWithConfig(t, func(c *agent.Config) {
c.DisableRemoteExec = agent.Bool(false)
})
defer a1.Shutdown()
waitForLeader(t, a1.httpAddr)

Expand Down
3 changes: 2 additions & 1 deletion website/source/docs/agent/options.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,8 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass

* <a name="disable_remote_exec"></a><a href="#disable_remote_exec">`disable_remote_exec`</a>
Disables support for remote execution. When set to true, the agent will ignore any incoming
remote exec requests.
remote exec requests. In versions of Consul prior to 0.8, this defaulted to false. In Consul
0.8 the default was changed to true, to make remote exec opt-in instead of opt-out.

* <a name="disable_update_check"></a><a href="#disable_update_check">`disable_update_check`</a>
Disables automatic checking for security bulletins and new version releases.
Expand Down
8 changes: 7 additions & 1 deletion website/source/docs/upgrade-specific.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ and update any scripts that passed a custom `-rpc-addr` to the following command

The [`acl_enforce_version_8`](/docs/agent/options.html#acl_enforce_version_8) configuration now defaults to `true` to enable [full version 8 ACL support](/docs/internals/acl.html#version_8_acls) by default. If you are upgrading an existing cluster with ACLs enabled, you will need to set this to `false` during the upgrade on **both Consul agents and Consul servers**. Version 8 ACLs were also changed so that [`acl_datacenter`](/docs/agent/options.html#acl_datacenter) must be set on agents in order to enable the agent-side enforcement of ACLs. This makes for a smoother experience in clusters where ACLs aren't enabled at all, but where the agents would have to wait to contact a Consul server before learning that.

#### <a name="raft_protocol"></a><a href="#raft_protocol">Raft Protocol Version Compatibility</a>
#### Remote Exec Is Now Opt-In

The default for [`disable_remote_exec`](/docs/agent/options.html#disable_remote_exec) was
changed to "true", so now operators need to opt-in to having agents support running
commands remotely via [`consul exec`](/docs/commands/exec.html).

#### Raft Protocol Version Compatibility

When upgrading to Consul 0.8.0 from a version lower than 0.7.0, users will need to
set the [`-raft-protocol`](/docs/agent/options.html#_raft_protocol) option to 1 in
Expand Down