Client security, Core #64
Merged
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
…n for recover randomness
|
updated with some comments regarding argon2 implementation |
…t-security-frontend
…nto client-security-frontend
…-wallet into client-security-frontend
Add use two otp options on create wallet
Client security: enable randomness by default
|
All items are done except argon2 speed optimization. I will create separate issues to document that (as already analyzed above) |
|
Yes, I also made a similar conclusion in #67 for different reasons |
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.
The recovery mechanism needs to be adjusted, since now it is no longer possible to brute-force the OTP.
The UI needs to be adapted to accommodate for the (optional) second OTP. Later we might add a function on smart contract to allow replacing the root, so the user can seamlessly transition a single OTP wallet to double OTP. For now we can just let the user decide upfront and ask them to create a new wallet if they want double OTP.
Some minor frontend logic needs to be updated to use the new hasher (argon2) with appropriate randomness (e.g. 16-17 bits). Later we can optimize the speed by parallelizing it (so we gain 1-3 more bits of randomness). For now we can just use a slightly lower randomness parameter to make it fast.
The sha256 hasher is already fast enough to support ~20 bits of randomness. Based on previous benchmark (see
/benchmarkand/legacy/clientfolders), double SHA256 can be done in-browser in ~1.5 second using a single thread on mid-tier iMac 2014, and ~1.2 second on a 2018 MacBook Pro, single-threaded.It should also be noted that the implementation relies on a customized Argon2 that made it more efficient for computing hashes in batch: https://github.com/polymorpher/argon2-browser. Note that there is still a lot of room for improvement in the customized version. For example, right now each hash computation still triggers a full initialization and destruction sequence in the underlying C code, which involves a large number of
mallocandfree. They can be improved by doingmallocandfreeonly once for the entire batch. The javascript code could also parallelize the workload and obtain at least 2-4x speed boost for today's typical hardware. A deeper dive into the C code may also allow us to removesaltand align each input to a single 32-byte word, and optimize the code further for 32-byte blocks of input. With these improvements, it can be expected that the randomness bits can be improved to ~20 with argon2, i.e. close to ~1M as originally proposed in Client Security.Other than these low hanging fruits, we can identify further bottleneck by comprehensively profiling and benchmarking the C code.