Upgrade RandomPool IncorporateEntropy from SHA-256 to SHA-384

[P2Poker]

Reason: more entropy Difficulty: pretty easy Risk: minimal

TODO:

• [x] determine best way to split a 384-bit hash into a 256-bit key + 128-bit seed
• [x] determine whether any other instances of SHA256 could benefit from a similar upgrade
• [x] determine whether any other code is affected negatively
• [x] confirm overall impact
• [x] test
• [x] update documentation

Theoretical example of possible modifications:

void RandomPool::IncorporateEntropy(const byte *input, size_t length)
{
    //SHA256 hash; // replace with SHA384
    SHA384 hash;

    hash.Update(m_key, 32);
    hash.Update(m_seed, 16); // add the current existing seed to the hash update cycle

    hash.Update(input, length);

    // Finalize using a 384-bit (48 byte) object, and split hash across the key and seed
    //hash.Final(m_key);
    FixedSizeAlignedSecBlock<byte, 48> hashResult;
    hash.Final(hashResult);
    ::memcpy(m_key,hashResult,32);
    ::memcpy(m_seed,(byte*)hashResult+32,16);

    m_keySet = false;
}

/* ... */

template <class BLOCK_CIPHER>
void AutoSeededX917RNG<BLOCK_CIPHER>::Reseed(bool blocking, const byte *input, size_t length)
{
    /* BLOCK_CIPHER::BLOCKSIZE + BLOCK_CIPHER::DEFAULT_KEYLENGTH
        will be 48 bytes (384 bits) when AES-256 is used as the block cipher. */

    SecByteBlock seed(BLOCK_CIPHER::BLOCKSIZE + BLOCK_CIPHER::DEFAULT_KEYLENGTH);
    const byte *key;
    do
    {
        OS_GenerateRandomBlock(blocking, seed, seed.size());
        if (length > 0)
        {
            //SHA256 hash; // replace with SHA384
            SHA384 hash;
            hash.Update(seed, seed.size());
            hash.Update(input, length);
            hash.TruncatedFinal(seed, UnsignedMin(hash.DigestSize(), seed.size()));
        }
        key = seed + BLOCK_CIPHER::BLOCKSIZE;
    }   // check that seed and key don't have same value
    while (memcmp(key, seed, STDMIN((unsigned int)BLOCK_CIPHER::BLOCKSIZE, (unsigned int)BLOCK_CIPHER::DEFAULT_KEYLENGTH)) == 0);

    Reseed(key, BLOCK_CIPHER::DEFAULT_KEYLENGTH, seed, NULLPTR);
}

/* ... */

class CRYPTOPP_DLL AutoSeededRandomPool : public RandomPool
{
public:
    CRYPTOPP_STATIC_CONSTEXPR const char* StaticAlgorithmName() { return "AutoSeededRandomPool"; }

    ~AutoSeededRandomPool() {}

    /// \brief Construct an AutoSeededRandomPool
    /// \param blocking controls seeding with BlockingRng or NonblockingRng
    /// \param seedSize the size of the seed, in bytes
    /// \details Use blocking to choose seeding with BlockingRng or NonblockingRng.
    ///   The parameter is ignored if only one of these is available.
    //explicit AutoSeededRandomPool(bool blocking = false, unsigned int seedSize = 32)
    //  {Reseed(blocking, seedSize);}
    explicit AutoSeededRandomPool(bool blocking = false, unsigned int seedSize = 48) // update default seedSize to 48 bytes
        {Reseed(blocking, seedSize);}

    /// \brief Reseed an AutoSeededRandomPool
    /// \param blocking controls seeding with BlockingRng or NonblockingRng
    /// \param seedSize the size of the seed, in bytes
    //void Reseed(bool blocking = false, unsigned int seedSize = 32);
    void Reseed(bool blocking = false, unsigned int seedSize = 48); // update default seedSize to 48 bytes
};

Thoughts:

• The random number generator routinely adds a high-resolution timestamp to the seed, for a little more entropy. This is fine, and would not be impacted by these modifications. More entropy is more entropy.
• It would be convenient if the key and seed were guaranteed to be stored contiguously in memory. (Are they already contiguous? It didn't seem so to me, and I figured it's never safe to assume contiguousness, unless it is explicitly defined e.g. arrays.) I'm pretty new to Crypto++, but I believe modifying the key and seed to share a contiguous 384-bit block might break some other things (which would in turn need to be modified as well) - I have not yet confirmed whether this approach is realistic, and I have my doubts.

[noloader]

@P2Poker ,

What is the problem? What problem are you trying to solve?

Most of the computation effort is expended in GenerateBlock and/or GenerateIntoBufferedTransformation, which produces the random number stream. The work is due to AES.

The system time is added to reduce the risk of state playback attacks in virtual machines. Also see the head notes at RandomPool Class Reference.

The memory for the key and seed are not contiguous. From randpool.h:

FixedSizeAlignedSecBlock<byte, 16, true> m_seed;
FixedSizeAlignedSecBlock<byte, 32> m_key;
member_ptr<BlockCipher> m_pCipher;
bool m_keySet;

[P2Poker]

@noloader Thanks for the info.

SHA256 just seemed like a little bit of a waste, since 256-bit AES expects 384 bits of input.

There is no problem, this isn't a bugfix. I just figure this would complete the picture, round out the algorithm, add a little more robustness, collect an extra initial 128-bits of entropy from the OS when auto-seeding, etc.

All of these things seem ultimately desirable, even if minor.

I figure, if the upgrade is this easy, let's just get it over with. I'll do some more work on this today, between other tasks.

[noloader]

Thanks @P2Poker.

I think we are going to pass on the PR. I prefer not to disturb the underlying RandomNumberGenerator interface. And while not readily apparent, the switch from SHA-256 to SHA-512 is a lot slower on some platforms. That's because we have SHA-256 intrinsics on Intel, ARM and Power8 machines.