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

rule(Rule Delete or rename shell history: warning): type restriction penalty warning #1418

Closed
fntlnz opened this issue Sep 26, 2020 · 4 comments · Fixed by #1423
Closed

rule(Rule Delete or rename shell history: warning): type restriction penalty warning #1418

fntlnz opened this issue Sep 26, 2020 · 4 comments · Fixed by #1423
Assignees
Labels

Comments

@fntlnz
Copy link
Contributor

fntlnz commented Sep 26, 2020

Describe the bug
I just tested the default ruleset on master and it looks like there's a warning there. Not sure that's what we wanted to have.

2020-09-26T14:29:13+0000: Falco version 0.26.0 (driver version 2aa88dcf6243982697811df4c1b484bcbe9488a2)
2020-09-26T14:29:13+0000: Falco initialized with configuration file /etc/falco/falco.yaml
2020-09-26T14:29:13+0000: Loading rules from file /etc/falco/falco_rules.yaml:
Rule Delete or rename shell history: warning (trailing-evttype):
(modify and (
  not evt.arg.name startswith /var/lib/docker and (
  evt.arg.name contains "bash_history" or
  evt.arg.name contains "zsh_history" or
  evt.arg.name contains "fish_read_history" or
  evt.arg.name endswith "fish_history" or
  evt.arg.oldpath contains "bash_history" or
  evt.arg.oldpath contains "zsh_history" or
  evt.arg.oldpath contains "fish_read_history" or
  evt.arg.oldpath endswith "fish_history" or
  evt.arg.path contains "bash_history" or
  evt.arg.path contains "zsh_history" or
  evt.arg.path contains "fish_read_history" or
  evt.arg.path endswith "fish_history"))) or
(open_write and (
  not fd.name startswith /var/lib/docker and (
  fd.name contains "bash_history" or
  fd.name contains "zsh_history" or
  fd.name contains "fish_read_history" or
  fd.name endswith "fish_history")) and evt.arg.flags contains "O_TRUNC")

         does not have all evt.type restrictions at the beginning of the condition,
         or uses a negative match (i.e. "not"/"!=") for some evt.type restriction.
         This has a performance penalty, as the rule can not be limited to specific event types.
         Consider moving all evt.type restrictions to the beginning of the rule and/or
         replacing negative matches with positive matches if possible.
2020-09-26T14:29:13+0000: Loading rules from file /etc/falco/falco_rules.local.yaml:
Rule Delete or rename shell history: warning (trailing-evttype):
(modify and (
  not evt.arg.name startswith /var/lib/docker and (
  evt.arg.name contains "bash_history" or
  evt.arg.name contains "zsh_history" or
  evt.arg.name contains "fish_read_history" or
  evt.arg.name endswith "fish_history" or
  evt.arg.oldpath contains "bash_history" or
  evt.arg.oldpath contains "zsh_history" or
  evt.arg.oldpath contains "fish_read_history" or
  evt.arg.oldpath endswith "fish_history" or
  evt.arg.path contains "bash_history" or
  evt.arg.path contains "zsh_history" or
  evt.arg.path contains "fish_read_history" or
  evt.arg.path endswith "fish_history"))) or
(open_write and (
  not fd.name startswith /var/lib/docker and (
  fd.name contains "bash_history" or
  fd.name contains "zsh_history" or
  fd.name contains "fish_read_history" or
  fd.name endswith "fish_history")) and evt.arg.flags contains "O_TRUNC")

This is happening because of the not evt.arg.name startswith /var/lib/docker condition added to the Delete or rename shell history rule in #1393

Unfortunately, this went into the 0.26.0 release.
How to reproduce it

  • Start Falco from master

  • The warning immediatelly shows
    Expected behaviour

  • Falco starts and there is no warning

@fntlnz
Copy link
Contributor Author

fntlnz commented Sep 26, 2020

We will probably need to fix this and push out a 0.26.1 release.

@leodido
Copy link
Member

leodido commented Sep 28, 2020

I agree, we need to fix this and publish 0.26.1 hotfix release.

Anyways, before doing this we'd need the 0.26.0 post-release steps (blog post, mailing list announcement, etc.) to be completed. Otherwise, users could be confused by two different releases with such short notice, IMHO.

/cc @falcosecurity/maintainers

@fntlnz
Copy link
Contributor Author

fntlnz commented Sep 30, 2020

From the community call: We decided to publish the 0.26.0 live on the call and we will skip the announcement since we have to release 0.26.1 with the fix. We will do announcement and blog post all together for 0.26.1 once this issue is fixed.

@fntlnz
Copy link
Contributor Author

fntlnz commented Sep 30, 2020

Just talked to @mstemm - Mark is looking into this
/assign @mstemm

mstemm added a commit that referenced this issue Sep 30, 2020
Move the "and not" checks to the end of the rule so all event type
checks are at the front.

Also break into 3 macros to make the rule easier to read.

This fixes #1418.

Signed-off-by: Mark Stemm <[email protected]>
poiana pushed a commit that referenced this issue Oct 1, 2020
Move the "and not" checks to the end of the rule so all event type
checks are at the front.

Also break into 3 macros to make the rule easier to read.

This fixes #1418.

Signed-off-by: Mark Stemm <[email protected]>
leogr pushed a commit to falcosecurity/rules that referenced this issue Dec 21, 2022
Move the "and not" checks to the end of the rule so all event type
checks are at the front.

Also break into 3 macros to make the rule easier to read.

This fixes falcosecurity/falco#1418.

Signed-off-by: Mark Stemm <[email protected]>
leogr pushed a commit to falcosecurity/rules that referenced this issue Dec 21, 2022
Move the "and not" checks to the end of the rule so all event type
checks are at the front.

Also break into 3 macros to make the rule easier to read.

This fixes falcosecurity/falco#1418.

Signed-off-by: Mark Stemm <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants