SPECK64 different output if input is passed in chunks

[guidovranken]

Reproducer:

#include <modes.h>
#include <filters.h>
#include <speck.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }

std::vector<uint8_t> Encrypt(const std::vector<uint8_t>& key, const std::vector<uint8_t>& iv, const std::vector<std::vector<uint8_t>>& cleartext) {
    ::CryptoPP::OFB_Mode<::CryptoPP::SPECK64>::Encryption enc(key.data(), key.size(), iv.data());
    ::CryptoPP::StreamTransformationFilter encryptor(enc, nullptr);
    for (const auto& part : cleartext) {
        encryptor.Put(part.data(), part.size());
    }
    encryptor.MessageEnd();
    std::vector<uint8_t> out(encryptor.MaxRetrievable());
    encryptor.Get(out.data(), out.size());
    return out;
}

std::vector<uint8_t> Decrypt(const std::vector<uint8_t>& key, const std::vector<uint8_t>& iv, const std::vector<uint8_t>& ciphertext) {
    ::CryptoPP::OFB_Mode<::CryptoPP::SPECK64>::Decryption dec(key.data(), key.size(), iv.data());
    ::CryptoPP::StreamTransformationFilter decryptor(dec, nullptr);
    decryptor.Put(ciphertext.data(), ciphertext.size());
    decryptor.MessageEnd();
    std::vector<uint8_t> out(decryptor.MaxRetrievable());
    decryptor.Get(out.data(), out.size());
    return out;
}

int main(void)
{
    const std::vector<std::vector<uint8_t>> cleartext{
#if !defined(CHUNKED)
        {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
#else
        {0x00},
              {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
                                                              {0x00, 0x00, 0x00, 0x00, 0x00},
                                                                                            {0x00},
                                                                                                  {0x00},
#endif
    };
    const std::vector<uint8_t> key(16);
    const std::vector<uint8_t> iv(8);
    const auto ciphertext = Encrypt(key, iv, cleartext);

    const auto cleartext2 = Decrypt(key, iv, ciphertext);

    for (size_t i = 0; i < cleartext2.size(); i++) {
        printf("%02X ", cleartext2[i]);
    }
    printf("\n");
end:
    return 0;
}

Compile this with and without -DCHUNKED. Both should output:

00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

But when compiled with -DCHUNKED, it outputs:

00 00 00 00 00 00 00 00 BD DD D9 80 6A 84 CF 2A

Tested on Linux x64, Crypto++ recent master branch.

[noloader]

OFB mode look broken at the moment. When opening config_asm.h and adding the define CRYPTOPP_DISABLE_ASM, then I get the following for encryption using full blocks and chunks:

$ ./test.exe
2C 69 2F 27 D5 48 04 68 F0 31 EE 33 DB 5F 63 D9

The CRYPTOPP_DISABLE_ASM result is probably correct. And it does not agree with the other two results using AdvancedProcessBlocks for encryption.

The current scorecard is ECB, CBC and CTR - Ok. OFB and probably CFB - Bad. All of the modes use AdvancedProcessBlocks.

[noloader]

Cleared at Commit 62ee118379f3.

I don't know when I'll have the time to look at this issue. It is low priority since 64-bit block ciphers are not likely to be used nowadays. The 128-bit ciphers are OK.

[mouse07410]

@noloader this commit appears to break my CI. The tests weren't updated?

[noloader]

@mouse07410,

Yeah, it broke mine, too.

The ECB tests were generated with SIMON and SPECK reference implementation. Whatever is going on is most likely on our side.

Let me take a look.

[noloader]

@mouse07410,

So it looks like the SIMON64 and SPECK64 keys were still being pre-splatted on the SSE4 path in UncheckedSetKey even though SSE4 was disabled in AdvancedProcessBlocks.

That break was cleared at Commit 84ab41902908.

[mouse07410]

Thanks!

BTW, you sure you want to drop SSE4 though? I'd probably prefer to live with chunking issues...?

[noloader]

@mouse07410,

you sure you want to drop SSE4 though? I'd probably prefer to live with chunking issues...?

Yeah, I know what you mean.

I think the library has to be correct before it can be fast. If it is not correct, then it does not matter if it is fast.

[noloader]

@mouse07410,

I removed the 64-bit AdvancedProcessBlocks, like AdvancedProcessBlocks64_4x1_SSE. The tempates were used by the 64-bit block ciphers, like SIMON64 and SPECK64.

If they are causing bad code then they are a hazard. Other people should not be using them.

I'll add a note about them in the head notes so someone can revert the change if they want the templates.