search

falcosecurity / rules

Falco rule repository
https://falcosecurity.github.io/rules/
Apache License 2.0
93 stars 68 forks source link

update: split `falco_rules.yaml` according to the rules maturity #149

Closed leogr closed 1 year ago

leogr commented 1 year ago

What type of PR is this?

/kind feature /kind cleanup /kind design /kind documentation

Any specific area of the project related to this PR?

/area rules /area registry /area build /area documentation

Proposed rule maturity level

This PR does not propose new rules or change the maturity level of any existing rule.

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

While splitting rules, I faced some issues that I needed to address in this PR

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing 671afa632fcdaec6e779b8185bb7492883a310e1 with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing e79989c12e3fb7900c135ec8f87cc6ff40c5f3fd with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing f8226e3481778321b835a47b30f11ef1132c44fc with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing d90939c2a0f143357c83988b765ecf3a1c774282 with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

incertum commented 1 year ago

@leogr 🚀 did a first pass and verified we didn't loose any rule or don't have a duplicated rules. Pushed an update to the py rules overview generator.

Will do a second pass review later.

Agreed with naming convention, looks good!

incertum commented 1 year ago

@leogr just reviewed again and LGTM! Seems we still need some more CI adjustments.

I think this is a great idea for now to duplicate the macros and lists in the respective rules files to ensure that they are self contained. In the future we can think of better ways to reduce duplication.

You also mentioned that the comments within the rules files are inconsistent -- I do agree, it would be a much appreciated follow up cleanup. In that line, perhaps should we have all macros first then all lists vs now some are at the top and some right before the rule uses the macro (mostly when its a very specific macro)? WDYT? Maybe even have the rules and macros listed in alphabetical order or by "topic"?

leogr commented 1 year ago

@leogr just reviewed again and LGTM! Seems we still need some more CI adjustments. Thank you! :pray:

re: CI adjustments The CI failing CI checks ( the 3 Rules / check-version... ) should be skipped in this case, since the previous versions of the rules files are non-existing. Let me see if I can fix it shortly, otherwise, we will do a follow-up PR.

I think this is a great idea for now to duplicate the macros and lists in the respective rules files to ensure that they are self contained. In the future we can think of better ways to reduce duplication.

Before sharing my final thoughts on that, I want to play a bit with the current approach when multiple rules files are loaded simultaneously. I will let you know.

You also mentioned that the comments within the rules files are inconsistent -- I do agree, it would be a much appreciated follow up cleanup. In that line, perhaps should we have all macros first then all lists vs now some are at the top and some right before the rule uses the macro (mostly when its a very specific macro)? WDYT? Maybe even have the rules and macros listed in alphabetical order or by "topic"?

The issues I found with comments are due to practical reasons. In particular:

However, all of this is for another PR for sure. I'll open an issue to track it.

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing bb404ca03e5943e2214d61e33b8d52d32754d26d with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing a0c034569ecd024a69768cc12aeba0785982f814 with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing e7507257093549dce877839693955bc689f258ef with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

github-actions[bot] commented 1 year ago

Rules files suggestions

falco_rules.yaml

Comparing 6651dbd69046499600ab8b86366526a466a6cabd with latest tag falco-rules-1.0.1

Major changes:

Patch changes:

leogr commented 1 year ago

@incertum

I think this is a great idea for now to duplicate the macros and lists in the respective rules files to ensure that they are self contained. In the future we can think of better ways to reduce duplication.

Before sharing my final thoughts on that, I want to play a bit with the current approach when multiple rules files are loaded simultaneously. I will let you know.

Ok, I did some experiments, and my conclusion is that we should keep duplicate entries for now. In this way, duplicated items are just silently overwritten. The only con is that the loading order affects the end results when the duplicate item is not identical (for example, if it has been modified in one file but not in the other).

The alternative would be to use an idiomatic syntax to make one rules file depend on an item defined in another other rule files, for example:

- macro: ansible_running_python
  append: true
  condition: " "

This :point_up: would force the user to load another rules file with the ansible_running_python macro definition. However, this is ugly and not necessarily the best option. In any case, the duplicated item issue should be separately discussed in another issue/PR (and likely to be postponed to Falco 0.37).

For the record, here's the list of dups:

Duplicated Macros:
Duplicated macro: ansible_running_python
        File: ../rules/falco_rules.yaml, Line: 248
        File: ../rules/falco-sandbox_rules.yaml, Line: 489
Duplicated macro: calico_node
        File: ../rules/falco-incubating_rules.yaml, Line: 383
        File: ../rules/falco-sandbox_rules.yaml, Line: 1022
Duplicated macro: open_write
        File: ../rules/falco_rules.yaml, Line: 37
        File: ../rules/falco-incubating_rules.yaml, Line: 29
        File: ../rules/falco-sandbox_rules.yaml, Line: 37
Duplicated macro: container_started
        File: ../rules/falco-incubating_rules.yaml, Line: 299
        File: ../rules/falco-sandbox_rules.yaml, Line: 467
Duplicated macro: exe_running_docker_save
        File: ../rules/falco-incubating_rules.yaml, Line: 351
        File: ../rules/falco-sandbox_rules.yaml, Line: 660
Duplicated macro: never_true
        File: ../rules/falco_rules.yaml, Line: 58
        File: ../rules/falco-deprecated_rules.yaml, Line: 37
        File: ../rules/falco-incubating_rules.yaml, Line: 46
        File: ../rules/falco-sandbox_rules.yaml, Line: 51
Duplicated macro: open_read
        File: ../rules/falco_rules.yaml, Line: 40
        File: ../rules/falco-incubating_rules.yaml, Line: 32
        File: ../rules/falco-sandbox_rules.yaml, Line: 40
Duplicated macro: modify
        File: ../rules/falco-incubating_rules.yaml, Line: 73
        File: ../rules/falco-sandbox_rules.yaml, Line: 81
Duplicated macro: allowed_aws_ecr_registry_root_for_eks
        File: ../rules/falco-incubating_rules.yaml, Line: 486
        File: ../rules/falco-sandbox_rules.yaml, Line: 1362
Duplicated macro: remove
        File: ../rules/falco-incubating_rules.yaml, Line: 70
        File: ../rules/falco-sandbox_rules.yaml, Line: 78
Duplicated macro: rename
        File: ../rules/falco-incubating_rules.yaml, Line: 67
        File: ../rules/falco-sandbox_rules.yaml, Line: 72
Duplicated macro: container
        File: ../rules/falco_rules.yaml, Line: 224
        File: ../rules/falco-deprecated_rules.yaml, Line: 133
        File: ../rules/falco-incubating_rules.yaml, Line: 296
        File: ../rules/falco-sandbox_rules.yaml, Line: 464
Duplicated macro: inbound_outbound
        File: ../rules/falco-deprecated_rules.yaml, Line: 55
        File: ../rules/falco-incubating_rules.yaml, Line: 218
        File: ../rules/falco-sandbox_rules.yaml, Line: 364
Duplicated macro: user_ssh_directory
        File: ../rules/falco_rules.yaml, Line: 309
        File: ../rules/falco-incubating_rules.yaml, Line: 358
        File: ../rules/falco-sandbox_rules.yaml, Line: 793
Duplicated macro: run_by_qualys
        File: ../rules/falco_rules.yaml, Line: 254
        File: ../rules/falco-incubating_rules.yaml, Line: 311
Duplicated macro: outbound
        File: ../rules/falco-deprecated_rules.yaml, Line: 44
        File: ../rules/falco-incubating_rules.yaml, Line: 207
        File: ../rules/falco-sandbox_rules.yaml, Line: 353
Duplicated macro: spawned_process
        File: ../rules/falco_rules.yaml, Line: 79
        File: ../rules/falco-incubating_rules.yaml, Line: 76
        File: ../rules/falco-sandbox_rules.yaml, Line: 84
Duplicated macro: veritas_driver_script
        File: ../rules/falco_rules.yaml, Line: 306
        File: ../rules/falco-sandbox_rules.yaml, Line: 627
Duplicated macro: etc_dir
        File: ../rules/falco_rules.yaml, Line: 95
        File: ../rules/falco-sandbox_rules.yaml, Line: 120
Duplicated macro: run_by_google_accounts_daemon
        File: ../rules/falco_rules.yaml, Line: 261
        File: ../rules/falco-incubating_rules.yaml, Line: 333
Duplicated macro: package_mgmt_ancestor_procs
        File: ../rules/falco-incubating_rules.yaml, Line: 166
        File: ../rules/falco-sandbox_rules.yaml, Line: 298
Duplicated macro: postgres_running_wal_e
        File: ../rules/falco_rules.yaml, Line: 433
        File: ../rules/falco-incubating_rules.yaml, Line: 386
Duplicated macro: package_mgmt_procs
        File: ../rules/falco-incubating_rules.yaml, Line: 163
        File: ../rules/falco-sandbox_rules.yaml, Line: 295
Duplicated macro: proc_name_exists
        File: ../rules/falco_rules.yaml, Line: 76
        File: ../rules/falco-incubating_rules.yaml, Line: 64
        File: ../rules/falco-sandbox_rules.yaml, Line: 69
Duplicated macro: chmod
        File: ../rules/falco-incubating_rules.yaml, Line: 79
        File: ../rules/falco-sandbox_rules.yaml, Line: 87
Duplicated macro: user_trusted_containers
        File: ../rules/falco-incubating_rules.yaml, Line: 525
        File: ../rules/falco-sandbox_rules.yaml, Line: 1399
Duplicated macro: run_by_chef
        File: ../rules/falco_rules.yaml, Line: 268
        File: ../rules/falco-sandbox_rules.yaml, Line: 501

Duplicated Lists:
Duplicated list: bash_config_files
        File: ../rules/falco-incubating_rules.yaml, Line: 228
        File: ../rules/falco-sandbox_rules.yaml, Line: 416
Duplicated list: user_mgmt_binaries
        File: ../rules/falco_rules.yaml, Line: 175
        File: ../rules/falco-incubating_rules.yaml, Line: 184
Duplicated list: trusted_images
        File: ../rules/falco_rules.yaml, Line: 612
        File: ../rules/falco-incubating_rules.yaml, Line: 511
        File: ../rules/falco-sandbox_rules.yaml, Line: 1385
Duplicated list: shadowutils_binaries
        File: ../rules/falco_rules.yaml, Line: 122
        File: ../rules/falco-incubating_rules.yaml, Line: 124
        File: ../rules/falco-sandbox_rules.yaml, Line: 171
Duplicated list: sysdigcloud_binaries
        File: ../rules/falco-incubating_rules.yaml, Line: 131
        File: ../rules/falco-sandbox_rules.yaml, Line: 203
Duplicated list: rfc_1918_addresses
        File: ../rules/falco-deprecated_rules.yaml, Line: 41
        File: ../rules/falco-incubating_rules.yaml, Line: 204
        File: ../rules/falco-sandbox_rules.yaml, Line: 350
Duplicated list: mail_binaries
        File: ../rules/falco_rules.yaml, Line: 187
        File: ../rules/falco-incubating_rules.yaml, Line: 196
Duplicated list: docker_binaries
        File: ../rules/falco_rules.yaml, Line: 1081
        File: ../rules/falco-incubating_rules.yaml, Line: 1165
        File: ../rules/falco-sandbox_rules.yaml, Line: 1999
Duplicated list: passwd_binaries
        File: ../rules/falco_rules.yaml, Line: 112
        File: ../rules/falco-incubating_rules.yaml, Line: 114
        File: ../rules/falco-sandbox_rules.yaml, Line: 134
Duplicated list: dev_creation_binaries
        File: ../rules/falco-incubating_rules.yaml, Line: 187
        File: ../rules/falco-sandbox_rules.yaml, Line: 313
Duplicated list: shell_config_files
        File: ../rules/falco-incubating_rules.yaml, Line: 244
        File: ../rules/falco-sandbox_rules.yaml, Line: 432
Duplicated list: nomachine_binaries
        File: ../rules/falco_rules.yaml, Line: 184
        File: ../rules/falco-incubating_rules.yaml, Line: 190
        File: ../rules/falco-sandbox_rules.yaml, Line: 316
Duplicated list: db_server_binaries
        File: ../rules/falco_rules.yaml, Line: 132
        File: ../rules/falco-incubating_rules.yaml, Line: 140
Duplicated list: sshkit_script_binaries
        File: ../rules/falco_rules.yaml, Line: 240
        File: ../rules/falco-sandbox_rules.yaml, Line: 474
Duplicated list: sematext_images
        File: ../rules/falco_rules.yaml, Line: 615
        File: ../rules/falco-incubating_rules.yaml, Line: 528
Duplicated list: userexec_binaries
        File: ../rules/falco_rules.yaml, Line: 172
        File: ../rules/falco-incubating_rules.yaml, Line: 174
Duplicated list: bash_config_filenames
        File: ../rules/falco-incubating_rules.yaml, Line: 225
        File: ../rules/falco-sandbox_rules.yaml, Line: 404
Duplicated list: shell_config_directories
        File: ../rules/falco-incubating_rules.yaml, Line: 247
        File: ../rules/falco-sandbox_rules.yaml, Line: 435
Duplicated list: shell_binaries
        File: ../rules/falco_rules.yaml, Line: 98
        File: ../rules/falco-incubating_rules.yaml, Line: 82
        File: ../rules/falco-sandbox_rules.yaml, Line: 127
Duplicated list: mail_config_binaries
        File: ../rules/falco_rules.yaml, Line: 194
        File: ../rules/falco-sandbox_rules.yaml, Line: 319
Duplicated list: login_binaries
        File: ../rules/falco_rules.yaml, Line: 105
        File: ../rules/falco-incubating_rules.yaml, Line: 107
Duplicated list: package_mgmt_binaries
        File: ../rules/falco_rules.yaml, Line: 164
        File: ../rules/falco-incubating_rules.yaml, Line: 160
        File: ../rules/falco-sandbox_rules.yaml, Line: 279
Duplicated list: shell_config_filenames
        File: ../rules/falco-incubating_rules.yaml, Line: 241
        File: ../rules/falco-sandbox_rules.yaml, Line: 429
Duplicated list: falco_privileged_images
        File: ../rules/falco_rules.yaml, Line: 637
        File: ../rules/falco-incubating_rules.yaml, Line: 548
Duplicated list: csh_config_filenames
        File: ../rules/falco-incubating_rules.yaml, Line: 232
        File: ../rules/falco-sandbox_rules.yaml, Line: 420
Duplicated list: rpm_binaries
        File: ../rules/falco_rules.yaml, Line: 149
        File: ../rules/falco-incubating_rules.yaml, Line: 145
        File: ../rules/falco-sandbox_rules.yaml, Line: 216
Duplicated list: python_package_managers
        File: ../rules/falco_rules.yaml, Line: 159
        File: ../rules/falco-incubating_rules.yaml, Line: 155
        File: ../rules/falco-sandbox_rules.yaml, Line: 274
Duplicated list: cron_binaries
        File: ../rules/falco_rules.yaml, Line: 232
        File: ../rules/falco-incubating_rules.yaml, Line: 305
Duplicated list: csh_config_files
        File: ../rules/falco-incubating_rules.yaml, Line: 235
        File: ../rules/falco-sandbox_rules.yaml, Line: 423
Duplicated list: deb_binaries
        File: ../rules/falco_rules.yaml, Line: 154
        File: ../rules/falco-incubating_rules.yaml, Line: 150
        File: ../rules/falco-sandbox_rules.yaml, Line: 253
Duplicated list: falco_containers
        File: ../rules/falco_rules.yaml, Line: 622
        File: ../rules/falco-incubating_rules.yaml, Line: 535
        File: ../rules/falco-sandbox_rules.yaml, Line: 1403
Duplicated list: zsh_config_filenames
        File: ../rules/falco-incubating_rules.yaml, Line: 238
        File: ../rules/falco-sandbox_rules.yaml, Line: 426
leogr commented 1 year ago

/hold cancel

leogr commented 1 year ago

Hey @falcosecurity/rules-maintainers

This PR is ready. PTAL :pray:

Once merged, I will tag each ruleset with version 2.0.0-rc1 so that we can test with the upcoming Falco 0.36 RC1 :pray:

leogr commented 1 year ago

cc @falcosecurity/falco-maintainers

poiana commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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: - ~~[OWNERS](https://github.com/falcosecurity/rules/blob/main/OWNERS)~~ [incertum,leogr] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
incertum commented 1 year ago

re https://github.com/falcosecurity/rules/pull/149#issuecomment-1704807897

  • There's no standard item's comment structure
    • I would love to define a simple Rule Doc convention (i.e., something similar to Go Doc Comments, but simpler)
  • There's no clear way to distinguish sections from item's comment

Agreed, instead of yet another doc, could we check how much of the comments could simply be removed (for example the engine version mentions could be removed) vs what information could be added to the desc since we now adopt an approach of the desc being a small paragraph vs what information could be generically shared on the existing Falco website rules sections? If it's at the end just about 4-5 special comments about unique macros or rules, we can leave them in the rules yaml where adopters are most likely to find them.

re the duplicate macros: Could we add a CI check ensuring macros and lists are the same in all files? And while we do that ensure upstream rules have no duplicative rules aka no overriding and also ensure macros and lists only appear once per rules file and as said ensure they match up across rules files?