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

file permissions for symlinked files are not checked correctly #665

Closed
vjeffrey opened this issue Apr 19, 2016 · 2 comments
Closed

file permissions for symlinked files are not checked correctly #665

vjeffrey opened this issue Apr 19, 2016 · 2 comments
Assignees

Comments

@vjeffrey
Copy link

this test fails:
control "xccdf_org.cisecurity.benchmarks_rule_1.5.2_Set_Permissions_on_etcgrub.conf" do
title "Set Permissions on /etc/grub.conf"
desc "Set permission on the /etc/grub.conf file to read and write for root only."
impact 1.0
describe file("/etc/grub.conf") do
it { should exist }
end
describe file("/etc/grub.conf") do
it { should_not be_executable.by "group" }
end
describe file("/etc/grub.conf") do
it { should_not be_readable.by "group" }
end
describe file("/etc/grub.conf") do
it { should_not be_writable.by "group" }
end
describe file("/etc/grub.conf") do
it { should_not be_executable.by "other" }
end
describe file("/etc/grub.conf") do
it { should_not be_readable.by "other" }
end
describe file("/etc/grub.conf") do
it { should_not be_writable.by "other" }
end
end

because:
screen shot 2016-04-19 at 11 12 24 am

@srenatus
Copy link
Contributor

🎉 good catch 😨

@vjeffrey vjeffrey self-assigned this Apr 19, 2016
@arlimus
Copy link
Contributor

arlimus commented Apr 20, 2016

@vjeffrey @srenatus Good use-case! the way that file currently works, is that it will check the item you are pointing to, without resolving any fancy links:

inspec> file('/dev/initctl').mode.to_s(8)
=> "777"
inspec> file('/dev/initctl').symlink?
=> true

The actual target is behind the api:

inspec> file('/dev/initctl').link_path
=> "/run/systemd/initctl/fifo"
inspec> file('/dev/initctl').link_target.mode.to_s(8)
=> "600"

This is great for being specific and not wasting effort each time we need to resolve a file, but you may be pointing to an underlying difficulty in that model, which is: Users might be more interested in always testing the target, i.e. having it resolved. What are your thoughts?

If we decide to change this behavior, let's do it in train and make it applicable everywhere.

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

3 participants