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

[Feature Request] Return affected resource FQDN in results JSON #58

Open
dinvlad opened this issue Sep 16, 2020 · 9 comments
Open

[Feature Request] Return affected resource FQDN in results JSON #58

dinvlad opened this issue Sep 16, 2020 · 9 comments

Comments

@dinvlad
Copy link
Contributor

dinvlad commented Sep 16, 2020

Hi Team,

We're trying to make sense of all of the CIS findings in some of our projects, and how to begin to address them.
Currently, to even identify the affected resources, all we have is a message like this:

"title": "[DB] Ensure that Cloud SQL database instances do not have public IPs",
"results": [{
  "status": "failed",
  "code_desc": "[{project}] CloudSQL {instance} type is expected not to include \"PRIMARY\"",
  "run_time": 0.000317756,
  "start_time": "2020-05-29T10:25:44+00:00",
  "message": "expected \"PRIMARY\" not to include \"PRIMARY\""
}]

As you can see, {instance} can only be identified programmatically by parsing code_desc message.
It would be really helpful if, for each finding, the CIS benchmark also returned the FQDN of the affected resource:

"code_desc": "[{project}] CloudSQL {instance} type is expected not to include \"PRIMARY\"",
"resource": "https://sqladmin.googleapis.com/sql/v1beta4/projects/{project}/instances/{instance}"

This would allow us to quickly detect which resources are affected, and possibly set up some automation for it, which is much harder to do if all we have is a human-readable suggestion.

Thanks!

@dinvlad dinvlad changed the title [Feature Request] Return affected resource in results JSON [Feature Request] Return affected resource FQDN in results JSON Sep 16, 2020
@binamov
Copy link
Member

binamov commented Sep 16, 2020

We've deliberately been baking in both project-id and resource name into the describe across the board, in this case: https://github.com/GoogleCloudPlatform/inspec-gcp-cis-benchmark/blob/master/controls/6.06-db.rb#L50
describe "[#{gcp_project_id}] CloudSQL #{db}" do

So the error messages should look like:
[bakh-good] CloudSQL bakh-good-mysql type is expected not to include \"PRIMARY\"

Output jsons seem to have that information too, I can see it eg when adding the json to https://heimdall-lite.mitre.org/

@dinvlad
Copy link
Contributor Author

dinvlad commented Sep 16, 2020

OK, makes sense. So in that case, we can treat this as a standardized message, so we can parse it I guess..

@binamov
Copy link
Member

binamov commented Sep 16, 2020

Yeah we wanted the message to be descriptive enough to help investigations, so we're following this approach across all our profiles.

@dinvlad
Copy link
Contributor Author

dinvlad commented Sep 16, 2020

The problem is, a lot of code messages don't follow this format or are not uniform across various controls though, for example:

ServiceAccount Keys for <sa_name>@<project_id>.iam.gserviceaccount.com older than 7776000 seconds is expected not to exist

Role:roles/editor Its member group:<group>@<domain> is expected to match /@<another_domain>/

CloudSQL <instance_id> is expected to have ip configuration require ssl

GCS Bucket <bucket>, Role: roles/storage.objectViewer members is expected not to include "allUsers"

default-allow-ssh should not allow SSH from 0.0.0.0/0

DNS Zone [<network_id>] DNSSEC is expected to equal "on"

So how do we parse it in a more general way, instead of having to create a regex for each control that's different?

@binamov
Copy link
Member

binamov commented Sep 16, 2020

Why parse and not just share the message as-is?

@dinvlad
Copy link
Contributor Author

dinvlad commented Sep 16, 2020

@binamov this is because we'd like to build additional automation around it, as explained :) I.e. we'd like to have a programmatic way to the findings that are machine-readable, as opposed to human-readable..

@binamov
Copy link
Member

binamov commented Sep 16, 2020

I'm not sure adding eg tag is supported from inside a describe block. We could possibly introduce some redundancy by saying something like:

its('parent') {should eq project_id}
its('name') {should eq name}

inside of each describe, but still that name field is different from resource to resource, eg instance_id , bucket etc.

@binamov
Copy link
Member

binamov commented Sep 16, 2020

@aaronlippold any thoughts on this?

@aaronlippold
Copy link
Contributor

Hi guys, I would have to look at the describe blocks in question to dig in ... but it seems like the use of a subject and perhaps switching the describe blocks to the expect syntax may help clean up the reporting of the test results.

The idea of using a rspec explicit subject - https://relishapp.com/rspec/rspec-core/v/3-9/docs/subject/explicit-subject - is to use the describe message as the 'context of the ideaand then theits` statements are just the details. You could further use variables in the description message to more precisely describe the group of items or item we are testing and that it did or did not meet what we expected.

Secondly, using the more detailed rspec expect syntax - https://docs.chef.io/inspec/migration/#expect-syntax-with-inspec - which in some cases we have found to be very useful in complex tests. We give some examples in our advanced inspec class here: https://mitre-inspec-advanced-developer.netlify.app/course/5.html#_5-3-checking-password-encryption

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