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

Fuzzing: Add 3 fuzzers #3458

Merged
merged 1 commit into from Dec 20, 2021
Merged

Fuzzing: Add 3 fuzzers #3458

merged 1 commit into from Dec 20, 2021

Conversation

AdamKorcz
Copy link
Contributor

@AdamKorcz AdamKorcz commented Jul 20, 2021

This PR adds 3 fuzzers for Distribution and a build script to run the fuzzers continuously through OSS-fuzz.

A PR has also been set up on the OSS-fuzz side to integrate Distribution: google/oss-fuzz#6014. Two of the fuzzers that were committed on the OSS-fuzz side are moved upstream in this PR and a third fuzzer is added.

Once this PR gets merged, I will amend the changes on OSS-fuzz to invoke the build script in this PR.

For those unfamiliar with fuzzing and/or OSS-fuzz: Fuzzing (or fuzz testing) is a way of testing software whereby pseudo-random data is passed to a target application with the goal of finding bugs and vulnerabilities. It has been useful in finding bugs, including security-critical bugs, in many open source Go projects to date. Some can be found here, https://github.com/dvyukov/go-fuzz#trophies and others include this CVE in Kubernetes and this CVE in the Go standard library found by fuzzing Go-ethereum.

OSS-fuzz is a free service offered by Google into which critical open source projects can integrate and have their fuzzers run continuously. If any bugs are found, maintainers will get notified with an email containing a link to a detailed bug reports.
The fuzzers in this PR are all implemented by way of go-fuzz which is the Golang engine supported by OSS-fuzz.
To complete the OSS-fuzz PR, at least one maintainers email address is needed for bug reports.

Other projects integrated into OSS-fuzz include Kubernetes, runC and containerd.

reference/fuzz.go Outdated Show resolved Hide resolved
@DavidKorczynski
Copy link

DavidKorczynski commented Oct 6, 2021

@brackendawson are there any updates on this PR? I think Adam resolved any issues and it would be great to get the fuzzing going. We have quite a bit more fuzzers for Distribution but am not sure what is the smartest approach to get them into the project and have them analyse Distribution if there is a fairly slow turnaround? It's a shame because we need to have the fuzzers in the project for them to be integrated with OSS-Fuzz (see the PR Adam mentioned google/oss-fuzz#6014) One solution is to create a dedicated repository containing the fuzzers where we can merge in faster. CC @caniszczyk

@brackendawson
Copy link
Contributor

@brackendawson are there any updates on this PR? I think Adam resolved any issues and it would be great to get the fuzzing going. We have quite a bit more fuzzers for Distribution but am not sure what is the smartest approach to get them into the project and have them analyse Distribution if there is a fairly slow turnaround? It's a shame because we need to have the fuzzers in the project for them to be integrated with OSS-Fuzz (see the PR Adam mentioned google/oss-fuzz#6014) One solution is to create a dedicated repository containing the fuzzers where we can merge in faster. CC @caniszczyk

I'm happy with the current state of this PR. It will need a maintainer to approve the github actions, approve & merge though.

@milosgajdos
Copy link
Member

You will need to rebase to re-kick the actions. There is no way I can kick the GHA run otherwise 🤔

@AdamKorcz
Copy link
Contributor Author

Great if we can finally get this merged @milosgajdos ! Could you see my initial message where I describe the OSS-Fuzz integration: are there any emails I can put in to receive bug reports?

Notice that there are quite a few more fuzzers ready for Distribution and we would like to get these merged in as well https://github.com/cncf/cncf-fuzzing/tree/main/projects/distribution as such, how do you want to go about this? I can create another PR that uploads all of the fuzzers in one go?

@milosgajdos
Copy link
Member

Great if we can finally get this merged @milosgajdos ! Could you see my initial message where I describe the OSS-Fuzz integration: are there any emails I can put in to receive bug reports?

What do you mean by "bug reports"? Security vulnerabilities found by the fuzzers?

Notice that there are quite a few more fuzzers ready for Distribution and we would like to get these merged in as well https://github.com/cncf/cncf-fuzzing/tree/main/projects/distribution as such, how do you want to go about this? I can create another PR that uploads all of the fuzzers in one go?

I'd welcome adding more fuzzers, indeed. Personally, I'd have no issue with having a single PR covering all of those, but I'm but a single maintainer. I can bring it up with other folks at the next call. In the meantime, can you please rebase so the GHA are run?

