Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use MAC::DIGESTSIZE in ECIES SymmetricEncrypt/SymmetricDecrypt #857

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

rectalogic
Copy link
Contributor

Fixes #856

@mouse07410 mouse07410 merged commit c80a7ad into weidai11:master Jul 2, 2019
@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019

@mouse07410,

We may want to hold-off on this commit until we know it maintains backwards compatibility.

It may not maintain it based on my tests.


Here's the output of a test using Crypto++ 5.6.2 ECIES:

$ ./test.exe
SHA1 digestsize: 20
HMAC/SHA1 keysize: 16
Private key:
3081C80201003081A406072A8648CE3D0201308198020101302006072A8648CE3D0101021500FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFF302C0414FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFC04141C97BEFC54BD7A8B65ACF89F81D4D4ADC565FA450429044A96B5688EF573284664698968C38BB913CBFC8223A628553168947D59DCC912042351377AC5FB3202150100000000000000000001F4C8F927AED3CA752257020101041C301A02010104150023A68821ABB99DBB8429ED2320D61A8EA4C6D81B
Plaintext:
Yoda said, Do or do not. There is no try.
Ciphertext:
04F6C1B1FAAC8AD5D396E713AEBD0CCE15CF44540863CCBF894DD0B838A13AB2907586827F9D9526A574133A74631171704C01A4080495696A91F0C0A4BD1EAA5957B8A9D2F77C98E3C5E3F44FA76E7383F31E0573A4EE6355FD6D31BB9E364C79D076C00DE9
Recovered:
Yoda said, Do or do not. There is no try.

I plan on using the key, plaintext and ciphertext for an interop test. We should be able to obtain the decryption using Crypto++ 8.0 ECIES<ECP,SHA1,NoCofactorMultiplication,false,true>.


Here's the test program using Crypto++ 5.6.2 ECIES.

$ cat test.cxx
#include "cryptlib.h"
#include "eccrypto.h"
#include "osrng.h"
#include "files.h"
#include "oids.h"
#include "hex.h"

#include "hmac.h"
#include "sha.h"

#include <iostream>
#include <string>

inline byte* C2B(char* ptr) {
    return reinterpret_cast<byte*>(ptr);
}

inline const byte* C2B(const char* ptr) {
    return reinterpret_cast<const byte*>(ptr);
}

int main(int argc, char* argv[])
{
    using namespace CryptoPP;

    AutoSeededRandomPool prng;
    HexEncoder encoder(new FileSink(std::cout));

    DL_PrivateKey_EC<ECP> key;
    key.Initialize(prng, ASN1::secp160r1());

    ECIES<ECP>::Decryptor decryptor(key);
    ECIES<ECP>::Encryptor encryptor(key);

    std::cout << "SHA1 digestsize: " << SHA1::DIGESTSIZE << std::endl;
    std::cout << "HMAC/SHA1 keysize: " << HMAC<SHA1>::DEFAULT_KEYLENGTH << std::endl;

    std::cout << "Private key:" << std::endl;
    decryptor.AccessKey().Save(encoder);
    std::cout << std::endl;

    const std::string plain = "Yoda said, Do or do not. There is no try.";
    size_t size = encryptor.CiphertextLength(plain.size());
    std::string cipher; cipher.resize(size);

    encryptor.Encrypt(prng, C2B(&plain[0]), plain.size(), C2B(&cipher[0]));

    std::cout << "Ciphertext:" << std::endl;
    StringSource(cipher, true, new Redirector(encoder));
    std::cout << std::endl;

    size = decryptor.MaxPlaintextLength(cipher.size());
    std::string recover; recover.resize(size);

    DecodingResult result = decryptor.Decrypt(prng, C2B(&cipher[0]), cipher.size(), C2B(&recover[0]));
    if (!result.isValidCoding)
        throw std::runtime_error("Failed to decrypt ciphertext");
    recover.resize(result.messageLength);

    std::cout << "Recovered:" << std::endl;
    std::cout << recover << std::endl;

    return 0;
}

mouse07410 added a commit that referenced this pull request Jul 3, 2019
Travis CI fails "deep tests" of DLIES with #857 applied. Let's revert it for now and get back to
```c++
    cipherKey = key + MAC::DEDAULT_KEYLENGTH;
```
and see if it improves the situation.
@mouse07410
Copy link
Collaborator

With reverted master:

$ clang++-mp-8.0 $CXXFLAGS -DDECRYPT -o test-ecies -I/opt/local/include/cryptopp test-ecies.cpp -L/opt/local/lib -lcryptopp-clang
$ ./test-ecies 
SHA1 digestsize: 20
HMAC/SHA1 keysize: 16
Private key:
3081C80201003081A406072A8648CE3D0201308198020101302006072A8648CE3D0101021500FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFF302C0414FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFC04141C97BEFC54BD7A8B65ACF89F81D4D4ADC565FA450429044A96B5688EF573284664698968C38BB913CBFC8223A628553168947D59DCC912042351377AC5FB3202150100000000000000000001F4C8F927AED3CA752257020101041C301A02010104150023A68821ABB99DBB8429ED2320D61A8EA4C6D81B
Plaintext:
Yoda said, Do or do not. There is no try.
Plaintext size: 41
Encryptor decided size = 102
Ciphertext:
04F6C1B1FAAC8AD5D396E713AEBD0CCE15CF44540863CCBF894DD0B838A13AB2907586827F9D9526A574133A74631171704C01A4080495696A91F0C0A4BD1EAA5957B8A9D2F77C98E3C5E3F44FA76E7383F31E0573A4EE6355FD6D31BB9E364C79D076C00DE9
Ciphertext size: 102
Expected recovered size: 41
Recovered:
Yoda said, Do or do not. There is no try.
Recovered size: 41
$ 

Here's the modified source:

#include "cryptlib.h"
#include "eccrypto.h"
#include "osrng.h"
#include "files.h"
#include "oids.h"
#include "hex.h"

#include "hmac.h"
#include "sha.h"

#include <iostream>
#include <string>


const char *PRIVKEY_HEX="3081C80201003081A406072A8648CE3D0201308198020101302006072A8648CE3D0101021500FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFF302C0414FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFC04141C97BEFC54BD7A8B65ACF89F81D4D4ADC565FA450429044A96B5688EF573284664698968C38BB913CBFC8223A628553168947D59DCC912042351377AC5FB3202150100000000000000000001F4C8F927AED3CA752257020101041C301A02010104150023A68821ABB99DBB8429ED2320D61A8EA4C6D81B";

#ifdef DECRYPT
const char *CT_HEX="04F6C1B1FAAC8AD5D396E713AEBD0CCE15CF44540863CCBF894DD0B838A13AB2907586827F9D9526A574133A74631171704C01A4080495696A91F0C0A4BD1EAA5957B8A9D2F77C98E3C5E3F44FA76E7383F31E0573A4EE6355FD6D31BB9E364C79D076C00DE9";
#endif /* DECRYPT */

using CryptoPP::byte;

inline byte* C2B(char* ptr) {
    return reinterpret_cast<byte*>(ptr);
}

inline const byte* C2B(const char* ptr) {
    return reinterpret_cast<const byte*>(ptr);
}

int main(int argc, char* argv[])
{
    using namespace CryptoPP;

    AutoSeededRandomPool prng;
    HexEncoder encoder(new FileSink(std::cout));
    StringSource spriv(PRIVKEY_HEX, true, new HexDecoder);

    DL_PrivateKey_EC<ECP> key;
    //key.Initialize(prng, ASN1::secp160r1());
    key.BERDecode(spriv.Ref());

#ifdef DECRYPT
    ECIES<ECP,SHA1,NoCofactorMultiplication,false,true>::Decryptor decryptor(key);
    ECIES<ECP,SHA1,NoCofactorMultiplication,false,true>::Encryptor encryptor(key);
#else /* DECRYPT */
    ECIES<ECP>::Decryptor decryptor(key);
    ECIES<ECP>::Encryptor encryptor(key);
#endif /* DECRYPT */

    std::cout << "SHA1 digestsize: " << SHA1::DIGESTSIZE << std::endl;
    std::cout << "HMAC/SHA1 keysize: " << HMAC<SHA1>::DEFAULT_KEYLENGTH << std::endl;

    std::cout << "Private key:" << std::endl;
    decryptor.AccessKey().Save(encoder);
    std::cout << std::endl;

    const std::string plain = "Yoda said, Do or do not. There is no try.";
    std::cout << "Plaintext:" << std::endl;
    std::cout << plain << std::endl << "Plaintext size: " << plain.size() << std::endl;
    
    std::string cipher; 
    size_t size = encryptor.CiphertextLength(plain.size());
    cipher.resize(size);

    std::cout << "Encryptor decided size = " << size << std::endl;

    encryptor.Encrypt(prng, C2B(&plain[0]), plain.size(), C2B(&cipher[0]));
    
#ifdef DECRYPT
    std::string ciph2;
    StringSource(CT_HEX, true, new HexDecoder(new StringSink(ciph2)));
    std::cout << "Ciphertext:" << std::endl;
    StringSource(ciph2, true, new Redirector(encoder));
    std::cout << std::endl;
    std::cout << "Ciphertext size: " << ciph2.size() << std::endl;
#else /* DECRYPT */
    std::cout << "Ciphertext:" << std::endl;
    StringSource(cipher, true, new Redirector(encoder));
    std::cout << std::endl;
    std::cout << "Ciphertext size: " << cipher.size() << std::endl;
#endif /* DECRYPT */

#ifdef DECRYPT
    size = decryptor.MaxPlaintextLength(ciph2.size());
    std::string recover; recover.resize(size);
    std::cout << "Expected recovered size: " << size << std::endl;
    
    DecodingResult result = decryptor.Decrypt(prng, C2B(&ciph2[0]), ciph2.size(), C2B(&recover[0]));
#else /* DECRYPT */
    size = decryptor.MaxPlaintextLength(cipher.size());
    std::string recover; recover.resize(size);
    std::cout << "Expected recovered size: " << size << std::endl;
    
    DecodingResult result = decryptor.Decrypt(prng, C2B(&cipher[0]), cipher.size(), C2B(&recover[0]));
#endif /* DECRYPT */
    if (!result.isValidCoding)
        throw std::runtime_error("Failed to decrypt ciphertext");
    recover.resize(result.messageLength);

    std::cout << "Recovered:" << std::endl;
    std::cout << recover << std::endl << "Recovered size: " << recover.size() << std::endl;

    return 0;
}

Trying with this patch enabled:

$ ./test-ecies 
SHA1 digestsize: 20
HMAC/SHA1 keysize: 16
Private key:
3081C80201003081A406072A8648CE3D0201308198020101302006072A8648CE3D0101021500FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFF302C0414FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFC04141C97BEFC54BD7A8B65ACF89F81D4D4ADC565FA450429044A96B5688EF573284664698968C38BB913CBFC8223A628553168947D59DCC912042351377AC5FB3202150100000000000000000001F4C8F927AED3CA752257020101041C301A02010104150023A68821ABB99DBB8429ED2320D61A8EA4C6D81B
Plaintext:
Yoda said, Do or do not. There is no try.
Plaintext size: 41
Encryptor decided size = 102
Ciphertext:
04F6C1B1FAAC8AD5D396E713AEBD0CCE15CF44540863CCBF894DD0B838A13AB2907586827F9D9526A574133A74631171704C01A4080495696A91F0C0A4BD1EAA5957B8A9D2F77C98E3C5E3F44FA76E7383F31E0573A4EE6355FD6D31BB9E364C79D076C00DE9
Ciphertext size: 102
Expected recovered size: 41
Recovered:
Yoda said, Do or do not. There is no try.
Recovered size: 41
$

So, I don't understand why Travis CI fails:

$ ./cryptest.exe tv`:
. . . . .
Testing AsymmetricCipher algorithm DLIES(NoCofactorMultiplication, KDF2(SHA-1), XOR, HMAC(SHA-1), DHAES).
.
AlgorithmType: AsymmetricCipher
Ciphertext: B11D906CC5A8E71CA8962A8CC0AC4CAFF2DA00DC130C370F42D11FCF5C37DE046EBC07C7D457CA351CE456A043695D14ED055ADAD2B58BE0DF992685EF8B0D21597A43D7B3D9634A077CB70C4590CD73C20FAAACBC5649413EECA0C7B3CBF469E531299398F61496C51FE9FFE48AE9FE6034F104EFC562DE9529C776B86ADD4025AD6B0C3687B012F92C7B9E82F794E4FBE247D644
Comment: 1024-bit DLIES key
KeyFormat: DER
Name: DLIES(NoCofactorMultiplication, KDF2(SHA-1), XOR, HMAC(SHA-1), DHAES)
Plaintext: 76
PrivateKey: 308201370201003082011706072a8648ce3804013082010a02818100ba3ed941 10332be99b77a345da72a33146ca960498a6fc2e0e207fdeaadf69c3e5650df7 3255475854900b75af7f6aac021de687a1c166ecb2ab6ec6b9da82ad4fb0f48a 966a2b968406e18ba50947d7ee3bb1f13511cb4dde191f0ade1933d089c5e82a b8d283943d85ef0102e173abf2635aeac2f84cfc9ec6c4e8f3fbc4130281805d 1f6ca0881995f4cdbbd1a2ed395198a3654b024c537e1707103fef556fb4e1f2 b286fb992aa3ac2a4805bad7bfb556010ef343d0e0b3765955b7635ced4156a7 d87a454b3515cb420370c5d284a3ebf71dd8f89a88e5a6ef0c8f856f0c99e844 e2f4155c6941ca1ec2f7808170b9d5f931ad75617c267e4f63627479fde20902 01030417021501fdc788cd93f07dba3af2de42ae5aa3ede219919d
PublicKey: 308201a23082011706072a8648ce3804013082010a02818100ba3ed94110332b e99b77a345da72a33146ca960498a6fc2e0e207fdeaadf69c3e5650df7325547 5854900b75af7f6aac021de687a1c166ecb2ab6ec6b9da82ad4fb0f48a966a2b 968406e18ba50947d7ee3bb1f13511cb4dde191f0ade1933d089c5e82ab8d283 943d85ef0102e173abf2635aeac2f84cfc9ec6c4e8f3fbc4130281805d1f6ca0 881995f4cdbbd1a2ed395198a3654b024c537e1707103fef556fb4e1f2b286fb 992aa3ac2a4805bad7bfb556010ef343d0e0b3765955b7635ced4156a7d87a45 4b3515cb420370c5d284a3ebf71dd8f89a88e5a6ef0c8f856f0c99e844e2f415 5c6941ca1ec2f7808170b9d5f931ad75617c267e4f63627479fde20902010303 81840002818029eaa5b193357c200e0d42f374d4c003c633c77f4778fe40ad0b d035b87ae5da4e74110ec2b15eefe1bd8b9357534c85328382946d314e15b79f 7b854227012dfaac9bd862e73a5630e01327b36319765a3eb1434e108ef6421c 659e3f9223966759611429b3c86ed9937563efbfad8bfedcfa92db3d7d2157fe 2c8a33f08636
Source: generated by Wei Dai using Crypto++ 5.1
Test: DecryptMatch

Test FAILED.
Skipping to next test.

AlgorithmType: AsymmetricCipher
Ciphertext: 8A33B0E212DB8155CA796B472F55CD77267C9106229B6055141EA3AAAE42AD27249D90E70F892B0CDC80D29D3D586A5CA6FE67D4BB44C58B03496708F80681125DCEF983B7453B1E4F927438BD2E3E506C1951E9F19BA70F9B687012440CD75C0BB78BDCFAB22AF535D3E2670ABD1F4D44ED95F3360536612B1A7DF35E2A88F66BD6E8C813EB9DC89D93A85C9A0BA13E4862B91171B681E64A0750197C6467B22566BC640E11
Comment: 1024-bit DLIES key
KeyFormat: DER
Name: DLIES(NoCofactorMultiplication, KDF2(SHA-1), XOR, HMAC(SHA-1), DHAES)
Plaintext: 89338CE80AFB62E9577A310E40311BB3F77F
PrivateKey: 308201370201003082011706072a8648ce3804013082010a02818100ba3ed941 10332be99b77a345da72a33146ca960498a6fc2e0e207fdeaadf69c3e5650df7 3255475854900b75af7f6aac021de687a1c166ecb2ab6ec6b9da82ad4fb0f48a 966a2b968406e18ba50947d7ee3bb1f13511cb4dde191f0ade1933d089c5e82a b8d283943d85ef0102e173abf2635aeac2f84cfc9ec6c4e8f3fbc4130281805d 1f6ca0881995f4cdbbd1a2ed395198a3654b024c537e1707103fef556fb4e1f2 b286fb992aa3ac2a4805bad7bfb556010ef343d0e0b3765955b7635ced4156a7 d87a454b3515cb420370c5d284a3ebf71dd8f89a88e5a6ef0c8f856f0c99e844 e2f4155c6941ca1ec2f7808170b9d5f931ad75617c267e4f63627479fde20902 01030417021501fdc788cd93f07dba3af2de42ae5aa3ede219919d
PublicKey: 308201a23082011706072a8648ce3804013082010a02818100ba3ed94110332b e99b77a345da72a33146ca960498a6fc2e0e207fdeaadf69c3e5650df7325547 5854900b75af7f6aac021de687a1c166ecb2ab6ec6b9da82ad4fb0f48a966a2b 968406e18ba50947d7ee3bb1f13511cb4dde191f0ade1933d089c5e82ab8d283 943d85ef0102e173abf2635aeac2f84cfc9ec6c4e8f3fbc4130281805d1f6ca0 881995f4cdbbd1a2ed395198a3654b024c537e1707103fef556fb4e1f2b286fb 992aa3ac2a4805bad7bfb556010ef343d0e0b3765955b7635ced4156a7d87a45 4b3515cb420370c5d284a3ebf71dd8f89a88e5a6ef0c8f856f0c99e844e2f415 5c6941ca1ec2f7808170b9d5f931ad75617c267e4f63627479fde20902010303 81840002818029eaa5b193357c200e0d42f374d4c003c633c77f4778fe40ad0b d035b87ae5da4e74110ec2b15eefe1bd8b9357534c85328382946d314e15b79f 7b854227012dfaac9bd862e73a5630e01327b36319765a3eb1434e108ef6421c 659e3f9223966759611429b3c86ed9937563efbfad8bfedcfa92db3d7d2157fe 2c8a33f08636
Source: generated by Wei Dai using Crypto++ 5.1
Test: DecryptMatch

Test FAILED.
Skipping to next test.
. . . . .

@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019 via email

@mouse07410
Copy link
Collaborator

Why is DLIES failing after an ECIES tweak???

I wish I knew. On the other hand, I think this tweak is for DLIES, not (just) ECIES - that would explain...

Maybe you need a distclean

I have it in my build scripts, and I repeat it when I build manually.

I'm thinking - the test-vectors are generated with 5.1 with the old defaults. Current tests are running with the new defaults (though this doesn't explain why in that case Travis started failing only now).

In any case, configuring ECIES fit the old defaults, I could decrypt your test both with this patch, and without it.

I'm stymied.

@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019 via email

@mouse07410
Copy link
Collaborator

So, what do we do? Keep the pre-patch version? Re-generate the test vectors with the patched code and the new defaults? Change the tests to instantiate DLAES with the old defaults...?

I still do not understand why I could decrypt your test (after I changed the defaults to the pre-5.6) both with the patched code, and with the patch reverted.

@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019 via email

@mouse07410
Copy link
Collaborator

mouse07410 commented Jul 3, 2019

Sounds like a plan. Except, this change affected not just ECP but DL. Probably we need a wider legacy check.

@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019

@rectalogic, @mouse07410,

I cut-in the legacy ECIES ECP test at Commit ce6d3c1306cf, and legacy ECIES EC2N test at Commit cd0d14563532. I also fixed my typo, and changed back to MAC::DEFAULT_KEYLENGTH.

Can you guys give it a go and let us know what breaks?

@noloader
Copy link
Collaborator

noloader commented Jul 3, 2019

Probably we need a wider legacy check.

Do you want to take a crack at it?

Checkout 5.6.2 with git checkout -f CRYPTOPP_5_6_2. Then, patch:

$ cat cryptopp562.diff
diff --git a/config.h b/config.h
index 4554a1c7..44045c44 100644
--- a/config.h
+++ b/config.h
@@ -1,6 +1,9 @@
 #ifndef CRYPTOPP_CONFIG_H
 #define CRYPTOPP_CONFIG_H

+#pragma GCC diagnostic ignored "-Wdeprecated-declaration"
+#pragma GCC diagnostic ignored "-Wterminate"
+
 // ***************** Important Settings ********************

 // define this if running on a big-endian CPU
diff --git a/wake.cpp b/wake.cpp
index c34165b8..30b1221f 100644
--- a/wake.cpp
+++ b/wake.cpp
@@ -23,15 +23,14 @@ void WAKE_Base::GenKey(word32 k0, word32 k1, word32 k2, word32 k3)
        signed int x, z, p;
        // x and z were declared as "long" in Wheeler's paper, which is a signed type. I don't know if that was intentional, but it's too late to change it now. -- Wei 7/4/2010
        CRYPTOPP_COMPILE_ASSERT(sizeof(x) == 4);
-       static int tt[10]= {
-               0x726a8f3b,                                                     // table
-               0xe69a3b5c,
-               0xd3c71fe5,
-               0xab3c73d2,
-               0x4d3a8eb3,
-               0x0396d6e8,
-               0x3d4c2f7a,
-               0x9ee27cf3, } ;
+       static unsigned int tt[10]= {
+               0x726a8f3b, 0xe69a3b5c,
+               0xd3c71fe5, 0xab3c73d2,
+               0x4d3a8eb3, 0x0396d6e8,
+               0x3d4c2f7a, 0x9ee27cf3,
+               0, 0
+       };
+
        t[0] = k0;
        t[1] = k1;
        t[2] = k2;

Then, make clean && make -j 4.

Then, use the test program to generate new data. There is already a keypair at TestData/dlie1024.dat, so it can be reused.

Finally, plug it into a function like ValidateDLIES_Legacy_Encrypt().

@mouse07410
Copy link
Collaborator

Sorry, I'm a bit dense today - so could you please provide more details?

Then, use the test program to generate new data.

Which test program? The test-ecies.cpp that is posted above? Or...?

There is already a keypair at TestData/dlie1024.dat, so it can be reused.

Re-used how and where? Do I need to copy that file from the 5.6.2 build? Or it didn't change since then?

Finally, plug it into a function like ValidateDLIES_Legacy_Encrypt().

You mean - write a new function ValidateDLIES_Legacy_Encrypt() that would use the above TestData/dlie1024.dat?

I'm not sure I understand the actual plan and the specific goals:

  • What's the semantics of cipherKey variable? Was it supposed to be plaintext_len plus KEYLEN or plus DIGESTSIZE? Is the patch actually fixing a bug, or a compatibility issue?
  • Are we going to move to adopting this patch (moving to MAC::DIGESTSIZE from MAC::DEFAULT_KEYLENGTH) in the future?
  • Do we know why/where/how the compatibility got broken, and why it only manifests itself with this patch?
  • Are we going to generate new test-vectors?
  • Are we going to have two validation suites for DLIES - one for the current, and one for legacy?
  • Who will generate the correct test-vectors, how and when?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ECIES and GetSymmetricKeyLength no longer uses MAC::DEFAULT_KEYLENGTH
3 participants