Refactor: Have a common base for Kyber/Dilithium structures #4024

reneme · 2024-04-19T14:50:10Z

This is essentially a complete overhaul of the Kyber and Dilithium implementations aiming at as much shared code as possible between the two algorithms. The rough structure is outlined below. I split-up the changes into three commits, that could be separate pull requests, if needed.

Module: `pqcrystals`

Polynomial Structures
This is a templated base for the polynomials (as well as poly vectors/matrices) used in both implementations. The base implementation shares math where possible and is pluggable where necessary. It also exposes its domain (NTT or "normal") in the type system. Also, it makes some initial optimization effort (mostly memory layout), but more work could be done here.
Polynomial Encoding/Decoding
Both algorithms contain bit-packing algorithms to efficiently encode polynomial coefficients of various known value ranges. Both Kyber and Dilithium now share a generic and pluggable implementation of this.

Module: `kyber`

This now makes use of the pqcrystals module. The polynomial instantiation happens in kyber_polynomial.h (replacing the old kyber_structures.h). Most low-level "algorithms" (pseudo-codes of the spec) live in kyber_algos.cpp. The indcpa "K-PKE" scheme is implemented in kyber_keys.cpp and the cca2-secure wrapper can be found in kyber.cpp. Auxiliary stuff like constants and strong types are in other headers.

Note that this now contains almost no verbatim code from the reference implementation anymore.

Module: `dilithium`

Likewise, this uses the pqcrystals module (instantiating the polynomials in dilithium_polynomial.h (replacing the almost verbatim reference implementation code). The dilithium_algos.cpp is home to most low-level "algorithms" (as seen in the spec). dilithium.cpp houses the actual signature/verification algorihtms that bind it all together..

Care has been taken to have a consistent implementation structure between Kyber and Dilithium. Note, though, that Dilithium's flexibility might require some love before a clean integration of ML-KEM (similar to what happened to Kyber in #3887).

Benchmark

~~Despite not explicitly aiming to increase speed,~~ ./botan speed shows nice gains for Kyber and no losses for Dilithium. ~~It might be worth looking into why Kyber is so much faster, though. Feels suspicious.~~ Edit: The new implementation pre-computes and stores the expanded public matrix A. So as long as one re-uses the PK_KEM_Encryptor for multiple encapsulations, one saves a lot of invocations to the XOF. That explains. 😏

GCC 13.2 vs. Clang 18.1 on Linux (in WSL)

Algorithm	This PR (GCC)	Master (GCC)	This PR (Clang)	Master (Clang)
Dilithium-4x4-r3	4602 sign/sec	4332 sign/sec	5358 sign/sec	4681 sign/sec
Dilithium-4x4-r3	28913 verify/sec	31303 verify/sec	37226 verify/sec	35459 verify/sec
Dilithium-4x4-AES-r3	5050 sign/sec	4689 sign/sec	6363 sign/sec	5007 sign/sec
Dilithium-4x4-AES-r3	27824 verify/sec	31180 verify/sec	38452 verify/sec	34830 verify/sec
Dilithium-6x5-r3	2648 sign/sec	2695 sign/sec	2893 sign/sec	2830 sign/sec
Dilithium-6x5-r3	19250 verify/sec	20999 verify/sec	26201 verify/sec	24643 verify/sec
Dilithium-6x5-AES-r3	2848 sign/sec	2850 sign/sec	3398 sign/sec	3033 sign/sec
Dilithium-6x5-AES-r3	18503 verify/sec	21491 verify/sec	26030 verify/sec	24351 verify/sec
Dilithium-8x7-r3	2389 sign/sec	2251 sign/sec	2853 sign/sec	2543 sign/sec
Dilithium-8x7-r3	12768 verify/sec	14551 verify/sec	18622 verify/sec	17211 verify/sec
Dilithium-8x7-AES-r3	2758 sign/sec	2490 sign/sec	3044 sign/sec	2970 sign/sec
Dilithium-8x7-AES-r3	13856 verify/sec	13881 verify/sec	18468 verify/sec	17629 verify/sec

Algorithm	This PR (GCC)	Master (GCC)	This PR (Clang)	Master (Clang)
Kyber-512-r3	69864 encrypt/sec	39354 encrypt/sec	59956 encrypt/sec	38009 encrypt/sec
Kyber-512-r3	51452 decrypt/sec	32157 decrypt/sec	48114 decrypt/sec	32214 decrypt/sec
Kyber-512-90s-r3	81350 encrypt/sec	39878 encrypt/sec	75388 encrypt/sec	44527 encrypt/sec
Kyber-512-90s-r3	57531 decrypt/sec	32740 decrypt/sec	57515 decrypt/sec	37094 decrypt/sec
Kyber-768-r3	50542 encrypt/sec	22767 encrypt/sec	45523 encrypt/sec	21180 encrypt/sec
Kyber-768-r3	37306 decrypt/sec	19418 decrypt/sec	36505 decrypt/sec	18593 decrypt/sec
Kyber-768-90s-r3	48522 encrypt/sec	22167 encrypt/sec	59785 encrypt/sec	24793 encrypt/sec
Kyber-768-90s-r3	34923 decrypt/sec	19070 decrypt/sec	46596 decrypt/sec	21613 decrypt/sec
Kyber-1024-r3	35053 encrypt/sec	14597 encrypt/sec	40132 encrypt/sec	14884 encrypt/sec
Kyber-1024-r3	26609 decrypt/sec	12840 decrypt/sec	32397 decrypt/sec	13338 decrypt/sec
Kyber-1024-90s-r3	36312 encrypt/sec	14504 encrypt/sec	43981 encrypt/sec	16118 encrypt/sec
Kyber-1024-90s-r3	27100 decrypt/sec	13312 decrypt/sec	34729 decrypt/sec	14435 decrypt/sec

coveralls · 2024-04-22T07:06:38Z

coverage: 91.697% (-0.02%) from 91.716%
when pulling 3ecb3f9 on Rohde-Schwarz:refactor/crystals_april
into 92c2079 on randombit:master.

reneme

Below are a few highlights and potential conversion starters of things I saw along the way. :)

reneme · 2024-06-20T15:45:41Z

src/lib/pubkey/kyber/kyber_common/kyber_helpers.h

+ // This is a mitigation for a potential side channel called "KyberSlash".
+ // It implements the division by Q using a multiplication and a shift. Most
+ // compilers would generate similar code for such a division by a constant.
+ // Though, in some cases, compilers might use a variable-time int division,
+ // resulting in a potential side channel.
+ //
+ // See "Hacker's Delight" (Second Edition) by Henry S. Warren, Jr.
+ // Chapter 10-9 "Unsigned Division by Divisors >= 1"
+ auto divide_by_q = [](uint32_t n) -> unsigned_T {
+ static_assert(KyberConstants::Q == 3329);
+ BOTAN_DEBUG_ASSERT(n < (1 << 23));
+
+ // These constants work for all values that appear in Kyber with the
+ // greatest being 3328 * 2^11 + Q // 2 = 6,817,408 < 2**23 = 8,388,608.
+ constexpr uint64_t m = 2580335;
+ constexpr size_t p = 33;
+ return static_cast<unsigned_T>((n * m) >> p);
+ };
+
+ constexpr unsigned_T mask = (1 << d) - 1;
+ return divide_by_q((static_cast<uint32_t>(x) << d) + KyberConstants::Q / 2) & mask;


I've adapted the KyberSlash countermeasure to also work for the compression instances with d = 10 and d = 11. Not sure it's necessary but doesn't hurt either. I'd definitely be happy about some other sharp pair of eyes.

reneme · 2024-06-20T15:49:01Z

src/lib/pubkey/pqcrystals/pqcrystals.h

+ // TODO: perhaps secure vector
+ std::vector<T> m_coeffs_storage;


Polynomials store their coefficients as a std::vector<>. Some of those store secret values and should therefore use secure_vector. Also, it might be beneficial to add first-level support for Polynomials (and friends) as Strong types. (Most likely this would be another pull request, though.)

reneme · 2024-06-20T15:58:19Z

src/lib/pubkey/pqcrystals/pqcrystals_encoding.h

+namespace detail {
+
+constexpr auto as_byte_source(BufferSlicer& slicer) {
+ return [&](std::span<uint8_t> out) { slicer.copy_into(out); };
+}
+
+constexpr auto as_byte_source(Botan::XOF& xof) {
+ return [&](std::span<uint8_t> out) { xof.output(out); };
+}
+
+} // namespace detail
+
+template <typename T>
+concept byte_source =
+ requires(T& t) { requires std::invocable<decltype(detail::as_byte_source(t)), std::span<uint8_t>>; };


I feel like this is a top-level concept we might want to establish deeper in the library. Perhaps as an (internal) replacement for the DataSource class.

Namely: A thing that can provide a concrete number of bytes where I don't really know how many bytes will be needed. The source may be infinite or not.

Things that could fall in this abstraction may be:

the RandomNumberGenerator

a XOF

read from some buffer (think BufferSlicer)

from some I/O source (e.g. the network)

In Kyber/Dithithium this is used to either sample some random polynomial from a XOF or to unmarshal it from a bitpacked encoding.

Other use cases may be:

parsing in general (TLS, X.509 and friends)

data loading, e.g. like:
load_be<uint64_t>(rng) to get a random 64 bit integer. (See also Internal: load_be/le should accept a BufferSlicer #4068)

among other things

From a technical perspective, one might even implement it like shown here: As a C++20 concept that tries to build a thin wrapper around "the source". That way, new data sources could just hook themselves into this "helper" and decide for themselves to be a "data source" or not.

Cool idea 👍. I'm not sure if we want to commit to a pure concept construction, though. In some cases, it may be more convenient to have a class like DataSource. We could still extend the DataSource class to fulfill the respective concept if one wants to juggle with templates.

reneme · 2024-06-20T16:01:35Z

src/lib/pubkey/kyber/kyber_common/kyber.cpp

+ // TODO: Remove the need for the montgomery transformation
+ //
+ // -> When calculating A*s below, A is not in montgomery form, but s is. The
+ // operation uses fqmul internally, which performs a montgomery reduction.
+ auto A = montgomery(kyber_sample_matrix(rho, false /* not transposed */, mode));


This was different in the reference implementation. @FAlbertDev we did look into this a while ago, I feel we should take this up again and get to the bottom of it.

I've just investigated the usage of Montgomery in Kyber. First, a short recap on Montgomery:
The main observation is that every Montgomery multiplication $c=a \cdot b$ adds a factor of $R^{-1}$ to the result c where $R$ is the Montgomery factor (i.e., we compute $a \cdot b \cdot R^{-1}$) At some point we need to multiply $R$ to neutralize this $R^{-1}$ factor. Therefore, we could multiply $R$ to either $a$ or $b$. However, we can also multiply $R$ to the result $c$.
In our Kyber and Dilithium code, the following operations add or remove the Montgomery factor.

the function montgomery adds a factor $R$

the function inv_ntt adds a factor $R$. Note that ntt doesn't add a Montgomery factor.

As already said, a Montgomery multiplication adds a factor $R^{-1}$ to each poly, vector, or matrix element. This holds for all multiplication types, i.e., polynomial-polynomial, vector-vector, matrix-vector multiplication, etc.

For encryption and decryption, this setup works well since there is always one multiplication (adding factor $R^{-1}$) followed by inv_ntt (adding factor $R$). This does not work here since we store the value t in NTT domain and, therefore, do not call inv_ntt. We have a factor of $R^{-1}$ to eliminate. Currently, we do that by adding the factor $R$ to the matrix beforehand using montgomery. Instead, I propose to add this factor to t after the multiplication. The fewer elements multiplied by $R$, the faster it should be.

auto A = kyber_sample_matrix(rho, false /* not transposed */, mode); auto s = ntt(ps.sample_polynomial_vector_cbd_eta1()); const auto e = ntt(ps.sample_polynomial_vector_cbd_eta1()); auto t = montgomery(A * s); t += e; t.reduce(); // End Algorithm 12 ---------------------------

Nice! Thanks for double-checking and the detailed write-up! It would probably be really nice to track the montgomery parameter in the type system as well, but it does feel like overkill.

Also: by applying montgomery() to the t vector, we can remove the overload for PolynomialMatrix altogether. :)

reneme · 2024-06-20T16:04:58Z

src/lib/pubkey/kyber/kyber_common/kyber_types.h

+using KyberPolyNTT = Botan::CRYSTALS::Polynomial<KyberPolyTraits, Botan::CRYSTALS::Domain::NTT>;
+using KyberPolyVecNTT = Botan::CRYSTALS::PolynomialVector<KyberPolyTraits, Botan::CRYSTALS::Domain::NTT>;
+using KyberPolyMat = Botan::CRYSTALS::PolynomialMatrix<KyberPolyTraits>;
+
+using KyberPoly = Botan::CRYSTALS::Polynomial<KyberPolyTraits, Botan::CRYSTALS::Domain::Normal>;
+using KyberPolyVec = Botan::CRYSTALS::PolynomialVector<KyberPolyTraits, Botan::CRYSTALS::Domain::Normal>;


As mentioned before: it would be good if these could be the basis for strong types to disambiguate them in interfaces E.g. to replace the multiple poly_pack_t0, ..._t1, ... methods in dilithium_algos.cpp with an overloaded poly_pack() that just "does the right thing" ™ based on the strong type.

src/lib/pubkey/pqcrystals/pqcrystals_helpers.h

src/lib/xof/xof.h

reneme · 2024-06-20T16:13:28Z

src/lib/pubkey/dilithium/dilithium_common/dilithium_constants.h

+ enum DilithiumTau : uint32_t { _39 = 39, _49 = 49, _60 = 60 };
+
+ enum DilithiumLambda : uint32_t { _128 = 128, _192 = 192, _256 = 256 };
+
+ enum DilithiumGamma1 : uint32_t { ToThe17th = (1 << 17), ToThe19th = (1 << 19) };
+
+ enum DilithiumGamma2 : uint32_t { Qminus1DevidedBy88 = (Q - 1) / 88, Qminus1DevidedBy32 = (Q - 1) / 32 };
+
+ enum DilithiumEta : uint32_t { _2 = 2, _4 = 4 };
+
+ enum DilithiumBeta : uint32_t { _78 = 78, _196 = 196, _120 = 120 };
+
+ enum DilithiumOmega : uint32_t { _80 = 80, _55 = 55, _75 = 75 };


Dilithium has many integer constants that change depending on the instantiation of the algorithm. I found it worthwhile to restrict them with explicit enums. I'm okay with this concept, also given the restricted (algorithm-internal) scope. But I feel there must be a better way for this. :)

I also like this idea. While the implied switch-cases are annoying, they guarantee we do not forget a value.

reneme · 2024-06-20T16:15:39Z

src/lib/pubkey/dilithium/dilithium_common/dilithium_algos.cpp

+ dilithium_poly_pack_t0(p, stuffer);
+ }
+
+ BOTAN_ASSERT_NOMSG(stuffer.full());


Those became ubiquitous and at this point I do start wondering whether we should just put that in the destructors of Stuffer and Slicer (perhaps explicitly disengagable). What do y'all think?

Sounds good to me.

I'll do that after 3.5.0 is cut, to avoid unwanted surprises.

Most notably, this is an abstract implementation to handle CRYSTALS polynomials, poly vectors and poly matrices. Also, a generic implementation for bit-packed encoding/decoding of coefficients useful for both Kyber and Dilithium. Co-Authored-By: Fabian Albert <[email protected]>

FAlbertDev

I've only looked into dilithium and the crystals module so far. The other sections (including Kyber) will come tomorrow.
For algorithms of this high complexity, the new structure looks very well-organized and clear :)
I've only found some minor nits so far.
(Btw sorry if I also marked lots of removed code. The VSCode extension for GitHub caused this)

src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp

src/lib/pubkey/dilithium/dilithium_common/dilithium_algos.cpp

src/lib/pubkey/dilithium/dilithium_common/dilithium_types.h

src/lib/pubkey/pqcrystals/pqcrystals.h

src/lib/pubkey/pqcrystals/pqcrystals_helpers.h

FAlbertDev · 2024-06-25T08:51:15Z

src/lib/pubkey/dilithium/dilithium_common/dilithium.cpp

 size_t Dilithium_PublicKey::key_length() const {
 return m_public->mode().public_key_bytes();
 }


I get confused by this functions every single time...

/** * Return an integer value best approximating the length of the * primary security parameter. For example for RSA this will be * the size of the modulus, for ECDSA the size of the ECC group, * and for McEliece the size of the code will be returned. */ virtual size_t key_length() const = 0;

I'm not sure which value fits best here. We should, at least, provide some size in bits.
Same for Kyber.

For Kyber, we could return the ML-KEM-x values: i.e. {512, 768, 1024}. What do these even mean though? My best guess: k*N as the coefficients in the private vector s?! At least that has some foundation in the names of the specified parameter sets, but its not a bit length. We could multiply those by 12 as the bit-length of Q, but that will result in fairly arbitrary values {6144, 9216, 12288}.

For Dilithium, we could go for the bit lengths of s1 and s2 perhaps?

@randombit Do you have any preference?

When looking at where key_length is used at all, it seems that it should be some metric beside estimated_strength to identify specific instances of an algorithm while being generic enough to be a number. Also, the higher the number, the higher the security. Therefore, I would guess everything with these properties should be sufficient, even {1,2,3}. Therefore, I think {512, 768, 1024} should suffice. But I'll submit to the judgment of our Botan master ;)

@FAlbertDev has the right idea about the intent. It doesn't really have anything to do with the length of the key in bytes in any direct way (after all a ECDSA P-256 public key is 64 bytes being two field elements, but saying the key length of ECDSA P-256 is 512 bits is not going to illuminate the situation!).

For Kyber I see quite commonly ML-KEM-{512,768,1024} so that seems the obvious choice. NIST uses the same naming.

For Dilithium I have seen less press. The NIST spec (as well as https://www.ietf.org/id/draft-ietf-lamps-dilithium-certificates-03.html) uses 44, 65, and 87. TBH I have no idea where those numbers come from, but it's as good as anything.

Co-authored-by: Fabian Albert <[email protected]>

Co-Authored-By: Fabian Albert <[email protected]>

t1 is part of the public key and thus independent of any other verification input. Precomputing it saves about 20% of verification runtime when performing more than a single verification with the same Botan::PK_Verifier. Co-Authored-By: Fabian Albert <[email protected]>

FAlbertDev

Final review iteration.
I really like your work. IMO it's the better reference implementation ;)

FAlbertDev · 2024-06-25T08:52:43Z

src/lib/pubkey/kyber/kyber_common/kyber.cpp

 size_t Kyber_PublicKey::key_length() const {
 // TODO: this should report 512, 768, 1024
- return m_public->mode().public_key_byte_length();
+ return m_public->mode().public_key_bytes();
 }


See dilithium comment above. I'm not sure if 512, 768, 1024 are the correct values, though.

FAlbertDev · 2024-06-25T11:44:20Z

src/lib/pubkey/kyber/kyber_common/kyber.cpp

+ // TODO: Remove the need for the montgomery transformation
+ //
+ // -> When calculating A*s below, A is not in montgomery form, but s is. The
+ // operation uses fqmul internally, which performs a montgomery reduction.
+ auto A = montgomery(kyber_sample_matrix(rho, false /* not transposed */, mode));


I've just investigated the usage of Montgomery in Kyber. First, a short recap on Montgomery:
The main observation is that every Montgomery multiplication $c=a \cdot b$ adds a factor of $R^{-1}$ to the result c where $R$ is the Montgomery factor (i.e., we compute $a \cdot b \cdot R^{-1}$) At some point we need to multiply $R$ to neutralize this $R^{-1}$ factor. Therefore, we could multiply $R$ to either $a$ or $b$. However, we can also multiply $R$ to the result $c$.
In our Kyber and Dilithium code, the following operations add or remove the Montgomery factor.

the function montgomery adds a factor $R$

the function inv_ntt adds a factor $R$. Note that ntt doesn't add a Montgomery factor.

As already said, a Montgomery multiplication adds a factor $R^{-1}$ to each poly, vector, or matrix element. This holds for all multiplication types, i.e., polynomial-polynomial, vector-vector, matrix-vector multiplication, etc.

For encryption and decryption, this setup works well since there is always one multiplication (adding factor $R^{-1}$) followed by inv_ntt (adding factor $R$). This does not work here since we store the value t in NTT domain and, therefore, do not call inv_ntt. We have a factor of $R^{-1}$ to eliminate. Currently, we do that by adding the factor $R$ to the matrix beforehand using montgomery. Instead, I propose to add this factor to t after the multiplication. The fewer elements multiplied by $R$, the faster it should be.

auto A = kyber_sample_matrix(rho, false /* not transposed */, mode); auto s = ntt(ps.sample_polynomial_vector_cbd_eta1()); const auto e = ntt(ps.sample_polynomial_vector_cbd_eta1()); auto t = montgomery(A * s); t += e; t.reduce(); // End Algorithm 12 ---------------------------

src/lib/pubkey/kyber/kyber_common/kyber_algos.h

src/lib/pubkey/kyber/kyber_common/kyber_polynomial.h

src/lib/rng/rng.h

FAlbertDev · 2024-06-26T06:39:39Z

src/tests/test_crystals.cpp

+ Botan_Tests::CHECK("hamming weight of polynomials",
+ [](Test::Result& res) {
+ Kyberish_Poly<Domain::Normal> p;
+ res.test_is_eq<size_t>("hamming weight of 0", p.hamming_weight(), 0);


already tested below

One is testing poly, the other polyvec. The more the merrier I would say.

FAlbertDev · 2024-06-26T06:44:50Z

src/tests/test_crystals.cpp

+
+} // namespace
+
+BOTAN_REGISTER_TEST_FN("pubkey", "CRYSTALS", test_extended_euclidean_algorithm, test_polynomial_basics, test_encoding);


Suggested change

BOTAN_REGISTER_TEST_FN("pubkey", "CRYSTALS", test_extended_euclidean_algorithm, test_polynomial_basics, test_encoding);

BOTAN_REGISTER_TEST_FN("pubkey", "crystals", test_extended_euclidean_algorithm, test_polynomial_basics, test_encoding);

it seems all other test suits use snake_case (see ./botan-test --help). Maybe, while you're at it, adapt the names for dilithium as well (e.g.dilithium_kat_4x4_AES_Deterministic, dilithium_kat_4x4_Randomized, etc.).

Fair. Regarding the dilithium KATs: These names are currently tightly coupled with the DilithiumMode constants. 😨 This will need a cleanup anyway, but lets not do that in this PR.

randombit

Partial review still need to look at changes in dilithium and kyber

src/lib/pubkey/pqcrystals/pqcrystals_helpers.h

src/lib/pubkey/pqcrystals/pqcrystals_encoding.h

src/lib/pubkey/pqcrystals/pqcrystals_helpers.h

src/tests/test_crystals.cpp

src/lib/pubkey/pqcrystals/info.txt

src/lib/pubkey/kyber/kyber_common/kyber_helpers.h

Co-Authored-By: Fabian Albert <[email protected]> Co-Authored-By: Jack Lloyd <[email protected]>

This comment was marked as resolved.

Sign in to view

reneme mentioned this pull request Apr 23, 2024

HSS-LMS Signature Algorithm Implementation #3716

Merged

This comment was marked as outdated.

Sign in to view

atreiber94 mentioned this pull request May 17, 2024

PQC: ML-DSA (Initial Public Draft) #4062

Draft

3 tasks

reneme mentioned this pull request May 29, 2024

🤖 Audit: Auto-update with latest upstream patches sehlen-bsi/botan-docs#212

Merged

reneme force-pushed the refactor/crystals_april branch from 64f9db8 to 066fc6d Compare May 31, 2024 13:24

reneme force-pushed the refactor/crystals_april branch 8 times, most recently from dd45e61 to c30b9fe Compare June 20, 2024 15:28

reneme marked this pull request as ready for review June 20, 2024 15:41

This comment was marked as outdated.

Sign in to view

reneme commented Jun 20, 2024

View reviewed changes

reneme force-pushed the refactor/crystals_april branch from c30b9fe to cf16bf3 Compare June 21, 2024 09:27

reneme added 2 commits June 21, 2024 11:36

Add to_underlying() (from C++23)

c5f6a94

Add XOF::output<count>() to obtain an array

3002915

reneme force-pushed the refactor/crystals_april branch from cf16bf3 to 3fc00ec Compare June 21, 2024 09:40

reneme requested a review from FAlbertDev June 21, 2024 14:28

reneme and others added 3 commits June 21, 2024 16:38

Rework Kyber using the common CRYSTALS structs

bc4739a

Rework Dilithium using the common CRYSTALS structs

8dcd72b

reneme force-pushed the refactor/crystals_april branch from 3fc00ec to 8dcd72b Compare June 21, 2024 14:38

FAlbertDev reviewed Jun 24, 2024

View reviewed changes

FAlbertDev reviewed Jun 25, 2024

View reviewed changes

reneme mentioned this pull request Jun 25, 2024

Faster pcurves reductions for P-256 and P-384 #4147

Open

Apply indisputable suggestions from code review

c021471

Co-authored-by: Fabian Albert <[email protected]>

reneme force-pushed the refactor/crystals_april branch from 3cbf069 to 2289246 Compare June 25, 2024 16:18

reneme and others added 2 commits June 26, 2024 08:43

more low-hanging review fruits

a73f47b

Co-Authored-By: Fabian Albert <[email protected]>

reneme force-pushed the refactor/crystals_april branch from 1a3ac57 to ae4298c Compare June 26, 2024 06:44

FAlbertDev approved these changes Jun 26, 2024

View reviewed changes

randombit reviewed Jun 26, 2024

View reviewed changes

reneme mentioned this pull request Jun 27, 2024

add RandomNumberGenerator::random_array<>() #4152

Merged

reneme force-pushed the refactor/crystals_april branch from a939cf1 to 33c6a10 Compare June 27, 2024 11:39

Review comments

3ecb3f9

Co-Authored-By: Fabian Albert <[email protected]> Co-Authored-By: Jack Lloyd <[email protected]>

reneme force-pushed the refactor/crystals_april branch from 33c6a10 to 3ecb3f9 Compare June 27, 2024 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: Have a common base for Kyber/Dilithium structures #4024

Refactor: Have a common base for Kyber/Dilithium structures #4024

reneme commented Apr 19, 2024 •

edited

Loading

coveralls commented Apr 22, 2024 •

edited

Loading

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

reneme left a comment

reneme Jun 20, 2024

reneme Jun 20, 2024

reneme Jun 20, 2024

reneme Jun 20, 2024

FAlbertDev Jun 24, 2024

reneme Jun 20, 2024

FAlbertDev Jun 25, 2024

reneme Jun 27, 2024 •

edited

Loading

reneme Jun 20, 2024

reneme Jun 20, 2024

FAlbertDev Jun 26, 2024

reneme Jun 20, 2024

randombit Jun 22, 2024

reneme Jun 25, 2024

FAlbertDev left a comment

FAlbertDev Jun 25, 2024

reneme Jun 25, 2024

FAlbertDev Jun 26, 2024

randombit Jun 27, 2024

FAlbertDev left a comment

FAlbertDev Jun 25, 2024

FAlbertDev Jun 25, 2024

FAlbertDev Jun 26, 2024

reneme Jun 27, 2024

FAlbertDev Jun 26, 2024

reneme Jun 27, 2024

randombit left a comment

		// TODO: perhaps secure vector
		std::vector<T> m_coeffs_storage;


		} // namespace

		BOTAN_REGISTER_TEST_FN("pubkey", "CRYSTALS", test_extended_euclidean_algorithm, test_polynomial_basics, test_encoding);

Refactor: Have a common base for Kyber/Dilithium structures #4024

Are you sure you want to change the base?

Refactor: Have a common base for Kyber/Dilithium structures #4024

Conversation

reneme commented Apr 19, 2024 • edited Loading

Module: pqcrystals

Module: kyber

Module: dilithium

Benchmark

coveralls commented Apr 22, 2024 • edited Loading

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

reneme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reneme Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FAlbertDev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FAlbertDev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

randombit left a comment

Choose a reason for hiding this comment

reneme commented Apr 19, 2024 •

edited

Loading

Module: `pqcrystals`

Module: `kyber`

Module: `dilithium`

coveralls commented Apr 22, 2024 •

edited

Loading

reneme Jun 27, 2024 •

edited

Loading