MHumm / DelphiEncryptionCompendium

Cryptographic library for Embarcadero Delphi and potentially for FPC as well
Apache License 2.0
257 stars 66 forks source link

Incorrect auth tag generated for GCM mode when feeding data using small chunks #52

Open denovosoftware opened 1 year ago

denovosoftware commented 1 year ago

Describe the bug Incorrect auth tag generated for GCM mode when feeding data using small chunks (at least when sized to cipher's block size). Using such small chunks appear to conform with DEC's documentation.

To Reproduce

  1. Apply commit af94b8b on top of latest master
  2. Add new test case to test chunked stream encoding, which uses existing test data set but feeds data in a smaller chunks (cipher's blocks sized) instead of whole stream at once
    procedure TestTDECGCM.TestEncodeStreamChunked;
    begin
    // Use cipher block size as max chunk size
    DoTestEncodeStream_LoadAndTestCAVSData(
    Max(FCipherAES.Context.BlockSize, FCipherAES.Context.BufferSize));
    end;
  3. Run test GCM suite and get auth tag failure in new test case

Expected and actual behavior Test should pass, proving that expected auth tag is generated when streaming data in small chunks.

MHumm commented 1 year ago

Have you checked that master is as new as development branch? Can you modify the code to add so that it can be added to current version of development branch please? If no change is necessary please tell me.

denovosoftware commented 1 year ago

I was able to apply patch on top of development branch, no conflicts. New test is still failing.

denovosoftware commented 1 year ago

If you just want to reproduce the issue without the patch and new test, then simplest way would be to modify existing TestEncodeStream to use chunking when calling EncodeStream:

    dataLeftToEncode := ptbStream.Size;
    curChunkSize := dataLeftToEncode;
    repeat
      curChunkSize := Min(dataLeftToEncode, FCipherAES.Context.BlockSize);
      FCipherAES.EncodeStream(ptbStream, ctbStream, curChunkSize);
      Dec(dataLeftToEncode, curChunkSize);
    until (dataLeftToEncode = 0);
MHumm commented 1 year ago

I applied the pull request #51 for the other issue and that passes the unit tests. I added the test suggested above now: procedure TestTDECGCM.TestEncodeStreamChunked; but this failes the unit test. Reading what you wrote above the pull request should have fixed that as well.

Can you please check what I did in development branch? So we can work on getting this one properly fixed as well.

MHumm commented 1 year ago

Did you find the time to test my version already?

denovosoftware commented 1 year ago

Hi,

Just to clarify, there were two separate issues: one was fixed in PR #51 and this one is another GCM-related issue, that was discovered while I was working on #51. That is why proposed test case was dependent on that PR, but #51 only provided fix for one of the issues.

Unfortunately, I don’t have a solution for this other issue, only the test case to reproduce it.

MHumm commented 1 year ago

I started to investigate but I'm not far yet. It is this plain text from the 128 bit test data which fails for me: 3feef98a976a1bd634f364ac428bb59cd51fb159ec1789946918dbd50ea6c9d594a3a31a5269b0da6936c29d063a5fa2cc8a1c. It fails if I specify a buffer size of < 51 byte. The last byte returned is wrong, the remaining ones seem ok. It is the output of CipherProc(Buffer[0], outBuffer[0], Bytes); in CipherProc(Buffer[0], outBuffer[0], Bytes); which is wrong. So much for today. Any help on this is appreciated.

danielmarschall commented 6 months ago

I think I have found the reason.

The vector that fails for me is from gcmEncryptExtIV128.rsp

[Keylen = 128]
[IVlen = 96]
[PTlen = 256]
[AADlen = 0]
[Taglen = 128]

Count = 0
Key = 9971071059abc009e4f2bd69869db338
IV = 07a9a95ea3821e9c13c63251
PT = f54bc3501fed4f6f6dfb5ea80106df0bd836e6826225b75c0222f6e859b35983
AAD = 
CT = 0556c159f84ef36cb1602b4526b12009c775611bffb64dc0d9ca9297cd2c6a01
Tag = 7870d9117f54811a346970f1de090c41

Error happens in TestTDECGCM.DoTestEncodeStream_TestSingleSet, with aSetIndex=105 and aDataIndex=0.

The chunksize is 16 byte, the stream size is 32 byte. Therefore, EncodeStream is called two times.

Just for testing, I added a 3rd call with empty data: FCipherAES.EncodeStream(ptbStream, ctbStream, 0);

and surprise: FCipherAES.CalculatedAuthenticationResult ~is empty~ is different !!!

Theory: FCipherAES.CalculatedAuthenticationResult only contains the Authentication Result of the LAST call of EncodeStream, not all calls combined.

Looking at TGCM.EncodeGCM :

  AuthTag := XOR_T128(CalcGaloisHash(DataToAuthenticate, Dest, Size), FE_K_Y0);
  Setlength(FCalcAuthenticationTag, FCalcAuthenticationTagLength);
  Move(AuthTag[0], FCalcAuthenticationTag[0], FCalcAuthenticationTagLength);

It appears like FCalcAuthenticationTag is dependent on DataToAuthenticate, Dest, Size, but not on any previously processed data? I might be wrong, because I didn't investigate how CalcGaloisHash works, if it might be considering previously processed data.