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

Falco alerts mapping #59

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

aleksandra-galara
Copy link
Member

No description provided.

@bzurkowski bzurkowski added this to the 0.2 milestone Apr 21, 2020
Copy link
Member

@bzurkowski bzurkowski left a comment

Choose a reason for hiding this comment

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

@aleksandra-galara Thanks for another contribution! Now we have a complete mapping for both Prometheus and Falco alerts. Great progress 😌I added two minor remarks for this change to consider.

Also, I noticed that the branch with this change (Falco_alerts_mapping) originates from Prometheus_alerts_mapping - it should originate from master branch instead. As a result, this PR includes all changes related to Prometheus mapping. Please, do the interactive rebase to remove the unwanted Prometheus commits, then rebase with the master branch, and lastly, squash your commits.

origin: kubernetes
kind: config_map
properties:
name: ka.req.configmap.name
Copy link
Member

Choose a reason for hiding this comment

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

Config maps are namespaced. To avoid conflicts you must filter by namespace property as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, due to k8s_audit_rules.yaml this one alert doesn't give namespace in the output:

- rule: Create/Modify Configmap With Private Credentials
  desc: >
     Detect creating/modifying a configmap containing a private credential (aws key, password, etc.)
  condition: kevt and configmap and kmodify and contains_private_credentials
  output: K8s configmap with private credential (user=%ka.user.name verb=%ka.verb configmap=%ka.req.configmap.name config=%ka.req.configmap.obj)
  priority: WARNING
  source: k8s_audit
  tags: [k8s]

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, that's a shame 😄I think we could contribute a fix for this in Falco.

Comment on lines 684 to 693
- name: "Untrusted Node Successfully Joined the Cluster"
source_mapping:
origin: kubernetes
kind: cluster
properties: {}
- name: "Untrusted Node Unsuccessfully Tried to Join the Cluster"
source_mapping:
origin: kubernetes
kind: cluster
properties: {}
Copy link
Member

Choose a reason for hiding this comment

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

At first, I had the same mapping intuition, however, if you extract the essence of these two alerts, they come down to a warning: The node is untrusted. The fact that a node is untrusted is the key information, not the fact that the node joined a cluster - it's a regular behaviour, we already record these events. I think it would be valuable for an operator to highlight in the graph the nodes that have been identified as untrusted.

Is it possible to map the above alerts to node entities?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, It's possible. I just wasn't sure if the node would be placed in the graph in case of "trying to join unsuccessfully", so I mapped the alert to the cluster kind. I'm gonna change it! ;)

It refers to openrca#35 and complete mapping
alerts due to list created in issue.

Signed-off-by: Aleksandra Galara <a.galara@samsung.com>
Copy link
Member

@bzurkowski bzurkowski left a comment

Choose a reason for hiding this comment

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

Looks good. Let's merge!

@bzurkowski bzurkowski merged commit c0cea8b into openrca:master Apr 21, 2020
@bzurkowski bzurkowski linked an issue Apr 22, 2020 that may be closed by this pull request
@bzurkowski bzurkowski removed this from the 0.2 milestone Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Falco alerts mapping
2 participants