-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
HC-256 Inconsistent with Other Libraries #1280
Comments
Thanks @Demonslay335. A few thoughts... We test Crypto++ on both little-endian and big-endian systems. HC-128 and HC-256 are both passing on them. If we had an endian issue, I would expect it to surface. I try to use reference implementations to generate test vectors. Here is the one I used for HC-256: https://github.com/noloader/cryptopp-test/tree/master/HC-256. (It is my GitHub, where I store reference implementations for posterity). The reference implementation includes a file called
In Crypto++, the tests are at located at:
I only see the first one in BouncyCastle. I do not see the second or third in the BouncyCastle tests. Can you run the test vectors using BouncyCastle? I know David, and he and I have met in real life. We try to stay consistent on implementations (ECIES was a hairy one). So it will be interesting to see where we differ. |
Ok, there is something very odd going on here then... let me explain. In short, it appears the HC-256 author's PDF description does not match his own C source code listed on the same page. (Phases 1-3 all contain the same PDF and C code zip, so it's not some draft/version mismatch). In the PDF, under the section "B The optimized C implementation of HC-256 (“hc256.h”)" on Page 20, there is the following code: void initialization(uint32 key[], uint32 iv[])
{
uint32 i,j;
//expand the key and iv into P and Q
for (i = 0; i < 8; i++) P[i] = key[i];
for (i = 8; i < 16; i++) P[i] = iv[i-8];
... Notice how there are no rotations. Also, my mistake, I did not use the direct ECRYPT version, but actually used this library that essentially does the same as the PDF version: https://github.com/peterferrie/hc256 So, it may come down to each library having derived off of either the C code in the PDF specification, or the ECRYPT I have also looked at a common Rust implementation and a few Python implementations, and they all preserve little-endian for the key and IV. Honestly so far, CryptoPP has been the only library that differs from what I can tell. But also, not many mainstream crypto libraries even support the algorithm to begin with... As extra context to my original post, the "external app" is actually a ransomware using the HC-256 algorithm. I was developing a decryptor in our C++ application, and noticed the output from I originally did not mention it being malware, as I did not want you to discredit it as a potentially "customized" version of the algorithm or something (that does happen, rarely). The malware's implementation completely matches output from BouncyCastle, which is how I originally identified and tested the algorithm during my reverse engineering. Concerning the unit tests, basically every test labeled from your "hc-256.c reference implementation" does not match BouncyCastle. I spot tested a couple dozen. To easily test yourself (without compiling anything), you can try my CryptoTester program. Internally, it uses BouncyCastle C#'s https://github.com/bcgit/bc-java/blob/0520ebcd3291d6d3176ea25c571f1c7e0a963847/core/src/main/java/org/bouncycastle/crypto/engines/HC256Engine.java#L105-L113 You can also conversely try basically any test vector from BouncyCastle's Set 6 in CryptoPP, just like my original bug PoC. (The C# project's vectors are much easier to read). I should also mention that CryptoPP and BouncyCastle do match not only when the key/IV are repeating bytes, but also if only the first byte is modified; aka the 3 original reference tests do work everywhere. It is very frustrating that they are so lacking, allowing this ambiguity to exist in the first place... literally any other algorithm I've worked with has some standardized test vectors of non zero keys for this very reason... |
Thanks for the work @Demonslay335,
Yeah, I've seen this before. For example, SIMON and SPECK did the same thing. In the case of SIMON and SPECK, we worked with the authors to get things to line up (the paper and ref implementation).
I would hazard a guess and say both are right (as unfortunate as that is). In Crypto++, we used the reference ECRYPT implementation because that's what we were aiming for. More specifically, we used eSTREAM Phase 3 implementation from https://www.ecrypt.eu.org/stream/hcp3.html. We should get this documented in the wiki at https://www.cryptopp.com/wiki/HC-128#HC-256. And ChaCha20 is another really bad example of interop problems. Crypto++ implemented ChaCha20 from Bernstein's paper and reference implementation. Then the IETF standardized ChaCha20 but changed the algorithm. So there are two versions of ChaCha20 floating around -- Bernstein's ChaCha20 and the IETF version that hijacked the ChaCha20 name. |
Can you provide a diff that makes Crypto++ work with BouncyCastle and friends? I'd like to think about how we can support both implementations. |
@noloader Sure thing, here ya go. I matched the method of initializing the key and IV with that of HC128 for consistency. Building with this patch makes my above PoC pass, but it will of course break most of your unit tests. diff --git a/hc256.cpp b/hc256.cpp
index 3a93f21e..96bb9743 100644
--- a/hc256.cpp
+++ b/hc256.cpp
@@ -101,14 +101,7 @@ void HC256Policy::CipherSetKey(const NameValuePairs ¶ms, const byte *userKey
CRYPTOPP_UNUSED(params); CRYPTOPP_UNUSED(keylen);
CRYPTOPP_ASSERT(keylen == 32);
- for (unsigned int i = 0; i < 8; i++)
- m_key[i] = 0;
-
- for (unsigned int i = 0; i < 32; i++)
- {
- m_key[i >> 2] = m_key[i >> 2] | userKey[i];
- m_key[i >> 2] = rotlConstant<8>(m_key[i >> 2]);
- }
+ GetUserKey(LITTLE_ENDIAN_ORDER, m_key.begin(), 8, userKey, keylen);
}
void HC256Policy::OperateKeystream(KeystreamOperation operation, byte *output, const byte *input, size_t iterationCount)
@@ -127,18 +120,11 @@ void HC256Policy::CipherResynchronize(byte *keystreamBuffer, const byte *iv, siz
CRYPTOPP_UNUSED(keystreamBuffer); CRYPTOPP_UNUSED(length);
CRYPTOPP_ASSERT(length == 32);
- /* initialize the iv */
- word32 W[2560];
- for (unsigned int i = 0; i < 8; i++)
- m_iv[i] = 0;
-
- for (unsigned int i = 0; i < 32; i++)
- {
- m_iv[i >> 2] = m_iv[i >> 2] | iv[i];
- m_iv[i >> 2] = rotlConstant<8>(m_iv[i >> 2]);
- }
+ /* initialize the iv */
+ GetUserKey(LITTLE_ENDIAN_ORDER, m_iv.begin(), 8, iv, length);
/* setup the table P and Q */
+ word32 W[2560];
for (unsigned int i = 0; i < 8; i++)
W[i] = m_key[i];
-- My guess for moving forward would be to fix the current implementation, and copy the "old" one to a new I had no idea this would end up being another ChaCha vs ChaChaTLS situation... |
Thanks @Demonslay335, I vaguely remember I wanted to use this with HC-256, but I had to switch because of the test vectors produced by the reference implementation. + GetUserKey(LITTLE_ENDIAN_ORDER, m_key.begin(), 8, userKey, keylen); |
Same thing happens for me, BouncyCastle (.NET/Java both) and CryptoPP HC256 implementations are different. |
While developing support for an external app that uses the HC-256 cipher, I noticed inconsistencies with the
CryptoPP::HC256
algorithm's output. Further digging showed that when either the key or IV contain non-repeating bytes (e.g. not just all00
or36
...), the output does not match that of other libraries, or the original reference library.I have tested against BouncyCastle and the original
hc256.c
ECRYPT source (which I had to use instead of CryptoPP for this project).Compiled CryptoPP 8.9.0 using Visual Studio (x64) on Windows 10.
I have found the problem seems to be with the endian flip happening with the key and IV at these two locations. Such an endian flip is not present in BouncyCastle or
hc256.c
.cryptopp/hc256.cpp
Lines 107 to 111 in 60f81a7
cryptopp/hc256.cpp
Lines 135 to 139 in 60f81a7
If I replace the loop bodies with these respective changes, the unit output matches that of BouncyCastle and
hc256.c
.Here is a test program that uses a BouncyCastle test vector to show the mismatch. The code throws an assertion with CryptoPP 8.9.0, and passes with the above changes.
Furthermore, the sample on the wiki is not decryptable with BouncyCastle.
The
CryptoPP::HC128
algorithm appears fine; it also does not do endian flipping of the key and IV, instead explicitly usingGetUserKey(LITTLE_ENDIAN_ORDER ...)
.The text was updated successfully, but these errors were encountered: