Skip to content

Commit

Permalink
Try fix ProcessData in CFB_CipherTemplate and AdditiveCipherTemplate
Browse files Browse the repository at this point in the history 
Also see GH #683, GH #1010, GH #1088, GH #1103
  • Loading branch information
@noloader
noloader committed Feb 13, 2022
1 parent a1e35ee commit db33299
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 60 deletions.
70 changes: 14 additions & 56 deletions strciphr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void AdditiveCipherTemplate<S>::GenerateBlock(byte *outString, size_t length)
}

PolicyInterface &policy = this->AccessPolicy();
unsigned int bytesPerIteration = policy.GetBytesPerIteration();
size_t bytesPerIteration = policy.GetBytesPerIteration();

if (length >= bytesPerIteration)
{
Expand Down Expand Up @@ -93,26 +93,8 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
CRYPTOPP_ASSERT(outString); CRYPTOPP_ASSERT(inString);
CRYPTOPP_ASSERT(length % this->MandatoryBlockSize() == 0);

// If this assert fires, then outString == inString. You could experience a
// performance hit. Also see https://github.com/weidai11/cryptopp/issues/1088'
CRYPTOPP_ASSERT(outString != inString);

PolicyInterface &policy = this->AccessPolicy();
unsigned int bytesPerIteration = policy.GetBytesPerIteration();

byte* savedOutString = outString;
size_t savedLength = length;
bool copyOut = false;

if (inString == outString)
{
// No need to copy inString to outString.
// Just allocate the space.
m_tempOutString.New(length);
m_tempOutString.SetMark(0);
outString = m_tempOutString.BytePtr();
copyOut = true;
}
size_t bytesPerIteration = policy.GetBytesPerIteration();

if (m_leftOver > 0)
{
Expand All @@ -124,13 +106,9 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
length -= len; m_leftOver -= len;
}

if (!length) {
if (copyOut)
std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
return;
}
if (!length) { return; }

const unsigned int alignment = policy.GetAlignment();
const word32 alignment = policy.GetAlignment();
const bool inAligned = IsAlignedOn(inString, alignment);
const bool outAligned = IsAlignedOn(outString, alignment);
CRYPTOPP_UNUSED(inAligned); CRYPTOPP_UNUSED(outAligned);
Expand All @@ -143,6 +121,10 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin
KeystreamOperation operation = KeystreamOperation(flags);
policy.OperateKeystream(operation, outString, inString, iterations);

// Try to tame the optimizer. This is GH #683, #1010, and #1088.
volatile byte* unused = const_cast<volatile byte*>(outString);
CRYPTOPP_UNUSED(unused);

inString = PtrAdd(inString, iterations * bytesPerIteration);
outString = PtrAdd(outString, iterations * bytesPerIteration);
length -= iterations * bytesPerIteration;
Expand Down Expand Up @@ -171,9 +153,6 @@ void AdditiveCipherTemplate<S>::ProcessData(byte *outString, const byte *inStrin

m_leftOver = bufferByteSize - length;
}

if (copyOut)
std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
}

template <class S>
Expand Down Expand Up @@ -244,28 +223,10 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
CRYPTOPP_ASSERT(outString); CRYPTOPP_ASSERT(inString);
CRYPTOPP_ASSERT(length % this->MandatoryBlockSize() == 0);

// If this assert fires, then outString == inString. You could experience a
// performance hit. Also see https://github.com/weidai11/cryptopp/issues/1088'
CRYPTOPP_ASSERT(outString != inString);

PolicyInterface &policy = this->AccessPolicy();
unsigned int bytesPerIteration = policy.GetBytesPerIteration();
byte *reg = policy.GetRegisterBegin();

byte* savedOutString = outString;
size_t savedLength = length;
bool copyOut = false;

if (inString == outString)
{
// No need to copy inString to outString.
// Just allocate the space.
m_tempOutString.New(length);
m_tempOutString.SetMark(0);
outString = m_tempOutString.BytePtr();
copyOut = true;
}

if (m_leftOver)
{
const size_t len = STDMIN(m_leftOver, length);
Expand All @@ -276,13 +237,9 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
m_leftOver -= len; length -= len;
}

if (!length) {
if (copyOut)
std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
return;
}
if (!length) { return; }

const unsigned int alignment = policy.GetAlignment();
const word32 alignment = policy.GetAlignment();
const bool inAligned = IsAlignedOn(inString, alignment);
const bool outAligned = IsAlignedOn(outString, alignment);
CRYPTOPP_UNUSED(inAligned); CRYPTOPP_UNUSED(outAligned);
Expand All @@ -292,6 +249,10 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
CipherDir cipherDir = GetCipherDir(*this);
policy.Iterate(outString, inString, cipherDir, length / bytesPerIteration);

// Try to tame the optimizer. This is GH #683, #1010, and #1088.
volatile byte* unused = const_cast<volatile byte*>(outString);
CRYPTOPP_UNUSED(unused);

const size_t remainder = length % bytesPerIteration;
inString = PtrAdd(inString, length - remainder);
outString = PtrAdd(outString, length - remainder);
Expand All @@ -314,9 +275,6 @@ void CFB_CipherTemplate<BASE>::ProcessData(byte *outString, const byte *inString
CombineMessageAndShiftRegister(outString, reg, inString, length);
m_leftOver = bytesPerIteration - length;
}

if (copyOut)
std::memcpy(savedOutString, m_tempOutString.BytePtr(), savedLength);
}

template <class BASE>
Expand Down
5 changes: 1 addition & 4 deletions strciphr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,7 @@ class CRYPTOPP_NO_VTABLE AdditiveCipherTemplate : public BASE, public RandomNumb
inline byte * KeystreamBufferBegin() {return this->m_buffer.data();}
inline byte * KeystreamBufferEnd() {return (PtrAdd(this->m_buffer.data(), this->m_buffer.size()));}

// m_tempOutString added due to GH #1010
AlignedSecByteBlock m_buffer, m_tempOutString;
AlignedSecByteBlock m_buffer;
size_t m_leftOver;
};

Expand Down Expand Up @@ -646,8 +645,6 @@ class CRYPTOPP_NO_VTABLE CFB_CipherTemplate : public BASE

void UncheckedSetKey(const byte *key, unsigned int length, const NameValuePairs &params);

// m_tempOutString added due to GH #1010
AlignedSecByteBlock m_tempOutString;
size_t m_leftOver;
};

Expand Down

0 comments on commit db33299

Please sign in to comment.