Signed-off-by: AdamKorcz <adam@adalogics.com>
@codecov-commenter
Copy link

Codecov Report

Merging #3458 (d0ca0c3) into main (01f589c) will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3458      +/-   ##
==========================================
- Coverage   56.38%   56.35%   -0.04%     
==========================================
  Files         102      101       -1     
  Lines        7324     7307      -17     
==========================================
- Hits         4130     4118      -12     
+ Misses       2541     2534       -7     
- Partials      653      655       +2     
Impacted Files Coverage Δ
...bution/distribution/manifest/ocischema/manifest.go 73.80% <0.00%> (-5.61%) ⬇️
...distribution/manifest/manifestlist/manifestlist.go 70.88% <0.00%> (-2.36%) ⬇️
...distribution/distribution/registry/handlers/app.go 45.91% <0.00%> (-0.26%) ⬇️
...com/distribution/distribution/registry/registry.go 45.79% <0.00%> (ø)
...ribution/distribution/contrib/token-server/main.go 0.00% <0.00%> (ø)
...bution/distribution/registry/handlers/basicauth.go 100.00% <0.00%> (ø)
...tribution/registry/storage/blobwriter_resumable.go 50.00% <0.00%> (ø)
.../github.com/distribution/distribution/uuid/uuid.go
.../distribution/distribution/configuration/parser.go 84.61% <0.00%> (+0.13%) ⬆️
...ution/distribution/registry/storage/driver/walk.go 75.00% <0.00%> (+29.54%) ⬆️

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 01f589c...d0ca0c3. Read the comment docs.

@DavidKorczynski
Copy link

What do you mean by "bug reports"? Security vulnerabilities found by the fuzzers?

The goal is to find security and also reliability issues, so the fuzzer will report on both. There is most likely some false positives too if a fuzzer executes code in a way it should not.

I'd welcome adding more fuzzers, indeed. Personally, I'd have no issue with having a single PR covering all of those, but I'm but a single maintainer. I can bring it up with other folks at the next call. In the meantime, can you please rebase so the GHA are run?

Great, we will get these uploaded when the whole OSS-Fuzz integraion has been completed. We can then migrate the fuzzers from https://github.com/cncf/cncf-fuzzing to here

Notice that I did join the meeting around the time of this PR to discuss this work. Essentially it's work we have been doing with the Linux Foundation to support various CNCF projects

@AdamKorcz
Copy link
Contributor Author

While we are waiting for review, which email address can we add for google/oss-fuzz#6014 ?

@milosgajdos
Copy link
Member

While we are waiting for review, which email address can we add for google/oss-fuzz#6014 ?

I dont think we have any specific email address to reach all maintainers of distribution, so unfortunately I've no answer for you here. I wonder if @SteveLasker might know?

@DavidKorczynski
Copy link

We can always add more later, i.e. it's fine to start with a single email address

Copy link
Collaborator

@wy65701436 wy65701436 left a comment

Choose a reason for hiding this comment

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

lgtm

@milosgajdos
Copy link
Member

@AdamKorcz you should be able to email cncf-distribution-security at lists.cncf.io

@milosgajdos milosgajdos merged commit 020bcce into distribution:main Dec 20, 2021
@DavidKorczynski
Copy link

DavidKorczynski commented Dec 20, 2021

Thanks @milosgajdos - using a list won't work unfortunately. It needs to be email accounts linked to Google accounts as you need to log in to watch the reports. Example project.yamls for CNCF projects:

@milosgajdos
Copy link
Member

Well, in that case I'd recommend grabbing the list of email addresses from https://github.com/distribution/distribution/blob/main/MAINTAINERS

Do we need to do anything extra to get this sorted? Would apreciate some guidance here if the above is not enough.

@DavidKorczynski
Copy link

Thanks Milos! I will put those on.

Nothing more now, we have everything in terms of getting the fuzzers up and running. Will ping you on the PR that does the OSS-Fuzz integration on the OSS-Fuzz repository.

@SteveLasker
Copy link
Collaborator

SteveLasker commented Dec 20, 2021

Hey folks,
Just saw this. I've added the new cncf-distribution-security@lists.cncf.io
#3549

I've opend a CNCF service desk to update Milo's email. Are there any others that should be added to the security email list above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants