Kyber optimizations

[randombit]

In aggregate these improve Kyber performance between 1.5x and 2.5x

cc @boricm @reneme

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (8460ca2) 88.12% compared to head (1123636) 88.13%.

❗ Current head 1123636 differs from pull request most recent head a7d7457. Consider uploading reports for the commit a7d7457 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3387   +/-   ##
=======================================
  Coverage   88.12%   88.13%           
=======================================
  Files         617      616    -1     
  Lines       70331    70303   -28     
  Branches     6985     6985           
=======================================
- Hits        61978    61960   -18     
+ Misses       5424     5401   -23     
- Partials     2929     2942   +13     

Impacted Files	Coverage Δ
src/lib/pubkey/kyber/kyber_common/kyber.cpp	96.96% <100.00%> (-0.17%)	⬇️
src/tests/test_kyber.cpp	95.37% <100.00%> (ø)

... and 14 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[reneme]

Nice! Thanks for taking this on. I have added a few minor suggestions.

Out of curiosity: Did you go through the code looking for optimization opportunities or was this the result of some profiling?

[reneme]

Side-note: Maybe a Hash_Function::process() with a templated output container would be great to avoid the copy. Similar to the new RandomNumberGenerator::random_vec():

botan/src/lib/rng/rng.h

Lines 202 to 212 in c810e6c

	template<typename T = secure_vector<uint8_t>>
	requires(concepts::contiguous_container<T> &&
	concepts::resizable_container<T> &&
	concepts::default_initializable<T> &&
	std::same_as<typename T::value_type, uint8_t>)
	T random_vec(size_t bytes)
	{
	T result;
	random_vec(result, bytes);
	return result;
	}

[randombit]

Yes I definitely had that in mind for a follow up

[randombit]

Did you go through the code looking for optimization opportunities or was this the result of some profiling?

This was sparked by seeing this PR adding X25519+Kyber key exchange for TLS ziglang/zig#14920 where the author quotes numbers for Kyber which were much better than reported by botan speed. I think part of that is his machine is faster since X25519 numbers are also faster.

Using his X25519 numbers versus what I see locally as a scale, Kyber encryption is now as fast as the Zig implementation. However decryption is still significantly slower. I'm still confused on that point, because that PR quotes decryption as faster than encryption. But indcpa_enc is the vast majority of the cost for encrypt or decrypt, and decrypt additionally has to do indcpa_dec. Perhaps there are some additional decryption-side precomputations that we are missing.

Edit: I realized I did not actually answer your question. This work was all based on profiling with valgrind's callgrind tool plus qcachegrind for visualization.

[bwesterb]

because that PR quotes decryption as faster than encryption. But indcpa_enc is the vast majority of the cost for encrypt or decrypt, and decrypt additionally has to do indcpa_dec. Perhaps there are some additional decryption-side precomputations that we are missing.

Encapsulation recomputes H(pk) whereas decapsulation doesn't. Yeah, Kyber is so fast that such a short hash makes a difference.

[bwesterb]

You're missing this trick which the Zig implementation uses.

[reneme]

@randombit I took the liberty to adapt this to the now merged #3297 and #3294. Due to merge-conflicts I needed to force push.

[randombit]

Thanks @reneme

I read @bwesterb's paper on reducing the number of Barrett reductions in the inverse NTT, seems like a nice win, I'll take it up in another PR, maybe this weekend.