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

sysctl-17 title/description does not match test #48

Open
rjhornsby opened this issue Jan 9, 2017 · 6 comments
Open

sysctl-17 title/description does not match test #48

rjhornsby opened this issue Jan 9, 2017 · 6 comments

Comments

@rjhornsby
Copy link

As per this inline comment[1], there's a mismatch between the title/description and the actual test for systcl-17[2], martian logging.

The title says we're testing to ensure martian logging is disabled, but the actual test verifies that the logging is enabled. If I'm understanding correctly it's possible, even likely, that this is just a simple oversight in commit bb7c532 where the test was updated correctly, but the title/description were missed.

Martian logging enabled matches the chef-os-hardening cookbook behavior[3].

The CIS standards agree martian logging should be enabled. However, the chef BaseOS compliance profile says it should be disabled, perhaps because as the sysctl-17 description states, this logging can create a DoS attack vector.

There's a valid argument to be made either way - enable the logging, or disable it. I don't know which is more correct. Seems like the Chef compliance profile is perhaps the odd man out here, and that logging should be enabled.

[1] bb7c532#commitcomment-20365340
[2]

control 'sysctl-17' do
impact 1.0
title 'Disable log martians'
desc 'log_martians can cause a denial of service attack to the host'
describe kernel_parameter('net.ipv4.conf.all.log_martians') do
its(:value) { should eq 1 }
end
end

[3] https://github.com/dev-sec/chef-os-hardening/blob/ea3c8b6634d1c75fa8e84d43b4122cb27293d78f/attributes/sysctl.rb#L124-L126

@mcgege
Copy link
Member

mcgege commented Jun 22, 2017

I think in production this definitly should be disabled because of the DoC risk

@atomic111
Copy link
Member

i agree it can produce a big logfile. @artem-sidorenko , @chris-rock your opinion? I am not aware anymore why we activate this

@artem-sidorenko
Copy link
Member

artem-sidorenko commented Jun 22, 2017

@atomic111 I agree on disabling this. What about making it configurable here and switch it off per default (and providing a comment why its disabled) ?

I guess it affects all implementations, chef-os-hardening, ansible-os-hardening and puppet-os-hardening. This change will probably lead to a new major release everywhere

@bitvijays
Copy link
Contributor

@mcgege @artem-sidorenko @atomic111 In my opinion ( I might be completely wrong ). With the size of hard-disk now a days in the range of 512GB/ 1-2 TB. It's a rare chance that the log file would become so big to become DoS attack. I would probably, keep it enabled to adhere with the CIS compliance.

@mcgege
Copy link
Member

mcgege commented Jul 2, 2017

Well, on my production servers the size of the root disk is way smaller ... but if it's required by compliance: How about enable this here in the baseline and make it configurable in the check/ansible/puppet implementation plus a warning in the documentation?

@artem-sidorenko
Copy link
Member

With the size of hard-disk now a days in the range of 512GB/ 1-2 TB. It's a rare chance that the log file would become so big to become DoS attack

@bitvijays this might be right for typical metal systems today, but you have a lot of systems as VMs with OS HDDs about 10-20GB, esp in the cloud environments

@atomic111 @chris-rock ping, whats your view: to keep it CIS complaint per default, or to make it operations friendly per default?

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

No branches or pull requests

5 participants