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

Improved container and host policy fuzzer #1891

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

prady0t
Copy link
Contributor

@prady0t prady0t commented Nov 10, 2024

Purpose of PR?:

Fixing bugs in fuzzers.
Seed value changed from yaml to json for checking for generation of invalid json inputs. ContainerPolicy and HostPolicy now returns err for generated errors.

@prady0t
Copy link
Contributor Author

prady0t commented Nov 10, 2024

FuzzHostPolicy still needs to be updated based on the review. @daemon1024 would love your reviews here

@prady0t
Copy link
Contributor Author

prady0t commented Nov 10, 2024

Inside FuzzContainerPolicy, I've added a new case :

len(policyEvent.Object.Spec.Selector.MatchLabels) == 0 && res.Status == pb.PolicyStatus_Invalid

Based on https://github.com/kubearmor/KubeArmor/blob/2cfc2e24d13eac6d857ad53723f9bd2025892b95/KubeArmor/core/unorchestratedUpdates.go#L234C13-L234C33

This also solved the error we were getting earlier with this value :

"{\"oBjeCt\":{\"metAdAtA\":{\"nAme\":\"0\"}}}"

There are a few more cases where fuzzer throws PolicyStatus_Invalid status, but are very hard for fuzzers to generate values for such cases.

if _, ok := secPolicy.Spec.Selector.MatchLabels["kubearmor.io/container.name"]; ok && len(secPolicy.Spec.Selector.MatchLabels) > 1 {

What do you think of these conditions?

@DelusionalOptimist
Copy link
Member

There are a few more cases where fuzzer throws PolicyStatus_Invalid status, but are very hard for fuzzers to generate values for such cases.

@prady0t Can you share more on why it is hard? Is it because it's not possible to add a new label through them or have the enforcer based check?

@prady0t
Copy link
Contributor Author

prady0t commented Nov 27, 2024

Yes because it's hard for fuzzer to generate such conditions. We can still add this condition inside fuzzer if we want

@DelusionalOptimist
Copy link
Member

Yes because it's hard for fuzzer to generate such conditions. We can still add this condition inside fuzzer if we want

I think it should be fine. Ideally these should be handled by policy validation.

@DelusionalOptimist
Copy link
Member

Please rebase @prady0t.

Signed-off-by: prady0t <[email protected]>
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.

2 participants