agent: show warning when enable_script_checks is enabled without safty net (#7437)

In order to enforce a bit security on Consul agents, add a new method in agent
to highlight possible security issues.

This does not return an error for now, but might in the future.

For now, it detects issues such as:

https://www.hashicorp.com/blog/protecting-consul-from-rce-risk-in-specific-configurations/

This would display this kind of messages:

```
2020-03-11T18:27:49.873+0100 [ERROR] agent: [SECURITY] issue: error="using enable-script-checks without ACLs and without allow_write_http_from is DANGEROUS, use enable-local-script-checks instead see https://www.hashicorp.com/blog/protecting-consul-from-rce-risk-in-specific-configurations/"
```
This commit is contained in:
Pierre Souchay 2020-04-02 09:59:23 +02:00 committed by GitHub
parent 0d1d5d0863
commit 2b8da952a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 39 additions and 0 deletions

View File

@ -384,6 +384,11 @@ func (a *Agent) Start() error {
c := a.config c := a.config
if err := a.CheckSecurity(c); err != nil {
a.logger.Error("Security error while parsing configuration: %#v", err)
return err
}
// Warn if the node name is incompatible with DNS // Warn if the node name is incompatible with DNS
if InvalidDnsRe.MatchString(a.config.NodeName) { if InvalidDnsRe.MatchString(a.config.NodeName) {
a.logger.Warn("Node name will not be discoverable "+ a.logger.Warn("Node name will not be discoverable "+
@ -3841,6 +3846,21 @@ func (a *Agent) getPersistedTokens() (*persistedTokens, error) {
return persistedTokens, nil return persistedTokens, nil
} }
// CheckSecurity Performs security checks in Consul Configuration
// It might return an error if configuration is considered too dangerous
func (a *Agent) CheckSecurity(conf *config.RuntimeConfig) error {
if conf.EnableRemoteScriptChecks {
if !conf.ACLsEnabled {
if len(conf.AllowWriteHTTPFrom) == 0 {
err := fmt.Errorf("using enable-script-checks without ACLs and without allow_write_http_from is DANGEROUS, use enable-local-script-checks instead, see https://www.hashicorp.com/blog/protecting-consul-from-rce-risk-in-specific-configurations/")
a.logger.Error("[SECURITY] issue", "error", err)
// TODO: return the error in future Consul versions
}
}
}
return nil
}
func (a *Agent) loadTokens(conf *config.RuntimeConfig) error { func (a *Agent) loadTokens(conf *config.RuntimeConfig) error {
persistedTokens, persistenceErr := a.getPersistedTokens() persistedTokens, persistenceErr := a.getPersistedTokens()
@ -4022,6 +4042,10 @@ func (a *Agent) loadLimits(conf *config.RuntimeConfig) {
// including all services, checks, tokens, metadata, dnsServer configs, etc. // including all services, checks, tokens, metadata, dnsServer configs, etc.
// It will also reload all ongoing watches. // It will also reload all ongoing watches.
func (a *Agent) ReloadConfig(newCfg *config.RuntimeConfig) error { func (a *Agent) ReloadConfig(newCfg *config.RuntimeConfig) error {
if err := a.CheckSecurity(newCfg); err != nil {
a.logger.Error("Security error while reloading configuration: %#v", err)
return err
}
// Bulk update the services and checks // Bulk update the services and checks
a.PauseSync() a.PauseSync()
defer a.ResumeSync() defer a.ResumeSync()

View File

@ -3383,6 +3383,21 @@ func TestAgent_loadTokens(t *testing.T) {
}) })
} }
func TestAgent_SecurityChecks(t *testing.T) {
t.Parallel()
hcl := `
enable_script_checks = true
`
a := &TestAgent{Name: t.Name(), HCL: hcl}
defer a.Shutdown()
data := make([]byte, 0, 8192)
bytesBuffer := bytes.NewBuffer(data)
a.LogOutput = bytesBuffer
assert.NoError(t, a.Start(t))
assert.Contains(t, bytesBuffer.String(), "using enable-script-checks without ACLs and without allow_write_http_from is DANGEROUS")
}
func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) { func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) {
t.Parallel() t.Parallel()
dataDir := testutil.TempDir(t, "agent") // we manage the data dir dataDir := testutil.TempDir(t, "agent") // we manage the data dir