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

target/p256: add a few more corpus entries #13

Merged
merged 1 commit into from Feb 9, 2019

Conversation

Projects
None yet
4 participants
@josharian
Copy link
Contributor

josharian commented Feb 9, 2019

No description provided.

@mmcloughlin mmcloughlin merged commit 68ea42f into mmcloughlin:master Feb 9, 2019

@mmcloughlin

This comment has been minimized.

Copy link
Owner

mmcloughlin commented Feb 9, 2019

Actually, I ran against P-256 and SHA3 for a while so I think I have some corpus entries sitting on S3 that I didn't commit. Let me go find them.

@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Feb 9, 2019

Actually, I ran against P-256 and SHA3 for a while so I think I have some corpus entries sitting on S3 that I didn't commit. Let me go find them.

Cool. A few duplicate corpus entries won't hurt too much, I think.

mmcloughlin added a commit that referenced this pull request Feb 9, 2019

@mmcloughlin

This comment has been minimized.

Copy link
Owner

mmcloughlin commented Feb 9, 2019

Okay, committed them.

Out of interest, are you using a personal computer, or did you try out the terraform EC2 config?

@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Feb 9, 2019

Personal computer. I'm hacking on go-fuzz, and using this as one of my test cases, since it is nicely packaged up, and since it is a relatively hard case from a fuzzer's perspective.

@mmcloughlin

This comment has been minimized.

Copy link
Owner

mmcloughlin commented Feb 9, 2019

Oh, sounds great. Well I appreciate the contributions, it's good to get more eyeballs on this project and Go crypto in general.

go-fuzz is such an easy project to use so I thought it was a good place to start, but I did have some doubts about it's effectiveness on targets such as these. Specifically:

  • Crypto algorithms typically have minimal structure to their inputs (unlike a network protocol or language syntax)
  • To my knowledge, go-fuzz has no visibility into the coverage of assembly functions

As someone with more insight into go-fuzz, what would you say to these concerns?

@dgryski

This comment has been minimized.

Copy link

dgryski commented Feb 9, 2019

/cc @dvyukov

@josharian

This comment has been minimized.

Copy link
Contributor Author

josharian commented Feb 9, 2019

  • Crypto algorithms typically have minimal structure to their inputs (unlike a network protocol or language syntax)

Dmitry can speak more authoritatively, but go-fuzz will still work on these, just be less efficient.

  • To my knowledge, go-fuzz has no visibility into the coverage of assembly functions

It can still use the Go implementation to guide coverage. If they have similar structures, it'll probably suffice.

@dvyukov

This comment has been minimized.

Copy link

dvyukov commented Feb 11, 2019

Absence of input structure should not be a problem.
Absence of coverage, or partial coverage, reduces fuzzing efficiency, but it still can find bugs. Some of out fuzzers that don't have coverage are still finding hundreds of bugs.
For some libraries that have optimized asm implementations and generic C implementations, we switched the build to C implementations for fuzzing (also possible to runs 2 side-by-side, even compare results of both implementations).
Some crypto algorithms are completely oblivious to the input data, though. Whatever is passed in just does not affect anything in the algorithm. These are less useful to fuzz, but that's for good.

@mmcloughlin

This comment has been minimized.

Copy link
Owner

mmcloughlin commented Feb 11, 2019

Thank you both.

For some libraries that have optimized asm implementations and generic C implementations, we switched the build to C implementations for fuzzing (also possible to runs 2 side-by-side, even compare results of both implementations).

This is an interesting idea. To clarify, do you mean:

  • Call the C reference implementation with cgo and fuzz with go-fuzz
  • Expose the Go asm version to C (somehow) and fuzz with afl (or similar)
@dvyukov

This comment has been minimized.

Copy link

dvyukov commented Feb 11, 2019

I was talking about testing C libraries that have asm. For Go libraries this corresponds to switching from asm to Go impl which is natively understood by go-fuzz (assuming you have corresponding Go implementations for side-by-side testing, etc). But testing Go (or asm) against C impl is possible too, e.g. I fuzzed regexp package against re2 side-by-side, which is super useful as it finds logical bugs, not just crashes.

@dgryski

This comment has been minimized.

Copy link

dgryski commented Feb 11, 2019

I've similarly fuzzed (via cgo) my Go version of farmhash against the reference C implementation. Found a few compatibility issues mostly around automatic integer promotion that needed to be explicitly handled in Go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment