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

Update fuzz target #184

Merged
merged 1 commit into from
Jun 17, 2020
Merged

Conversation

ionut-arm
Copy link
Member

This commit provides a few updates to the fuzz testing framework.

  • all recent changes to the service and interface are pulled in
  • the initial corpus for the fuzzer is removed and recreated at build
    time from Rust native operation objects
  • the fuzzer is restricted/focused on the TPM provider for now

Signed-off-by: Ionut Mihalcea [email protected]

@ionut-arm ionut-arm added the enhancement New feature or request label Jun 10, 2020
@ionut-arm ionut-arm added this to the Parsec production ready milestone Jun 10, 2020
@ionut-arm ionut-arm requested a review from hug-dev June 10, 2020 14:49
@ionut-arm ionut-arm self-assigned this Jun 10, 2020
fuzz/Cargo.toml Outdated
libfuzzer-sys = "0.3.0"
flexi_logger = "0.14.5"
log = "0.4.8"
toml = "0.4.2"
lazy_static = "1.4.0"
arbitrary = { version = "0.4.0", features = ["derive"] }
mockstream = "0.0.3"
Copy link
Member Author

Choose a reason for hiding this comment

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

Still need to replace my handrolled Mockstream with this

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I removed this dependency because we'd have to implement Arbitrary by hand on the mockstream if we use this one, so it doesn't make much of a difference.

@ionut-arm
Copy link
Member Author

I'll be running this on our server before this becomes a proper PR

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

It is nice to have those binary files removed from the tree! Why do we want to focus the tests only on the TPM provider?

[[package]]
name = "fixedbitset"
version = "0.1.9"
version = "0.2.0"
Copy link
Member

Choose a reason for hiding this comment

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

Nice one 👏

@ionut-arm
Copy link
Member Author

It is nice to have those binary files removed from the tree! Why do we want to focus the tests only on the TPM provider?

I was thinking that since our work is mostly geared towards that one for now, it makes sense to focus the fuzzer on it. I'm debating trying to create a fuzz target for just the provider and seeing how that goes too.

@hug-dev
Copy link
Member

hug-dev commented Jun 11, 2020

It is nice to have those binary files removed from the tree! Why do we want to focus the tests only on the TPM provider?

I was thinking that since our work is mostly geared towards that one for now, it makes sense to focus the fuzzer on it. I'm debating trying to create a fuzz target for just the provider and seeing how that goes too.

Since all paths are explored with fuzzing, only enabling the TPM provider would provide results faster for that provider but theoretically enabling them all would provide the same results only taking longer? As in, it is only a few bits that change in the request?

@ionut-arm
Copy link
Member Author

Since all paths are explored with fuzzing, only enabling the TPM provider would provide results faster for that provider but theoretically enabling them all would provide the same results only taking longer? As in, it is only a few bits that change in the request?

Yes, I'd say so - when the fuzzer starts messing with the bits for the provider ID, the backend handler will simply return an error if another provider is not available (well, hopefully it does)

@ionut-arm ionut-arm marked this pull request as ready for review June 15, 2020 16:09
@ionut-arm
Copy link
Member Author

The fuzz test is now running on our private server!

This commit provides a few updates to the fuzz testing framework.
* all recent changes to the service and interface are pulled in
* the initial corpus for the fuzzer is removed and recreated at build
time from Rust native operation objects
* the fuzzer is restricted/focused on the TPM provider for now

Signed-off-by: Ionut Mihalcea <[email protected]>
@ionut-arm
Copy link
Member Author

I've modified the operations that we use as starting corpus to use the same key names for creating, destroying, exporting, signing and verifying, one for ECDSA one for RSA, so that they can actually end up calling the TPM operations instead of just attempting to use non-existent keys. I guess this would be an advantage of fuzzing the provider on its own - we can control what the environment looks like to begin with!

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Nice! Can't wait to see what our first bug is 🤓

@ionut-arm ionut-arm merged commit 7b62874 into parallaxsecond:master Jun 17, 2020
@ionut-arm ionut-arm deleted the fuzz-update branch June 17, 2020 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants