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

feat(os-14) add rule to check noexec, nosuid and nodev mount options #164

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

cmhe
Copy link
Contributor

@cmhe cmhe commented Oct 26, 2021

Setting the noexec, nosuid and nodev mount options for mount
points where those features are not required, limits possible attack
vectors.

Closes: #163

Signed-off-by: Claudius Heine [email protected]

@cmhe cmhe force-pushed the ISSUE-163 branch 3 times, most recently from 32b870b to 8d0cbe9 Compare October 26, 2021 09:05
controls/os_spec.rb Outdated Show resolved Hide resolved
controls/os_spec.rb Outdated Show resolved Hide resolved
controls/os_spec.rb Outdated Show resolved Hide resolved
title 'Check mount options (noexec, nodev, nosuid)'
desc 'Use the noexec, nodev and nosuid mount options to limit attack vectors via mount points'

inspec.file('/proc/self/mountinfo').content.split(/\n/).map { |l| l.split[4] }.each do |mnt_point|
Copy link
Member

Choose a reason for hiding this comment

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

Its very dangerous to just run over all mounts. Is there a reason why we need this for all? This also makes the logic about blocklist and approvelist more complex. Is there another way to achieve the result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unix systems can mount everywhere, and therefore a static list of mounts might not be enough.

TBH, I currently don't see the danger in iterating over mounted volumes and reporting on their current mount options. Maybe you can elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

You can run into may issues that are unexpected here. I propose the following:

  • We optimize os-14 to cover the matrix that is outlined in https://github.com/CISOfy/lynis/blob/3.0.6/include/tests_filesystems#L562
  • I like the ability to test all mountpoints. I discussed with @atomic111 about the approach and we agree that we do not understand the effects on production systems yet. General guidance should be to disallowed mounting for unprivileged users in the first place. And we know a few places where this control would break existing deploys.

Therefore I propose we add a specific control that scans all mount points (minus the one already tested on os-14). That control should be flagged as optional, therefore will not be activated as default yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the dynamic mount check, and now only check the mount points specified.

I haven't found any precedence of optionally flagged controls in dev-sec, how should we go about that?

Copy link
Member

Choose a reason for hiding this comment

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

You would define an attribute and then add a only_if option if the variable is active

see
https://github.com/dev-sec/cis-docker-benchmark/blob/master/controls/host_configuration.rb#L52-L70 for inspiration

title 'Check mount options (noexec, nodev, nosuid)'
desc 'Use the noexec, nodev and nosuid mount options to limit attack vectors via mount points'

inspec.file('/proc/self/mountinfo').content.split(/\n/).map { |l| l.split[4] }.each do |mnt_point|
Copy link
Member

Choose a reason for hiding this comment

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

You can run into may issues that are unexpected here. I propose the following:

  • We optimize os-14 to cover the matrix that is outlined in https://github.com/CISOfy/lynis/blob/3.0.6/include/tests_filesystems#L562
  • I like the ability to test all mountpoints. I discussed with @atomic111 about the approach and we agree that we do not understand the effects on production systems yet. General guidance should be to disallowed mounting for unprivileged users in the first place. And we know a few places where this control would break existing deploys.

Therefore I propose we add a specific control that scans all mount points (minus the one already tested on os-14). That control should be flagged as optional, therefore will not be activated as default yet.

controls/os_spec.rb Outdated Show resolved Hide resolved
Setting the `noexec`, `nosuid` and `nodev` mount options for mount
points where those features are not required, limits possible attack
vectors.

Closes: dev-sec#163

Signed-off-by: Claudius Heine <[email protected]>
@chris-rock
Copy link
Member

This looks great. Thank you @cmhe

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.

Add checks for mount options (noexec, nosuid, nodev)
2 participants