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

Add fuzzer with OSS-fuzz build script #14202

Merged
merged 1 commit into from
Jan 25, 2021
Merged

Add fuzzer with OSS-fuzz build script #14202

merged 1 commit into from
Jan 25, 2021

Conversation

AdamKorcz
Copy link
Contributor

This PR adds a fuzzer implemented with the go-fuzz fuzzing engine. Furthermore a build script is added to set up continuous fuzzing by way of OSS-fuzz.

Fuzzing is a way of testing programs wherby pseudo-random data is passed to a target application with the goal of finding bugs and vulnerabilities. In this fuzzer, the entrypoint to Cilium is the UnmarshalJSON implementatin in the labels package.

Integrating Cilium into OSS-fuzz allows Google to run all fuzzers in the Cilium project continuously free of charge. If bugs are found, maintainers get notified with detailed bug reports that include stack trace and reproducible test case. The service is offered free of charge to open source project with an implied expectation that bugs are fixed, and it has contributed to finding vulnerabilities in major open source projects like Kubernetes.

If there is interest in integrating Cilium, I will be happy to complete the integration on the OSS-fuzz side. For this at least one maintainer email address is needed.

Signed-off-by: AdamKorcz adam@adalogics.com

@AdamKorcz AdamKorcz requested a review from a team as a code owner November 27, 2020 15:57
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 27, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Nov 27, 2020
@maintainer-s-little-helper
Copy link

Commit bb44428dd151a795e3045c910072b113ccf91e57 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Nov 27, 2020
@maintainer-s-little-helper
Copy link

Commit bb44428dd151a795e3045c910072b113ccf91e57 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@twpayne
Copy link
Contributor

twpayne commented Dec 4, 2020

Thanks for this @AdamKorcz :)

Yes, it would be great to get Cilium fuzzed regularly by OSS-Fuzz. Looking at OSS-Fuzz's guidelines for new projects it looks like Cilium is well qualified.

I've opened google/oss-fuzz#4784 to get Cilium added to OSS-Fuzz as having a PR from a Cilium maintainer skips a manual email verification step.

There are obviously several entry points for fuzzing and Cilium and this is great start, thank you. Let's wait to see if Cilium gets accepted and then merge this and add more.

@twpayne twpayne self-assigned this Dec 4, 2020
@twpayne
Copy link
Contributor

twpayne commented Dec 7, 2020

@AdamKorcz note that for this PR to be merged, all commits will need the Signed-off-by line as described in the contributing guide. You'll need to squash your commits.

@twpayne twpayne added the release-note/misc This PR makes changes that have no direct user impact. label Dec 7, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Dec 7, 2020
@AdamKorcz
Copy link
Contributor Author

@twpayne Thanks for the heads up!

@AdamKorcz
Copy link
Contributor Author

@twpayne Let me know if anything else is missing from my side.

@nebril
Copy link
Member

nebril commented Dec 18, 2020

@AdamKorcz can this script be moved out of the tests directory? Currently tests holds our older bash based test scripts and I think it would be better to put any new features in either test dir, or a completely new one.

@AdamKorcz
Copy link
Contributor Author

@nebril that is not an issue. The build file has hereby been moved.

Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

@AdamKorcz thanks, I think @nebril was referring to both files, oss-fuzz-build.sh and fuzz/fuzz.go to be moved into test/ directory.

@maintainer-s-little-helper
Copy link

Commit be696169abc2894124c30b3dc72601ea817f5a70 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 30, 2020
@maintainer-s-little-helper
Copy link

Commit be696169abc2894124c30b3dc72601ea817f5a70 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit 98ea01ff46669eb78125a3c5f4c7617ce1f5b7ff does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 30, 2020
@AdamKorcz
Copy link
Contributor Author

@aanm Oh sorry about that. Both files have now been moved to /test/fuzzing

Signed-off-by: AdamKorcz <adam@adalogics.com>
@aanm
Copy link
Member

aanm commented Jan 6, 2021

test-me-please

@AdamKorcz
Copy link
Contributor Author

Let me know if there is anything missing from my side.

@twpayne
Copy link
Contributor

twpayne commented Jan 22, 2021

retest-runtime

@aanm aanm merged commit b7dbf0a into cilium:master Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants