ECIES with no HMAC

33 views
Skip to first unread message

Andrew Wason

unread,
Jul 1, 2019, 2:07:06 PM7/1/19
to Crypto++ Users
We have been using the code below with libcrypto++6 on Ubuntu for years. It uses a custom NULLHash implementation to reduce the size of the final ciphertext since we are not concerned with message integrity.

I recently tested this with libcrypto 8.2 and the decrypted text is now partially corrupted, also I noticed plaintext is visible in the encrypted ciphertext. I then built a non-optimized libcrypto 6 and realized it also fails in the same manner.

So I built git tags CRYPTOPP_8_2_0 and CRYPTOPP_6_0_0 both with default CXXFLAGS and with CXXFLAGS=-g3. Only CRYPTOPP_6_0_0 with default CXXFLAGS works (and the Ubuntu libcrypto++6 package), the other combinations all fail in the same way.

I found earlier discussions of someone trying to do something similar here and here

Anyone see what the issue is here? We have years of ciphertext encrypted this way and would like to upgrade to libcryptopp 8, but we need to be able to decrypt existing ciphertext and correctly encrypt new plaintext.

Test results:

$ apt install libcrypto++6 libcrypto++-dev
$ c++ -o cryptestu6 cryptest.cpp -lcryptopp
$ ./cryptestu6
Encrypted: \x04\xcc\xc2g\x00\x9f\x98E\xc7\xca\xa2!\xdcT\xd1\x07\x87\xaa\xfc\xc7\x16g1\x89h\xd5P\xb8"\xcaA\x80\x9b\x83/\x97g7\x17\xbd;f\xcbYI\xdfu\xdb\x026/)3\xf7\x09\x0e\xb2@\xb9Rt\xb1\xe0>\x19\x80\xe1`\xd1)\xee\xa5\x1f\x83\xdd\xff\xb5nt\xddq\xc0\x12\xdb{
Decrypted: This is a big secret

$ cd cryptopp6/cryptopp; make
$ cd ../..; c++ -o cryptest6 -Icryptopp6 -Lcryptopp6/cryptopp cryptest.cpp -lcryptopp
$ ./cryptest6
Encrypted: \x04\xd5\xe4\xdb5\xaa\xb4\xa7 \xad\x9f\x0f\xa9~\x9f\xcd?\x9f\xb5\x8drC\x96\x83\x91\x8a\xb0\xc5r\xcb\xee"\x0e\x87\x88\xe3\xf3\xe2q\xc6\xbd\xb5\x16\x86\xb4i\x1d\xae\x19\xdb\x8a\xf8~\xe7=\xcd\xfd\xf9\xae8\xe0K\x0a\xe5[\xad\x8d,aR<s @ big secret
Decrypted: This is a big secret

$ cd cryptopp6/cryptopp; CXXFLAGS=-g3 make
$ cd ../..; c++ -o cryptest6 -Icryptopp6 -Lcryptopp6/cryptopp cryptest.cpp -lcryptopp
$ ./cryptest6
Encrypted: \x04 \xceG\x92\xe3\xaa\xd8\xad\xc7_A\x84\xe4\x8d\xe3\x83\xc6\xf8\xab\x94\xcbp\x0etpe\xc7K4>\xd3\x11Q\xa9I\xe7s\xb3\xdd\xfd\xb3a\x86\xff\xc6\x9f\x98\xe0\x16,@H\xcb3w9\xfe*0Q\xf8\xdaO\x122\xed \x1d\xdc<s @ big secret
Decrypted: This is q big se\x02{\xaaK

$ cd cryptopp8/cryptopp; make
$ cd ../..; c++ -o cryptest8 -Icryptopp8 -Lcryptopp8/cryptopp cryptest.cpp -lcryptopp
$ ./cryptest8
Encrypted: \x04\xc2U\x0c\x85\xd8\xfa_\x9f\xac\x98\x93\xf0+\xe2K/\x14\x92 B\xf2(\xf5oi|h\x9d\xa8|\xac\xbd\xde\x1b\xd5\x06I|\xb7x5\xe7\xb2\x96\xcb\x8a<\xa4\x1b'\xcc[\x1b\xceC.\xd0\xf6\xc8\x90\xde\xd0\xf5Km\x054H8?s @ big secret
Decrypted: This is a big se\xd4het

$ cd cryptopp8/cryptopp; CXXFLAGS=-g3 make
$ cd ../..; c++ -o cryptest8 -Icryptopp8 -Lcryptopp8/cryptopp cryptest.cpp -lcryptopp
$ ./cryptest8
Encrypted: \x04S\x1f6\xb8\xa5\x1a{\xcc\xdd$Q\xeaxh\xbe\xd7^S\xe4\x06{\x1d\x92\x14T\x01\xb6Y\xb3\xc6\xc7\xab\x1a( w\x0c\x8e&K\x0c\xc9\xe6\xca\xa5\xd6>\xdc\xc7u\xff\xc4\xc6R\xc8\xe03\xe1@ks\xd0x\xa5\x08e[h\xcf<s @ big secret
Decrypted: This is \x01 big se\x83V\xfaE

cryptest.cpp:

#include <stdio.h>
#include <assert.h>
#include <cryptopp/osrng.h>
#include <cryptopp/eccrypto.h>
#include <cryptopp/oids.h>

#if CRYPTOPP_VERSION >= 600
#define byte CryptoPP::byte
#endif

void hexdump(byte* data, int length) {
   
for (int i = 0; i < length; i++) {
       
if (std::isprint(data[i]))
            printf
("%c", data[i]);
       
else
            printf
("\\x%02x", data[i]);
   
}
    printf
("\n");
}

class NULLHash : public CryptoPP::IteratedHashWithStaticTransform
   
<CryptoPP::word32, CryptoPP::BigEndian, 32, 0, NULLHash, 0>
{
public:
   
static void InitState(HashWordType *state) {}
   
static void Transform(CryptoPP::word32 *digest, const CryptoPP::word32 *data) {}
   
static const char *StaticAlgorithmName() {return "NULL HASH";}
};

template <class EC, class COFACTOR_OPTION = CryptoPP::IncompatibleCofactorMultiplication, bool DHAES_MODE = true>
   
struct ECIES_PY
   
: public CryptoPP::DL_ES<
       
CryptoPP::DL_Keys_EC<EC>,
       
CryptoPP::DL_KeyAgreementAlgorithm_DH<typename EC::Point, COFACTOR_OPTION>,
       
CryptoPP::DL_KeyDerivationAlgorithm_P1363<typename EC::Point, DHAES_MODE, CryptoPP::P1363_KDF2<CryptoPP::SHA1> >,
       
CryptoPP::DL_EncryptionAlgorithm_Xor<CryptoPP::HMAC<NULLHash>, DHAES_MODE>,
        ECIES_PY
<EC> >
   
{
       
static std::string CRYPTOPP_API StaticAlgorithmName() {return "ECIES_PY";}
   
};

int main() {
   
const char *plaintext = "This is a big secret";
    ECIES_PY
<CryptoPP::ECP>::PrivateKey private_key;
    ECIES_PY
<CryptoPP::ECP>::PublicKey public_key;

   
CryptoPP::AutoSeededRandomPool rng;

    private_key
.Initialize(rng, CryptoPP::ASN1::secp256k1());
    private_key
.MakePublicKey(public_key);

    ECIES_PY
<CryptoPP::ECP>::Encryptor encryptor;
    encryptor
.AccessKey().AccessGroupParameters().Initialize(CryptoPP::ASN1::secp256k1());
    encryptor
.AccessKey().SetPublicElement(public_key.GetPublicElement());
    encryptor
.AccessKey().ThrowIfInvalid(rng, 3);

   
unsigned long data_encrypted_length = encryptor.CiphertextLength(strlen(plaintext));
   
byte data_encrypted[data_encrypted_length];
    encryptor
.Encrypt(rng, reinterpret_cast<const byte *>(plaintext), strlen(plaintext), data_encrypted);
    printf
("Encrypted: ");
    hexdump
(data_encrypted, data_encrypted_length);
    ECIES_PY
<CryptoPP::ECP>::Decryptor decryptor;
    decryptor
.AccessKey().Initialize(CryptoPP::ASN1::secp256k1(), private_key.GetPrivateExponent());
    decryptor
.AccessKey().ThrowIfInvalid(rng, 3);

   
unsigned long data_decrypted_max_length = decryptor.MaxPlaintextLength(data_encrypted_length);
   
byte data_decrypted[data_decrypted_max_length];
   
CryptoPP::DecodingResult res = decryptor.Decrypt(rng, reinterpret_cast<const byte *>(data_encrypted), data_encrypted_length, data_decrypted);
   
assert(res.isValidCoding);
    printf
("Decrypted: ");
    hexdump
(data_decrypted, res.messageLength);
   
return 0;
}



Jeffrey Walton

unread,
Jul 1, 2019, 2:51:39 PM7/1/19
to Crypto++ Users


On Monday, July 1, 2019 at 2:07:06 PM UTC-4, Andrew Wason wrote:
We have been using the code below with libcrypto++6 on Ubuntu for years. It uses a custom NULLHash implementation to reduce the size of the final ciphertext since we are not concerned with message integrity.

I recently tested this with libcrypto 8.2 and the decrypted text is now partially corrupted, also I noticed plaintext is visible in the encrypted ciphertext. I then built a non-optimized libcrypto 6 and realized it also fails in the same manner.
...

I think this may have something to do with https://cryptopp.com/wiki/Elliptic_Curve_Integrated_Encryption_Scheme#Bouncy_Castle_Patch . That section needs to clearly state when the change occirs but it does not. I'll look up the info and get it added.

Under Crypto++ 8.0, try to use LABEL_OCTETS=true. The other options of interest are NoCofactorMultiplication and DHAES_MODE=false. Also see the comments at https://github.com/weidai11/cryptopp/blob/master/eccrypto.h#L617 .

Jeff

Andrew Wason

unread,
Jul 1, 2019, 5:38:28 PM7/1/19
to Crypto++ Users


On Monday, July 1, 2019 at 2:51:39 PM UTC-4, Jeffrey Walton wrote:

I think this may have something to do with https://cryptopp.com/wiki/Elliptic_Curve_Integrated_Encryption_Scheme#Bouncy_Castle_Patch . That section needs to clearly state when the change occirs but it does not. I'll look up the info and get it added.

Ugh, I assumed Ubuntu libcrypto++6  was based on 6, but it's 5.6.4. So I need to be comparing CRYPTOPP_5_6_4 and CRYPTOPP_6_0_0
https://packages.ubuntu.com/bionic/libcrypto++6

It turns out the problem does occur in CRYPTOPP_6_0_0 regardless of how I compile, and does not occur with CRYPTOPP_5_6_4.

I believe I found the issue.

DL_EncryptionAlgorithm_Xor::GetSymmetricKeyLength changed between 5.6.4 and 6.0.0:

     size_t GetSymmetricKeyLength(size_t plaintextLength) const
-               {return plaintextLength + MAC::DEFAULT_KEYLENGTH;}
+        {return plaintextLength + static_cast<size_t>(MAC::DIGESTSIZE);}



https://github.com/weidai11/cryptopp/compare/CRYPTOPP_5_6_4...CRYPTOPP_6_0_0#diff-bcbf193158734eaa1daf3c464a1e95b1L503

https://github.com/weidai11/cryptopp/compare/CRYPTOPP_5_6_4...CRYPTOPP_6_0_0#diff-bcbf193158734eaa1daf3c464a1e95b1R688

For my NULLHash, MAC::DIGESTSIZE is 0. This would result in a too-short buffer being allocated for derivedKey here

    SecByteBlock derivedKey(encAlg.GetSymmetricKeyLength(plaintextLength));


this is too short because in CryptoPP::DL_EncryptionAlgorithm_Xor::SymmetricEncrypt it offsets by MAC::DEFAULT_KEYLENGTH into derivedKey:

  cipherKey = key + MAC::DEFAULT_KEYLENGTH;
 
In 5.6.4 key was 36 bytes long (plaintextLength + MAC::DEFAULT_KEYLENGTH), in 6.0 it is only 20 bytes (just plaintextLength)
https://github.com/weidai11/cryptopp/blob/CRYPTOPP_6_0_0/gfpcrypt.h#L701

So cipherKey ends up pointing to only the tail end 4 bytes of the key, so when we xorbuf we walk off the end of cipherKey and use whatever is in memory beyond that and leave a lot of plaintext visible in the ciphertext.

In 6.0.0 and later, should SymmetricEncrypt be indexing into cipherKey by MAC::DIGESTSIZE instead of MAC::DEFAULT_KEYLENGTH?

Jeffrey Walton

unread,
Jul 1, 2019, 5:53:45 PM7/1/19
to Andrew Wason, Crypto++ Users
On Mon, Jul 1, 2019 at 5:38 PM Andrew Wason <recta...@gmail.com> wrote:
>
> On Monday, July 1, 2019 at 2:51:39 PM UTC-4, Jeffrey Walton wrote:
>>
>>
>> I think this may have something to do with https://cryptopp.com/wiki/Elliptic_Curve_Integrated_Encryption_Scheme#Bouncy_Castle_Patch . That section needs to clearly state when the change occirs but it does not. I'll look up the info and get it added.
>
> Ugh, I assumed Ubuntu libcrypto++6 was based on 6, but it's 5.6.4. So I need to be comparing CRYPTOPP_5_6_4 and CRYPTOPP_6_0_0
> https://packages.ubuntu.com/bionic/libcrypto++6
>
> It turns out the problem does occur in CRYPTOPP_6_0_0 regardless of how I compile, and does not occur with CRYPTOPP_5_6_4.
>
> I believe I found the issue.
>...
>
> In 6.0.0 and later, should SymmetricEncrypt be indexing into cipherKey by MAC::DIGESTSIZE instead of MAC::DEFAULT_KEYLENGTH?

I don't recall why that particular change happened. I'm probably the
guy who should remember why... Give me some time to think about it,
the reason may come to me.

Uri, Marcel - Do you guys remember why that particular change was made?

Jeff

Jeffrey Walton

unread,
Jul 1, 2019, 6:31:26 PM7/1/19
to Crypto++ Users
Looking at the changes, I would not be surprised if that was a copy/paste typo that flew under the radar because digest size was non-0.

We opened a bug report to track changes at https://github.com/weidai11/cryptopp/issues/856 .

Jeff

Uri Blumenthal

unread,
Jul 1, 2019, 6:48:57 PM7/1/19
to Jeffrey Walton, Crypto++ Users
If memory serves, ECIES standard required ^authenticated* encryption. That means - null hash wasn't allowed. 

Besides, there's Moxy Marlinspike principle: "If you don't enforce integrity, sooner or later you'll lose confidentiality as well."

Having said that, I don't recall why that particular change was made, and am willing to experiment to see what would happen if it's replaced with MAC::DEFAULT_KEYLENGTH (but we'll need to review the algorithm to recall what is doing there!).

Sent from my test iPhone
--
You received this message because you are subscribed to "Crypto++ Users". More information about Crypto++ and this group is available at http://www.cryptopp.com and http://groups.google.com/forum/#!forum/cryptopp-users.
---
You received this message because you are subscribed to the Google Groups "Crypto++ Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cryptopp-user...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/cryptopp-users/80a88957-276c-4eb8-b182-c353f1822e5e%40googlegroups.com.

Jeffrey Walton

unread,
Jul 1, 2019, 7:00:59 PM7/1/19
to Crypto++ Users


On Monday, July 1, 2019 at 6:48:57 PM UTC-4, Mouse wrote:
If memory serves, ECIES standard required ^authenticated* encryption. That means - null hash wasn't allowed. 

Besides, there's Moxy Marlinspike principle: "If you don't enforce integrity, sooner or later you'll lose confidentiality as well."

Having said that, I don't recall why that particular change was made, and am willing to experiment to see what would happen if it's replaced with MAC::DEFAULT_KEYLENGTH (but we'll need to review the algorithm to recall what is doing there!).

We have test cases for ECIES in validat8.cpp (https://github.com/weidai11/cryptopp/blob/master/validat8.cpp), but we don't have one for the Null hash. Based on the test cases, I think we should be OK to use MAC::DEFAULT_KEYLENGTH.

Jeff

Uri Blumenthal

unread,
Jul 1, 2019, 7:15:21 PM7/1/19
to Jeffrey Walton, Crypto++ Users
Hopefully it will not impact BouncyCastle interoperability. Now - also Botan interoperability.


Sent from my test iPhone
--
You received this message because you are subscribed to "Crypto++ Users". More information about Crypto++ and this group is available at http://www.cryptopp.com and http://groups.google.com/forum/#!forum/cryptopp-users.
---
You received this message because you are subscribed to the Google Groups "Crypto++ Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cryptopp-user...@googlegroups.com.

Jeffrey Walton

unread,
Jul 1, 2019, 7:21:16 PM7/1/19
to Uri Blumenthal, Crypto++ Users
On Mon, Jul 1, 2019 at 7:15 PM Uri Blumenthal <mous...@gmail.com> wrote:
>
> Hopefully it will not impact BouncyCastle interoperability. Now - also Botan interoperability.

Yeah, looking at the ECIES tests, it looks like pairwise consistency -
generate a key and then round trip some plaintext.

I think we should add two new tests. First, a legacy ECIES test, and
then an interop test using Botan. For the Botan test, we should
encrypt with Botan and decrypt with Crypto++ (or vice-versa).

Let me get with Jack and gather some ECIES test data.

Jeff

Mobile Mouse

unread,
Jul 1, 2019, 9:58:12 PM7/1/19
to Andrew Wason, Crypto++ Users
I believe I found the issue.

DL_EncryptionAlgorithm_Xor::GetSymmetricKeyLength changed between 5.6.4 and 6.0.0:

     size_t GetSymmetricKeyLength(size_t plaintextLength) const
-               {return plaintextLength + MAC::DEFAULT_KEYLENGTH;}
+        {return plaintextLength + static_cast<size_t>(MAC::DIGESTSIZE);}


After some thinking:

1. ECIES is an authenticated encryption mechanism - which means plaintext digest is appended to the plaintext itself.
2. Encryption is basically a hash-based stream-cipher. Which means - the “symmetric key” in this context is the key stream to be XORed with the plaintext.

It is obvious from the above, that the key stream generator must produce the number of bits/bytes equal to the length of the plaintext itself plus the length of the digest that provides integrity.

So, no - the above change does not look like a bug.

Comments? Questions?

Andrew Wason

unread,
Jul 2, 2019, 9:54:13 AM7/2/19
to Crypto++ Users

On Monday, July 1, 2019 at 9:58:12 PM UTC-4, Mouse wrote:

So, no - the above change does not look like a bug.
 

I don't think that change is the bug, the bug is that SymmetricEncrypt (and
SymmetricDecrypt) did not also change to use MAC::DIGESTSIZE at the same time. It continues to use MAC::DEFAULT_KEYLENGTH

Fixing that in master and it works properly with NULLHash:

diff --git a/gfpcrypt.h b/gfpcrypt.h
index
1b26a56b..f15c397e 100644
--- a/gfpcrypt.h
+++ b/gfpcrypt.h
@@ -716,7 +716,7 @@ public:
         
if (DHAES_MODE)
         
{
             macKey
= key;
-            cipherKey = key + MAC::DEFAULT_KEYLENGTH;
+            cipherKey = key + MAC::DIGESTSIZE;
         
}
         
else
         
{
@@ -748,7 +748,7 @@ public:
         
if (DHAES_MODE)
         
{
             macKey
= key;
-            cipherKey = key + MAC::DEFAULT_KEYLENGTH;
+            cipherKey = key + MAC::DIGESTSIZE;
         
}
         
else
         
{



Mouse

unread,
Jul 2, 2019, 11:04:10 AM7/2/19
to Andrew Wason, Crypto++ Users
What do you know, you're right! Thank you for finding the issue, and proposing the fix.

Applied in the master.

--
You received this message because you are subscribed to "Crypto++ Users". More information about Crypto++ and this group is available at http://www.cryptopp.com and http://groups.google.com/forum/#!forum/cryptopp-users.
---
You received this message because you are subscribed to the Google Groups "Crypto++ Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cryptopp-user...@googlegroups.com.


--
Regards,
Mouse

Jeffrey Walton

unread,
Jul 3, 2019, 2:49:49 PM7/3/19
to Crypto++ Users


On Tuesday, July 2, 2019 at 11:04:10 AM UTC-4, Mouse wrote:
What do you know, you're right! Thank you for finding the issue, and proposing the fix.

Applied in the master.

On Tue, Jul 2, 2019 at 9:54 AM Andrew Wason <recta...@gmail.com> wrote:

On Monday, July 1, 2019 at 9:58:12 PM UTC-4, Mouse wrote:

So, no - the above change does not look like a bug.
 

I don't think that change is the bug, the bug is that SymmetricEncrypt (and
SymmetricDecrypt) did not also change to use MAC::DIGESTSIZE at the same time. It continues to use MAC::DEFAULT_KEYLENGTH

Fixing that in master and it works properly with NULLHash:

Andrew, I think we got things sorted out. We added some ECIES known answer tests using Crypto++ 5.6.2 as the source. Uri also added a NullHash test. Then we fixed SymmetricEncrypt.

The kat's are working as expected with Crypto++ 5.6.2 and Master.

Can you verify the issue has been cleared with your code?

Jeff

Andrew Wason

unread,
Jul 3, 2019, 3:05:34 PM7/3/19
to Crypto++ Users

On Wednesday, July 3, 2019 at 2:49:49 PM UTC-4, Jeffrey Walton wrote:

Can you verify the issue has been cleared with your code?

Yes, my test program is working against master and I verified our existing code linked with master is able to decrypt data we had encrypted previously with cryptopp 5.6.4

Thanks!

Jeffrey Walton

unread,
Jul 3, 2019, 3:20:51 PM7/3/19
to Crypto++ Users
OK, so the root cause looks like a typo on my part that flew under the radar due to a missing known answer test.

Thanks for pointing it out.

Jeff
Reply all
Reply to author
Forward
0 new messages