Skip to content
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

AuthenticatedDecryptionFilter with AAD throws an exception when used with a FileSource #817

Closed
Nyk72 opened this issue Mar 6, 2019 · 6 comments
Labels

Comments

@Nyk72
Copy link

Nyk72 commented Mar 6, 2019

I found a strange behaviour using a FileSource to decrypt a text encrypted with AES/EAX using both channels provided by AuthenticatedEncryptionFilter and AuthenticatedDecryptionFilter: it seems that the operations sequence used (successfully) with a StringSource cannot be used with a FileSource instead.
Digging into the library code, I think the reason is that AuthenticatedDecryptionFilter does not redefine the ChannelPutModifiable2 method, but only the ChannelPut2 one. I don't know whether AuthenticatedDecryptionFilter is really bugged or not, but it sounds odd to me that the code working with a StringSource does not work with a FileSource, so I searched a way to remove this discrepancy.
The attached code, adapted from the Crypto++ Wiki example EAX-AEAD-Test.zip, shows what I found:

  • sections from 1 to 4 are more or less like the original code;
  • section 5 decrypts (successfully) the text in cipher using a StringSource by first putting the authenticated data into the AAD channel and then pumping all the source contents;
  • section 6 writes cipher into a file;
  • section 7 tries to decrypt this file using a FileSource and the same operations used in section 5, but the library throws an InvalidArgument exception;
  • section 8 decrypts (successfully) the file like in section 7, but using a modified version of AuthenticatedDecryptionFilter with the method ChannelPutModifiable2 redefined;
  • section 9 does the same as section 8, with another modified version of AuthenticatedDecryptionFilter (I don't know if either this "fix" or the other could be right: they are simply two ways to mimic what happens in the StringSource case, i.e. to call m_hashVerifier.ForceNextPut() at the right moment).

The program output is the following:

Crypto++ Version: 810

key: 0000000000000000000000000000000000000000000000000000000000000000
iv: 000000000000000000000000
adata: 00000000000000000000000000000000
pdata: 00000000000000000000000000000000
cipher: A67F548596FDF6C457402F467A10E2B270586FD20C3A1564051C48C436873C57
adata (received): 00000000000000000000000000000000
pdata (recovered): 00000000000000000000000000000000
adata (received): 00000000000000000000000000000000
pdata (recovered from string): 00000000000000000000000000000000
Caught InvalidArgument...
AES/EAX: additional authenticated data (AAD) cannot be input after data to be encrypted or decrypted
adata (received): 00000000000000000000000000000000
pdata (recovered from file - bug fixed 1): 00000000000000000000000000000000
adata (received): 00000000000000000000000000000000
pdata (recovered from file - bug fixed 2): 00000000000000000000000000000000

I'm using Crypto++ 8.1.0 build with Visual Studio 2012 and the OS is Windows 10 Pro (1803).

CryptoPP_test.cpp

@noloader
Copy link
Collaborator

noloader commented Jun 6, 2019

@Nyk72,

My apologies I did not Ack earlier. I've been mulling over this...

... it sounds odd to me that the code working with a StringSource does not work with a FileSource

Yes, agreed. Something is bent/broke if you can't swap-in a source for any other source.

I'm going to need some time to sort through it. The pipelines can be tricky to maintain, so we have to be careful of knocking something loose.

@noloader
Copy link
Collaborator

noloader commented Sep 27, 2019

@Nyk72,

Can you provide a stand alone test case demonstrating the problem. I'm having a little trouble creating the stand alone reproducer. I think it includes your Step (7), but I am not clear on what Step (5) has to do with things.

Also, as far as I know, you can only use the pipeline with confidential data. The default Sources put all their data on the DEFAULT_CHANNEL. None of the Sources put their data on the AAD_CHANNEL.

Put another way, there's no Source that says, "take some of the file and put it on the AAD_CHANNEL, and take the rest of the file and put it on the DEFAULT_CHANNEL". A custom Source would probably be able to do it, though.

@Nyk72
Copy link
Author

Nyk72 commented Sep 27, 2019

@noloader,
my section 5 demonstrates only the steps used to recover the encrypted data using a StringSource:

  1. configure the Filter with MAC_AT_END;
  2. put the authenticate data in the AAD_CHANNEL;
  3. pump the encrypted data into the Source.

As you note, it is not necessary to show the problem: section 7 is what is needed, so I have reduced the code in CryptoPP_short_test.cpp.

@LiKao
Copy link

LiKao commented Oct 23, 2019

I am having a similar problem. I could trace back the problem to the following position:

if (m_totalFooterLength > MaxFooterLength())
{
	if (MaxFooterLength() == 0)
		throw InvalidArgument(AlgorithmName() + ": additional authenticated data (AAD) cannot be input after data to be encrypted or decrypted");
	else
		throw InvalidArgument(AlgorithmName() + ": footer length of " + IntToString(m_totalFooterLength) + " exceeds the maximum of " + IntToString(MaxFooterLength()));
}

in the method AuthenticatedSymmetricCipherBase::TruncatedFinal() in file authenc.cpp (line 141 for me).

I checked, and all implementations of MaxFooterLength() seem to return 0, so this code must always throw. In my case m_totalFooterLength is set to 16, I am guessing this is related to the TAG_SIZE.

@LiKao
Copy link

LiKao commented Oct 23, 2019

Relevant parts of the backtrace:

#6  0x00005555555c3f26 in CryptoPP::AuthenticatedSymmetricCipherBase::TruncatedFinal (this=<optimized out>, mac=<optimized out>, macSize=<optimized out>) at /usr/lib/gcc/x86_64-pc-linux-gnu/8.3.0/include/g++-v8/ext/new_allocator.h:86
#7  0x000055555560a6a0 in CryptoPP::HashTransformation::TruncatedVerify (this=0x7fffffffc158, digest=0x555555a9bcf0 "\036\224\341\217x\362G\225\vp\251γ\277Ǝ ", digestLength=16) at cryptlib.cpp:410
#8  0x000055555568e1ad in CryptoPP::HashVerificationFilter::LastPut (this=0x7fffffffc400, inString=0x555555a9bcf0 "\036\224\341\217x\362G\225\vp\251γ\277Ǝ ", length=16) at filters.cpp:933
#9  0x000055555568fdfe in CryptoPP::FilterWithBufferedInput::PutMaybeModifiable (this=0x7fffffffc400, inString=<optimized out>, length=<optimized out>, messageEnd=-1, blocking=<optimized out>, modifiable=<optimized out>) at filters.cpp:431
#10 0x000055555568fdfe in CryptoPP::FilterWithBufferedInput::PutMaybeModifiable (this=0x7fffffffc370, inString=<optimized out>, length=<optimized out>, messageEnd=-1, blocking=<optimized out>, modifiable=<optimized out>) at filters.cpp:431
#11 0x0000555555690ee9 in CryptoPP::FilterWithBufferedInput::Put2 (blocking=true, messageEnd=-1, length=0, inString=0x0, this=0x7fffffffc370) at filters.h:331
#12 CryptoPP::AuthenticatedDecryptionFilter::ChannelPut2 (this=0x7fffffffc370, channel=..., begin=0x0, length=0, messageEnd=-1, blocking=<optimized out>) at filters.cpp:1029
#13 0x0000555555609416 in CryptoPP::BufferedTransformation::ChannelMessageEnd (blocking=true, propagation=<optimized out>, channel=..., this=0x5555559f84b0) at cryptlib.h:2156
#14 CryptoPP::BufferedTransformation::TransferMessagesTo2 (blocking=true, channel=..., messageCount=@0x7fffffffbf7c: 0, target=..., this=0x7fffffffc300) at cryptlib.cpp:655
#15 CryptoPP::BufferedTransformation::TransferMessagesTo2 (this=0x7fffffffc300, target=..., messageCount=@0x7fffffffbf7c: 0, channel=..., blocking=<optimized out>) at cryptlib.cpp:635
#16 0x0000555555609629 in CryptoPP::BufferedTransformation::TransferAllTo2 (blocking=<optimized out>, channel=..., target=..., this=<optimized out>) at cryptlib.cpp:696
#17 CryptoPP::BufferedTransformation::TransferAllTo2 (this=0x7fffffffc300, target=..., channel=..., blocking=<optimized out>) at cryptlib.cpp:684

noloader added a commit that referenced this issue Dec 31, 2019
Thanks to @Nyk72 and @LiKao on GitHub for diagnosing and fixing the issue
noloader added a commit to noloader/cryptopp that referenced this issue Dec 31, 2019
@noloader
Copy link
Collaborator

noloader commented Dec 31, 2019

@Nyk72, @LiKao,

I checked in the fix provided by @Nyk72 in Item 9:

class CRYPTOPP_DLL AuthenticatedDecryptionFilter
{
    ...
    size_t ChannelPutModifiable2(const std::string &channel, byte *begin, size_t length, int messageEnd, bool blocking)
        { return ChannelPut2(channel, begin, length, messageEnd, blocking); }
};

Also see Commit ff110c6e183e.

Thanks for the help, and sorry about the delay.

@noloader noloader added the Bug label Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants