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

delete(query): Remove false positive host_aliases_undefined_or_empty k8s rule #5077

Merged
merged 4 commits into from
Apr 18, 2022

Conversation

Churro
Copy link
Contributor

@Churro Churro commented Mar 28, 2022

Proposed Changes

  • Remove the rule entirely as, to best of my knowledge, the high severity warning is a false positive:
    • According to official docs, the kubelet manages the hosts file. Regular DNS resolution can be overridden using hostAliases if no resolver is available. This is no security issue though.
    • The rule description mentions that containers should not be able to modify the hosts file after they are started. However, specifying hostAliases would not prevent this (as long as readOnlyRootFilesystem: false).
    • Requiring a hostAliases definition seems flawed in the first place: usually, you want to use kube-dns and not list combinations of hostname and IP address manually. This is analogous to how we use the Internet generally, nowadays.

I submit this contribution under the Apache-2.0 license.

@kicsbot
Copy link
Contributor

kicsbot commented Mar 28, 2022

Scan submitted to Checkmarx

@kicsbot
Copy link
Contributor

kicsbot commented Mar 28, 2022

Logo
Checkmarx SAST - Scan Summary & Details

Cx-SAST Summary

Total of 5 vulnerabilities
High 0 High
Medium 0 Medium
Low 5 Low
Info 0 Info

Violation Summary

No policy violation found

@rafaela-soares rafaela-soares added query New query feature community Community contribution labels Mar 29, 2022
@rafaela-soares
Copy link
Contributor

Hello, @Churro!

Your PR has not been forgotten. Thank you so much for being so collaborative 😊

@rafaela-soares
Copy link
Contributor

rafaela-soares commented Apr 12, 2022

Hello again, @Churro!

Our security team confirmed that you are right! Thank you so much for noticing and reporting! 🚀

We will be glad to add your contribution to the next release. Can we ask you to also delete the query kics/assets/queries/terraform/kubernetes/host_aliases_undefined_or_empty, please?

@Churro
Copy link
Contributor Author

Churro commented Apr 14, 2022

Hi @rafaela-soares,

Thank you for following up on this! As requested, I've now also deleted the TF rule.

@rafaela-soares
Copy link
Contributor

rafaela-soares commented Apr 18, 2022

Hello again, @Churro!

Thank you so much!

Sorry for bothering again. Can you please merge the branch with the KICS master? We had a bug in the validate-queries-metadata test.

@Churro
Copy link
Contributor Author

Churro commented Apr 18, 2022

Sure, no problem. Seems to work fine now 👍

Copy link
Contributor

@rafaela-soares rafaela-soares left a comment

Choose a reason for hiding this comment

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

Thank you much, @Churro! 🚀

@rafaela-soares rafaela-soares merged commit f036a7b into Checkmarx:master Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution query New query feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants