search

MHumm / DelphiEncryptionCompendium

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

Version 6.4 broke 2DES output #34

Closed pierangelodalben closed 3 years ago

pierangelodalben commented 3 years ago

Decrypting data and encrypting back the result of first step should output the starting value. This has been broken with lastest version, using class TCipher_2DES. Not tested all the des function, possible bug also in other des classes due the new refactoring on the DES unit.

Test output with version 6.4

DES/CBC (DEC 6.4)----------------------------- Key : 0011223344556677 Data : B71FAAB72D3C4F50 Decrypt : 8FE5901009548428 Encrypt : B71FAAB72D3C4F50 Match : True

2DES/CBC (DEC 6.4)---------------------------- Key : 00112233445566778899AABBCCDDEEFF Data : B71FAAB72D3C4F50 Decrypt : 8FE5901009548428 Encrypt : 45C874C1880C21FB Match : >>>>> FALSE <<<<<

AES/CBC (DEC 6.4)----------------------------- Key : 00112233445566778899AABBCCDDEEFF Data : B71FAAB72D3C4F50B71FAAB72D3C4F50 Decrypt : 0899A8624B026B88E9238173EB5B5475 Encrypt : B71FAAB72D3C4F50B71FAAB72D3C4F50 Match : True

Test output with version 6.3

DES/CBC (DEC 6.3)----------------------------- Key : 0011223344556677 Data : B71FAAB72D3C4F50 Decrypt : 8FE5901009548428 Encrypt : B71FAAB72D3C4F50 Match : True

2DES/CBC (DEC 6.3)---------------------------- Key : 00112233445566778899AABBCCDDEEFF Data : B71FAAB72D3C4F50 Decrypt : 31C35143663E7A00 Encrypt : B71FAAB72D3C4F50 Match : True

AES/CBC (DEC 6.3)----------------------------- Key : 00112233445566778899AABBCCDDEEFF Data : B71FAAB72D3C4F50B71FAAB72D3C4F50 Decrypt : 0899A8624B026B88E9238173EB5B5475 Encrypt : B71FAAB72D3C4F50B71FAAB72D3C4F50 Match : True

MHumm commented 3 years ago

That sounds a bit strange, as there are unit tests for all ciphers implemented and they pass 100%. But: the tests do not exercise all cipher algorithms with all block chaining modes. The test data used is normally the same for encryption and decryption.

Your tests indicate that the failure happens between 6.3 and 6.4. I need to diff that one, but I don't remember a difference in that part of the code, but I'll have to check. Can you supply a minimal test application demoing the failure? That would help me investigating this.

pierangelodalben commented 3 years ago

Hi

I was using the same test app to compare your library with others library (example dot not) and noticed that with the latest version the 2DES computation has been broken. I'm attaching my test app: just set in the project file the path of DEC source.

I'm wrapping the call to DEC function though a wrapper class defined in the unit CryptoUtilsDec.

Thanks Pierangelo

Il giorno gio 11 nov 2021 alle ore 09:24 Markus @.***> ha scritto:

That sounds a bit strange, as there are unit tests for all ciphers implemented and they pass 100%. But: the tests do not exercise all cipher algorithms with all block chaining modes. The test data used is normally the same for encryption and decryption.

Your tests indicate that the failure happens between 6.3 and 6.4. I need to diff that one, but I don't remember a difference in that part of the code, but I'll have to check. Can you supply a minimal test application demoing the failure? That would help me investigating this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MHumm/DelphiEncryptionCompendium/issues/34#issuecomment-966086066, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJDWNPFLGGCDOY5IG2O4OLULN4THANCNFSM5HZ3KZXQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

MHumm commented 3 years ago

Either I'm too stupid or I don't know, but I don't see any attachment with your code. If I'd get that sample I'd try to have a look at it as soon as time permits.

pierangelodalben commented 3 years ago

There is an attached rar on the previous email If you prefer I can resend to another email address. Let me know.

In data 11 novembre 2021 09:47:46 Pierangelo Dal Ben @.***> ha scritto:

Hi

I was using the same test app to compare your library with others library (example dot not) and noticed that with the latest version the 2DES computation has been broken. I'm attaching my test app: just set in the project file the path of DEC source.

I'm wrapping the call to DEC function though a wrapper class defined in the unit CryptoUtilsDec.

Thanks Pierangelo

Il giorno gio 11 nov 2021 alle ore 09:24 Markus @.***> ha scritto:

That sounds a bit strange, as there are unit tests for all ciphers implemented and they pass 100%. But: the tests do not exercise all cipher algorithms with all block chaining modes. The test data used is normally the same for encryption and decryption. Your tests indicate that the failure happens between 6.3 and 6.4. I need to diff that one, but I don't remember a difference in that part of the code, but I'll have to check. Can you supply a minimal test application demoing the failure? That would help me investigating this. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

pierangelodalben commented 3 years ago

dec_test.zip

please you can also check some online website to verify that TDES value calculated from version 6.4 are wrong, while was ok on prev. version.

Example http://tripledes.online-domain-tools.com/

Key : 00112233445566778899AABBCCDDEEFF IV : 0000000000000000 Data : B71FAAB72D3C4F50

A = Decrypt(Data) = 31C35143663E7A00 B = Encrypt(A) = B71FAAB72D3C4F50

Version 6.4 output diffent values

MHumm commented 3 years ago

This link works. I'll put all these issues on my ToDo list and try to find time to look after them.

MHumm commented 3 years ago

I started to look at your test application. Thanks for providing it, but it seems to have a flaw itself ;-) It talks about 3DES but creates a 2DES instance...

MHumm commented 3 years ago

Enough programming for today. One thing I saw which is different between 6.3 and 6.4 is, that I created a common base class for all DES related ciphers where I extracted one method into so that the 2DES etc. classes do not need to inherit from 1DES. Could it be related to that one?

pierangelodalben commented 3 years ago

Yes my test app use 2des because that was the scope my test and this is still a triple des with 16 byte key and 8 byte data.

I don't have investigated on your code but it have verified that the previous version computation was correct and verified with other libraries and resources on-line like the one reported in previous email. I don't know where the code has been broken but as 1 des is correct I think has been broken in the derived classes where the triple des is applied.

In data 14 novembre 2021 17:56:18 Markus @.***> ha scritto:

Enough programming for today. One thing I saw which is different between 6.3 and 6.4 is, that I created a common base class for all DES related ciphers where I extracted one method into so that the 2DES etc. classes do not need to inherit from 1DES. Could it be related to that one?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

MHumm commented 3 years ago

I fixed it in development branch now but would like to keep the issue open until I provide this fix in an official release in the near future. I'd also like to add a unit test for this to make sure we do not regress in the future. You might help me getting your test data in the format required by the unit tests (I don't mean to convert it to this "escape format" from the test vectors I inherited when I took the project over).

MHumm commented 3 years ago

I added your test data to the unit tests for 2DES now, as this should ensure we do not regress in the future if further changes might be necessary. I hope to have an official 6.4.1 release soon.

pierangelodalben commented 3 years ago

Good thanks

In data 20 novembre 2021 17:43:55 Markus @.***> ha scritto:

I added your test data to the unit tests for 2DES now, as this should ensure we do not regress in the future if further changes might be necessary. I hope to have an official 6.4.1 release soon. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

MHumm commented 3 years ago

Fixed in the just released 6.4.1 release so I close this one.