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

Display a security warning when enable_scripts_checks is enabled without security #7437

Conversation

pierresouchay
Copy link
Contributor

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/"

@pierresouchay
Copy link
Contributor Author

cc @i0rek

@pierresouchay pierresouchay changed the title Display a security warning when enable_scripts_checks is enable without security Display a security warning when enable_scripts_checks is enabled without security Mar 11, 2020
@pierresouchay pierresouchay force-pushed the security_warnings_for_security_issues branch 3 times, most recently from 0c9d411 to 65f8ff7 Compare March 12, 2020 09:15
@ShimmerGlass
Copy link
Contributor

LGTM

@pierresouchay pierresouchay force-pushed the security_warnings_for_security_issues branch 2 times, most recently from 4b42260 to 0fcd120 Compare March 25, 2020 15:46
@pierresouchay
Copy link
Contributor Author

unstable integration test, force-pushing to fix...

@pierresouchay pierresouchay force-pushed the security_warnings_for_security_issues branch 3 times, most recently from 8c6c585 to 3cc86a3 Compare March 27, 2020 10:35
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this warning a lot! Could you add a test to demonstrate it is working as intended?

agent/agent.go Outdated Show resolved Hide resolved
agent/agent.go Outdated Show resolved Hide resolved
@hanshasselberg hanshasselberg self-assigned this Apr 1, 2020
@hanshasselberg hanshasselberg added the waiting-reply Waiting on response from Original Poster or another individual in the thread label Apr 1, 2020
…out security

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/"
```
@pierresouchay pierresouchay force-pushed the security_warnings_for_security_issues branch from 3cc86a3 to 98ef7b0 Compare April 1, 2020 09:16
@pierresouchay
Copy link
Contributor Author

I like this warning a lot! Could you add a test to demonstrate it is working as intended?

@i0rek DONE, added unit test

@ghost ghost removed waiting-reply Waiting on response from Original Poster or another individual in the thread labels Apr 1, 2020
Copy link
Member

@hanshasselberg hanshasselberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hanshasselberg hanshasselberg merged commit 2a8bf45 into hashicorp:master Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants