GoogleCloudPlatform / inspec-gcp-cis-benchmark

GCP CIS 1.1.0 Benchmark InSpec Profile
Apache License 2.0
129 stars 53 forks source link

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

Open dinvlad opened 4 years ago

dinvlad commented 4 years ago

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!

binamov commented 4 years ago

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 commented 4 years ago

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

binamov commented 4 years ago

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

dinvlad commented 4 years ago

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 commented 4 years ago

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

dinvlad commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@aaronlippold any thoughts on this?

aaronlippold commented 4 years ago

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