[Refactor] Remove OpenSSL#2352
Merged
furszy merged 15 commits intoPIVX-Project:masterfrom May 12, 2021
Merged
Conversation
Fuzzbawls
force-pushed
the
2021_remove-openssl
branch
from
e327846 to
c316548
Compare
|
Gitian at c316548dcb9b8db6131725fe5df0057e2db45b17 |
random-zebra
previously approved these changes
May 9, 2021
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
Great PR.
Tested BIP38/Scrypt changes with encryption/decryption between this and master.
Tested resync on mainnet and testnet, and builds (local/depends/gitian) on linux.
All good. Just a minor non-blocking nit, which can be ignored.
ACK c316548dcb9b8db6131725fe5df0057e2db45b17
Note: this is going to have conflicts with #2314
On the ::SLOW or ::SLEEP paths, we would feed our RNG output back into OpenSSL using RAND_add. This commit removes that functionality. RAND_add(): https://www.openssl.org/docs/manmaster/man3/RAND_add.html RAND_add() mixes the num bytes at buf into the internal state of the random generator. This function will not normally be needed, as mentioned above. The randomness argument is an estimate of how much randomness is contained in buf, in bytes, and should be a number between zero and num.
On the ::SLOW path we would use OpenSSL as an additional source of random bytes. This commit removes that functionality. Note that this was always only an additional source, and that we never checked the return value RAND_bytes(): https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html RAND_bytes() puts num cryptographically strong pseudo-random bytes into buf.
Also the hanging declaration of DecodeBase64Secure
also cleanup some stray comments
Fuzzbawls
force-pushed
the
2021_remove-openssl
branch
from
c316548 to
5563331
Compare
Collaborator
Author
|
Rebased now that #2314 was merged and removed the redundant hasher object in |
|
Gitian [Merge 5563331 into 44ddf8a] |
furszy
reviewed
May 11, 2021
furszy
left a comment
furszy
left a comment
There was a problem hiding this comment.
Gitian here:
Linux:
945643f6a19fe942ddad9176fb2d618b00176835e26d5b24c2e87151eeca63a7 pivx-5.1.99-aarch64-linux-gnu-debug.tar.gz
9cb8733096290c73e5ff63ed321d68497e9f8e883e273c3a0d8139912ddccebb pivx-5.1.99-aarch64-linux-gnu.tar.gz
16de180100dbe1d96e21c677921037ca7c6334a9ad2c58ae3d47d7209f4627ea pivx-5.1.99-arm-linux-gnueabihf-debug.tar.gz
44ec7d4e506bfe2a74ec6f632462463313b5ace5b29b9740440a0f0d11667178 pivx-5.1.99-arm-linux-gnueabihf.tar.gz
f2a6711658c982281539b6cdb11b369b3f9e8d29f5da291a2871aa2600b47e35 pivx-5.1.99-i686-pc-linux-gnu-debug.tar.gz
08fdb40f785b2ec49968a96ad58f9427d16ed1be617046c78cb2ac91702c998a pivx-5.1.99-i686-pc-linux-gnu.tar.gz
cb40140db46d73c07b9ab69d44595ebc8e0489db41c78a2c442eb01204d16845 pivx-5.1.99-x86_64-linux-gnu-debug.tar.gz
6ccfae0c11590ffa41e622794dc8ce3392096f6a9709034eaa84acac11be6c3d pivx-5.1.99-x86_64-linux-gnu.tar.gz
b14f25d8519712de954f741075df482caae513083fbdd8a8c4ed55ff040df19a src/pivx-5.1.99.tar.gz
501f92949acda149b01350d75f401ea5baa833a2576f7cbf821af1452e86e062 pivx-linux-6.0-res.yml
Windows:
7ab71bf03357bbe3a7d6a186fb675e71cb246bd8d756440c92ed795997e4ef39 pivx-5.1.99-win-unsigned.tar.gz
f18575bbcd1280b06af96526c35e22cfe276315ec023de7d799244a1e3d428b4 pivx-5.1.99-win64-debug.zip
0b1df2a8c7d7b50a066cf9a0f1adbfcecaafaec212b3c7ae2668e6df00b544bf pivx-5.1.99-win64-setup-unsigned.exe
c767d07858190d4ad02f3b316570d2bc14400a3b74d275413adb50658b1c04e7 pivx-5.1.99-win64.zip
b14f25d8519712de954f741075df482caae513083fbdd8a8c4ed55ff040df19a src/pivx-5.1.99.tar.gz
730d7980b5020b032682e918826fce7a3d3ad8bfa4fda4053174616e4a459d41 pivx-win-6.0-res.yml
furszy
approved these changes
May 12, 2021
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 natural follow-up to #2278, #2286, and #2288. With these three PRs merged, there are only a few minor pieces of code that still rely on OpenSSL:
RAND_bytesduring the ::SLOW path of ProcRandRAND_addduring the ::SLOW and ::SLEEP paths.utilstrencodings.cpp(DecodeBase64Secure(), now removed)crypto/scrypt.cpp, now switched to using our native HMAC_SHA256 headerhash.h(std::string Hash(std::string input)), now removedUpstream PRs backported: bitcoin#17265, bitcoin#17515, and bitcoin#18825
The changes to bip38 were tested by doing two-way encryption/decryption between
masterand this PR