Fuzzing updates: all-clusters-app #27858
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
github-actions
bot
added
tools
examples
scripts
integrations
transport
platform
lib
config
core
labels
|
PR #27858: Size comparison from 08af5fa to c1b35c0 Increases (11 builds for bl602, bl702, bl702l, cc32xx, k32w, linux, nrfconnect, qpg)
Decreases (2 builds for bl602, cc32xx)
Full report (17 builds for bl602, bl702, bl702l, cc32xx, k32w, linux, mbed, nrfconnect, qpg)
|
|
PR #27858: Size comparison from 08af5fa to 266918b Increases (38 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, nrfconnect, psoc6, qpg, telink)
Decreases (11 builds for bl602, bl702, cc32xx, cyw30739, efr32, nrfconnect, psoc6)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
| // | ||
| // To build with this flag, pass 'treat_warnings_as_errors=false' to gn/ninja. | ||
| // | ||
| #define CHIP_CONFIG_SECURITY_FUZZ_MODE 0 |
There was a problem hiding this comment.
Shouldn't this be the default value, though? And if so, why are we adding it to all the project configs?
| // Skip encryption and message integrity! | ||
| #if CHIP_CONFIG_SECURITY_FUZZ_MODE | ||
| #warning \ | ||
| "Warning: CHIP_CONFIG_SECURITY_FUZZ_MODE=1 bypassing encryption! Node can only communicate with other nodes built with this flag set. Requires build flag 'treat_warnings_as_errors=false'." |
There was a problem hiding this comment.
The "requires build flag" bit is:
- Not really in need of being documented here.
- Seems to not be needed at all, if this code just has
#elsearound the bits we want to turn off....
Same for Decrypt.
| @@ -739,8 +750,15 @@ void SessionManager::SecureUnicastMessageDispatch(const PacketHeader & partialPa | |||
| "Received a duplicate message with MessageCounter:" ChipLogFormatMessageCounter | |||
| " on exchange " ChipLogFormatExchangeId, | |||
| packetHeader.GetMessageCounter(), ChipLogValueExchangeIdFromReceivedHeader(payloadHeader)); | |||
|
|
|||
| #if CHIP_CONFIG_SECURITY_FUZZ_MODE | |||
| #warning "Warning: CHIP_CONFIG_SECURITY_FUZZ_MODE=1 bypassing duplicate message check!" | |||
There was a problem hiding this comment.
This could use some documentation explaining why we would want to do that.
…ature verification
(and fallback to use this for secure unicast messages)
(manually captured commissioning + door lock/unlock sequence using chip-tool and all-clusters-app)
|
PR #27858: Size comparison from 5d2beb7 to 6d99874 Increases (39 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, nrfconnect, psoc6, qpg, telink)
Decreases (6 builds for cc32xx, efr32, psoc6)
Full report (58 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
pullapprove
bot
requested review from
amitnj,
andy31415,
anush-apple,
arkq,
carol-apple,
cecille and
chrisdecenzo
pullapprove
bot
requested review from
tima-q,
tobiasgraf,
turon,
vijs,
vivien-apple,
woody-apple,
xylophone21,
younghak-hwang,
yufengwangca and
yunhanw-google
|
Our usage of |
pullapprove
bot
requested review from
andyg-apple,
aronrosenberg,
mlepage-google,
mrjerryjohns,
mspang,
mkardous-silabs and
rcasallas-silabs
|
PR #27858: Size comparison from b516ff4 to 5d9bf14 Increases (45 builds for bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
Decreases (7 builds for cc32xx, efr32, psoc6)
Full report (66 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
PR not updated in >1 year and has merge conflicts. Closing as stale. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As mentioned in another pull request, a colleague and I have completed some research investigating fuzzing.
This pull request summarises some of our findings that enable deeper fuzzing of the all-clusters-app fuzz target.
We note that the all-clusters-app target is not currently added to OSS-Fuzz integration - and most of the proposed changes below are more severe code changes (which may limit their appeal). Nonetheless, we hope at the very least this record is useful for future fuzzing efforts.
The below changes resulted in an additional ~5000 lines (45%) of all-clusters fuzzing coverage; notably including the addition of successful parsing of app and interaction model commands (+350% coverage of src/app).
all-clusters-app-fuzzing
Other:
CHIP_CONFIG_SECURITY_FUZZ_MODEto disable encryption and signature verificationsessionId=1) session (and fallback to use this for secure unicast messages)/tmp/chip_kvscredentials over prior to commencing fuzzing.For example: