libtomcrypt icon indicating copy to clipboard operation
libtomcrypt copied to clipboard

Better documentation for CFB

Open MarekKnapek opened this issue 2 years ago • 1 comments

Prerequisites

  • [x] Checked the developer manual
  • [x] Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=repo%3Alibtom%2Flibtomcrypt
  • [x] Checked that your issue isn't related to TomsFastMath's limitation that PK operations can by default only be done with max. 2048bit keys

Description

If you look at the definition of CFB mode (for example in NIST SP 800-38A document), you will see that CFB mode can accept integer parameter (called s) telling the CFB (roughly) how many bits shall process at a time. The parameter is often incorporated into the mode's name (such as 1-bit CFB mode, the 8-bit CFB mode, the 64-bit CFB mode, or the 128-bit CFB mode).

The problem is, that libtomcrypt doesn't document which CFB variant it uses.

Therefore in this issue I suggest to improve libtomcrypt's documentation to tell its users that it always uses the "full width" version of CFB ("full-width" meaning 128-bit CFB for AES cipher (I didn't test with other ciphers, yet)). This might apply to other modes as well (I didn't test other modes, yet).

Steps to Reproduce

Write AES + CFB implementation from scratch (yes, I'm masochist reinventing wheel) and test its correctness against other crypto library (such as libtomcrypt) and discover that both libraries behave differently.

Version

Latest git head, develop branch, 1.17. Windows 10, x86 + x64, Visual Studio 2022.

Additional Information

It would be nice if libtomcrypt would implement the CFB s parameter, but this is not subject of this issue.

Best regards, Marek.

MarekKnapek avatar Apr 29 '22 07:04 MarekKnapek

Well, the documentation already states the following:

https://github.com/libtom/libtomcrypt/blob/06a81aeb227424182125363f7554fad5146d6d2a/doc/crypt.tex#L828

Looks like that isn't sufficient, maybe could require a bit more clarification.

Implementing the full spec would also be an option.

I'll leave this issue open until the doc is updated or the full spec is implemented, whichever comes first ;)

sjaeckel avatar Apr 29 '22 07:04 sjaeckel