Securely generate network key when resetting adapters #5833
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
`qrand` is a wrapper around `rand`, which is entirely unsuitable for generating cryptographic keys.
No more than 16 sequential bytes are ever generated by `fallbackRandom` so this is probably an inconsequential change but the Mersenne Twister is not cryptographically secure, while `std::random_device` is pulling from a high-quality source on all platforms. This is "good enough" for a fallback.
|
Thanks, very nice good spot on the Mersenne Twister use. The fallback is likely not used much as in most installations OpenSSL should be available. |
moovy2
approved these changes
Nov 5, 2022
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.
qrand()is not suitable for generating the network key when factory resetting an adapter.The first call to
qrand()sets the public PAN ID. This leaks enough RNG state to make bruteforcing the full state feasible from a single captured packet and thus allows the network key to be easily computed.Unoptimized PoC: https://github.com/puddly/deconz-network-key-leak-poc
I can't figure out how to test this branch with the deCONZ Docker image so while the code compiles, I'm not certain of its correctness. Feel free to close this PR and re-implement it yourself, the changes are pretty minor.