Kleines Weihnachtsgeschenk von der DEC

[TurboMagic]

Hallo,

so, Weihnachten naht mit großen Schritten und ich hab's endlich mal wieder geschafft was an der DEC
(Delphi Encryption Compendium) zu machen, u.a. getrieben durch einen Beitrag unseres Schweizer MVP Kollegen Christoph.

Das "Geschenk" ist noch im Development Zweig hier:
https://github.com/MHumm/DelphiEncry...ee/development

Was ist es?

• Unterstützung des PKCS#7 Paddings bei den Verschlüsselungsalgorithmen (das schließt auch PKCS#5 ein)
• Erweiterte Cipher_FMX Demo
• Erweiterte VCL Progress Demo
• TFormat_UTF8 und TFormat_UTF16 (letzteres ist nur ein Alias für TFormat_BigEndian16)
• In anderen Pull Requests beigesteuerte Fehlerkorrekturen z. B. in TDECCipher.DecodeRawByteString

Die Doku wurde auch gleich erweitert.
Achtung: es ist bekannt, dass derzeit nicht alle Unit Tests fehlerfrei durchlaufen.
Das betrifft jedoch nur den GCM und den Keccak Algorithmus.Ursachen muss ich noch suchen. Beim Keccak
liegt es an irgendwelchen Stringverarbeitungen und der Fehler ist im Unit Test, sehr wahrscheinlich nicht im Algorithmus,
da andere Tests für diesen bestanden werden.

Was ich noch super fände wäre, wenn mal jemand der FPC installiert hat prüft, ob das
auch unter FPC noch alles compiliert.

So, jetzt aber frohe Weihnachten und einen guten Rutsch ins Jahr 2025!

[Kas Ob.]

Happy holidays to you too and too everyone !

Nice and thank you for work and contribution!

One very small thing i saw here
https://github.com/MHumm/DelphiEncry...as#L1451-L1453
This should be exception raised in consistent with the used style and coding, if the length is 0 this PKCS7 padding is absent and broken.


on side note: these exception in general, not only in PKCS7 or any specific unit
raise EDECCipherException.Create('xxxxxxxxxx'); Can affect performance due to code for string handling inserted by the compiler, although using TBytes itself does that too, so the following is not so important performance wise in this case per se, but..
replacing these calls with something centralized would be contain the usage of the strings and open the door for localization or at least just translating, it is cosmetic suggestion mostly.
example
as procedure DECExceptionRaise(DECEXCEPT_CIPHER_PADDING_PKCS7_INVALID;...); And this could be applied to other exception across the library to make it useful, see these
https://github.com/MHumm/DelphiEncry...Modes.pas#L483

Delphi-Quellcode:

resourcestring
  sInvalidMessageLength = 'Message length for mode %0:s must be a multiple of %1:d bytes';
  sInvalidBlockSize     = 'Block size must be %0:d bit for selected mode %1:s';
  sInvalidModeForMethod = 'Invalid mode for this method. Mode must be %0:s';

.....
procedure TDECCipherModes.SetDataToAuthenticate(const Value: TBytes);
begin
  if (FMode = cmGCM) then
    FGCM.DataToAuthenticate := Value
  else
    raise EDECCipherException.CreateResFmt(@sInvalidModeForMethod, ['cmGCM']);
end;

[Kas Ob.]

More thinking about this RemovePKCS7Padding i have a suggestion, and i am really sorry that i am doing this here not on GitHub

Refactor RemovePKCS7Padding where it could be split into a new public function HasValidPKCS7 return boolean and RemovePKCS7Padding can call it, this is more useful.

[Christoph Schneider]

I agree, in case of empty chiffre we should throw an exception.

To your second proposal. What is the workflow? Do you really have a chiffre where you dont know, if PKCS#7 padding was added or not?
The problem is from my point of view, if we would offer such check method, we run decrypt two times, one in this check and one after it for getting the result. Than an option for a facultative padding would be better but result in a more complex interface.

[Kas Ob.]

Zitat von Christoph Schneider:

To your second proposal. What is the workflow? Do you really have a chiffre where you dont know, if PKCS#7 padding was added or not?

No, i don't have specific logic where we are in doubt if there is a PKCS7 padding or not, this should not be the case.

Zitat von Christoph Schneider:

The problem is from my point of view, if we would offer such check method, we run decrypt two times, one in this check and one after it for getting the result. Than an option for a facultative padding would be better but result in a more complex interface.

My suggestion is small cosmetic, the refactor i suggested is based on the fact PKCS7 padding is used in few other places not just symmetric encryption or in PKCS7 CMS itself, this padding is the most used in asymmetric encryption and signing, also it is de facto the most used data padding scheme in general, so extracting the check for validity will be helpful, instead of repeating that loop, we can have a tidy small check for validity and correctness, .... well naming could be better like HasValidPKCS7Padding (i missed typed it as HasValidPKCS7).

[Kas Ob.]

One more thing i missed up there,
Signing and hashing in many case does need padding and in this case we don't need to extract, we need the correctness check only.
Have a look at this question and answers
https://crypto.stackexchange.com/que...y-need-padding

Fun stuff

[TurboMagic]

Would you like to suggest some code for the implementation of this check?

[Kas Ob.]

Zitat von TurboMagic:

Would you like to suggest some code for the implementation of this check?

Just looked again and saw that this padding handling is declared as private, and this makes things weird to begin with, PKCS7 padding scheme might be its own class as auxiliary.

Looking more.. i can't find more direct and correct way to add this class except in DECFormat, but all classes there are TFormat_XXX so it might be TFormat_PKCS7Padder ( had to check for padder as word, and it does exist), well it is not up to me to decide naming.

The checker is exactly as you did it, the last byte value is repeated "value" times, it should not be 0 and obviously can't be more than 255, so implementing it is trivial, but with an important thing in mind:

There is no scheme or standard in my recollection that demand a limitation, meaning PKCS7 padding when used for symmetric encryption then the target padded size must be satisfy
n*blocksize; n > 0
and this in contrary to the most usual usage of pad to one block size, so for AES the minimum padding is 1 byte to satisfy the full length to a multiply of 16 bytes, but it could be 32, 48... and will still correct and valid,(many stick to pad AES encryption to one block), the scheme by its design when de-padding (removing the padding) will remove all the residue, although not PKCS7 but in TOR as example padding done up to 512 or a multiple of 512 byte for obfuscation, same can be used with PKCS7 you can choose to pad to 128 bytes so all entries in db no matter how long names or addresses are encrypted as the same size.

back to suggested class, in case you decided to add it as separate class, it should accept block_size as optional input (parameter) along the data, so
block_size =0 then the checking or the loop will only make sure the all last trailing k bytes have the value k and k >0
block_size >0 then the checking will be the same as above with extra check that length of the data length is equal to block_size * m, because in this case the size must be padded (aligned)

that is it, removing the padding by truncating the data length by k, padding is the same, so for padding, it could have the block size parameter along with requested length, the actual padding must be more or equal to than the requested length and equal to multiple block size, padding length can't be more than 255 bytes.

[TurboMagic]

I do not really like the idea to implement this padding as a new format class.
While padding could be used outside block ciphers it would be really uncommon
in my eyes. Using the format classes outside that scenario is not so uncommon.

PKCS#7 btw. is defined in RFC 5652, which can be found here:
https://tools.ietf.org/html/rfc5652

[Kas Ob.]

Zitat von TurboMagic:

I do not really like the idea to implement this padding as a new format class.
While padding could be used outside block ciphers it would be really uncommon
in my eyes. Using the format classes outside that scenario is not so uncommon.

PKCS#7 btw. is defined in RFC 5652, which can be found here:
https://tools.ietf.org/html/rfc5652

Well that is up to you, but on side note most well known libraries do use padding classes as external implementation away from any specific implementation, example, the famous BouncyCastle
https://downloads.bouncycastle.org/j...erPadding.html
https://downloads.bouncycastle.org/j...S7Padding.html

But again, if DEC doesn't do any padding and currently is internal for symmetric encryption then no need to expand on this, and the only missing feature to to pad to more than one block, as i explained above, this nice feature to harden the encryption by obfuscation the length, but this is up top to you to introduce.