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

brontide: deterministic fuzz targets #7618

Merged
merged 4 commits into from
May 23, 2023

Conversation

morehouse
Copy link
Collaborator

Fuzz targets that are truly random make it difficult to reproduce failures. This PR seeds the private key RNG from fuzzer input so that any crashes found during the initial handshake are reproducible.

Tested with 35 CPU-hours of fuzzing for each FuzzRandom* target, with no crashes found.

@guggero guggero requested a review from Crypt-iQ April 20, 2023 07:34
@Crypt-iQ
Copy link
Collaborator

Concept ACK

@lightninglabs-deploy
Copy link

@Crypt-iQ: review reminder

Copy link
Collaborator

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice, LGTM 🎉

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🏸

Just needs release notes, or the label to ignore that part for CI

brontide/fuzz_test.go Show resolved Hide resolved
brontide/fuzz_test.go Show resolved Hide resolved
The nilAndPanic function does not actually nil the Curve fields as it
claims. dumpAndFail is a more descriptive name.
Move the functionality directly into completeHandshake instead. If a
failure does happen at any point during the handshake, it is beneficial
to know which line it happens on for debugging. The helper function was
hiding this information.
It is best to have deterministic fuzz targets, so that if a failure
occurs, it can be easily reproduced.

This commit swaps the cryptographically secure RNG for a deterministic
one seeded from fuzzer input.
@morehouse
Copy link
Collaborator Author

Rebased and added release note.

@guggero guggero merged commit 15525ff into lightningnetwork:master May 23, 2023
@morehouse morehouse deleted the derandomize_brontide_fuzz branch May 23, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants