Add TurboSHAKE XOF functions.

[MarekKnapek]

TurboSHAKE128 and TurboSHAKE256 as described in https://keccak.team/files/TurboSHAKE.pdf.

#691

Checklist

• documentation is added or updated
• tests are added or updated

[levitte]

That PDF refers to https://datatracker.ietf.org/doc/draft-irtf-cfrg-kangarootwelve/, which also describes the algo, and provides test vectors. Might I suggest adding some tests, based on those test vectors?

[sjaeckel]

And TBH I'd prefer to have this only merged once the RFC is finalized.
I've been burned too often by standards changing in some future version before release.

[MarekKnapek]

I updated my branch, I added some unit tests. Regarding the standard being in draft stage. It seems to be draft since 2019-08-06 until 2025-02-21 and continuing to be draft until today, not sure what changed in the meantime. How would you like to continue? I have few ideas:

• Keep the TurboSHAKE functions in the library, but behind some #ifdef that clearly communicates to the user that this feature is "beta" or "standard not finished yet" and that it might change in the future or something like that.
• Do not add this functionality to your product until the RFC standardization is done. (You seem to like this approach.)
• Add this functionality to your product and if it breaks in the future, say to your customers "bad luck" your problem using unfinished standard, not mine. (You seem to don't like this approach.)
• Any more ideas?

[levitte]

I could go with having this as an unstable feature, only to be enabled on demand... especially if libtomcrypt already have infrastructure for that sort of thing (er, I forget if it has). If that ends up too hard, I'd go with keeping this PR as is until the standard is finalized.

I was a bit involved in the addition of TLS 1.3 in OpenSSL, and we did hold off a release with that until the RFC was actually published... and we did see some changes very late in the RFC process. So that is to say, there's quite some precedence to @sjaeckel's worries.

[sjaeckel]

Thanks for adding the tests and thanks for your input!

So a long lived PR this will be, until cfrg comes to a conclusion ...

[sjaeckel]

So a long lived PR this will be, until cfrg comes to a conclusion ...

That was faster than expected - https://datatracker.ietf.org/doc/rfc9861/

[MarekKnapek]

Nice. I just checked the test vectors with the RFC. They are the same. I updated my branch to reflect the RFC number.

[MarekKnapek]

Hi @sjaeckel, do you like this feature and this pull request? Are you willing to consider including it in the LibTomCrypt project? Should I improve this code or commit message in some way? Should I add the KT12 hash also?

[sjaeckel]

Thanks again for the PR :)

do you like this feature and this pull request? Are you willing to consider including it in the LibTomCrypt project? Should I add the KT12 hash also?

yes to all those :) and thanks already in advance!

Should I improve this code or commit message in some way?

Not sure about the code ... The newly added functions are 99% copies, would it maybe make sense to add generic implementations and call them? (or did I miss some other change in the copies?)

static LTC_INLINE void s_keccak_f(ulong64 s[25], int max_rounds, int rc_offset)
{
   /* ... */
}

static void s_keccakf(ulong64 s[25])
{
   s_keccak_f(s, SHA3_KECCAK_ROUNDS, 0);
}
static void s_keccak_turbo_f(ulong64 s[25])
{
   s_keccak_f(s, SHA3_KECCAK_TURBO_ROUNDS, SHA3_KECCAK_TURBO_RC_OFFSET);
}

typedef void (*process_fn)(ulong64 s[25]);
static LTC_INLINE int s_sha3_process(hash_state *md, const unsigned char *in, unsigned long inlen, process_fn proc_f)
{
   /* ... */
}

int sha3_process(hash_state *md, const unsigned char *in, unsigned long inlen)
{
   return s_sha3_process(md, in, inlen, s_keccakf);
}
int sha3_turbo_process(hash_state *md, const unsigned char *in, unsigned long inlen)
{
   return s_sha3_process(md, in, inlen, s_keccak_turbo_f);
}

[MarekKnapek]

Yes, the functions are almost identical. I'm not sure about your / the project's philosophy. I have two ideas. One is to make two separate "main" functions having all the meat-and-potatoes in them. Second option is to have single function with run-time parameter switching between Shake and TurboShake. There are pros and cons for both options.

Two separate functions. As it is today in my pull request. Pros: If the user wants only one functionality Shake or TurboShake, they are not bothered with the other one. Possibly making the code a bit faster (no need to do run-time if). Possibly making the code smaller (not including the other functionality). I think this is the philosophy of LibTomCrypt - include only what you use and don't pay for what you don't use. Implementation wise: There is copy & pasta so this could cause confusion for anybody reading the code. There is high maintainability cost of having duplicate code. I could use C++ templates to instantiate two versions of the function having only single source code and switching between them at compile-time (think of template dispatch or constexpr-if). But this project is not C++, in C I could use macros, x-macros or some similar shenanigans. But I guess this would not fly.

Single function with run-time parameter. Pros: Simpler code, better understandability, better maintainability, smaller code for those that need both. Cons: Bigger code, slower code for those that need only single one.

Should I consolidate the two functions with added boolean parameter switching between the two variants?

// Edit: Instead of boolean parameter, use number of rounds and round starting offset as you suggested.

[sjaeckel]

I guess you're missing something here.

But this project is not C++, in C I could use macros, x-macros or some similar shenanigans. But I guess this would not fly.

Why so complicated? 🙂

Single function with run-time parameter. Pros: Simpler code, better understandability, better maintainability, smaller code for those that need both. Cons: Bigger code, slower code for those that need only single one.

Simply use an optionally inline function, as suggested. It combines the best of both worlds. The code is simple and the same, so we have less LOC. If someone cares about compiled code size they disable the inline function and get a shared implementation. If they want speed they keep inline enabled.

I took the liberty to do those changes and squashed it with the other two commits together at https://github.com/libtom/libtomcrypt/tree/turboshake (please check, maybe you want to keep some part of the other commit messages, I simply only kept the first one)

[sjaeckel]

Minor nitpick I just saw.

sha3_shake_turbo_process
sha3_shake_turbo_done
sha3_turbo_shake_test

I'd propose to rename them to:

turbo_shake_process
turbo_shake_done
turbo_shake_test

since the official name is TurboSHAKE.

Also there's
a) no need for a separate sha3_turbo_process
b) turbo_shake_init is missing
c) please add the option to separately enable it via LTC_TURBO_SHAKE, similar to LTC_KECCAK

And please squash everything together in a single commit :)