-
Notifications
You must be signed in to change notification settings - Fork 38.8k
fuzz: add I2P harness #30230
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
fuzz: add I2P harness #30230
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
|
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
b9a754f to
193c748
Compare
dergoegge
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 193c748
Thanks for picking this up!
I fuzzed the i2p harrness for a few hundred CPU hours and even without the dictionary it reaches almost everything. The dictionary will still help though, so adding it to qa-assets later would be good.
My only note: I think it would be worthwhile to point out in the PR description what changes you have made to the original harness and why they were necessary.
Done. Thanks for all the help @dergoegge. |
brunoerg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 193c748
vasild
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 193c748
193c748 fuzz: add I2P harness (marcofleon) Pull request description: Addresses bitcoin#28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as `GetTime` is called at points in `i2p`. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in `i2p` every time, so the fuzzer can eventually make progress through the target code. Re-working this harness also led me and dergoegge to resolve a couple of issues in `FuzzedSock`, which allows for full coverage of the `i2p` code. Those changes can be seen in bitcoin#30211. The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness. <details> <summary>I2P dict</summary> ``` "HELLO VERSION" "HELLO REPLY RESULT=OK VERSION=" "HELLO REPLY RESULT=NOVERSION" "HELLO REPLY RESULT=I2P_ERROR" "SESSION CREATE" "SESSION STATUS RESULT=OK DESTINATION=" "SESSION STATUS RESULT=DUPLICATED_ID" "SESSION STATUS RESULT=DUPLICATED_DEST" "SESSION STATUS RESULT=INVALID_ID" "SESSION STATUS RESULT=INVALID_KEY" "SESSION STATUS RESULT=I2P_ERROR MESSAGE=" "SESSION ADD" "SESSION REMOVE" "STREAM CONNECT" "STREAM STATUS RESULT=OK" "STREAM STATUS RESULT=INVALID_ID" "STREAM STATUS RESULT=INVALID_KEY" "STREAM STATUS RESULT=CANT_REACH_PEER" "STREAM STATUS RESULT=I2P_ERROR MESSAGE=" "STREAM ACCEPT" "STREAM FORWARD" "DATAGRAM SEND" "RAW SEND" "DEST GENERATE" "DEST REPLY PUB= PRIV=" "DEST REPLY RESULT=I2P_ERROR" "NAMING LOOKUP" "NAMING REPLY RESULT=OK NAME= VALUE=" "DATAGRAM RECEIVED DESTINATION= SIZE=" "RAW RECEIVED SIZE=" "NAMING REPLY RESULT=INVALID_KEY NAME=" "NAMING REPLY RESULT=KEY_NOT_FOUND NAME=" "MIN" "MAX" "STYLE" "ID" "SILENT" "DESTINATION" "NAME" "SIGNATURE_TYPE" "CRYPTO_TYPE" "SIZE" "HOST" "PORT" "FROM_PORT" "TRANSIENT" "STREAM" "DATAGRAM" "RAW" "MASTER" "true" "false" ``` </details> I'll add this dict to qa-assets later on. ACKs for top commit: dergoegge: tACK 193c748 brunoerg: ACK 193c748 vasild: ACK 193c748 Tree-SHA512: 09ae4b3fa0738aa6f159f4d920493bdbce786b489bc8148e7a135a881e9dba93d727b40f5400c9510e218dd2cfdccc7ce2d3ac9450654fb29c78aac59af92ec3
193c748 fuzz: add I2P harness (marcofleon) Pull request description: Addresses bitcoin#28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as `GetTime` is called at points in `i2p`. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated in `i2p` every time, so the fuzzer can eventually make progress through the target code. Re-working this harness also led me and dergoegge to resolve a couple of issues in `FuzzedSock`, which allows for full coverage of the `i2p` code. Those changes can be seen in bitcoin#30211. The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness. <details> <summary>I2P dict</summary> ``` "HELLO VERSION" "HELLO REPLY RESULT=OK VERSION=" "HELLO REPLY RESULT=NOVERSION" "HELLO REPLY RESULT=I2P_ERROR" "SESSION CREATE" "SESSION STATUS RESULT=OK DESTINATION=" "SESSION STATUS RESULT=DUPLICATED_ID" "SESSION STATUS RESULT=DUPLICATED_DEST" "SESSION STATUS RESULT=INVALID_ID" "SESSION STATUS RESULT=INVALID_KEY" "SESSION STATUS RESULT=I2P_ERROR MESSAGE=" "SESSION ADD" "SESSION REMOVE" "STREAM CONNECT" "STREAM STATUS RESULT=OK" "STREAM STATUS RESULT=INVALID_ID" "STREAM STATUS RESULT=INVALID_KEY" "STREAM STATUS RESULT=CANT_REACH_PEER" "STREAM STATUS RESULT=I2P_ERROR MESSAGE=" "STREAM ACCEPT" "STREAM FORWARD" "DATAGRAM SEND" "RAW SEND" "DEST GENERATE" "DEST REPLY PUB= PRIV=" "DEST REPLY RESULT=I2P_ERROR" "NAMING LOOKUP" "NAMING REPLY RESULT=OK NAME= VALUE=" "DATAGRAM RECEIVED DESTINATION= SIZE=" "RAW RECEIVED SIZE=" "NAMING REPLY RESULT=INVALID_KEY NAME=" "NAMING REPLY RESULT=KEY_NOT_FOUND NAME=" "MIN" "MAX" "STYLE" "ID" "SILENT" "DESTINATION" "NAME" "SIGNATURE_TYPE" "CRYPTO_TYPE" "SIZE" "HOST" "PORT" "FROM_PORT" "TRANSIENT" "STREAM" "DATAGRAM" "RAW" "MASTER" "true" "false" ``` </details> I'll add this dict to qa-assets later on. ACKs for top commit: dergoegge: tACK 193c748 brunoerg: ACK 193c748 vasild: ACK 193c748 Tree-SHA512: 09ae4b3fa0738aa6f159f4d920493bdbce786b489bc8148e7a135a881e9dba93d727b40f5400c9510e218dd2cfdccc7ce2d3ac9450654fb29c78aac59af92ec3
Addresses #28803. This updated harness sets mock time at the beginning of each iteration and deletes the private key file at the end of each iteration. Mock time is used to make the fuzz test more stable, as
GetTimeis called at points ini2p. Deleting the private key file ensures that each iteration is independent from the last. Now, a new key is generated ini2pevery time, so the fuzzer can eventually make progress through the target code.Re-working this harness also led me and dergoegge to resolve a couple of issues in
FuzzedSock, which allows for full coverage of thei2pcode. Those changes can be seen in #30211.The SAM protocol for interacting with I2P requires some specifc inputs so it's best to use a dictionary when running this harness.
I2P dict
I'll add this dict to qa-assets later on.