-
Notifications
You must be signed in to change notification settings - Fork 191
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
add attributes to exclude fstypes and paths from suid_check find #94
Conversation
Is there a reason rubocop is still set to ruby 1.9? Can it be increased to something like 2.2? |
We should bump rubocop and the ruby version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @mattlqx for that proposed solution. Let me think about the breaking change behaviour
output.stdout.split(/\r?\n/) | ||
end | ||
|
||
def exclude_fstype_paths | ||
mounts = inspec.command('mount') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we open a ticket in inspec to expose their point parsing? https://github.com/inspec/inspec/blob/master/lib/resources/mount.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears they're just inspecting files to see if they are mounts and getting their options from mount if so (https://github.com/inspec/inspec/blob/master/lib/resources/file.rb#L94), rather than parsing all mounts. I don't think the two are really compatible. Individual objects vs. "walking" mounts.
@exclude_paths = exclude_paths || default_exclude_paths | ||
end | ||
|
||
def default_exclude_fstypes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By adding those, we change the default behaviour, which results in a breaking change. @arlimus @atomic111 we have two options:
- release a new major version
- keep the old behaviour but allow the override
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My $0.02 is that it should be the default behavior and have a major version bump because the current default of walking network filesystems is prohibitively expensive and redundant when you have a lot of nodes mounting the same share. If you're using the audit cookbook to run this profile in that scenario, some nodes could take longer than the Chef run interval to even complete. If you're using push jobs on large groups, the job will take as long as the longest node, so using the current version of this profile has increased deploy times as well. IMO, if you want to check network shares for SUID, that should be done by a single ad-hoc process somewhere.
@mattlqx and @chris-rock i like this approach, but then we should check the mount point that they are mounted with nosuid. Then this solution would work without decreasing the security aspects of it. This means we need a control that checks the fstab and the current mounted filesystems, like nfs, nfs4 and so on. |
This profile has been blocking systems with network mounts for extended periods because find walks all of /. This pull by default will ignore network filesystems in the find as well as /sys, /proc and /var/lib/lxd/containers. They are also able to be overriden with the `find_exclude_paths` and `find_exclude_fstypes` attributes. This should speed up the profile execute greatly on systems with large network mounts. Signed-off-by: Matt Kulka <[email protected]>
This profile has been blocking systems with network mounts for extended periods because find walks all of /. This pull by default will ignore network filesystems in the find as well as /sys, /proc and /var/lib/lxd/containers. They are also able to be overriden with the
find_exclude_paths
andfind_exclude_fstypes
attributes. This should speed up the profile execute greatly on systems with large network mounts.Fixes #53 #78