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 Continuous Fuzzing Integration to Fuzzit #3093

Merged
merged 1 commit into from Aug 18, 2019

Conversation

@yevgenypats
Copy link
Contributor

commented Aug 7, 2019

This feature introduce continuous fuzzing with the following
features:

Do not merge yet as this is work in progress but comments are welcome.

  • Ruzzing: fuzz-targets are run continuously on master
    ( the fuzzers are updated every time new code is pushed to master)
  • Regresion: In addition to unit-tests travis runs all fuzz
    targets through the generated corpus to catch bugs early on
    in the CI process before merge.

1. Why is this pull request needed and what does it do?

2. Which issues (if any) are related?

3. Which documentation changes (if any) need to be made?

4. Does this introduce a backward incompatible change or deprecation?

@yevgenypats yevgenypats force-pushed the fuzzitdev:fuzzit-integration branch from a5cd029 to eded3bf Aug 7, 2019

@miekg
Copy link
Member

left a comment

Thanks for picking this up so quickly. Think it's very good to have this automatically enabled.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@miekg

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

Regarding the go module it's an open issue. I'm not sure it's an easy fix but it will be supported. Hopefully it will also be supported as part of go standard toolchain. We'll have to workaround GOMODULES for now.

@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

@miekg I'm having trouble building the go-fuzz targets without the libfuzzer support (This is even not related to this PR).

go-fuzz-build -tags fuzz ./plugin/chaos

prints

/go/src/github.com/go-acme/lego/certificate/authorization.go:6: cannot find package "github.com/go-acme/lego/v3/acme" in any of:
        /tmp/go-fuzz-build563253197/goroot/src/github.com/go-acme/lego/v3/acme (from $GOROOT)
        /tmp/go-fuzz-build563253197/gopath/src/github.com/go-acme/lego/v3/acme (from $GOPATH)
/go/src/github.com/go-acme/lego/certificate/certificates.go:16: cannot find package "github.com/go-acme/lego/v3/acme/api" in any of:
        /tmp/go-fuzz-build563253197/goroot/src/github.com/go-acme/lego/v3/acme/api (from $GOROOT)
        /tmp/go-fuzz-build563253197/gopath/src/github.com/go-acme/lego/v3/acme/api (from $GOPATH)

I guess something connected to go modules as well.

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

mm. Yes it might help though I was able to compile the targets on my mac somehow. I'm not able to reproduce it again on a linux docker. But it would be best not to wait for go-fuzz support imho and try to workaround the issue, this should work even with repos that support go modules.

@yevgenypats yevgenypats force-pushed the fuzzitdev:fuzzit-integration branch 2 times, most recently from 5ab6ea5 to 0cc849d Aug 18, 2019

Add Continuous Fuzzing Integration to Fuzzit
This feature introduce continuous fuzzing with the following
features:

* Ruzzing: fuzz-targets are run continuously on master
( the fuzzers are updated every time new code is pushed to master)
* Regresion: In addition to unit-tests travis runs all fuzz
targets through the generated corpus to catch bugs early  on
in the CI process before merge.

@yevgenypats yevgenypats force-pushed the fuzzitdev:fuzzit-integration branch from 0cc849d to 43dd423 Aug 18, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Aug 18, 2019

Codecov Report

Merging #3093 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3093      +/-   ##
==========================================
+ Coverage   55.73%   55.75%   +0.01%     
==========================================
  Files         210      210              
  Lines       10516    10516              
==========================================
+ Hits         5861     5863       +2     
+ Misses       4229     4228       -1     
+ Partials      426      425       -1
Impacted Files Coverage Δ
plugin/clouddns/clouddns.go 82.72% <0%> (-2.73%) ⬇️
plugin/route53/route53.go 86.61% <0%> (+2.11%) ⬆️
plugin/file/reload.go 74.28% <0%> (+5.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b53d822...43dd423. Read the comment docs.

@@ -44,6 +44,26 @@ ifeq ($(TEST_TYPE),coverage)
fi; \
done
endif
ifeq ($(TEST_TYPE),fuzzit)
# skip fuzzing for PR
if [ "$(TRAVIS_PULL_REQUEST)" = "false" ] || [ "$(FUZZIT_TYPE)" = "local-regression" ] ; then \

This comment has been minimized.

Copy link
@miekg

miekg Aug 18, 2019

Member

this comes out as if [ "3093" = "false" ] || [ "fuzzing" = "local-regression" ] ; then \ is that correct?

go get -u github.com/dvyukov/go-fuzz/go-fuzz-build; \
go get -u -v .; \
cd ../../go-acme/lego; \
git checkout v2.5.0; \

This comment has been minimized.

Copy link
@miekg

miekg Aug 18, 2019

Member

oh wow, this is pretty horrible :/

I think it make sense to make this into Makefile.fuzz under some phony install target, so we can call make -f Makefile.fuzz install here. or $(MAKE) or somethign

This comment has been minimized.

Copy link
@yevgenypats

yevgenypats Aug 18, 2019

Author Contributor

This is the workaround for the strange glitch with go-acme v3. How do you suggest working around that? (not sure I got it regarding the make install)

@yevgenypats yevgenypats changed the title WIP: Add Continuous Fuzzing Integration to Fuzzit Add Continuous Fuzzing Integration to Fuzzit Aug 18, 2019

@corbot

This comment has been minimized.

Copy link

commented Aug 18, 2019

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked dilyevsky (via /OWNERS) for a review.
Note this is not an exclusive request. Anyone is free to provide a review of this pull request.

If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository.

The bot understands the commands that are listed here.

@corbot corbot bot requested a review from dilyevsky Aug 18, 2019

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

thanks for making this work!

Got a couple of optimizations that could be done in this PR or in a follow up one.

The API key def. needs some docs and where to get it/change it/remove it. And maybe some words on that it is safe to put in source code.

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

@miekg Thanks for your review:) I suggest to do the optimization in a following PR.

You need to signup at https://app.fuzzit.dev and I'll add you to the core-dns group so you can view crashes as well as get notified of new crashes/bugs (there is one bug so far as far as I can see).

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

@miekg miekg merged commit c33fc9e into coredns:master Aug 18, 2019

4 checks passed

ci/circleci: kubernetes-tests Your tests passed on CircleCI!
Details
codecov/project 55.75% (target 50%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
stickler-ci No lint errors found.
@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

@yevgenypats

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

Success it works! https://travis-ci.org/coredns/coredns/jobs/573356225

Here is the link to the crash (it classifies incorrectly as leak, Im aware of that and will push a fix soon):
https://app.fuzzit.dev/orgs/coredns/targets/rewrite/ZcyF7mWmglds6Z589dKJ

@miekg

This comment has been minimized.

Copy link
Member

commented Aug 18, 2019

this is awesome!

@bookmoons bookmoons referenced this pull request Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.