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

5.4.1 and 5.4.2 Consider adding a test for public access #12

Open
karikarshivani opened this issue Dec 6, 2022 · 2 comments
Open

5.4.1 and 5.4.2 Consider adding a test for public access #12

karikarshivani opened this issue Dec 6, 2022 · 2 comments

Comments

@karikarshivani
Copy link
Contributor

karikarshivani commented Dec 6, 2022

The guidance states that authorized IP addresses should be added to an allowlist. To me, this sounds like 0.0.0.0/0 should lead to a failed test - even if the user adds it to the allow list - because allowing requests from all IP addresses essentially defeats the purpose of an allowlist.

Maybe we can modify the describe block so that the control doesn't pass if the user just looks at the following output and adds 0.0.0.0/0 to the input:

Screen Shot 2022-12-06 at 1 14 59 PM

actual_allowlist.each do |cidr|
  describe 'Cluster allowlist should match expected allowlist' do
    subject { cidr }
      it { should be_in expected_allowlist }
      it { should_not eq '0.0.0.0/0' } # or something like that
  end
end

I understand if the implication from guidance is insufficient to include a test like this in the control.

@karikarshivani karikarshivani changed the title 5.4.1 Consider adding a test for public access 5.4.1 and 5.4.2 Consider adding a test for public access Dec 6, 2022
@karikarshivani
Copy link
Contributor Author

After discussing with @ejaronne, one approach that would be helpful is to have an if statement that checks for 0.0.0.0/0 in the input first and asks for a manual review if it's found; else go through the list of IP addresses configured on the instance to compare with the allowlist input. Also, language from the guidance can be used for the manual review in the skip statement. Thanks @ejaronne for the recommendation!

@wdower
Copy link
Collaborator

wdower commented Dec 7, 2022

This makes sense. Note that we're pretty much requiring people to specifically define a restricted IP range as an input; we can't give them a default value for the input since we won't know what the IP range will be.

I re-read the CIS benchmark and noticed that:
| "If you specify no CIDR blocks, then the public API server endpoint receives requests from all (0.0.0.0/0) IP addresses."
So we need to make sure the allowlist is not empty as well --

actual_allowlist.each do |cidr|
  describe 'Cluster allowlist should match expected allowlist' do
    subject { cidr }
      it { should be_in expected_allowlist }
      it { should_not eq '0.0.0.0/0' } # or something like that
      it { should_not eq [] } # etc etc
  end
end

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

2 participants