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

update(rules): ehanced rules tagging for inventory / threat modeling #2167

Merged
merged 3 commits into from
Dec 15, 2022

Conversation

incertum
Copy link
Contributor

@incertum incertum commented Aug 17, 2022

Signed-off-by: Melissa Kilby melissa.kilby.oss@gmail.com

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

/kind release

If contributing rules or changes to rules, please make sure to also uncomment one of the following line:

/kind rule-update

/kind rule-create

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area build

/area engine

/area rules

/area tests

/area proposals

/area CI

What this PR does / why we need it:

Proposing subtle changes in tagging rules for inventory and thread modeling purposes.

(1) Add Mitre TTP code(s) [1, 2] to rules under existing tags.
Besides better inventory and threat modeling, this will allow us to quantify areas of improvements for Falco from a threat modeling perspective. Mitre TTP code(s) also provide end users less familiar with system level threats and exploitations with additional information.

(2) Adjust container and host tags usage.

container - The rule only works inside containers -> The rule works inside containers
host - The rule only works outside of containers -> The rule works outside of containers

Why not add host and/or container (either only one or both if applicable) to each rule? Could be more informative for inventory purposes when parsing and aggregating fields in the yaml to for example be able to know if certain threats are only addressed for container workloads or also on the underlying host etc.

(3) [Future Options]

While we do have the priority tag A case-insensitive representation of the severity of the event. sometimes wonder if additional tags around accuracy or scope of the rule could be useful for everyone in addition to existing tags.

"scope" - ["narrow", "medium", "broad", "unknown"] - Is this a more generic or extremely specific detection? For example does the rule only capture one way to elevate privileges or does the rule capture an entire class of vulnerabilities?
"accuracy" - ["low", "medium", "high", "unknown"] - While it depends on the actual workloads and environments, could some sort of FP ratio anticipation still be captured for the rules that are enabled by default? 

References:

[1] https://attack.mitre.org/tactics/enterprise/
[2] https://raw.githubusercontent.com/mitre/cti/master/enterprise-attack/enterprise-attack.json

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

CC @leogr

Does this PR introduce a user-facing change?:

rules(tagging): enhanced rules tagging for inventory / threat modeling

@jasondellaluce
Copy link
Contributor

/milestone 0.34.0

@poiana poiana added this to the 0.34.0 milestone Sep 7, 2022
@darryk10
Copy link
Contributor

darryk10 commented Sep 9, 2022

Hi @incertum, I totally agre with tags container and host. In addition AFAIK you can also load rules based on tags so you may want to use the host ruleset and avoid the container rules. We might need to check and adjust the other rules but I find it really useful.

I do really like the "accuracy" tag idea. However it's hard to evaluate the FP ratio in advance and in reliable way since it's often based on "feeling" about the possible noise. However if we agree how to set this ratio, it would be a good way to split noisy rules from high confident security rules and enable them based on the use cases.

@incertum
Copy link
Contributor Author

Hi @incertum, I totally agree with tags container and host. In addition AFAIK you can also load rules based on tags so you may want to use the host ruleset and avoid the container rules. We might need to check and adjust the other rules but I find it really useful.

Ok, for rules that work for both host and in a container, like this one https://github.com/falcosecurity/falco/pull/2167/files#diff-ea7e90b2fb13b5aa2c8e6799c1ea20612e8bd29a13c61dfb3653f03f0235436eR1738 confirming that it is ok to add both tags, host and container? That way yes you can filter for the host rules, including the ones that would also work in containers. I think that was the disconnect I brought up above that currently the tag host is only used for rules that work only on host, but not in a container which is not too intuitive to me. Also this way we can create nice summary aggregation tables showing number of rules for host and containers and more.

I do really like the "accuracy" tag idea. However it's hard to evaluate the FP ratio in advance and in reliable way since it's often based on "feeling" about the possible noise. However if we agree how to set this ratio, it would be a good way to split noisy rules from high confident security rules and enable them based on the use cases.

Agreed, if you have ideas on how to set these ratios, that would be cool. Even very imperfect ratios could help setting expectations for each rule.

@leogr
Copy link
Member

leogr commented Sep 13, 2022

cc @Kaizhe 🤗

@incertum incertum force-pushed the rules-enhanced-tagging branch from 1aaab8c to 9ab4650 Compare November 17, 2022 07:06
@poiana poiana added size/XL and removed size/S labels Nov 17, 2022
@incertum incertum changed the title [WIP] update(rules): ehanced rules tagging for inventory / threat modeling update(rules): ehanced rules tagging for inventory / threat modeling Nov 17, 2022
@incertum
Copy link
Contributor Author

incertum commented Nov 17, 2022

Published a HackMD preview of the new markdown doc here.

In addition to updating the rules tags added a new rules_inventory folder. Motivation is to create a useful overview for the community. First pass of cleanup and adding Mitre TTP tags can be regarded as complete. Would appreciate help in reviewing the tags to ensure the best available Mitre TTPs were selected 🙏 .

@incertum incertum force-pushed the rules-enhanced-tagging branch from 9ab4650 to d6d26c5 Compare November 17, 2022 18:59
@leogr
Copy link
Member

leogr commented Nov 23, 2022

cc @darryk10 @Kaizhe

@darryk10
Copy link
Contributor

Hi @incertum amazing work! Both inventory and the Mitre TTP mapping is really helpful to understand the coverage.
My suggestion is to generate the mitre coverage map with the tag you updated. In this way we could just report the mitre matrix coverage pictures generated without the tables I see in the doc regarding the tag and the percentage coverage. It would be more intuitive and easy to read. I think all the project can benefit from it.
of course it could stay an idea for the future. :)

@leogr
Copy link
Member

leogr commented Dec 15, 2022

Hi @incertum amazing work! Both inventory and the Mitre TTP mapping is really helpful to understand the coverage. My suggestion is to generate the mitre coverage map with the tag you updated. In this way we could just report the mitre matrix coverage pictures generated without the tables I see in the doc regarding the tag and the percentage coverage. It would be more intuitive and easy to read. I think all the project can benefit from it. of course it could stay an idea for the future. :)

I agree with @darryk10 , however, I think it's ok to address the points you raised in follow-up PRs.

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>
* add ad-hoc python script to generate Falco default rules overview markdown document
* init rules_inventory/rules_overview.md doc

Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>
Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>
@leogr leogr force-pushed the rules-enhanced-tagging branch from d6d26c5 to ce2d337 Compare December 15, 2022 15:03
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PS I had to rebase the PR because of a conflict.

@poiana
Copy link
Contributor

poiana commented Dec 15, 2022

LGTM label has been added.

Git tree hash: a2ad7811d96d93ba32f7fe91622c7bcf39454777

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Amazing work!

@poiana
Copy link
Contributor

poiana commented Dec 15, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants