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.

Sign up for GitHub

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

Jump to bottom

Address rules fps #1028

Merged
merged 7 commits into from
Feb 3, 2020
Merged

Address rules fps #1028

merged 7 commits into from
Feb 3, 2020

Conversation

mstemm
Copy link
Contributor

@mstemm mstemm commented Jan 31, 2020 •
edited
Loading

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

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 examples

/area rules

/area integrations

/area tests

/area proposals

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

rule(Write below etc): Add "dsc_host" as a MS OMS program
rule(Write below etc): Let mcafee write to /etc/cma.d 
rule(Write below etc): Let avinetworks supervisor write some ssh cfg
rule(Write below etc): Alow writes to /etc/pki from openshift secrets dir
rule(Write below root): Let runc write to /exec.fifo
rule(Change thread namespace): Let cilium-cni change namespaces
rule(Run shell untrusted): Let puma reactor spawn shells

mstemm added 3 commits January 30, 2020 17:15
@mstemm
Add "dsc_host" as a MS OMS program
f01ffc3 
Sample Falco alert:

```
File below /etc opened for writing (user=<NA> command=dsc_host
/opt/dsc/output PerformRequiredConfigurationChecks 1 parent=python
pcmdline=python
/opt/microsoft/omsconfig/Scripts/PerformRequiredConfigurationChecks.py
file=/etc/opt/omi/conf/omsconfig/con...
```

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm
Let mcafee write to /etc/cma.d
315206d 
Sample Falco alert:

```
File below /etc opened for writing (user=root command=macompatsvc
self_start parent=macompatsvc pcmdline=macompatsvc self_start
file=/etc/cma.d/lpc.conf program=macompatsvc gparent=macompatsvc
ggparent=systemd gggparent=<NA> CID1 image=<NA>)
```

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm
Let avinetworks supervisor write some ssh cfg
8ddcaba 
Sample Falco alert:

```
File below /etc opened for writing (user=root command=se_supervisor.p
/opt/avi/scripts/se_supervisor.py -d parent=systemd pcmdline=systemd
file=/etc/ssh/ssh_monitor_config_10.24.249.200 program=se_supervisor.p
gparent=docker-containe ggparent=docker-con...
```

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@poiana poiana requested review from fntlnz and mfdii January 31, 2020 01:18
@poiana poiana added the size/S label Jan 31, 2020
@mstemm mstemm force-pushed the address-rules-fps branch from bc56610 to a523016 Compare January 31, 2020 01:18
@leodido
Copy link
Member

leodido commented Jan 31, 2020

/cc @leodido

@poiana poiana requested a review from leodido January 31, 2020 11:13
@leodido
Copy link
Member

leodido commented Jan 31, 2020

/check-dco

leodido
leodido reviewed Jan 31, 2020
View reviewed changes
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

Thanks for sending this Mark!

FPs really concern me.

Anyway, would you also update the integration tests for them? Some of them are clearly failing now

mstemm added 4 commits January 31, 2020 16:59
@mstemm
Alow writes to /etc/pki from openshift secrets dir
683738f 
Sample falco alert:

```
File below /etc opened for writing (user=root command=cp
/run/secrets/kubernetes.io/serviceaccount/ca.crt
/etc/pki/ca-trust/source/anchors/openshift-ca.crt parent=bash
pcmdline=bash -c #!/bin/bash\nset -euo pipefail\n\n# set by the node
image\nunset KUB...
```

The exception is conditioned on containers.

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm
Let runc write to /exec.fifo
cc105eb 
Sample Falco alert:

```
File below / or /root opened for writing (user=<NA>
command=runc:[1:CHILD] init parent=docker-runc-cur file=/exec.fifo
program=runc:[1:CHILD] CID1 image=<NA>)
```

This github issue provides some context:
opencontainers/runc#1698

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm
Let cilium-cni change namespaces
527d700 
Sample Falco alert:

```
Namespace change (setns) by unexpected program (user=root
command=cilium-cni parent=cilium-cni host CID2 CID1 image=<NA>)
```

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm
Let puma reactor spawn shells
c7cb49c 
Sample Falco alert:

```
Shell spawned by untrusted binary (user=git shell=sh parent=puma reactor
cmdline=sh -c pgrep -fl "unicorn.* worker\[.*?\]" pcmdline=puma reactor
gparent=puma ggparent=runsv aname[4]=ru...
```

https://github.com/puma/puma says it is "A Ruby/Rack web server built
for concurrency".

Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
@mstemm mstemm force-pushed the address-rules-fps branch from 1e55309 to c7cb49c Compare February 1, 2020 01:00
@mstemm
Copy link
Contributor Author

mstemm commented Feb 1, 2020

Yay for regression tests! I fixed the rules and they’re passing now.

fntlnz
fntlnz approved these changes Feb 3, 2020
View reviewed changes
Copy link
Contributor

@fntlnz fntlnz left a comment

Choose a reason for hiding this comment

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

LGTM

@poiana
Copy link
Contributor

poiana commented Feb 3, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz

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

@poiana poiana added the lgtm label Feb 3, 2020
@poiana
Copy link
Contributor

poiana commented Feb 3, 2020

LGTM label has been added.

Git tree hash: c5ade4dbbe5a8f9a154d7215f9f264d884ca017b

@poiana poiana added the approved label Feb 3, 2020
@fntlnz
Copy link
Contributor

fntlnz commented Feb 3, 2020

This looks good as always @mstemm .
We need better release notes for this before the merge or at least before the next release.

See: https://github.com/falcosecurity/falco/blob/dev/CONTRIBUTING.md#rule-type

@leodido leodido changed the base branch from dev to master February 3, 2020 15:13
@poiana poiana merged commit 3693b16 into master Feb 3, 2020
@poiana poiana deleted the address-rules-fps branch February 3, 2020 15:14
@mstemm
Copy link
Contributor Author

mstemm commented Feb 3, 2020

Okay, updated the release notes with the rule names and changes.

@fntlnz
Copy link
Contributor

fntlnz commented Feb 3, 2020

Thank you @mstemm !

@fntlnz fntlnz added this to the 0.19.1 milestone Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leodido leodido leodido left review comments

@fntlnz fntlnz fntlnz approved these changes

@mfdii mfdii Awaiting requested review from mfdii

Assignees

@fntlnz fntlnz

Labels
approved area/rules dco-signoff: yes kind/bug kind/rule-update lgtm release-note size/S
Projects
None yet
Milestone
0.20.0
Development

Successfully merging this pull request may close these issues.
4 participants
@mstemm @leodido @poiana @fntlnz