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

tools: initial fuzzers #179

Closed
wants to merge 5 commits into from
Closed

tools: initial fuzzers #179

wants to merge 5 commits into from

Conversation

ghedo
Copy link
Member

@ghedo ghedo commented Sep 22, 2019

This adds a couple of fuzzers based on honggfuzz that can be used to
fuzz incoming packet processing from both the server and client side.

@ghedo ghedo added the hold Do not merge label Sep 22, 2019
@ghedo ghedo requested a review from a team as a code owner September 22, 2019 20:25
@ghedo ghedo changed the title WIP tools: initial fuzzers tools: initial fuzzers Sep 23, 2019
@ghedo ghedo removed the hold Do not merge label Sep 23, 2019
@ghedo
Copy link
Member Author

ghedo commented Sep 23, 2019

This should be good to go now.

@LPardue
Copy link
Contributor

LPardue commented Sep 23, 2019

failing CI. Is this a Rust version problem or should cfg fuzzing be cfg feature fuzzing?

@ghedo
Copy link
Member Author

ghedo commented Sep 23, 2019

No, the build command was wrong (can't pass --cfg directly to cargo apparently). But in any case I changed this to use the actual command to build the fuzzers (using honggfuzz) rather than try to simulate it.

LPardue
LPardue previously approved these changes Sep 23, 2019
@LPardue
Copy link
Contributor

LPardue commented Sep 23, 2019

given the number of file additions, I wonder how this will affect the build times for Docker on Mac

@ghedo
Copy link
Member Author

ghedo commented Sep 23, 2019

🤷‍♂️ though all those files can probably be left out in that case.

@LPardue
Copy link
Contributor

LPardue commented Sep 23, 2019

I understand this is focused on fuzzing the transport right now. But it seems not too hard to extend it to HTTP/3. For example,

packet_server.rs

      fuzz!(|data: &[u8]| {
            let mut buf = data.to_vec();
            let mut conn =
                quiche::accept(&server_scid, None, &mut config).unwrap();

            let mut h3_conn = quiche::h3::Connection::with_transport(
                &mut conn,
                &h3_config,
            ).unwrap();

            conn.recv(&mut buf).ok();
            h3_conn.poll(conn.as_mut()).ok();
        });

inside http3::with_transport

if cfg!(not(fuzzing)) {
       http3_conn.send_settings(conn)?;
}

This adds a couple of fuzzers based on honggfuzz that can be used to
fuzz incoming packet processing from both the server and client side.
This way more of the packet processing code can be fuzzed (e.g.
including frame parsing).
@ghedo
Copy link
Member Author

ghedo commented Sep 23, 2019

So, I renamed the fuzzers and made the server fuzzer accept cert and key paths from environment. This is needed to run the fuzzers on Mayhem (also added configuration for that).

@LPardue yeah, we can add more fuzzers, though I'd like to concentrate on these two until we have the whole infrastructure properly setup.

@LPardue
Copy link
Contributor

LPardue commented Sep 23, 2019

Totally agree, after gaining a bit more context I realise there are complexities for HTTP/3. Let's avoid this this PR getting bogged down by that.

@junhochoi
Copy link
Contributor

Just curious where those corpus come from? Is it from actual packet dumps?

@ghedo
Copy link
Member Author

ghedo commented Sep 25, 2019

@junhochoi they were generated randomly by honggfuzz.

@ghedo
Copy link
Member Author

ghedo commented Sep 25, 2019

But also, I managed to generate a few test cases manually, which seem to increase the coverage, so I'll add those as well.

@ghedo
Copy link
Member Author

ghedo commented Sep 25, 2019

Too bad honggfuz doesn't support minimizing inputs...

@ghedo
Copy link
Member Author

ghedo commented Sep 26, 2019

In retrospect going with honggfuzz might have been a mistake, as the lack of corpus minimization is pretty annoying and it's generally not as well supported as libfuzzer in general. So i'll try to put something together based on libfuzzer/cargo-fuzz to see if that's better (and also see how it fares on Mayhem).

@ghedo
Copy link
Member Author

ghedo commented Sep 27, 2019

Closing in favor of #186.

@ghedo ghedo closed this Sep 27, 2019
@ghedo ghedo deleted the fuzz branch September 29, 2019 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants