Using Cryptopp's Shamir's Secret Sharing to securely divide in memory byte blobs and then disperse the shares over network.

[Whou Lee]

Hi, all!

I am trying to make CryptoPP::SecretSharing  to work with in memory byte objects.

My current solution partly works. I can reconstruct the original secret except for the last byte or two, depends on the length of original secret.

Could you please suggest me how to fix this.

The code:

std::vector<Bytes> SecretShareBytes(const Bytes& secret, int threshold, int nShares)

{

CryptoPP::AutoSeededRandomPool rng;

CryptoPP::ChannelSwitch *channelSwitch;

CryptoPP::ArraySource source( secret.data(), secret.size(), false,new CryptoPP::SecretSharing( rng, threshold, nShares, channelSwitch = new CryptoPP::ChannelSwitch) );

        std::vector<Bytes> shares( nShares );

CryptoPP::vector_member_ptrs<CryptoPP::ArraySink> arraySinks( nShares );

std::string channel;

for (int i = 0; i < nShares; i++)

{

shares[i] = Bytes( secret.size() + sizeof(int) );

arraySinks[i].reset( new CryptoPP::ArraySink((byte*)shares[i].data(), shares[i].size()) );

channel = CryptoPP::WordToString<word32>(i);

                arraySinks[i]->Put( (byte *)channel.data(), 4 );

  channelSwitch->AddRoute( channel,*arraySinks[i],CryptoPP::BufferedTransformation::NULL_CHANNEL );

}

source.PumpAll();

return shares;

}

Bytes SecretRecoverBytes(std::vector<Bytes>& shares, int threshold)

{

Bytes bytes( shares[0].size() - sizeof( int ) );

CryptoPP::SecretRecovery recovery( threshold, new CryptoPP::ArraySink(bytes.data(), bytes.size()) );

CryptoPP::SecByteBlock channel(4);

for (int i = 0; i < threshold; i++)

{

CryptoPP::ArraySource arraySource(shares[i].data(), shares[i].size(), false);

arraySource.Pump(4);

arraySource.Get( channel, 4 );

arraySource.Attach( new CryptoPP::ChannelSwitch( recovery, std::string( (char *)channel.begin(), 4) ) );

arraySource.PumpAll();

}

return bytes;

}

[Linmao Song]

I had the same issue as in this (very) old unanswered thread below. It took me quite a while to figure out why. In case it might help someone else puzzled by the same, the root cause is insufficient buffer size. Specifically, the "Shares[i]" should be made larger.

Based on the original poster's code,  the "last byte or two" problem can be shown by:

    const Bytes secret = {0x01, 0x02, 0x03, 0x04, 0x05};

    const auto shares = SecretShareBytes(secret, 3, 7);

    std::vector<Bytes> partial;

    partial.push_back(shares[6]);

    partial.push_back(shares[3]);

    partial.push_back(shares[5]);

    const auto recovered = SecretRecoverBytes(partial, partial.size());

    std::cout << "Secret    -> " << std::dec << secret.size() << " bytes: ";

    for (const auto & uch : secret)

        std::cout << std::hex << std::setfill('0') << std::setw(2)

                  << static_cast<unsigned>(uch) << " ";

    std::cout << std::endl;

    std::cout << "Recovered -> " << std::dec << recovered.size() << " bytes: ";

    for (const auto & uch : recovered)

        std::cout << std::hex << std::setfill('0') << std::setw(2)

                  << static_cast<unsigned>(uch) << " ";

    std::cout << std::endl;

The output will be something like:

Secret    -> 5 bytes: 01 02 03 04 05

Recovered -> 5 bytes: 01 02 03 04 7a

What I found is that if the output is to FileSink (as in https://github.com/weidai11/cryptopp/blob/master/test.cpp#L657), the share/recovery works perfectly. Yet strangely ArraySink always gives trouble. The culprit seems to be in:

shares[i] = Bytes( secret.size() + sizeof(int) );

This should be given a larger size. Otherwise the outputs of SecretSharing are chopped off. With the incomplete shares, re-construction can't be successful.

There might be a way to precisely decide the correct buffer size for the shares. However, I find it easier to use ostringstream to manage the buffers. Counter-intuitively, ArraySink will have to be replaced by FileSink, due to the fact that ArraySink only accepts fixed length buffers.

With this change, the original poster's code would look like:

std::vector<Bytes> SecretShareBytes(const Bytes& secret, int threshold, int nShares)

{

    CryptoPP::AutoSeededRandomPool rng;

    CryptoPP::ChannelSwitch *channelSwitch;

    CryptoPP::ArraySource source( secret.data(), secret.size(), false,new CryptoPP::SecretSharing( rng, threshold, nShares, channelSwitch = new CryptoPP::ChannelSwitch) );

    std::vector<std::ostringstream> shares( nShares );

    CryptoPP::vector_member_ptrs<CryptoPP::FileSink> sinks( nShares );

    std::string channel;

    for (int i = 0; i < nShares; i++)

    {

        sinks[i].reset( new CryptoPP::FileSink(shares[i]));

        channel = CryptoPP::WordToString<word32>(i);

        sinks[i]->Put( (byte *)channel.data(), 4 );

        channelSwitch->AddRoute( channel,*sinks[i], DEFAULT_CHANNEL);

    }

    source.PumpAll();

    std::vector<Bytes> ret;

    for (const auto &share : shares)

    {

        const auto & piece = share.str();

        ret.push_back(Bytes(piece.begin(), piece.begin() + piece.size()));

    }

    return move(ret);

}

Bytes SecretRecoverBytes(std::vector<Bytes>& shares, int threshold)

{

    std::ostringstream out;

    CryptoPP::SecretRecovery recovery( threshold, new CryptoPP::FileSink(out) );

    CryptoPP::SecByteBlock channel(4);

    for (int i = 0; i < threshold; i++)

    {

        CryptoPP::ArraySource arraySource(shares[i].data(), shares[i].size(), false);

        arraySource.Pump(4);

        arraySource.Get( channel, 4 );

        arraySource.Attach( new CryptoPP::ChannelSwitch( recovery, std::string( (char *)channel.begin(), 4) ) );

        arraySource.PumpAll();

    }

    const auto & secret = out.str();

    return Bytes(secret.begin(), secret.begin() + secret.size());

}

And the print out will be correct.

Secret    -> 5 bytes: 01 02 03 04 05

Recovered -> 5 bytes: 01 02 03 04 05

[Jeffrey Walton]

Good find Linmao.

Using ArraySink is one of those sharp edges of the library. Array's are fixed size, so its [undesired] expected behavior. The library depends on the truncation in a few places.

You can duplicate the issue with other objects, like AES and StreamTransformationFilter.

Like you stated, you can use a StringSink or FileSink to sidestep the issue.

Stepping back and doing the postmortem, what would help here? Should we add an ASSERT in filters like Channel and StreamTransformationFilter to alert of a potential problem (i.e., a Put() did not process all data)? Should the example in test.cpp be reworked? Should we do something else?

We can't add an ASSERT to ArraySink since its doing its job. If we did, then the assert would spuriously fire as extra data intended to be discarded was dropped (the library does this in a few places). The only other place I can think of is immediately upstream (the "parent filter").

Jeff

[Jeffrey Walton]

On Thursday, April 13, 2017 at 8:53:15 AM UTC-4, Linmao Song wrote:

I had the same issue as in this (very) old unanswered thread below. It took me quite a while to figure out why. In case it might help someone else puzzled by the same, the root cause is insufficient buffer size. Specifically, the "Shares[i]" should be made larger.

...

On Friday, 11 December 2015 10:34:42 UTC, Whou Lee wrote:

...

I am trying to make CryptoPP::SecretSharing  to work with in memory byte objects.

My current solution partly works. I can reconstruct the original secret except for the last byte or two, depends on the length of original secret.

Could you please suggest me how to fix this.

...

We added comments to the source code at https://github.com/weidai11/cryptopp/commit/cf160e91c4e919d9. We also added a note on ArraySink's wiki page at https://www.cryptopp.com/wiki/ArraySink.

Is there anything else we can do?

Jeff

[Linmao Song]

Many thanks Jefferey. The added documentation is very helpful. As you say, it seems difficult to protect against such misuse. I also thought about making the name ArraySink more advisory, e.g. something like FixedSizeArraySink. However, that appears unnecessarily verbose, as in C++ context, "Array" itself already indicates "fixed size". So it seems to me the added documentation should suffice.