-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Replace Crypto++ ECIES #3634
base: master
Are you sure you want to change the base?
Replace Crypto++ ECIES #3634
Conversation
7d44a1d to
c93edbb
Compare
Codecov Report
@@ Coverage Diff @@
## master #3634 +/- ##
=========================================
+ Coverage 59.94% 65.55% +5.6%
=========================================
Files 337 306 -31
Lines 27306 22666 -4640
Branches 3173 0 -3173
=========================================
- Hits 16369 14858 -1511
+ Misses 9857 7808 -2049
+ Partials 1080 0 -1080
|
1af4f47 to
f0f8ed5
Compare
# Conflicts: # test/unittests/libdevcrypto/crypto.cpp
…cies # Conflicts: # libdevcrypto/CryptoPP.cpp # libp2p/RLPXFrameCoder.h # test/unittests/libdevcrypto/crypto.cpp # test/unittests/libp2p/rlpx.cpp
|
Please add a test for PBKDF2 (maybe use these vectors http://stackoverflow.com/questions/5130513/pbkdf2-hmac-sha2-test-vectors). Also probably would be nicer to split the PBKDF2 change into a separate PR. |
| return; | ||
|
|
||
| bytes msg(1 + Public::size + h128::size + cipherText.size() + 32); | ||
| msg[0] = 0x04; |
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.
Probably I would create a helper which creates a SEC.1 key.
SEC.1 (http://www.secg.org/sec1-v2.pdf) section 2.3.3 describes the rules.
| // interop w/go ecies implementation | ||
|
|
||
| // io_cipher[0] must be 2, 3, or 4, else invalidpublickey | ||
| if (_cipher.empty() || _cipher[0] < 2 || _cipher[0] > 4) |
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.
Probably better to have a class for SEC.1 key (mentioned above) and just use validations on that class here.
| bytes io = _plain.toBytes(); | ||
| Secp256k1PP::get()->encrypt(_k, io); | ||
| o_cipher = std::move(io); | ||
| encryptECIES(_k, _plain, o_cipher); |
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.
encrypt & decrypt are now the same as encryptECIES & decryptECIES, do we still need both?
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.
I will take a look. Probably not.
|
@axic requested some wrappers for Pubkey, Seckey to be added to handle conversion to/from secp256k1 library format nicely. I think it is good idea and I will start working on it soon. |
No description provided.