Custom finding type classifier securityhub

[Lux-CC]

add type_classifier key for security hub which will be appended to all finding types that both have a namespace and a category already set.

Note that this is an 'all or nothing' approach for type classifiers (either all types get the classifier appended or none), but IMO this is not a bad thing.

Original issue #4115:

In security hub, finding types are formatted as: namespace/category/classifier, where the category and classifier are optional. (https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-findings-format.html)

Currently in CloudCustodian, only predefined namespaces and categories are supported (e.g. "Effects/Data Exposure", but not a custom classier (e.g. "Effects/Data Exposure/Public Bucket"). This does not allow for granular filtering of Findings in Security Hub's 'Insights' tab.

One of the problems is that Security Hub's default findings have a trailing slash (e.g. for public buckets, the filter is: "TYPE PREFIX Effects/Data Exposure/". The trailing slash filter out Cloud Custodian's "Effects/Data Exposure" Type, and is therefore not visible as an insight.

[kapilt]

Thanks for the pull request! i didn't realize this could be customized. If you don't mind could you sign the CLA at the bottom of the README? Direct link here: https://docs.google.com/forms/d/e/1FAIpQLSfwtl1s6KmpLhCY6CjiY8nFZshDwf_wrmNYx1ahpsNFXXmHKw/viewform

[Lux-CC]

Done and signed! I noticed a check is failing. Is there anything I can do to fix this?

[kapilt]

re failing check its mostly a question of adding in a unit test that uses this functionality. i'm wondering if we want to just make the types free form, and only enforce top level namespace. ie let the user define category and classifier themselves.

from the referenced doc page

One or more finding types in the format of namespace/category/classifier that classify a finding.

Type: array of 50 strings max

Valid namespace values are Software and Configuration Checks | TTPs | Effects | Unusual Behaviors | Sensitive Data Identifications.

Namespace must be a value from the predefined set of namespace values.

Category might be any value, but it's recommended that findings providers use categories from the finding type taxonomy in Types Taxonomy of the AWS Security Finding.

Classifier might be any value, but it's recommended that FPs use the identifier verbatim defined by published standards whenever possible.

[Lux-CC]

True, but then the questions is about how to map the the categories / classifiers to namespaces.
We could make a structure like this:

EDIT: this one is better

types={
  "type": "array",
  "items": {
    "oneOf": [
      {
        "type": "object",
        "required": [
          "namespace"
        ],
        "properties": {
          "namespace": {
            "type": "string",
            "enum": build_vocabulary()
          },
          "category": {
            "type": "string"
          }
        }
      },
      {
        "type": "object",
        "required": [
          "namespace",
          "category"
        ],
        "properties": {
          "namespace": {
            "type": "string",
            "enum": build_vocabulary()
          },
          "category": {
            "type": "string"
          },
          "classifier": {
            "type": "string"
          }
        }
      }
    ]
  }
}

will update later today or tomorrow :)

[kapilt]

Thats interesting re array of dicts, although its worthwhile noting we have backward compatibilities guarantees to adhere to as well. is there any particular value in splitting out the components (besides less horizontal scrolling ;-)? we could just do a custom validator in code to assert the namespace prefix matches up to the defined vocabulary and let the user continue to use strings.

[kapilt]

thanks looks good, i think we want to move that check into the validate method on the action, and then add a test and this should be good to go. i can tackle that later this week if you don't get to it first.

[kapilt]

lgtm, i went ahead added in a unit test for coverage and removed the unused category/classifier static definitions