search

MHumm / DelphiEncryptionCompendium

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

Fix auth tag issues when using GCM cipher with stream-based methods #48

Closed denovosoftware closed 2 years ago

denovosoftware commented 2 years ago

Incorrect auth tag is generated by DecodeStream method due to buffer reuse. Also addresses issue with auth tags for empty data streams. Solution is to update logic similar to logic in DecodeBytes methods.

MHumm commented 2 years ago

Thanks for providing this pull request. I try to find the time to review it in the near future!

MHumm commented 2 years ago

I had a look at your commits now. While most things look clear enough to me theres one issue: there is no TestTDECGCM.TestDecodeStream method in Unit Tests/Tests/TestDECCipherModesGCM.pas, at least not in the current development or master branch.

There is no // FIXME comment at all in the whole project group at the moment. How to accept the first two commits but leaving out the other one? Or what would happen if I accept a pull request which wants to modify source which is not there?

Ah, sorry. I'm not too accustomed with pull requests yet and overlooked the commit adding the TestDecodeStream method.

MHumm commented 2 years ago

I merged your pull request but: your two new unit tests TestDecodeStream and TestEncodeStream do crash now with access violations and invalid pointer operations etc. It looks like the calls with a 0 byte Size and empty Source and Dest params in DecodeGCM(Source, Dest: TBytes; Size: Integer) are the problem.

Since this version is online noew in development and master branch: could you please have a look and try to provide a fix as soon as time permits?

denovosoftware commented 2 years ago

Hi, sorry about that. Pull request #49 should fix those AV issues.